Skip to content

Commit

Permalink
Improves #794: Update error message on duplicate submission when orig…
Browse files Browse the repository at this point in the history
…inal is soft deleted (#1316)

* Improves #794: Update error message on duplicate submission when original is soft deleted

* Update API docs

* use Problem.translate
  • Loading branch information
sadiqkhoja authored Dec 5, 2024
1 parent 217a111 commit fbcdf07
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 3 deletions.
2 changes: 1 addition & 1 deletion docs/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9642,7 +9642,7 @@ paths:

* Central supports the `HEAD` request preflighting recommended by the specification, but does not require it. Because our supported authentication methods do not follow the try/retry pattern, only preflight your request if you want to read the `X-OpenRosa-Accept-Content-Length` header or are concerned about the other issues listed in the standards document, like proxies.

* As stated in the standards document, it is possible to submit multimedia attachments with the `Submission` across multiple `POST` requests to this API. _However_, we impose the additional restriction that the Submission XML (`xml_submission_file`) _may not change_ between requests. If Central sees a Submission with an `instanceId` it already knows about but the XML has changed in any way, it will respond with a `409 Conflict` error and reject the submission.
* As stated in the standards document, it is possible to submit multimedia attachments with the `Submission` across multiple `POST` requests to this API. _However_, we impose the additional restriction that the Submission XML (`xml_submission_file`) _may not change_ between requests. If Central sees a Submission with an `instanceId` it already knows about but the XML has changed in any way, it will respond with a `409 Conflict` error and reject the submission. Additionally, a `409 Conflict` error is returned if the Submission has been deleted in Central while attempting to send further POST requests.

* Central will never return a `202` in any response from this API.

Expand Down
6 changes: 5 additions & 1 deletion lib/resources/submissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,11 @@ module.exports = (service, endpoint) => {
return Promise.all([
Submissions.createNew(partial, form, query.deviceID, userAgent),
Forms.getBinaryFields(form.def.id)
]).then(([ saved, binaryFields ]) => SubmissionAttachments.create(saved, form, binaryFields, files));
])
.then(([ saved, binaryFields ]) => SubmissionAttachments.create(saved, form, binaryFields, files))
// This is only true when submission is soft deleted, if it is hard deleted then there will no error
// and if it is not deleted then `extant` will be not null and this block will not execute.
.catch(Problem.translate(Problem.user.uniquenessViolation, noargs(Problem.user.duplicateSubmission)));
});
})
.then(always(createdMessage({ message: 'full submission upload was successful!' }))))));
Expand Down
4 changes: 3 additions & 1 deletion lib/util/problem.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ const problems = {

entityVersionConflict: problem(409.15, ({ current, provided }) => `Current version of the Entity is '${current}' and you provided '${provided}'. Please correct the version number or pass '?force=true' in the URL to forcefully update the Entity.`),

datasetNameConflict: problem(409.16, ({ current, provided }) => `A dataset named '${current}' exists and you provided '${provided}' with the same name but different capitalization.`)
datasetNameConflict: problem(409.16, ({ current, provided }) => `A dataset named '${current}' exists and you provided '${provided}' with the same name but different capitalization.`),

duplicateSubmission: problem(409.18, () => `This submission has been deleted. You may not resubmit it.`)

},
internal: {
Expand Down
34 changes: 34 additions & 0 deletions test/integration/api/submissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,26 @@ describe('api: /submission', () => {
.then(({ body }) => { body.toString('utf8').should.equal('this is test file one'); })
])))));

it('should reject resubmission of soft-deleted submission', testService(async (service) => {
const asAlice = await service.login('alice');

await asAlice.post('/v1/projects/1/submission')
.set('X-OpenRosa-Version', '1.0')
.attach('xml_submission_file', Buffer.from(testData.instances.simple.one), { filename: 'data.xml' })
.expect(201);

await asAlice.delete('/v1/projects/1/forms/simple/submissions/one')
.expect(200);

await asAlice.post('/v1/projects/1/submission')
.set('X-OpenRosa-Version', '1.0')
.attach('xml_submission_file', Buffer.from(testData.instances.simple.one), { filename: 'data.xml' })
.expect(409)
.then(({ text }) => {
text.should.match(/This submission has been deleted. You may not resubmit it./);
});
}));

context('versioning', () => {
it('should reject if the deprecatedId is not known', testService((service) =>
service.login('alice', (asAlice) =>
Expand Down Expand Up @@ -1153,6 +1173,20 @@ describe('api: /forms/:id/submissions', () => {
]);
})
])))));

it('should reject duplicate submissions', testService(async (service) => {
const asAlice = await service.login('alice');

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

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

describe('[draft] POST', () => {
Expand Down

0 comments on commit fbcdf07

Please sign in to comment.