Update Submissions response (getodk#783)

* include currentVersion in submission response

* updated API docs

* add submitter details for current version if extended data is requested
read logical submission from database on submission update

* minor changes in API doc

Author: sadiqkhoja

docs/api.md

@@ -32,6 +32,11 @@ Finally, **system information and configuration** is available via a set of spec
 
 Here major and breaking changes to the API are listed by version.
 
+### ODK Central v2023.2
+
+**Changed**:
+- The response of `GET`, `POST`, `PUT` and `PATCH` methods of [Submissions](#reference/submissions/listing-all-submissions-on-a-form) endpoint has been updated to include metadata of the `currentVersion` of the Submission. 
+
 ### ODK Central v2023.1
 
 **Added**:
@@ -2105,6 +2110,8 @@ Like how `Form`s are addressed by their XML `formId`, individual `Submission`s a
 
 As of version 1.4, a `deviceId` and `userAgent` will also be returned with each submission. The client device may transmit these extra metadata when the data is submitted. If it does, those fields will be recognized and returned here for reference. Here, only the initial `deviceId` and `userAgent` will be reported. If you wish to see these metadata for any submission edits, including the most recent edit, you will need to [list the versions](/reference/submissions/submission-versions/listing-versions).
 
+As of version 2023.2, this API returns `currentVersion` that contains metadata of the most recent version of the Submission.
+
 This endpoint supports retrieving extended metadata; provide a header `X-Extended-Metadata: true` to return a `submitter` data object alongside the `submitterId` Actor ID reference.
 
 + Parameters
@@ -2121,6 +2128,11 @@ This endpoint supports retrieving extended metadata; provide a header `X-Extende
 
     + Attributes (Extended Submission)
 
++ Response 301 (text/html)
+    Returns 301 with URL of the specific Submission version if `instanceId` of a Submission edit is provided
+
+    + Attributes
+
 + Response 403 (application/json)
     + Attributes (Error 403)
 
@@ -2574,12 +2586,12 @@ This endpoint supports retrieving extended metadata; provide a header `X-Extende
 + Response 200 (application/json)
     This is the standard response, if Extended Metadata is not requested:
 
-    + Attributes (array[Submission])
+    + Attributes (array[SubmissionVersion])
 
 + Response 200 (application/json; extended)
     This is the Extended Metadata response, if requested via the appropriate header:
 
-    + Attributes (array[Extended Submission])
+    + Attributes (array[Extended SubmissionVersion])
 
 + Response 403 (application/json)
     + Attributes (Error 403)
@@ -2596,12 +2608,12 @@ This endpoint supports retrieving extended metadata; provide a header `X-Extende
 + Response 200 (application/json)
     This is the standard response, if Extended Metadata is not requested:
 
-    + Attributes (Submission)
+    + Attributes (SubmissionVersion)
 
 + Response 200 (application/json; extended)
     This is the Extended Metadata response, if requested via the appropriate header:
 
-    + Attributes (Extended Submission)
+    + Attributes (Extended SubmissionVersion)
 
 + Response 403 (application/json)
     + Attributes (Error 403)
@@ -4237,17 +4249,29 @@ These are in alphabetic order, with the exception that the `Extended` versions o
 
 ## Submission (object)
 + instanceId: `uuid:85cb9aff-005e-4edd-9739-dc9c1a829c44` (string, required) - The `instanceId` of the `Submission`, given by the Submission XML.
-+ instanceName: `village third house` (string, optional) - The `instanceName`, if any, given by the Submission XML in the metadata section.
-+ submitterId: `23` (number, required) - The ID of the `Actor` (`App User`, `User`, or `Public Link`) that submitted this `Submission`.
-+ deviceId: `imei:123456` (string, optional) - The self-identified `deviceId` of the device that collected the data, sent by it upon submission to the server. On overall ("logical") submission requests, the initial submission `deviceId` will be returned here. For specific version listings of a submission, the value associated with the submission of that particular version will be given.
-+ userAgent: `Enketo/3.0.4` (string, optional) - The self-identified `userAgent` of the device that collected the data, sent by it upon submission to the server.
++ submitterId: `23` (number, required) - The ID of the `Actor` (`App User`, `User`, or `Public Link`) that originally submitted this `Submission`.
++ deviceId: `imei:123456` (string, optional) - The self-identified `deviceId` of the device that collected the data, sent by it upon submission to the server. The initial submission `deviceId` will be returned here. 
++ userAgent: `Enketo/3.0.4` (string, optional) - The self-identified `userAgent` of the device that collected the data, sent by it upon submission to the server. The initial submission `userAgent` will be returned here. 
 + reviewState: `approved` (Submission Review State, optional) - The current review state of the submission.
 + createdAt: `2018-01-19T23:58:03.395Z` (string, required) - ISO date format. The time that the server received the Submission.
 + updatedAt: `2018-03-21T12:45:02.312Z` (string, optional) - ISO date format. `null` when the Submission is first created, then updated when the Submission's XML data or metadata is updated.
++ currentVersion: (SubmissionVersion) - The current version of the `Submission`.
+
+## SubmissionVersion (object)
++ instanceId: `uuid:85cb9aff-005e-4edd-9739-dc9c1a829c44` (string, required) - The `instanceId` of the `Submission` version, given by the Submission XML.
++ instanceName: `village third house` (string, optional) - The `instanceName`, if any, given by the Submission XML in the metadata section.
++ submitterId: `23` (number, required) - The ID of the `Actor` (`App User`, `User`, or `Public Link`) that submitted this `Submission` version.
++ deviceId: `imei:123456` (string, optional) - The self-identified `deviceId` of the device that submitted the `Submission` version. 
++ userAgent: `Enketo/3.0.4` (string, optional) - The self-identified `userAgent` of the device that submitted the `Submission` version.
++ createdAt: `2018-01-19T23:58:03.395Z` (string, required) - ISO date format. The time that the server received the `Submission` version.
++ current: `true` (boolean, required) - Whether the version is current or not.
 
 ## Extended Submission (Submission)
 + submitter (Actor, required) - The full details of the `Actor` that submitted this `Submission`.
-+ formVersion: `1.0` (string, optional) - The version of the form the submission was initially created against. Only returned with specific Submission Version requests.
+
+## Extended SubmissionVersion (SubmissionVersion)
++ submitter (Actor, required) - The full details of the `Actor` that submitted this version of the `Submission`.
++ formVersion: `1.0` (string, optional) - The version of the form the submission version was created against. Only returned with specific Submission Version requests.
 
 ## Review State Counts (object)
 + received: `3` (number, required) - The number of submissions receieved with no other review state.

lib/model/frames/submission.js

@@ -24,7 +24,9 @@ class Submission extends Frame.define(
   'instanceId', readable,               'submitterId',  readable,
   'deviceId',   readable,               'createdAt',    readable,
   'updatedAt',  readable,               'reviewState',  readable, writable,
-  embedded('submitter')
+  'userAgent',  readable,
+  embedded('submitter'),
+  embedded('currentVersion')
 ) {
   get def() { return this.aux.def; }
   get xml() { return (this.aux.xml != null) ? this.aux.xml.xml : null; }

lib/model/query/submissions.js

@@ -48,7 +48,17 @@ select ins.*, def.id as "submissionDefId" from ins, def;`)
           localKey: partial.def.localKey,
           encDataAttachmentName: partial.def.encDataAttachmentName,
           signature: partial.def.signature,
-          createdAt: submissionData.createdAt
+          createdAt: submissionData.createdAt,
+          userAgent
+        }),
+        currentVersion: new Submission.Def({
+          instanceId: partial.instanceId,
+          createdAt: submissionData.createdAt,
+          deviceId,
+          userAgent,
+          instanceName: partial.def.instanceName,
+          current: true,
+          submitterId: submissionData.submitterId
         }),
         xml: new Submission.Xml({ xml: partial.xml })
       }));
