Prevent local entity create/update from older spec version forms #6394
Conversation
| @@ -54,11 +56,11 @@ public void processBindAttribute(String name, String value, DataBinding binding) | |||
|
|
|||
| @Override | |||
| public void processFormDef(FormDef formDef) throws XFormParser.ParseException { | |||
| if (!versionPresent && EntityFormParser.getEntityElement(formDef.getMainInstance()) != null) { | |||
| if (version == null && EntityFormParser.getEntityElement(formDef.getMainInstance()) != null) { | |||
There was a problem hiding this comment.
Why do we need to check if EntityFormParser.getEntityElement(formDef.getMainInstance()) != null here?
Can't we just throw that exception if version is null?
There was a problem hiding this comment.
Hmm good point! Let me play with that.
There was a problem hiding this comment.
Ah, because if we do that, we end up exploding when there's no version and no entity element. We only care about validating the version if there's an entity element.
There was a problem hiding this comment.
Ok so it determines if a form contains entities or not? If so please create a method with a descriptive name that could be used here instead of that enigmatic
EntityFormParser.getEntityElement(formDef.getMainInstance()) != null
There was a problem hiding this comment.
Gotha! I can probably wrap the whole method in that check as well to make it clearer.
entities/src/main/java/org/odk/collect/entities/javarosa/parse/EntityFormParseProcessor.java
Show resolved
Hide resolved
|
Tested with success! Verified on device with Android 14 Verified cases:
|
|
Tested with success! Verified on device with Android 10 |
Closes #6384
Why is this the best possible solution? Were any other approaches considered?
The biggest change ended up being in
EntityFormParseProcessor. This now only adds theEntityFormExtraif the entity spec version being used isv2024.1(or a patch of that). For earlier versions there's actually now nothing we need to do with theentitynode (that isn't already handled by JavaRosa), so this made sense as a simple change to disable local entities for earlier spec versions. As a point of interest, we'd only been exposing entities after form finalization in these older versions for debug purposes and to verify the spec made sense - this is us explicitly makingv2024.1the cut-off for local entities.How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
This should only affect entity forms, but it's worth quickly check form entry for a non-entity form as well. The biggest change will be the intended one that now old entity forms (v2023.1 and older) will no longer create or update entities until a submit/refresh happens.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still passDateFormatsTest