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

Additions needed for offline entity updates #749

Merged
merged 7 commits into from
Mar 11, 2024

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Feb 23, 2024

Blocked by #748

This adds new features to JavaRosa to allow clients to handle offline updates to Entities:

  • Entities updated using an update action are now exposed from the Entities extra on FormEntryModel
  • Entity now has a version field that is set to 1 for created Entities
  • Updated Entities have their version set to the entity element's baseVersion attribute incremented by 1
  • Forms that do an "upsert" (contain both a truthy create and update) expose an Entity as an update (version is baseVersion + 1)
  • Forms that do an "upsert", but do not define a baseVersion expose an Entity as a create (version is set to 1)
  • Forms that do an update, but do not contain a label node for the Entity, expose an Entity with a null label so clients know not to override an existing label if they have it

What has been done to verify that this works as intended?

New tests and verified with new Collect features at getodk/collect#5982.

Why is this the best possible solution? Were any other approaches considered?

I did consider leaving the version incrementing to clients, but that would mean them making a decision about how to deal with the "upsert" cases. It felt more natural to me for JavaRosa to be the one making decisions about how to interpret the spec.

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?

We now care a lot more about the entity element at finalization as we parse more out of it. I think this probably means that there is more scope for crashes during form finalization due to incorrect forms. I think we should collect those cases and handle in a follow up rather than holding this up however as we're mostly concerned right now with getting an experimental version of offline entities together (and the feature will still be opt in). It also feels easier to me to play with bad forms once we're able to do it Collect.

The XForms spec states: "If both are truthy, the spec consumer processing
submissions should do both and one of them will fail."

We might want this to throw an exception at parse instead, but
that's something to discuss.
@seadowg seadowg marked this pull request as ready for review February 28, 2024 10:28
@lognaturel
Copy link
Member

we should collect those cases and handle in a follow up rather than holding this up however as we're mostly concerned right now with getting an experimental version of offline entities together

That sounds right to me.

We also know there are very likely going to be additions to the spec to support offline submission runs (one client submits N updates to one entity and another also submits updates in parallel).

With those things in mind, I did a pretty quick pass to make sure the tests match the cases discussed in the issues and that there are no major sources of risk that I can think of. We'll want to come back to this more carefully before a release.

@lognaturel lognaturel merged commit 6a32b4b into getodk:master Mar 11, 2024
3 checks passed
@seadowg seadowg deleted the entity-updates branch March 11, 2024 16:31
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.

2 participants