@@ -64,15 +74,24 @@ const createVersion = (partial, deprecated, form, deviceIdIn = null, userAgentIn
   const deviceId = blankStringToNull(deviceIdIn);
   const userAgent = blankStringToNull(userAgentIn);
 
+  const _unjoiner = unjoiner(Submission, Submission.Def.into('currentVersion'));
+
   // we already do transactions but it just feels nice to have the cte do it all at once.
   return one(sql`
 with logical as (update submissions set "reviewState"='edited', "updatedAt"=clock_timestamp()
-  where id=${deprecated.submissionId})
-, upd as (update submission_defs set current=false where "submissionId"=${deprecated.submissionId})
-${_defInsert(deprecated.submissionId, partial, form.def.id, actorId, null, deviceId, userAgent)}`)
-    // TODO/HACK: lame that we are reconstructing this this way.
-    .then((saved) => new Submission({ instanceId: partial.instanceId },
-      { def: new Submission.Def(saved), xml: new Submission.Xml({ xml: partial.xml }) }));
+  where id=${deprecated.submissionId} 
+  returning * )
+, upd as (update submission_defs set current=false where "submissionId"=${deprecated.submissionId} returning *)
+, def as (
+  ${_defInsert(deprecated.submissionId, partial, form.def.id, actorId, null, deviceId, userAgent)}
+)
+select ${_unjoiner.fields} from (
+  select logical.*, upd."userAgent" from logical
+  join upd on logical.id = upd."submissionId" and root
+) submissions
+join def submission_defs on submissions.id = submission_defs."submissionId"
+`)
+    .then(_unjoiner);
 };
 
 createVersion.audit = (partial, deprecated, form) => (log) =>
@@ -134,20 +153,34 @@ order by path asc, value asc`)
 ////////////////////////////////////////////////////////////////////////////////
 // SUBMISSION GETTERS
 
