feat: FORMS-1963 Hard deletion of form submissions #1760
Conversation
Introduces a new FormClassificationSettings component for configuring data retention and classification in form settings. Updates the form designer to support classificationType, retentionDays, and classificationDescription fields. Adds internationalization strings for the new settings in multiple languages. Backend changes include new audit and deletion logic, model/table updates, and related tests for form deletion and retention features.
jasonchung1871
changed the title
Refactors the hard deletion service for form submissions to support retention policy modes, force deletion, and targeted deletion by submission IDs. Adds detailed audit triggers for submission deletions, improves error handling, and introduces a diagnostic function for analyzing retention eligibility. Updates related controller, service, and unit tests to cover new logic and options.
The AuditActions object was removed from the constants module.
Extracted logic for fetching deleted submissions, retrieving submission details with form info, and retention policy checks into separate helper methods. This improves code readability, modularity, and maintainability in the hard deletion process for form submissions.
This comment was marked as outdated.
This comment was marked as outdated.
Updated the 'id' column in the 'form_audit' table from a UUID to an integer generated by default as identity. This change standardizes the primary key type for the audit table.
This comment has been minimized.
This comment has been minimized.
jasonchung1871
requested review from
RyanBirtch-aot,
nimya-aot,
revanth-banala and
usingtechnology
The cronjob routes were making requests to http://chefs-app-pr-1760:8080/pr-1760/api/v1/public/submission_deletion when it should have been https://chefs-dev.apps.silver.devops.gov.bc.ca/pr-1760/api/v1/public/submission_deletion or the proper namespace environment for the other environments. This was also incorrect for the submission reminders, but this should be resolved now.
…ithub.com/jasonchung1871/common-hosted-form-service into FORMS-1963-hard-deletion-form-submissions
|
This comment has been minimized.
This comment has been minimized.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Fixing sonarqube code smells. Prevented command injection by avoiding directly interpolating untrusted data into shell commands. Unexpected negated condition, flipped the conditional logic and used if (enabled) instead of if (!enabled). Replaced all instances of global with globalThis in the deletionService.spec.js test
Replaced global with globalThis
This comment has been minimized.
This comment has been minimized.
|
I left a looooooong comment on the JIRA ticket about making this a non-core feature and a pathway to externalize records management at a later stage without changing CHEFS. |
Retention logic is separated from the form in the backend. This also undoes much of the previous changes. The frontend needs to be updated to reflect these changes.
The cronjob path is updated to use the new route
This comment was marked as resolved.
This comment was marked as resolved.
|
Cypress tests failing on this PR. I assume you are already working on it |
Deleted classificationType, retentionDays, and classificationDescription from the initial form state in form.js as they are no longer used. Also removed an unnecessary blank line in service.js for code cleanliness.
This comment has been minimized.
This comment has been minimized.
Renamed the form deletion audit migration file for consistency and updated the CREATED_BY constant to 'form-deletion-audit'. Recreated the migration since it's not working in the live environment.
Replaces knex.raw('gen_random_uuid()') with uuidv4() for default UUID values in 'retention_policy' and 'scheduled_submission_deletion' tables. This change ensures UUIDs are generated using the uuidv4 function instead of relying on database-specific functions.
This comment has been minimized.
This comment has been minimized.
Controller methods now return 204 when no retention policy exists, instead of treating it as an error. Added UUID generation for new RetentionPolicy and ScheduledSubmissionDeletion records. Updated and expanded unit tests to cover these cases and ensure correct behavior.
This comment has been minimized.
This comment has been minimized.
Moved the currentUser middleware to be applied per-route instead of globally in recordsManagement/routes.js. Updated controller to use req.currentUser instead of req.user for user identification. Also refactored retentionClassification model to use shared 'stamps' schema for timestamp fields.
Updated the curl command in the cronjob to use HTTP POST instead of GET for the deletions process endpoint, aligning with expected API usage.
Replaces the 'user' property with 'currentUser' in the mock request object for the recordsManagement controller unit test to match updated controller expectations.
This comment has been minimized.
This comment has been minimized.
Added missing 'await' when starting a transaction in deleteSubmissionAndRelatedData to ensure the transaction is properly initialized before proceeding.
This comment has been minimized.
This comment has been minimized.
The submission_audited_func needs to return an OLD object upon deletion.
This comment has been minimized.
This comment has been minimized.
|
I like this. Looks great... slight issue is the Form Setting is kind of "sticky". Then created a second form, published, went to set the schedule and the settings were carried over from the previous form (Custom, 1 day). I did change it to 2 days. Same thing happened when I created a third form (initialized with Custom for 2 days). Could just be a frontend caching issue? Just grabbing the last retention schedule? I didn't look in the db or look at ids in the JSON for the form before saving. |
Eliminates retention policy configuration during new form creation in FormDesigner.vue. Also ensures formRetentionPolicy is reset to its initial state on 404 errors in recordsManagement store to prevent displaying outdated policy data.
|
|
Release 49ba53c deployed at https://chefs-dev.apps.silver.devops.gov.bc.ca/pr-1760 |
|
@jasonchung1871 Yesterday, I set up a one-day custom period for form deletion. I deleted my submission five minutes after submitting it. But it is still showing up on filter "Show deleted submissions". It should remove from there as well, right? |
This should be resolved now |
I'm going to wait a day but it should only actually delete every 24 hours. We go through a batch of deleted submissions every 24 hours and check if submissions are eligible for hard deletion, if so, then they are deleted. |
|
@jasonchung1871 Now messages removed from "show deleted messages". It may be calculating time in different way may be. |
3 participants
Description
Introduces a new FormClassificationSettings component for configuring data retention and classification in form settings. Updates the form designer to support classificationType, retentionDays, and classificationDescription fields. Adds internationalization strings for the new settings in multiple languages. Backend changes include new audit and deletion logic, model/table updates, and related tests for form deletion and retention features.
See: https://citz-do.atlassian.net/browse/FORMS-1963
Type of Change
feat (a new feature)
build (change in build system or dependencies)
docs (change to documentation)
test (add missing tests or correct existing tests)
Checklist
Further comments