Added a form deletion script for genie requests #35514
Conversation
dimagimon
added
the
Risk: High
| parser.add_argument('--resume_id', help='form ID to start at, within the CSV file') | ||
|
|
||
| def handle(self, domain, filename, resume_id=None, **options): | ||
| # expects the filename to have a CSV with a header containing "Deletion Status" and "Form ID" fields |
There was a problem hiding this comment.
Do we actually use the "Deletion Status" field? I only see the "Form ID" field being used. My thinking here is if there is a Deletion Status field with values under it, someone's going to expect those values to be used for something, and if we're not going to use them, then we should require the CSV not have that column.
There was a problem hiding this comment.
This is working with the CSV that Enveritas submitted to us. If I simplify this script to expect a sheet without that column, then I'd need to write another script to port the Envertias CSV over. Do you think that's something we should do long term?
There was a problem hiding this comment.
Addressed in 4fccff9. As mentioned offline, I think the process going forward should be that customers making this request provide us a CSV file that only contains a single column representing form ID. In this case, the customer provided an initial column called "Deletion Status" which only seems to contain ready_to_request. However, if one of the rows contained something like do_not_delete instead, we wouldn't want to have the responsibility for interpreting that field.
| form_ids = [row[INDEX_FORM_ID] for row in chunk] | ||
|
|
||
| try: | ||
| deleted_form_ids = set(XFormInstance.objects.hard_delete_forms( |
There was a problem hiding this comment.
I feel squeemish about hard-delete, though obviously it is what we're being asked to do. An alternate pattern would be to soft delete and mark for automatic hard-deletion in 90 days. But that is probably overkill for the present situation. Maybe what I would ask is just to rename the command to hard_delete_forms.py, since in some places delete means soft-delete, and making that name explicit will remove any kind of ambiguity for anyone reusing this script.
There was a problem hiding this comment.
Good point, I can definitely do that
There was a problem hiding this comment.
Addressed with 4fccff9
| parser.add_argument('--resume_id', help='form ID to start at, within the CSV file') | ||
|
|
||
| def handle(self, domain, filename, resume_id=None, **options): | ||
| # expects the filename to have a CSV with a header containing "Deletion Status" and "Form ID" fields |
There was a problem hiding this comment.
I think this comment is wrong now?
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Product Description
Technical Summary
Associated ticket: https://dimagi.atlassian.net/browse/SAAS-16071
This is a PR in response to a genie request to delete forms for a client. This PR creates a script to give us a command to run in the future.
The command is intended to be run as:
./manage.py delete_forms <domain> <path_to_csv_file>and we can record a log by piping its output to
tee.Safety Assurance
Safety story
Verified this locally, by creating new forms, and putting those form IDs (and bogus form IDs) inside a csv file.
Automated test coverage
Added new tests to verify the changes made to
hard_delete_formsQA Plan
No QA
Rollback instructions
Labels & Review