-const _get = extender(Submission)(Actor.into('submitter'))((fields, extend, options, projectId, xmlFormId, draft) => sql`
-select ${fields} from submissions
-join forms on forms."xmlFormId"=${xmlFormId} and forms.id=submissions."formId"
-join projects on projects.id=${projectId} and projects.id=forms."projectId"
-${extend|| sql`left outer join actors on actors.id=submissions."submitterId"`}
-where ${equals(options.condition)} and draft=${draft} and submissions."deletedAt" is null
-order by submissions."createdAt" desc, submissions.id desc
-${page(options)}`);
+// Helper function to assign submission.currentVersionSubmitter to submission.currentVersion.submitter
+// Current there is no way to create such complex object using `extender` and `unjoiner` framework functions
+const assignCurrentVersionSubmitter = (x) => x.withAux('currentVersion', x.aux.currentVersion.withAux('submitter', x.aux.currentVersionSubmitter));
+
+const _get = extender(Submission, Submission.Def.into('currentVersion'))(Actor.into('submitter'), Actor.alias('current_version_actors', 'currentVersionSubmitter'))((fields, extend, options, projectId, xmlFormId, draft) => sql`
+  select ${fields} from 
+  (
+    select submissions.*, submission_defs."userAgent" from submissions
+    join submission_defs on submissions.id = submission_defs."submissionId" and root
+  ) submissions
+  join submission_defs on submissions.id = submission_defs."submissionId" and submission_defs.current
+  join forms on forms."xmlFormId"=${xmlFormId} and forms.id=submissions."formId"
+  join projects on projects.id=${projectId} and projects.id=forms."projectId"
+  ${extend|| sql`
+    left outer join actors on actors.id=submissions."submitterId"
+    left outer join actors current_version_actors on current_version_actors.id=submission_defs."submitterId"
+  `}
+  where ${equals(options.condition)} and draft=${draft} and submissions."deletedAt" is null
+  order by submissions."createdAt" desc, submissions.id desc
+  ${page(options)}`);
 
 const getByIds = (projectId, xmlFormId, instanceId, draft, options = QueryOptions.none) => ({ maybeOne }) =>
-  _get(maybeOne, options.withCondition({ instanceId }), projectId, xmlFormId, draft);
+  _get(maybeOne, options.withCondition({ 'submissions.instanceId': instanceId }), projectId, xmlFormId, draft)
+    .then(x => x.map(assignCurrentVersionSubmitter));
 
 const getAllForFormByIds = (projectId, xmlFormId, draft, options = QueryOptions.none) => ({ all }) =>
-  _get(all, options, projectId, xmlFormId, draft);
+  _get(all, options, projectId, xmlFormId, draft)
+    .then(map(assignCurrentVersionSubmitter));
 
 const getById = (submissionId) => ({ maybeOne }) =>
   maybeOne(sql`select * from submissions where id=${submissionId} and "deletedAt" is null`)
@@ -244,10 +277,12 @@ const _exportUnjoiner = unjoiner(Submission, Submission.Def, Submission.Xml, Sub
 // the /only/ place we need to do this in the entire codebase right now. so for now
 // we just use the terrible hack.
 const { raw } = require('slonik-sql-tag-raw');
-const _exportFields = raw(_exportUnjoiner.fields.sql.replace(
-  'submission_defs."xml" as "submission_defs!xml"',
-  '(case when submission_defs."localKey" is null then submission_defs.xml end) as "submission_defs!xml"'
-));
+const _exportFields = raw(_exportUnjoiner.fields.sql
+  .replace(',submissions."userAgent" as "submissions!userAgent"', '')
+  .replace(
+    'submission_defs."xml" as "submission_defs!xml"',
+    '(case when submission_defs."localKey" is null then submission_defs.xml end) as "submission_defs!xml"'
+  ));
 
 const _export = (formId, draft, keyIds = [], options) => {
   const encrypted = keyIds.length !== 0;

lib/resources/submissions.js

@@ -94,7 +94,7 @@ module.exports = (service, endpoint) => {
                     Forms.getBinaryFields(form.def.id),
                     SubmissionAttachments.getForFormAndInstanceId(form.id, deprecatedId, draft)
                   ]).then(([ saved, binaryFields, deprecatedAtts ]) =>
-                    SubmissionAttachments.create(saved, form, binaryFields, files, deprecatedAtts))))
+                    SubmissionAttachments.create(saved.aux.currentVersion.with({ def: saved.aux.currentVersion }), form, binaryFields, files, deprecatedAtts))))
 
               // ((no deprecatedId given))
               : Submissions.getAnyDefByFormAndInstanceId(form.id, partial.instanceId, draft)
