Recover deleted draft submissions by soft-deleting them when updating the form draft #1299
Conversation
| @@ -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 | |||
There was a problem hiding this comment.
Now that this endpoint will no longer result in a new form draft that's ready to be purged, I think we can remove this line. It'd be a nice way to simplify the endpoint. I also think it'd make things clearer if there was only one thing that called clearUnneededDrafts() (namely, the purge task).
There was a problem hiding this comment.
One possible reason to leave it here is when there are no submissions, the draft def can be cleaned up right away. There are some tests in "purging form fields of unneeded drafts" (in api: /projects/:id/forms (drafts)) that are checking for these defs to be purged immediately. I could change or remove those tests, though.
There was a problem hiding this comment.
Oh I see, interesting. No strong feelings, either way!
|
I've added tests for the 4 main scenarios in the issue, and I've also added some tests showing, experimentally, what it would be like to bring some of these deleted draft submissions back. It gets a little complicated depending on the scenario. I also had to change the DB submission uniqueness constraint to allow for draft submissions that have been deleted to be able to reuse instance IDs. I am not 100% confident on the uniqueness constraint AND the trigger that exists to check instance ID uniqueness. |
Co-authored-by: Matthew White <[email protected]>
ktuite
force-pushed
the
ktuite/recover_deleted_draft_forms
branch
from
ac2552a to
9dd22ac
Compare
| .then(({ body }) => { | ||
| body.length.should.equal(2); | ||
| }); | ||
| })); | ||
| }); |
There was a problem hiding this comment.
What happens when currentDefId is the same as draftDefId?
ktuite
force-pushed
the
ktuite/recover_deleted_draft_forms
branch
from
90ab687 to
2e76ed0
Compare
Closes getodk/central#749
WIP while adding more tests and thinking about encrypted forms.
What has been done to verify that this works as intended?
Why is this the best possible solution? Were any other approaches considered?
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Does this change require updates to the API documentation? If so, please update docs/api.yaml as part of this PR.
Before submitting this PR, please make sure you have:
make testand confirmed all checks still pass OR confirm CircleCI build passes