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

Regenerate snapshots when downloading XForm XML #5128

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jnm
Copy link
Member

@jnm jnm commented Sep 27, 2024

Checklist

  1. If you've added code that should be tested, add tests
  2. If you've changed APIs, update (or create!) the documentation
  3. Ensure the tests pass
  4. Make sure that your code lints and that you've followed our coding style
  5. Write a title and, if necessary, a description of your work suitable for publishing in our release notes
  6. Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE)
  7. Open an issue in the docs if there are UI/UX changes

Description

Invokes pyxform every time "Download XML" is clicked, which avoids serving stale XForm XML in certain situations. For example, prior to this work, if a form had not been changed since its XForm XML was last generated, but the behavior of pyxform had changed, clicking "Download XML" would have ignored the pyxform changes.

Please note that the XForm XML obtained from "Download XML" is not identical to the XForm XML generated when deploying a form. Deploying or redeploying a form was not affected by this stale XML issue, even prior to this change.

Notes

I did not add a unit test or attempt to mock the response by pyxform to simulate changes to its behavior. The reviewer may make the determination about the sufficiency of my approach.

Internal discussion: https://chat.kobotoolbox.org/#narrow/stream/4-Kobo-Dev/topic/Pyxform.20updates/near/458832

Avoids serving stale XML in certain situations, such as when the form
has *not* been changed but pyxform has been updated
@tiritea
Copy link

tiritea commented Sep 27, 2024

Curious, what is/was the reason behind needing to persist snapshots in the first place, given they're not the XML XForm definition that is going to get deployed in any case?

Are snapshots just being done (and basically cached) for Enketo-based preview functionality? If so, then we would seen to be in much the same situation with Preview: unless you happen to have done an "XML Download" on all your forms after a pyxform change, the saved snapshot that Enketo renders will still be effectively out-of-date [and its not intuitive that "XML Download" would be a prerequisite for Preview...]

@noliveleger noliveleger changed the base branch from beta-refactored to main October 7, 2024 18:30
@noliveleger noliveleger changed the base branch from main to release/2.024.36 October 7, 2024 18:49
@noliveleger noliveleger changed the base branch from release/2.024.36 to main October 7, 2024 18:49
Copy link
Contributor

@noliveleger noliveleger left a comment

Choose a reason for hiding this comment

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

As per our discussion, let's add a minimal unit test for that

https://chat.kobotoolbox.org/#narrow/stream/4-Kobo-Dev/topic/Pyxform.20updates/near/466261

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants