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

Tell clients to delete entities when their submissions have not been approved #685

Open
matthew-white opened this issue Jul 11, 2024 · 6 comments
Assignees
Labels
backend Requires a change to the API server enhancement New feature or behavior entities Multiple Encounter workflows needs discussion Discussion needed before work can begin

Comments

@matthew-white
Copy link
Member

There's a question of how offline entities should work for an entity list that requires submission approval when creating entities from submissions. The OpenRosa spec proposal for offline entities mentions a use case for the integrityUrl (#668) that involves submission approval:

An Entity List is configured to only create Entities on submission approval. Collect will create Entities offline in that case. If an Entity-creating submission is rejected, the Entity should be deleted from Collect

In that case, Central would return <deleted>true</deleted> for an entity that would have been created from a rejected submission. Actually, Central could return <deleted>true</deleted> for any new entity that it's seen in a submission (any new entity that it's even partially processed). Whether or not the submission is approved or rejected, the entity no longer needs to remain on the device as long as we know it's on the server and awaiting action there. As @lognaturel writes on Slack:

There’s a related and possibly even more complicated case which is when the submission has been made but not yet approved or rejected. Ideally that would also mean the Entity gets deleted locally.

Another idea we've discussed is to use the OpenRosa manifest to indicate to Collect that the entity list requires submission approval. The idea is that if an entity list is identified as requiring submission approval, Collect shouldn't create offline entities for the entity list. Forms could still share the entity list (reducing storage needs), but forms wouldn't add offline entities to the list.

If we go that route, one edge case to consider is if the submission approval setting is toggled after Collect downloads the OpenRosa manifest. In that case, Collect could end up creating offline entities for an entity list that didn't initially require submission approval, but was later set up to. As above, there would maybe be cases where Collect would need to be told to delete some of the offline entities.

@matthew-white matthew-white added enhancement New feature or behavior needs discussion Discussion needed before work can begin backend Requires a change to the API server entities Multiple Encounter workflows labels Jul 11, 2024
@github-project-automation github-project-automation bot moved this to 🕒 backlog in ODK Central Jul 16, 2024
@ktuite ktuite changed the title Offline entities and submission approval Tell client to delete entities when their submissions have not been approved Sep 3, 2024
@lognaturel lognaturel changed the title Tell client to delete entities when their submissions have not been approved Tell clients to delete entities when their submissions have not been approved Sep 3, 2024
@matthew-white
Copy link
Member Author

Actually, Central could return <deleted>true</deleted> for any new entity that it's seen in a submission (any new entity that it's even partially processed).

I just wanted to explain the phrase "partially processed". I'm thinking about submissions that ultimately create entities, but only after the submission is processed twice. Even when an entity list requires submission approval for entity creation, every submission.create event will still be run through Entities.processSubmissionEvent(). processSubmissionEvent() will eventually bail once it realizes in _createEntity() that there's nothing to do yet, but it does do some processing before reaching that point. For example, it parses the submission XML and validates the dataset attribute. This is what I mean by "partial" processing. The submission will be fully processed once Entities.processSubmissionEvent() is run a second time, for the submission.update event.

My idea here is that by the time Entities.processSubmissionEvent() bails for the submission.create event, it will have parsed the entity UUID. Even if Entities.processSubmissionEvent() doesn't end up creating an entity, it could persist the UUID somewhere. Then when Collect asks about the UUID, Central could say: I've seen a submission with this entity UUID; Collect doesn't need to store it on-device anymore.

@sadiqkhoja
Copy link
Contributor

Do we really need to tell Collect that Submission is "not approved"? Or should we just inform it when a Submission is "rejected".

I am thinking of the cases when decision is not made yet and if tell Collect that we know about an entity which is not approved yet so Collect should delete it - wouldn't that be wrong?

@matthew-white
Copy link
Member Author

I don't feel like I totally understand the use case. I think @lognaturel would know more. But the way we've been discussing things lately, if an entity list requires submission approval, the entity list just shouldn't be using offline entities.

@sadiqkhoja
Copy link
Contributor

Scenario:

  • Collect creates a Submission, which led to an Offline Entity
  • Submission is sent to Central, it requires approval for Entity creation
  • Nothing is done on the Central, hence no Entity is created
  • Collect receives entities.csv from the Central which doesn't include the Entity
  • Collect sends request to integrity URL to check the status of that Entity
  • Central tells Collect that Entity is not approved yet so delete it at your end as well
  • Collect deletes the Entity and goes offline
  • Submission is approved in Central and Entity is created
  • Collect wants to update the Entity but it is not there ‼️ (it will receive the entity in the next entities.csv though)

if an entity list requires submission approval, the entity list just shouldn't be using offline entities

I guess it will require update to Form definition? We keep approvalRequired flag on the Central only as of now.

@lognaturel
Copy link
Member

Do we really need to tell Collect that Submission is "not approved"?

Otherwise there's the possibility of the user doing offline work on that Entity even though it doesn't exist yet. In particular, a submission can be edited from Central before approval so that could lead to scenarios that are hard to reconcile.

wouldn't that be wrong?

It would bring Central and Collect in alignment. I can imagine a future in which Collect knows that a particular Entity-creating form requires approval before creating Entities so that it doesn't create that Entity in the first place. But I don't think we're going to prioritize that any time soon. Having Central tell Collect to delete an Entity that Collect knows about but that Central knows won't be created until it's approved makes that case less bad.

if an entity list requires submission approval, the entity list just shouldn't be using offline entities

We don't currently have a way of opting out of offline Entity creation and update so it's more about hoping that users design their workflows with this in mind. For example, it's totally fine to have a workflow like https://docs.getodk.org/tutorial-community-reporting/ that requires submission approval because the create and updates are on different devices. I guess we can say that the workflow opts out of offline Entity creation in a way.

The idea with telling clients to delete Entities created from submissions that haven't been approved yet is to limit the chances of a nonsense offline chain happening. I don't think it's hugely high priority, though, and happy to drop it if we think it's more confusing than helpful.

@sadiqkhoja
Copy link
Contributor

a submission can be edited from Central before approval.

Interesting! If editing and approval happen fast enough then entity version on Central and Collect would be same but data would be different or Collect always patches the data even when version is same.

I guess Collect needs to stop creating offline entities if approval is required.

@sadiqkhoja sadiqkhoja self-assigned this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Requires a change to the API server enhancement New feature or behavior entities Multiple Encounter workflows needs discussion Discussion needed before work can begin
Projects
Status: 🕒 backlog
Development

No branches or pull requests

3 participants