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

Set root node for created submissions to the XForms configured root node #1853

Merged
merged 3 commits into from
Aug 6, 2020

Conversation

DavisRayM
Copy link
Contributor

@DavisRayM DavisRayM commented Jul 27, 2020

Changes / Features implemented

  • Set the root node to data. By default the root node of an XForm is always set to data unless specifically set to something else

  • Make change backward compatible with forms that have the root node name set to something else other than the default(data)

  • Add management command that helps replace the XForm ID String as the root node for Instances

Steps taken to verify this change does what is intended

  • Updated tests
  • QA

Things to Note

Submissions made using RapidPro before the changes in this PR are in effect, will still have the different root node name issue. Suggestion: Those submissions should be updated manually on a case by case basis(Server wide submission XML edits seem a bit too unsafe)

@DavisRayM DavisRayM force-pushed the enketo_different_root_nodes_fix branch 2 times, most recently from 0b9e462 to 4bb6562 Compare July 27, 2020 11:48
@DavisRayM DavisRayM changed the title [WIP] Change root node for created submissions to data Change root node for created submissions to data Jul 27, 2020
@DavisRayM DavisRayM force-pushed the enketo_different_root_nodes_fix branch from 4bb6562 to acf673d Compare July 27, 2020 13:56
@@ -46,7 +46,7 @@ def create_submission(request, username, data_dict, xform_id):
"""
Returns validated data object instances
"""
xml_string = dict2xform(data_dict, xform_id)
xml_string = dict2xform(data_dict, xform_id, root='data')
Copy link
Member

Choose a reason for hiding this comment

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

What happens to older forms that still use the filename of the XLSForm that is in our historical data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's something I did not initially consider... I'll look into retrieving the root node name from the form. I believe pyxform provides this information on the survey object, I'll confirm and update the PR

Copy link
Contributor Author

@DavisRayM DavisRayM Jul 30, 2020

Choose a reason for hiding this comment

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

Updated the PR. Now trying to retrieve the root node name from the XForm if a username is passed alongside the form_id, This should ensure all new submissions have the correct root node for the form.

@DavisRayM DavisRayM changed the title Change root node for created submissions to data [WIP] Change root node for created submissions to data Jul 28, 2020
@DavisRayM DavisRayM force-pushed the enketo_different_root_nodes_fix branch 10 times, most recently from 54ac36c to 5eaee15 Compare July 29, 2020 14:30
@DavisRayM DavisRayM changed the title [WIP] Change root node for created submissions to data Change root node for created submissions to data Jul 29, 2020
@DavisRayM DavisRayM force-pushed the enketo_different_root_nodes_fix branch from 5eaee15 to e87e511 Compare July 30, 2020 06:05
ukanga
ukanga previously approved these changes Aug 4, 2020
By default XLSForms set the default node to 'data'
@DavisRayM DavisRayM force-pushed the enketo_different_root_nodes_fix branch from e87e511 to a2b9698 Compare August 4, 2020 08:13
@DavisRayM DavisRayM changed the title Change root node for created submissions to data Set root node for created submissions to the XForms configured root node Aug 4, 2020
@faith-mutua
Copy link

faith-mutua commented Aug 4, 2020

@DavisRayM New RapidPro submissions can now be edited in Enketo whereas old submissions cannot. The attached error is returned when editing old RapidPro submissions.
Screen Shot 2020-08-04 at 13 37 34

@DavisRayM DavisRayM changed the title Set root node for created submissions to the XForms configured root node [WIP] Set root node for created submissions to the XForms configured root node Aug 4, 2020
@DavisRayM DavisRayM changed the title [WIP] Set root node for created submissions to the XForms configured root node Set root node for created submissions to the XForms configured root node Aug 4, 2020
@DavisRayM DavisRayM force-pushed the enketo_different_root_nodes_fix branch 2 times, most recently from 34523fd to 8ff96fe Compare August 5, 2020 09:17
@DavisRayM
Copy link
Contributor Author

DavisRayM commented Aug 5, 2020

Old submissions from RapidPro will have to be updated via the management command

python manage.py replace_form_id_root_node -i <instance_ids>  # Prints out the changes that are going to be made on the XML
python manage.py replace_form_id_root_node -i <instance_ids> -c  # Makes changes to the Instances XML and saves them

@DavisRayM New RapidPro submissions can now be edited in Enketo whereas old submissions cannot. The attached error is returned when editing old RapidPro submissions.
Screen Shot 2020-08-04 at 13 37 34

@DavisRayM DavisRayM force-pushed the enketo_different_root_nodes_fix branch from 8ff96fe to 997d01f Compare August 5, 2020 09:37
ivermac
ivermac previously approved these changes Aug 5, 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.

This looks good to me but might require @ukanga 's eyes as well

initial_xml = inst.xml
form_id = re.escape(inst.xform.id_string)
if not root:
root = inst.xform.survey.name
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the value of root always be data?

Copy link
Contributor Author

@DavisRayM DavisRayM Aug 5, 2020

Choose a reason for hiding this comment

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

Not always, it'll be data as long as the user doesn't configure the form to have another root node name.

Copy link
Member

pld commented Aug 5, 2020

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

Complexity increasing per file
==============================
- onadata/apps/logger/management/commands/replace_form_id_root_node.py  4
         

See the complete overview on Codacy

@DavisRayM DavisRayM merged commit d79754d into master Aug 6, 2020
@DavisRayM DavisRayM deleted the enketo_different_root_nodes_fix branch August 6, 2020 06:55
@DavisRayM DavisRayM mentioned this pull request Aug 10, 2020
1 task
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.

5 participants