Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSV Import: Handle re-importing of select_multiple questions #1852

Merged
merged 7 commits into from
Sep 7, 2020

Conversation

DavisRayM
Copy link
Contributor

@DavisRayM DavisRayM commented Jul 21, 2020

Changes / Features implemented

  • Handle cases where extra details are added on an extra column.
  • Flatten split select_multiple questions into one column

Steps taken to verify this change does what is intended

  • Added test that replicates the issue and confirmed the test passes with the fix
  • QA

Side effects of implementing this change

  • Possible loss of data? Chance that the string value passed in the select_multiple field is actually useful? Should we store the data in the metadata ?

Extra Info

Seems the issue documented in #1655 occurs when a user adds in a column(matching a select_multiple question) for extra information. i.e When using a form such as this and trying to import this kind of data

Closes #1655

@DavisRayM DavisRayM force-pushed the 1655-csv-import-error branch 3 times, most recently from c731fb9 to 8b7a44f Compare July 30, 2020 14:05
@DavisRayM DavisRayM marked this pull request as ready for review July 30, 2020 14:22
@DavisRayM DavisRayM changed the title Handle 'AttributeError' raised during csv import [WIP] Handle 'AttributeError' raised during csv import Jul 30, 2020
ivermac
ivermac previously approved these changes Aug 3, 2020
Copy link
Contributor

@ivermac ivermac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@DavisRayM DavisRayM changed the title [WIP] Handle 'AttributeError' raised during csv import Handle 'AttributeError' raised during csv import Aug 3, 2020
@DavisRayM DavisRayM force-pushed the 1655-csv-import-error branch 2 times, most recently from b57afa7 to 7f65f4a Compare August 3, 2020 11:56
Comment on lines +75 to +78
if isinstance(result[k], str) and \
k in override_keys:
result[k] = {}
result[k] = merge_list_of_dicts([result[k], v])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should happen in the else part of this check? Should we re-raise the exception or silently ignore as this currently does? Does it mean that an empty submission is made or the row is skipped?

Copy link
Contributor Author

@DavisRayM DavisRayM Aug 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would mean the row would be skipped & the previous value would be used on the field. I think re-raising the error and stopping the CSV Import process here would be the best option...

ukanga
ukanga previously approved these changes Aug 4, 2020
Copy link
Member

pld commented Aug 12, 2020

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- onadata/libs/utils/dict_tools.py  2
         

See the complete overview on Codacy

@DavisRayM DavisRayM force-pushed the 1655-csv-import-error branch 3 times, most recently from 32ffc97 to a13e705 Compare August 26, 2020 13:57
@DavisRayM DavisRayM changed the title Handle 'AttributeError' raised during csv import CSV Import: Handle re-importing of select_multiple questions Aug 26, 2020
@DavisRayM DavisRayM force-pushed the 1655-csv-import-error branch 2 times, most recently from 7829944 to 2eaa305 Compare September 2, 2020 09:41
The 'AttributeError' is usually raised during the CSV Import process if
a user imports a CSV form that has an extra column(usually containing
some extra information that may be useful for the users own analysis)
Handle cases where extra details are added on a select_multiple question
column
select_multiple keys can have duplicates depending on the number of
choices. This change ensures that the 'select_question_keys' list only
has one entry for each question key and that the 'meta' key is excluded.
@DavisRayM DavisRayM merged commit f0869e8 into master Sep 7, 2020
@DavisRayM DavisRayM deleted the 1655-csv-import-error branch September 7, 2020 07:12
@DavisRayM DavisRayM mentioned this pull request Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError: 'str' object has no attribute 'items'
4 participants