Skip to content

Commit 35a6f0f

Browse files
authored
Return specific Problem when form version in submission not found (getodk#766)
* Return specific Problem when form version in submission not found * Simplifying code
1 parent c90025b commit 35a6f0f

File tree

3 files changed

+27
-4
lines changed

3 files changed

+27
-4
lines changed

lib/resources/submissions.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,9 @@ module.exports = (service, endpoint) => {
120120
openRosaSubmission('/projects/:projectId/submission', false, (auth, { projectId }, xmlFormId, Forms, version) =>
121121
Forms.getByProjectAndXmlFormId(projectId, xmlFormId, false, version)
122122
.then(getOrNotFound)
123-
.then(ensureDef)
123+
// This replaces ensureDef(form). If the form was found with the project ID and form ID
124+
// constraints but the def was not found, that suggest the problem was with the version.
125+
.then(rejectIf(((form) => (form.def.id == null)), () => Problem.user.formVersionNotFound({ formVersion: version })))
124126
.then(rejectIf(((form) => !form.acceptsSubmissions()), noargs(Problem.user.notAcceptingSubmissions)))
125127
.then((form) => auth.canOrReject('submission.create', form)));
126128

@@ -206,7 +208,9 @@ module.exports = (service, endpoint) => {
206208
restSubmission('/projects/:projectId/forms/:formId', false, ({ projectId, formId }, Forms, version) =>
207209
Forms.getByProjectAndXmlFormId(projectId, formId, false, version) // TODO: okay so this is exactly the same as the func above..
208210
.then(getOrNotFound)
209-
.then(ensureDef)
211+
// This replaces ensureDef(form). If the form was found with the project ID and form ID
212+
// constraints but the def was not found, that suggest the problem was with the version.
213+
.then(rejectIf(((form) => (form.def.id == null)), () => Problem.user.formVersionNotFound({ formVersion: version })))
210214
.then(rejectIf(
211215
(form) => !form.acceptsSubmissions(),
212216
() => Problem.user.notAcceptingSubmissions()

lib/util/problem.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ const problems = {
136136
// deprecated id does not exist.
137137
deprecatedIdNotFound: problem(404.5, ({ deprecatedId }) => `You tried to update an existing submission, but we can't find that submission to update (${deprecatedId})`),
138138

139+
// form version in submission does not exist
140+
formVersionNotFound: problem(404.6, ({ formVersion }) => `The form version specified in this submission '${formVersion}' does not exist.`),
141+
139142
// { allowed: [ acceptable formats ], got }
140143
unacceptableFormat: problem(406.1, ({ allowed }) => `Requested format not acceptable; this resource allows: ${allowed.join()}.`),
141144

test/integration/api/submissions.js

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ describe('api: /submission', () => {
8282
asAlice.post('/v1/projects/1/submission')
8383
.set('X-OpenRosa-Version', '1.0')
8484
.attach('xml_submission_file', Buffer.from('<data id="simple" version="-1"><orx:meta><orx:instanceID>one</orx:instanceID></orx:meta></data>'), { filename: 'data.xml' })
85-
.expect(404))));
85+
.expect(404)
86+
.then(({ text }) => {
87+
text.should.match(/The form version specified in this submission/);
88+
}))));
8689

8790
it('should save the submission to the appropriate form', testService((service) =>
8891
service.login('alice', (asAlice) =>
@@ -932,7 +935,11 @@ describe('api: /forms/:id/submissions', () => {
932935
asAlice.post('/v1/projects/1/forms/simple/submissions')
933936
.send(Buffer.from('<data id="simple" version="-1"><meta><instanceID>one</instanceID></meta></data>'))
934937
.set('Content-Type', 'text/xml')
935-
.expect(404))));
938+
.expect(404)
939+
.then(({ body }) => {
940+
body.code.should.equal(404.6);
941+
body.message.should.match(/The form version specified in this submission/);
942+
}))));
936943

937944
it('should submit if all details are provided', testService((service) =>
938945
service.login('alice', (asAlice) =>
@@ -1089,6 +1096,15 @@ describe('api: /forms/:id/submissions', () => {
10891096
asAlice.get('/v1/projects/1/forms/simple/submissions/one').expect(404),
10901097
asAlice.get('/v1/projects/1/forms/simple/draft/submissions/one').expect(200)
10911098
])))));
1099+
1100+
it('should accept even if the version in the submission is wrong', testService((service) =>
1101+
service.login('alice', (asAlice) =>
1102+
asAlice.post('/v1/projects/1/forms/simple/draft')
1103+
.expect(200)
1104+
.then(() => asAlice.post('/v1/projects/1/forms/simple/draft/submissions')
1105+
.send(Buffer.from('<data id="simple" version="-1"><meta><instanceID>one</instanceID></meta></data>'))
1106+
.set('Content-Type', 'application/xml')
1107+
.expect(200)))));
10921108
});
10931109

10941110
describe('/:instanceId PUT', () => {

0 commit comments

Comments
 (0)