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 task to permanently delete soft-deleted submissions #2446

Merged
merged 8 commits into from
Jul 3, 2023

Conversation

KipSigei
Copy link
Contributor

Changes / Features implemented

  • Add task to delete soft-deleted submissions after a certain period

Steps taken to verify this change does what is intended

  • Added tests

Side effects of implementing this change

  • Soft deleted submissions will be permanently deleted after a certain configurable period

Before submitting this PR for review, please make sure you have:

  • Included tests
  • Updated documentation

Closes #

@KipSigei KipSigei force-pushed the automated-submission-deletions branch 4 times, most recently from 6bbaf9a to 653c14d Compare June 27, 2023 08:25
@KipSigei KipSigei marked this pull request as ready for review June 27, 2023 08:25
@KipSigei KipSigei force-pushed the automated-submission-deletions branch from 653c14d to 0e8ae54 Compare June 27, 2023 12:52
Copy link
Member

@ukanga ukanga left a comment

Choose a reason for hiding this comment

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

Should we also check that the records have been removed from the InstanceHistory?
Also, could we verify attachments have been removed from storage locations?

"""
Task to periodically delete soft deleted submissions from db
"""
submissions_lifespan = getattr(settings, "INACTIVE_SUBMISSIONS_LIFESPAN", 365)
Copy link
Member

Choose a reason for hiding this comment

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

This line seems to define a hard limit of 365 days if the setting does not exist. Could we take the option of simply not taking any action if the setting is not defined/set? That will maintain the status quo as is now instead of on deployment all deleted items older than a year being removed. For some installations, this may not be ideal and prior communication may be necessary.

onadata/apps/api/tests/views/test_tasks.py Outdated Show resolved Hide resolved
onadata/apps/api/tests/views/test_tasks.py Outdated Show resolved Hide resolved
@KipSigei KipSigei force-pushed the automated-submission-deletions branch from 7165fe6 to d253557 Compare June 30, 2023 12:46
@KipSigei KipSigei requested a review from ukanga June 30, 2023 12:50
@KipSigei KipSigei force-pushed the automated-submission-deletions branch from d253557 to a79d95a Compare June 30, 2023 12:56
FrankApiyo
FrankApiyo previously approved these changes Jun 30, 2023
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
@KipSigei KipSigei force-pushed the automated-submission-deletions branch from df6a028 to 86055b4 Compare July 3, 2023 06:47
Comment on lines 126 to 127
Q(deleted_at__isnull=False) | Q(deleted_at__gte=time_threshold),
date_created__lte=time_threshold,
Copy link
Member

Choose a reason for hiding this comment

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

Should this check be based only on the time since the record was last deleted hence using the deleted_at field for checks and not when the record was created?

@KipSigei KipSigei force-pushed the automated-submission-deletions branch 2 times, most recently from 82055d3 to d172194 Compare July 3, 2023 09:15
ukanga
ukanga previously approved these changes Jul 3, 2023
@KipSigei KipSigei force-pushed the automated-submission-deletions branch from 713b55a to 00fe783 Compare July 3, 2023 12:43
@KipSigei KipSigei enabled auto-merge July 3, 2023 13:26
@KipSigei KipSigei disabled auto-merge July 3, 2023 13:26
@KipSigei KipSigei merged commit 690b83e into main Jul 3, 2023
@KipSigei KipSigei deleted the automated-submission-deletions branch July 3, 2023 13:26
@KipSigei KipSigei mentioned this pull request Jul 3, 2023
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.

3 participants