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

XForm with reserved entity property name can be uploaded in new version #784

Closed
matthew-white opened this issue Feb 27, 2023 · 4 comments · Fixed by #810
Closed

XForm with reserved entity property name can be uploaded in new version #784

matthew-white opened this issue Feb 27, 2023 · 4 comments · Fixed by #810
Assignees
Labels
behavior verified Behavior has been manually verified bug entities Multiple Encounter workflows

Comments

@matthew-white
Copy link
Member

When you upload an XLSForm that would create entities, pyxform will check that a reserved name is not used for an entity property (i.e., name or label). pyxform returns an error message like this:

The XLSForm could not be converted: [row : 2] Invalid save_to name: the entity property name 'name' is reserved.

If you upload an XForm instead of an XLSForm, then Backend will complete the check. If you try to create a new form using such an XForm, Backend will return an error message like this:

The entity definition within the form is invalid. Invalid Dataset property.

However, it looks like Backend only checks this when a new form is created, not when a new version is created for an existing form. I was able to upload an XForm with the entity property names name and label here: https://test.getodk.cloud/#/projects/444. I think we should complete this check whenever an XForm is uploaded, not just when a new form is created.

@matthew-white matthew-white added bug entities Multiple Encounter workflows labels Feb 27, 2023
@ktuite ktuite self-assigned this Feb 28, 2023
@matthew-white
Copy link
Member Author

@ktuite, am I right in thinking that this was addressed in #810?

@matthew-white matthew-white added the needs testing Needs manual testing label Apr 4, 2023
@ktuite
Copy link
Member

ktuite commented Apr 24, 2023

Yes, this was addressed in #810. (Just looked and confirmed.)

I also just added a test for this case in #845 when I went back and did more validation of entity/dataset property names.

@ktuite ktuite closed this as completed Apr 24, 2023
@matthew-white matthew-white linked a pull request Apr 24, 2023 that will close this issue
2 tasks
@dbemke
Copy link

dbemke commented May 17, 2023

Tested With Success!

1 similar comment
@srujner
Copy link

srujner commented May 17, 2023

Tested With Success!

@srujner srujner added behavior verified Behavior has been manually verified and removed needs testing Needs manual testing labels May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior verified Behavior has been manually verified bug entities Multiple Encounter workflows
Projects
Status: ✅ done
Development

Successfully merging a pull request may close this issue.

4 participants