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 creation #748

Merged
merged 5 commits into from
Feb 27, 2024

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Feb 22, 2024

This adds two things needed to implement offline entities in Collect:

  • Expose entity ID and label in EntityFormFinalizationProcessor
  • Add processor for ExternalDataInstance parsing that can be added to a new ExternalInstanceParser

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

New tests.

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

There's a lot of different ways we could go to get offline entities working. This is a first simple pass that focuses on getting the feature working and doesn't take into account additional goals like reducing the memory footprint of external secondary instances.

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 only adds things so should be very low risk.

@seadowg seadowg marked this pull request as ready for review February 22, 2024 15:12
@seadowg seadowg changed the title Additions needed for offline entities Additions needed for offline entity creation Feb 23, 2024
@seadowg
Copy link
Member Author

seadowg commented Feb 23, 2024

@lognaturel I've updated this so that we can override how external instances are parsed rather than just the XForm as a whole so we can intercept the parse in both the fresh XForm and the cached case. This is ready to go!

@seadowg
Copy link
Member Author

seadowg commented Feb 26, 2024

@lognaturel I've switched this to the SNAPSHOT version as we discussed.

Copy link
Member

@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.

Everything here looks reasonable and safe. I didn't think too deeply about "is this the best way" because we're treating this as relatively experimental for now.

@lognaturel lognaturel merged commit 312ff3c into getodk:master Feb 27, 2024
3 checks passed
@seadowg seadowg deleted the entity-additions branch February 28, 2024 10:07
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