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

[Feature] Delete saved draft of QuestionnaireResponses when clicking "delete draft" #3605

Open
9 tasks
f-odhiambo opened this issue Nov 13, 2024 · 15 comments · May be fixed by #3631
Open
9 tasks

[Feature] Delete saved draft of QuestionnaireResponses when clicking "delete draft" #3605

f-odhiambo opened this issue Nov 13, 2024 · 15 comments · May be fixed by #3631

Comments

@f-odhiambo
Copy link
Contributor

f-odhiambo commented Nov 13, 2024

Describe the feature request.
Implement a feature to delete all saved drafts of QuestionnaireResponses from the local SQLite database when the user opts to save a questionnaire as a draft. This will help manage storage and ensure that outdated drafts do not clutter the database.

Additional context
Currently, when a questionnaire is saved as a draft, it remains in the local database. This can lead to confusion and unnecessary data accumulation. By purging these drafts, we can maintain a cleaner state in the database.

Follow up ticket for #3541

delete_draft 2024-11-14 09-27-30

Acceptance criteria

  • Implement a function to delete all QuestionnaireResponses marked as drafts.
  • Ensure that the function is called whenever a questionnaire is saved as a draft.
  • Verify that the database no longer contains any QuestionnaireResponses marked as drafts after the deletion.
  • Write unit tests to confirm the functionality works as expected.

Area path

  1. Login to the app.
  2. Open the Navigation bar.
  3. Click on Questionnaires.
  4. Select a Questionnaire to edit.
  5. Click on Save as Draft.
  6. Verify that all drafts are deleted from the local database.

Implementation plan (For Engineers)

  • Add a new Application Workflow name LAUNCH_DELETE_DRAFT_FORM
  • Add a new AlertDialogFragment class that calls the showCancelAlert function. Implement the logic for Open Draft, Delete Draftand Cancel buttons
  • Open Draft - Launch the questionnaire with the latest draft values
  • Delete Draft - Update the status of the QuestionnaireResponse containing the draft changes to stopped
  • Handle the delete draft workflow in the configextensions. This should launch the delete draft form dialog (AlertDialogFragment defined above)
@f-odhiambo
Copy link
Contributor Author

Q. QR Status - in-progress | completed | amended | entered-in-error | stopped - which QR status do we use for the different user journey flows?
@Rkareko
I propose we use a new state stopped to flag all QR that are to be flagged for deletion

@f-odhiambo
Copy link
Contributor Author

As discussed

  1. We are going to add a new status to the QR - stopped then we run the background task to propagate to other devices
  2. If people work offline we need to figure out how to delete all stopped QR's from the other devices
  3. Can we propagate the delete to other devices whilst syncing the same records?

TBD
CC @Rkareko

@Rkareko
Copy link
Contributor

Rkareko commented Nov 14, 2024

  1. Can we propagate the delete to other devices whilst syncing the same records?

@f-odhiambo

  1. Purge - This only purges a resource from the database without deleting data from the server.
  2. Delete - This deletes the resource from the app db and propagates the changes to the server. However, from the tests carried out so far, it appears that the deletion is not propagated from the server to other devices.

Since the deletes cannot be propagated to other devices the following is likely to occur :

  1. Data inconsistency - A draft which has been deleted on device A still existing on another device B
  2. Sync issues - If the draft on device B is updated locally, syncing those changes to the server will result in an error. Sync uploaded changes to an existing resource as patches, the server will reject the patch update since the resource has already been deleted on the server

@f-odhiambo
Copy link
Contributor Author

f-odhiambo commented Nov 15, 2024

Some issues TBD

  1. Propagating drafts on devices. Deleting & Purging drafts
  2. I edit and save QR on device A and I have the same QR edit in progress on device B

@pld
Copy link
Member

pld commented Nov 15, 2024

isn't this replaced by #3611 ?

@Rkareko
Copy link
Contributor

Rkareko commented Nov 15, 2024

isn't this replaced by #3611 ?

@pld cc @f-odhiambo One of the assumptions for the Save Draft MVP is that You will only have 1 draft per question at a time . There is a bug with the current implentation whereby a new QR is generated every time a draft is saved. #3611 fixes this bug.