@@ -200,7 +200,7 @@ module.exports = (service, endpoint) => {
             SubmissionAttachments.getForFormAndInstanceId(form.id, deprecatedId, draft),
           ])
             .then(([ submission, binaryFields, deprecatedAtt ]) =>
-              SubmissionAttachments.create(submission, form, binaryFields, undefined, deprecatedAtt)
+              SubmissionAttachments.create(submission.aux.currentVersion.with({ def: submission.aux.currentVersion }), form, binaryFields, undefined, deprecatedAtt)
                 .then(always(submission))));
       })));
   };

test/assertions.js

@@ -91,29 +91,32 @@ should.Assertion.add('User', function() {
 should.Assertion.add('Submission', function() {
   this.params = { operator: 'to be a Submission' };
 
-  Object.keys(this.obj).should.containDeep([ 'instanceId', 'createdAt', 'updatedAt', 'submitterId' ]);
+  Object.keys(this.obj).should.containDeep([ 'instanceId', 'createdAt', 'updatedAt', 'submitterId', 'currentVersion', 'userAgent' ]);
   this.obj.instanceId.should.be.a.String();
   this.obj.submitterId.should.be.a.Number();
   this.obj.createdAt.should.be.an.isoDate();
+  this.obj.userAgent.should.be.a.String();
+  this.obj.currentVersion.should.be.a.SubmissionDef();
   if (this.obj.updatedAt != null) this.obj.updatedAt.should.be.an.isoDate();
 });
 
 // eslint-disable-next-line space-before-function-paren, func-names
 should.Assertion.add('ExtendedSubmission', function() {
   this.params = { operator: 'to be an extended Submission' };
 
-  Object.keys(this.obj).should.containDeep([ 'instanceId', 'createdAt', 'updatedAt', 'submitter' ]);
+  Object.keys(this.obj).should.containDeep([ 'instanceId', 'createdAt', 'updatedAt', 'submitter', 'currentVersion' ]);
   this.obj.instanceId.should.be.a.String();
   this.obj.submitter.should.be.an.Actor();
   this.obj.createdAt.should.be.an.isoDate();
+  this.obj.currentVersion.should.be.a.ExtendedSubmissionDef();
   if (this.obj.updatedAt != null) this.obj.updatedAt.should.be.an.isoDate();
 });
 
 // eslint-disable-next-line space-before-function-paren, func-names
 should.Assertion.add('SubmissionDef', function() {
   this.params = { operator: 'to be a Submission' };
 
-  Object.keys(this.obj).should.containDeep([ 'submitterId', 'createdAt', 'instanceName' ]);
+  Object.keys(this.obj).should.containDeep([ 'instanceId', 'submitterId', 'createdAt', 'instanceName', 'current', 'userAgent', 'deviceId' ]);
   this.obj.submitterId.should.be.a.Number();
   this.obj.createdAt.should.be.an.isoDate();
   if (this.obj.instanceName != null) this.obj.instanceName.should.be.a.String();

test/integration/api/submissions.js

@@ -1132,11 +1132,21 @@ describe('api: /forms/:id/submissions', () => {
         asAlice.post('/v1/projects/1/forms/simple/submissions')
           .send(testData.instances.simple.one)
           .set('Content-Type', 'application/xml')
+          .set('user-agent', 'node1')
           .expect(200)
           .then(() => asAlice.put('/v1/projects/1/forms/simple/submissions/one')
             .set('Content-Type', 'text/xml')
             .send(withSimpleIds('one', 'two'))
-            .expect(200))
+            .set('user-agent', 'node2')
+            .expect(200)
+            .then(({ body }) => {
+              body.should.be.a.Submission();
+              body.instanceId.should.be.eql('one');
+              body.currentVersion.instanceId.should.be.eql('two');
+
+              body.userAgent.should.be.eql('node1');
+              body.currentVersion.userAgent.should.be.eql('node2');
+            }))
           .then(() => asAlice.get('/v1/projects/1/forms/simple/submissions/one.xml')
             .expect(200)
             .then(({ text }) => { text.should.equal(withSimpleIds('one', 'two')); })))));