Skip to content

Commit

Permalink
wip: soft-delete draft submissions when drafts get replaced
Browse files Browse the repository at this point in the history
  • Loading branch information
ktuite committed Nov 16, 2024
1 parent afa0397 commit 31ce9ce
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 6 deletions.
8 changes: 7 additions & 1 deletion lib/model/query/forms.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,14 +485,20 @@ const _draftFilter = (form, project) =>
? sql`and forms."projectId" = ${project.id}`
: sql``));

// NOTE: copypasta alert! The following SQL also appears in 20220209-01-purge-unneeded-drafts.js
// NOTE: copypasta alert! Similar SQL also appears in 20220209-01-purge-unneeded-drafts.js
// Purges draft form defs that are not referenced by any forms AND have no associated submission defs.
const clearUnneededDrafts = (form = null, project = null) => ({ run }) =>
run(sql`
DELETE FROM form_defs
USING forms
WHERE form_defs."formId" = forms.id
AND form_defs."publishedAt" IS NULL
AND form_defs.id IS DISTINCT FROM forms."draftDefId"
AND NOT EXISTS (
SELECT 1
FROM submission_defs
WHERE submission_defs."formDefId" = form_defs.id
)
${_draftFilter(form, project)}`)
.then(() => run(sql`
DELETE FROM form_schemas
Expand Down
5 changes: 4 additions & 1 deletion lib/model/query/submissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ const clearDraftSubmissions = (formId) => ({ run }) =>
const clearDraftSubmissionsForProject = (projectId) => ({ run }) =>
run(sql`DELETE FROM submissions USING forms WHERE submissions."formId" = forms.id AND forms."projectId" = ${projectId} AND submissions.draft=true`);

const softDeleteDraftSubmissions = (formId) => ({ run }) =>
run(sql`UPDATE submissions SET "deletedAt"=now() WHERE "formId"=${formId} AND "draft"=true AND "deletedAt" IS NULL`);

////////////////////////////////////////////////////////////////////////////////
// SELECT-MULTIPLE VALUES

Expand Down Expand Up @@ -475,7 +478,7 @@ select count(*) from deleted_submissions`);

module.exports = {
createNew, createVersion,
update, del, restore, purge, clearDraftSubmissions, clearDraftSubmissionsForProject,
update, del, restore, purge, clearDraftSubmissions, clearDraftSubmissionsForProject, softDeleteDraftSubmissions,
setSelectMultipleValues, getSelectMultipleValuesForExport,
getByIdsWithDef, getSubAndDefById,
getByIds, getAllForFormByIds, getById, countByFormId, verifyVersion,
Expand Down
6 changes: 3 additions & 3 deletions lib/resources/forms.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ module.exports = (service, endpoint) => {
: getPartial(Forms, request, project, Keys)))
.then((partial) => Promise.all([
Forms.createVersion(partial, form, false),
Submissions.clearDraftSubmissions(form.id)
Submissions.softDeleteDraftSubmissions(form.id)
]))
.then(() => Forms.clearUnneededDrafts(form))) // remove drafts made obsolete by new draft
.then(success)));
Expand All @@ -162,7 +162,7 @@ module.exports = (service, endpoint) => {
.then(() => Forms.getByProjectAndXmlFormId(params.projectId, params.id, false, Form.DraftVersion))
.then(getOrNotFound)
: resolve(form)))
.then(((form) => Promise.all([ Forms.publish(form), Submissions.clearDraftSubmissions(form.id) ])))
.then(((form) => Promise.all([ Forms.publish(form), Submissions.softDeleteDraftSubmissions(form.id) ])))
.then(success)));

