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

713: support offline entities spec v2024.1.0 via opt-in setting #719

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

lindsay-stevens
Copy link
Contributor

Closes #713

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

Implements #713: opt-in via a new entities setting offline with a value yes. If not opted-in, retain all current behaviour. If opted-in, for the create case, use the new spec version. If opted-in, for the update case, use the new spec version and emit trunkVersion/branchId properties and bindings.

Deviations to the requirements:

  1. Bindings readonly property. I used the existing function in entity_declaration.py, EntityDeclaration._get_bind_node(), which sets the attribute readonly="true()", which presumably is correct since this function is used for the existing baseVersion property with a similar structure.
  2. Opt-in behaviour. Like other yes/no settings, the exact value yes is accepted, or it's aliases e.g. Yes, true, etc.
  3. Opt-out behaviour. A user can opt out either by omitting the offline column, or by providing a value other than an alias for yes (such as no, bananas, or anything else). This seemed more consistent with the requirements than raising an error if the column is present but the value is not an alias for yes, although an error may make sense too.

What are the regression risks?

Low risk, should be backwards compatible as per the tests suite.

In future when the new spec version is the default, presumably the offline setting will be removed, meaning that from then on forms will have to remove that setting or else get an error about an unexpected column. This would be a minor point of friction but beneficial in that the setting would do nothing (all forms would use spec 2024.1.0) which may be confusing it were kept/allowed.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Updates to the entities spec and ODK docs are already in progress. The new offline setting is intended to be kept for only a few months, so I'm not sure if that's worth an update to the XLSForm template, especially if the intended audience during that period is insiders/advanced users. The xlsform.org page doesn't mention entities at all.

Before submitting this PR, please make sure you have:

  • included test cases for core behavior and edge cases in tests
  • run python -m unittest and verified all tests pass
  • run ruff format pyxform tests and ruff check pyxform tests to lint code
  • verified that any code or assets from external sources are properly credited in comments

@lindsay-stevens lindsay-stevens changed the title add: support offline entities spec v2024.1.0 via opt-in setting 713: support offline entities spec v2024.1.0 via opt-in setting Sep 5, 2024
Copy link
Contributor

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

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

Thanks for all the nice refinements and for calling them out explicitly in the PR description! I agree with all the decisions you made. 👍

@lognaturel lognaturel merged commit d209a84 into XLSForm:master Sep 5, 2024
10 checks passed
@lindsay-stevens lindsay-stevens deleted the pyxform-713 branch September 6, 2024 04:38
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.

Add support for offline Entity create and update
2 participants