Add instanceId clause for submission audit query

In order to return delete and restore events in Submission audit endpoint we need to filter by instanceId as well because delete and restore actions on Submission log only instanceId (submissionId is not logged).
sadiqkhoja · Nov 28, 2024 · 3f3720f · 3f3720f
1 parent 57c4da5
commit 3f3720f
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 13 deletions.
diff --git a/lib/model/query/audits.js b/lib/model/query/audits.js
@@ -103,7 +103,7 @@ const get = (options = QueryOptions.none) => ({ all }) =>
     });
   });
 
-const _getBySubmissionId = extender(Audit)(Option.of(Actor), Option.of(Entity), Option.of(Entity.Def.alias('current_entity_def', 'currentVersion')))((fields, extend, options, submissionId) => sql`
+const _getBySubmissionId = extender(Audit)(Option.of(Actor), Option.of(Entity), Option.of(Entity.Def.alias('current_entity_def', 'currentVersion')))((fields, extend, options, submissionId, instanceId) => sql`
 SELECT ${fields} FROM audits
 ${extend|| sql`
   LEFT OUTER JOIN actors ON actors.id=audits."actorId"
@@ -113,13 +113,13 @@ ${extend|| sql`
   -- join with current entity def even if there is a specific def linked in event
   LEFT JOIN entity_defs AS current_entity_def ON current_entity_def."entityId" = entities.id AND current
 `}
-  WHERE (audits.details->>'submissionId')::INTEGER = ${submissionId}
+  WHERE ((audits.details->>'submissionId')::INTEGER = ${submissionId} OR (audits.details->>'instanceId')::VARCHAR = ${instanceId})
     -- suppress this one event that is used for offline entity ordering/processing
     AND audits.action != 'submission.reprocess'
   ORDER BY audits."loggedAt" DESC, audits.id DESC
   ${page(options)}`);
 
-const getBySubmissionId = (submissionId, options) => ({ all }) => _getBySubmissionId(all, options, submissionId)
+const getBySubmissionId = (submissionId, instanceId, options) => ({ all }) => _getBySubmissionId(all, options, submissionId, instanceId)
   .then(map(audit => {
     if (audit.aux.entity == null)
       return audit;

diff --git a/lib/resources/submissions.js b/lib/resources/submissions.js
@@ -388,7 +388,7 @@ module.exports = (service, endpoint) => {
         .then(auth.canOrReject('submission.read')) // TODO: this could be split to audit.read on form
         .then(() => Submissions.getByIds(params.projectId, params.formId, params.instanceId, draft))
         .then(getOrNotFound)
-        .then((submission) => Audits.getBySubmissionId(submission.id, queryOptions))));
+        .then((submission) => Audits.getBySubmissionId(submission.id, params.instanceId, queryOptions))));
 
     // if we match the logical id, we stick around. otherwise we redirect.
     const getOrRedirect = (Forms, Submissions, { params, auth, originalUrl, queryOptions }) =>

diff --git a/test/integration/api/submissions.js b/test/integration/api/submissions.js
@@ -3362,22 +3362,34 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
             .send(withSimpleIds('one', 'two'))
             .set('Content-Type', 'text/xml')
             .expect(200))
+          .then(() => asAlice.delete('/v1/projects/1/forms/simple/submissions/one')
+            .expect(200))
+          .then(() => asAlice.post('/v1/projects/1/forms/simple/submissions/one/restore')
+            .expect(200))
           .then(() => oneFirst(sql`select id from submissions`))
           .then((submissionId) => all(sql`SELECT id FROM submission_defs WHERE "submissionId" = ${submissionId} ORDER BY id desc`)
             .then(map(o => o.id))
             .then(submissionDefIds => asAlice.get('/v1/projects/1/forms/simple/submissions/one/audits')
               .expect(200)
               .then(({ body }) => {
-                body.length.should.equal(3);
+                body.length.should.equal(5);
                 for (const audit of body) audit.should.be.an.Audit();
-                body[0].action.should.equal('submission.update.version');
-                body[0].details.should.eql({ instanceId: 'two', submissionId, submissionDefId: submissionDefIds[0] });
-                body[1].action.should.equal('submission.update');
-                body[1].details.reviewState.should.equal('rejected');
-                body[1].details.submissionId.should.equal(submissionId);
-                should.exist(body[1].details.submissionDefId);
-                body[2].action.should.equal('submission.create');
-                body[2].details.should.eql({ instanceId: 'one', submissionId, submissionDefId: submissionDefIds[1] });
+                body[0].action.should.equal('submission.restore');
+                body[0].details.should.eql({ instanceId: 'one' });
+
+                body[1].action.should.equal('submission.delete');
+                body[1].details.should.eql({ instanceId: 'one' });
+
+                body[2].action.should.equal('submission.update.version');
+                body[2].details.should.eql({ instanceId: 'two', submissionId, submissionDefId: submissionDefIds[0] });
+
+                body[3].action.should.equal('submission.update');
+                body[3].details.reviewState.should.equal('rejected');
+                body[3].details.submissionId.should.equal(submissionId);
+                should.exist(body[3].details.submissionDefId);
+
+                body[4].action.should.equal('submission.create');
+                body[4].details.should.eql({ instanceId: 'one', submissionId, submissionDefId: submissionDefIds[1] });
               }))))));
 
     it('should not expand actor when not extended', testService((service) =>