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

Add remaining functionality for entity deletion #710

Closed
matthew-white opened this issue Sep 18, 2024 · 9 comments · Fixed by getodk/central-frontend#1119
Closed

Add remaining functionality for entity deletion #710

matthew-white opened this issue Sep 18, 2024 · 9 comments · Fixed by getodk/central-frontend#1119
Assignees
Labels
backend Requires a change to the API server enhancement New feature or behavior entities Multiple Encounter workflows frontend Requires a change to the UI needs design review Needs verification from designer needs testing Needs manual testing

Comments

@matthew-white
Copy link
Member

matthew-white commented Sep 18, 2024

It is currently possible to delete an entity either via the API or from Frontend. However, we don't ever purge entities. There also isn't a way to list deleted entities via OData or to restore/undelete a deleted entity. Eventually, we want both entity deletion and submission deletion to match all the functionality we offer around form deletion. Here is the functionality as things stand with the release of v2024.2:

  Form deletion Submission deletion Entity deletion
Delete via API
Delete from individual page #709
Delete from table N/A #709
Purge after fixed interval ⚪️
List deleted via REST API ⚪️
List deleted via OData N/A #709 ⚪️
Restore via API ⚪️
Restore from Frontend #709 ⚪️

This issue is to complete what's remaining under the "Entity deletion" column. There are related release criteria here.

Any existing deleted entities in the database should be immediately purged. That is, they shouldn't show up anywhere in Frontend or linger for 30 days. I think we did something similar when we first rolled out form purging.


As a technical note, one thing we've talked about is the need to tombstone (keep a record of) purged entities. We do so for purged forms via the actees table. We don't tombstone purged submissions, but we can retrieve all the information we need for them from the audit log. One specific reason we've talked about tombstoning purged entities has to do with #668. Basically, we never want the same UUID to refer to two different entities, even if one has been purged. If that were possible, then we could have a scenario like this:

  • An entity is created offline, then sent to Central.
  • The entity is deleted and purged in Central. As soon as the device that created the entity checks its status via the integrity URL, it will see that the entity has been deleted and can be safely removed from the device.
  • But before the device has a chance to check that, another device or some other process creates a different entity with the same UUID as the first one. Now when the first device checks, it won't see that the entity has been deleted. That could be a problem if it results in the device never removing the entity or continuing to use its local version instead of the new one that's been created on the server.

To prevent this sort of case, when an entity is created, we should check that its UUID has never been used, even for a previously purged entity. Right now, the uniqueness constraint on the entities table does all that we need, but we'll probably need something more once we work on this issue.

@matthew-white matthew-white added enhancement New feature or behavior backend Requires a change to the API server frontend Requires a change to the UI needs testing Needs manual testing needs design review Needs verification from designer entities Multiple Encounter workflows and removed needs testing Needs manual testing needs design review Needs verification from designer labels Sep 18, 2024
@matthew-white
Copy link
Member Author

There also isn't a way to list deleted entities

I'm realizing that this isn't totally accurate. It looks like it is possible to list deleted entities via the REST API (…/entities?deleted=true). However, we also need to be able to list deleted entities via OData. I'll update the issue description to make this distinction.

One thing to note is that unlike entities, it isn't possible to list deleted submissions via the REST API. It's only possible via OData. I don't think we need to add support for that, but I wanted to note that that will be one piece of asymmetry between submission deletion and entity deletion.

@sadiqkhoja
Copy link
Contributor

@getodk/testers this issue is ready for testing.

@srujner
Copy link

srujner commented Feb 3, 2025

@sadiqkhoja @matthew-white
Hey, we need some more information here, because this PR has been started a long time ago and probably some things changed during adding remaining functionalities.
What exactly should be tested by us and is it possible to verify it on staging?
Should we also test API and OData?

@sadiqkhoja
Copy link
Contributor

sadiqkhoja commented Feb 3, 2025

Following changes are done on the UI:

  • You can now view deleted Entities
  • You can undelete Entities

Backend:

  • Deleted entities are purged (delete forever) after 30 days or you can forcefully purge them from CLI. If you want we can configure the purge duration to 1 day, if this is how test Submissions were performed. Only caveat here is that it will purge all deleted items (forms, submission, entities).

If you tested undelete(restore) API and changes related to delete in Odata feed for Submission then I would suggest to test them for Entities as well.

@srujner
Copy link

srujner commented Feb 6, 2025

I have one case where I need to make sure.
I Used Voter registration and Card distribution forms.

I have project on Central and I scanned it on Collect.
I've created two voters using Voter registration form: Test 1 and Test 2
On Central in Entities tab I've deleted Test 1.
On Collect I'm still able to choose the Test 1 Voter in the Card distribution form until I do a form upgrade and send it correctly to Central.

And now I'm thinking of all the cases where someone may have an old form state with an entity, will have the automatic download of a new version of the form disabled, and will be able to send a form with data that was deleted some time ago.

Is there some solution for this or are you aware of that in this is fully expected?
I know that this topic was already discussed some time ago but I couldn't find the conclusions.

@srujner
Copy link

srujner commented Feb 6, 2025

Hey, there are small differences in UI between deleting entities and submissions:

  1. Text in the "deleted at" column is not red
  2. There is no information that deleted entities are deleted after 30 days in the Trash

Image

@srujner
Copy link

srujner commented Feb 7, 2025

Deleted entities are purged (delete forever) after 30 days or you can forcefully purge them from CLI. If you want we can configure the purge duration to 1 day, if this is how test Submissions were performed. Only caveat here is that it will purge all deleted items (forms, submission, entities).

Last time we tested purge using CLI commands provided by Matt, could you send me ones for purging deleted entities? (can be on slack)

If you tested undelete(restore) API and changes related to delete in Odata feed for Submission then I would suggest to test them for Entities as well.

We didn't test API and OData feed

@sadiqkhoja
Copy link
Contributor

sadiqkhoja commented Feb 13, 2025

For the case you described about deleted entity available on Collect, we are implementing #668 and corresponding change in Collect.

Good catch about the missing purging instruction and red deletedAt column. I will fix it in a separate PR.

I have sent you the CLI command on the slack.

@srujner
Copy link

srujner commented Feb 19, 2025

I also see that the messages in Server Audit logs are probably not formatted correctly?
Is this in the scope of this PR?

Image

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 frontend Requires a change to the UI needs design review Needs verification from designer needs testing Needs manual testing
Projects
Status: ✅ done
Development

Successfully merging a pull request may close this issue.

3 participants