// Entity/Dataset-specific endpoint that is used to show how publishing
Expand Down Expand Up @@ -244,7 +244,7 @@ module.exports = (service, endpoint) => {
.then(rejectIf(((form) => form.draftDefId == null), noargs(Problem.user.notFound)))
.then((form) => Promise.all([
Forms.clearDraft(form).then(() => Forms.clearUnneededDrafts(form)),
Submissions.clearDraftSubmissions(form.id),
Submissions.softDeleteDraftSubmissions(form.id),
Audits.log(auth.actor, 'form.update.draft.delete', form, { oldDraftDefId: form.draftDefId })
]))
.then(success)));
Expand Down
5 changes: 5 additions & 0 deletions lib/task/purge.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@ const purgeTask = task.withContainer((container) => async (options = {}) => {
const count = await Forms.purge(options.force, options.formId, options.projectId, options.xmlFormId);
return `Forms purged: ${count}`;
} else {
// Purge both Forms and Submissions according to options
const formCount = await Forms.purge(options.force, options.formId, options.projectId, options.xmlFormId);
const submissionCount = await Submissions.purge(options.force, options.projectId, options.xmlFormId, options.instanceId);

// Related to form purging: deletes draft form defs that are not in use by any form and have no associated submission defs
await Forms.clearUnneededDrafts();

return `Forms purged: ${formCount}, Submissions purged: ${submissionCount}`;
}
} catch (error) {
Expand Down
62 changes: 62 additions & 0 deletions test/integration/api/forms/draft.js
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,68 @@ describe('api: /projects/:id/forms (drafts)', () => {
.then((counts) => counts.should.eql([ 1, 4, 3 ])))));
});
});

describe('preserving submissions from old or deleted drafts', () => {
it('should soft-delete submissions of undeeded draft when a new version is uploaded', testService(async (service, { oneFirst }) => {
const asAlice = await service.login('alice');

await asAlice.post('/v1/projects/1/forms/simple/draft')
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty"'))
.set('Content-Type', 'application/xml')
.expect(200);

await asAlice.post('/v1/projects/1/forms/simple/draft/submissions')
.send(testData.instances.simple.one)
.set('Content-Type', 'text/xml')
.expect(200);

let subs = await oneFirst(sql`select count(*) from submissions`);
subs.should.equal(1);

await asAlice.post('/v1/projects/1/forms/simple/draft')
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty2"'))
.set('Content-Type', 'application/xml')
.expect(200);

subs = await oneFirst(sql`select count(*) from submissions`);
subs.should.equal(1);

const fds = await oneFirst(sql`select count(*) from form_defs as fd join forms as f on fd."formId" = f.id where f."xmlFormId"='simple'`);
fds.should.equal(3); // Old draft has not been deleted. Count also includes published and new draft.
}));
});

it('should purge old draft submissions after 30 days', testService(async (service, { oneFirst, run, Forms, Submissions }) => {
const asAlice = await service.login('alice');

await asAlice.post('/v1/projects/1/forms/simple/draft')
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty"'))
.set('Content-Type', 'application/xml')
.expect(200);

await asAlice.post('/v1/projects/1/forms/simple/draft/submissions')
.send(testData.instances.simple.one)
.set('Content-Type', 'text/xml')
.expect(200);

let subs = await oneFirst(sql`select count(*) from submissions`);
subs.should.equal(1);

await asAlice.post('/v1/projects/1/forms/simple/draft')
.send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty2"'))
.set('Content-Type', 'application/xml')
.expect(200);

await run(sql`update submissions set "deletedAt" = '1999-1-1T00:00:00Z' where "deletedAt" is not null`);
await Submissions.purge();
await Forms.clearUnneededDrafts();

subs = await oneFirst(sql`select count(*) from submissions`);
subs.should.equal(0);

const fds = await oneFirst(sql`select count(*) from form_defs as fd join forms as f on fd."formId" = f.id where f."xmlFormId"='simple'`);
fds.should.equal(2); // Old draft has now been deleted. Count also includes published and new draft.
}));
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/integration/api/submissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3294,7 +3294,7 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
.expect(200)
.then(({ body }) => { body.should.eql([]); })))));

it('should not carry over draft keys when a draft is replaced', testService((service) =>
it.only('should not carry over draft keys when a draft is replaced', testService((service) =>

Check failure on line 3297 in test/integration/api/submissions.js

View workflow job for this annotation

GitHub Actions / standard-tests

it.only not permitted
service.login('alice', (asAlice) =>
asAlice.post('/v1/projects/1/forms?publish=true')
.send(testData.forms.encrypted)
Expand Down

0 comments on commit 31ce9ce

Please sign in to comment.