This ticket ( #3605 ) deals with deleting an existing draft. See this comment that details the UI/UX for this

@pld
Copy link
Member

pld commented Nov 15, 2024

OK, so this will delete the 1 draft that may exist?

@Rkareko
Copy link
Contributor

Rkareko commented Nov 15, 2024

OK, so this will delete the 1 draft that may exist?

Yes. That is the goal of this ticket.
However there are some issues that have come up that are detailed here and here

@Rkareko
Copy link
Contributor

Rkareko commented Nov 15, 2024

@f-odhiambo @dubdabasoduba The SDK has a ConflictResolver.
It has 2 conflict resolution strategies

  1. AcceptLocalConflictResolver - Accepts the local change and rejects the remote change.
  2. AcceptRemoteConflictResolver - Accepts the remote change and rejects the local change.

On FHIRCore AcceptLocalConflictResolver is set.

@pld pld changed the title [Feature] Delete all saved drafts of QuestionnaireResponses from the local SQLite database [Feature] Delete saved draft of QuestionnaireResponses when clicking "delete draft" Nov 15, 2024
@Rkareko
Copy link
Contributor

Rkareko commented Nov 21, 2024

@f-odhiambo @dubdabasoduba The SDK has a ConflictResolver. It has 2 conflict resolution strategies

1. `AcceptLocalConflictResolver` - Accepts the local change and rejects the remote change.

2. `AcceptRemoteConflictResolver` - Accepts the remote change and rejects the local change.

On FHIRCore AcceptLocalConflictResolver is set.

@f-odhiambo @dubdabasoduba As a follow up, the Changes to handle conflicts during download PR states that Basic implementation of ConflictResolver provided with AcceptLocalConflictResolver and AcceptRemoteConflictResolver that Developer Applications can use as-is. For advanced heuristics based conflict resolution, the Developer Applications may provide custom implementations.

@Rkareko
Copy link
Contributor

Rkareko commented Nov 22, 2024

Delete options

  1. Hard delete - Delete the QuestionnaireResponse from the app db and from the server as well. The issues with this is that currently there is no way to propagate the deletion to other devices which leads to the following issues :
  • Data inconsistency - A draft which has been deleted on device A still existing on another device B
  • Sync issues - If the draft on device B is updated locally, syncing those changes to the server will result in an error. Sync uploaded changes to an existing resource as patches, the server will reject the patch update since the resource has already been deleted on the server
  1. Soft delete - Update the QuestionnaireResponse status to stopped. This approach will suffer from the following issue since the nature of the save/delete draft functionality is that this can be done by multiple devices :
  • Data inconsistency - Device a can delete a draft, but this can be reverted by changes made on device be i.e device saves the from as a draft or completes it.

Options for propagating deletes across devices

A background worker would have to be implemented that checks for a resource indicating that a certain QuestionnaireResponse has been deleted and then applies the deletion on the said QuestionnaireResponse. If a draft is deleted on device A and changes synced to the server and then down to device B, the background worker would run on device B and delete the draft. The implementation for this background worker will be tracked on another ticket. The following are the options that have been identified so far for propagation of deletes across devices -

  1. Add an entry in one of the QuestionnaireResponse fields indicating that it has been deleted. This is only applicable for soft deletes. Problem found :
  • If device A soft deletes a QuestionnaireResponse then pushes the changes to the server. When device B syncs down while it has local changes for that QuestionnaireResponse then the server side changes will be lost since the conflict resolution strategy prioritizes local changes.
  1. Use a Flag to indicate that a QuestionnaireResponse has been deleted.
  • Hard delete - This will not work since deletion of the QuestionnaireResponse on the server will leave the Flag with a broken subject reference.
  • Soft delete - In theory this will work. The downside is that we are introducing an extra resource.
  1. Use an Audit trail - @dubdabasoduba please provide more context here.

Next steps

  1. Make a decision on whether hard or soft deletion will be utilized.
  2. Decide on the mode of propagating the deletion to other devices.

@f-odhiambo @dubdabasoduba @ndegwamartin

@Rkareko Rkareko linked a pull request Nov 22, 2024 that will close this issue
11 tasks
@ndegwamartin
Copy link
Contributor

I would go with a soft delete and then instead of introducing the new (Flag) resource use an additive field on the QR to track a deleted resource, for example the Meta tags. That way when the background worker runs to do deletes it will always find the flag unaltered by any other subsequent updates to the QR. We should confirm that the SDK also guarantees deletes of any unsynced local (patch) changes on the device that may be linked to the QR.

@Rkareko
Copy link
Contributor

Rkareko commented Nov 22, 2024

I would go with a soft delete and then instead of introducing the new (Flag) resource use an additive field on the QR to track a deleted resource, for example the Meta tags. That way when the background worker runs to do deletes it will always find the flag unaltered by any other subsequent updates to the QR. We should confirm that the SDK also guarantees deletes of any unsynced local (patch) changes on the device that may be linked to the QR.

@ndegwamartin This is option 1 as stated above. It fails due to the following

If device A soft deletes a QuestionnaireResponse then pushes the changes to the server. When device B syncs down while it has local changes for that QuestionnaireResponse then the server side changes will be lost since the conflict resolution strategy prioritizes local changes.

This means that the additive field (e.g metatag) is not added to the QuestionnaireResponse on device B during sync down.

@dubdabasoduba
Copy link
Member

@Rkareko I agree with @ndegwamartin Let's go with the soft delete

Based on the review we had today we can approach it as follows

  1. The extra resource (Flag or Audit trail) will tell the background worker which QuestionniareResponse is deleted.
  2. The worker should do the following
    • Update the QuestionniareResponse.status for the QR referenced on the extra resource (Flag or Audit trail) to stopped
      • This will then create an entry on the device's LocalIndexEntity table. This entry will be a PATCH, meaning the server will pick it.
      • This will also allow us to keep the changes made by that device and send them to the server.

Note: The option that uses the FLAG resources is faster to implement because we don't have to think much about other product relations. However, the Audit Trail would be the best for this kind of thing, and it gives us the opportunity to start the audit trailing work.

We can also a 3rd option which is to add the ability to define Conflict resolution strategies per resources. This is much more complicated and needs changes on the SDK.

@f-odhiambo f-odhiambo removed the Draft label Nov 22, 2024
@f-odhiambo
Copy link
Contributor Author

Based on the discussion we had on Nov 26th @Rkareko , let's use a flag and in the identifier we pass the QR ID to link them as an entry point for the background worker

Here is a sample Flag and FHIR Path expression to query it Flag.code.coding.code = 'delete_draft'

{
  "resourceType": "Flag",
  "id": "example",
  "identifier": [
    {
      "value": "QuestionnaireResponse/12345"
    }
  ],
  "status": "active",
  "code": {
    "coding": [
      {
        "system": "http://smartregister.org",
        "code": "delete_draft",
        "display": "Delete Draft"
      }
    ],
    "text": "QR Draft has been deleted"
  },
  "subject": {
    "reference": "Patient/example",
    "display": "Peter Patient"
  },
  "period": {
    "start": "2024-11-26",
    "end": "2024-11-26"
  }
}

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

Successfully merging a pull request may close this issue.

5 participants