Remove use of submissionCreate from entity audit event (#932)

* Remove use of submissionCreate from entity audit event

* Simplify basic details fetching submission instance id

* Refactoring test utility

* Changing structure to put submission and event in details.source

* Minor PR comments

src/components/entity/activity.vue

@@ -57,17 +57,23 @@ const feed = computed(() => {
   let versionIndex = entityVersions.length - 1;
   for (const audit of audits) {
     if (audit.action === 'entity.update.version') {
-      const { submission } = audit.details;
+      const { submission } = audit.details.source;
       const entityVersion = entityVersions[versionIndex];
       versionIndex -= 1;
       groups.push([{ entry: audit, submission, entityVersion }]);
     } else if (audit.action === 'entity.create') {
       const group = [{ entry: audit }];
       const { details } = audit;
-      if (details.sourceEvent?.action === 'submission.update')
-        group.push({ entry: details.sourceEvent });
-      if (details.submissionCreate != null)
-        group.push({ entry: details.submissionCreate, submission: details.submission });
+      // this will insert a feed entry for the submission approval event
+      if (details.source?.event?.action === 'submission.update')
+        group.push({ entry: details.source.event });
+      // this will insert a feed entry for the submission creation
+      if (details.source?.submission != null) {
+        group.push({
+          entry: { action: 'submission.create', loggedAt: details.source.submission.createdAt },
+          submission: details.source.submission
+        });
+      }
       groups.push(group);
     } else {
       groups.push([{ entry: audit }]);

src/components/entity/basic-details.vue

@@ -18,16 +18,16 @@ except according to the terms contained in the LICENSE file.
           <dt>{{ $t('entity.entityId') }}</dt>
           <dd>{{ entity.uuid }}</dd>
         </div>
-        <div v-if="instanceId != null">
+        <div v-if="submission != null">
           <dt>{{ $t('creatingSubmission') }}</dt>
           <dd id="entity-basic-details-creating-submission">
-            <router-link v-if="submission != null"
-              :to="submissionPath(projectId, submission.xmlFormId, instanceId)">
-              {{ submission.currentVersion.instanceName ?? instanceId }}
+            <router-link v-if="submission.currentVersion != null"
+              :to="submissionPath(projectId, submission.xmlFormId, submission.instanceId)">
+              {{ submission.currentVersion.instanceName ?? submission.instanceId }}
             </router-link>
             <template v-else>
               <span class="icon-trash" v-tooltip.sr-only></span>
-              <span>{{ instanceId }}</span>
+              <span>{{ submission.instanceId }}</span>
               <span class="sr-only">&nbsp;{{ $t('submissionDeleted') }}</span>
             </template>
           </dd>
@@ -65,23 +65,16 @@ const projectId = inject('projectId');
 // created.
 const { entity, audits } = useRequestData();
 
-// Using `ref` and watchEffect() for instanceId and `submission` rather than
-// `computed` so that they don't change whenever `audits` is refreshed.
-const instanceId = ref(undefined);
+// Using `ref` and watchEffect() for `submission` rather than
+// `computed` so that it doesn't change whenever `audits` is refreshed.
 const submission = ref(undefined);
 watchEffect(() => {
-  if (instanceId.value !== undefined || !audits.dataExists) return;
+  if (submission.value !== undefined || !audits.dataExists) return;
   const audit = audits.find(({ action }) => action === 'entity.create');
   // `audit` should always exist in production, but it doesn't always exist in
   // testing.
   if (audit == null) return;
-  const { submissionCreate } = audit.details;
-  if (submissionCreate != null) {
-    instanceId.value = submissionCreate.details.instanceId;
-    submission.value = audit.details.submission;
-  } else {
-    instanceId.value = null;
-  }
+  submission.value = audit.details.source?.submission;
 });
 const { submissionPath } = useRoutes();
 </script>

src/components/entity/feed-entry.vue

@@ -15,20 +15,20 @@ except according to the terms contained in the LICENSE file.
     <template #title>
       <template v-if="entry.action === 'submission.create'">
         <span class="icon-cloud-upload"></span>
-        <i18n-t v-if="submission != null"
+        <i18n-t v-if="submission.currentVersion != null"
           keypath="title.submission.create.notDeleted">
           <template #instanceName>
             <router-link :to="creatingSubmissionPath">
               {{ submission.currentVersion.instanceName ?? submission.instanceId }}
             </router-link>
           </template>
-          <template #submitter><actor-link :actor="entry.actor"/></template>
+          <template #submitter><actor-link :actor="submission.submitter"/></template>
         </i18n-t>
         <i18n-t v-else keypath="title.submission.create.deleted.full">
           <template #deletedSubmission>
             <span class="deleted-submission">{{ deletedSubmission }}</span>
           </template>
-          <template #name><actor-link :actor="entry.actor"/></template>
+          <template #name><actor-link :actor="submission.submitter"/></template>
         </i18n-t>
       </template>
       <template v-else-if="entry.action === 'submission.update'">
@@ -44,7 +44,7 @@ except according to the terms contained in the LICENSE file.
       </template>
       <template v-else-if="entry.action === 'entity.create'">
         <span class="icon-magic-wand"></span>
-        <i18n-t v-if="entry.details.submissionCreate != null"
+        <i18n-t v-if="entry.details.source?.submission != null"
           keypath="title.entity.create.submission">
           <template #label>
             <span class="entity-label">{{ entity.currentVersion.label }}</span>
@@ -63,8 +63,8 @@ except according to the terms contained in the LICENSE file.
       <template v-else-if="entry.action === 'entity.update.version'">
         <span class="icon-pencil"></span>
         <span class="title">
-        <template v-if="entry.details.submissionCreate != null">
-          <i18n-t v-if="submission != null"
+        <template v-if="submission != null">
+          <i18n-t v-if="submission.currentVersion != null"
             keypath="title.entity.update_version.submission.notDeleted">
             <template #instanceName>
               <router-link :to="creatingSubmissionPath">
@@ -148,18 +148,13 @@ const creatingSubmissionPath = computed(() => submissionPath(
 ));
 const { t } = useI18n();
 
-// This function pulls out the submission instance ID when the event
-// is about a submission (creation, approval) and the ID is directly
-// in the event details.
 const deletedSubmission = computed(() => {
-  const id = props.entry.details.instanceId;
+  const id = props.submission.instanceId;
   return t('title.submission.create.deleted.deletedSubmission', { id });
 });
 
-// This function pulls out the submission instance ID in events about
-// entities, where the instance ID is found deep inside the submissionCreate event.
 const deletedSubmissionEntityEvent = computed(() => {
-  const id = props.entry.details.submissionCreate.details.instanceId;
+  const id = props.submission.instanceId;
   return t('title.entity.update_version.submission.deleted.deletedSubmission', { id });
 });
 const { reviewStateIcon } = useReviewState();

src/components/entity/version-link.vue

@@ -44,10 +44,10 @@ const versionPath = computed(() => {
 const { t } = useI18n();
 const text = computed(() => {
   const { source } = props.version;
-  const { submissionCreate } = source;
-  if (submissionCreate != null) {
-    const nameOrId = source.submission?.currentVersion?.instanceName ??
-      submissionCreate.details.instanceId;
+  const { submission } = source;
+  if (submission != null) {
+    const nameOrId = submission.currentVersion?.instanceName ??
+      submission.instanceId;
     return t('submission', { instanceName: nameOrId });
   }
   return t('api', { name: props.version.creator.displayName });

test/components/entity/activity.spec.js

@@ -38,17 +38,17 @@ const resolveConflict = () => {
   testData.extendedEntities.createPast(1, { uuid: 'e' });
   testData.extendedAudits.createPast(1, {
     action: 'entity.create',
-    details: {}
+    details: { source: {} }
   });
   testData.extendedEntityVersions.createPast(1);
   testData.extendedAudits.createPast(1, {
     action: 'entity.update.version',
-    details: {}
+    details: { source: {} }
   });
   testData.extendedEntityVersions.createPast(1, { baseVersion: 1 });
   testData.extendedAudits.createPast(1, {
     action: 'entity.update.version',
-    details: {}
+    details: { source: {} }
   });
   testData.extendedEntities.resolve(-1);
   testData.extendedAudits.createPast(1, { action: 'entity.update.resolve' });
@@ -102,7 +102,7 @@ describe('EntityActivity', () => {
       testData.extendedEntities.createPast(1);
       testData.extendedAudits.createPast(1, {
         action: 'entity.create',
-        details: sourceDetails
+        details: { source: sourceDetails }
       });
       const component = mountComponent();
       component.findAll('.feed-entry-group').length.should.equal(1);
@@ -119,7 +119,7 @@ describe('EntityActivity', () => {
       testData.extendedEntities.createPast(1);
       testData.extendedAudits.createPast(1, {
         action: 'entity.create',
-        details: sourceDetails
+        details: { source: sourceDetails }
       });
       const component = mountComponent();
       component.findAll('.feed-entry-group').length.should.equal(1);
@@ -137,7 +137,7 @@ describe('EntityActivity', () => {
       testData.extendedEntities.createPast(1);
       testData.extendedAudits.createPast(1, {
         action: 'entity.create',
-        details: sourceDetails
+        details: { source: sourceDetails }
       });
       const component = mountComponent();
       component.findAll('.feed-entry-group').length.should.equal(1);
@@ -187,13 +187,13 @@ describe('EntityActivity', () => {
       });
       testData.extendedAudits.createPast(1, {
         action: 'entity.create',
-        details: {}
+        details: { source: {} }
       });
       for (let version = 2; version <= 50; version += 1) {
         testData.extendedEntityVersions.createPast(1);
         testData.extendedAudits.createPast(1, {
           action: 'entity.update.version',
-          details: {}
+          details: { source: {} }
         });
       }
     };
@@ -258,7 +258,7 @@ describe('EntityActivity', () => {
           });
           testData.extendedAudits.createPast(1, {
             action: 'entity.update.version',
-            details: {}
+            details: { source: {} }
           });
           return testData.standardEntities.last();
         })

test/components/entity/basic-details.spec.js

@@ -47,20 +47,18 @@ describe('EntityBasicDetails', () => {
       const submission = testData.extendedSubmissions
         .createPast(1, { instanceId: 's', ...submissionOptions })
         .last();
-      const submissionCreate = testData.extendedAudits
-        .createPast(1, {
-          action: 'submission.create',
-          details: { instanceId: submission.instanceId }
-        })
-        .last();
-
       testData.extendedEntities.createPast(1, { uuid: 'e' });
       const details = {
         entity: { uuid: 'e' },
-        submissionCreate
+        source: {}
       };
       if (!submissionDeleted)
-        details.submission = { ...submission, xmlFormId: 'f' };
+        details.source = { submission: { ...submission, xmlFormId: 'f' } }; // Use entire submission, augmented with form id
+      else {
+        // If submission is deleted, these are the only fields we pass through in the audit log
+        const { instanceId, submitter, createdAt } = submission;
+        details.source = { submission: { instanceId, submitter, createdAt } };
+      }
       testData.extendedAudits.createPast(1, {
         action: 'entity.create',
         details
@@ -147,7 +145,7 @@ describe('EntityBasicDetails', () => {
           });
           testData.extendedAudits.createPast(1, {
             action: 'entity.update.version',
-            details: {}
+            details: { source: {} }
           });
           return testData.standardEntities.last();
         })

test/components/entity/feed-entry.spec.js

@@ -50,18 +50,25 @@ describe('EntityFeedEntry', () => {
 
   describe('submission.create audit event', () => {
     const createSubmission = ({ deleted = false, ...options } = {}) => {
-      const submission = testData.extendedSubmissions
+      // create a submission
+      const fullSubmission = testData.extendedSubmissions
         .createPast(1, { instanceId: 's', ...options })
         .last();
+      // return either full or partial submission info depending on if submission was deleted
+      const submission = (deleted)
+        ? { instanceId: fullSubmission.instanceId, submitter: fullSubmission.submitter, createdAt: fullSubmission.createdAt }
+        : { ...fullSubmission, xmlFormId: 'f' }; // full submission augmented with form id
+
       const audit = testData.extendedAudits
         .createPast(1, {
           action: 'submission.create',
-          details: { instanceId: 's' }
+          details: { instanceId: submission.instanceId }
         })
         .last();
+
       return {
         entry: audit,
-        submission: deleted ? null : { ...submission, xmlFormId: 'f' }
+        submission
       };
     };
 
@@ -160,13 +167,12 @@ describe('EntityFeedEntry', () => {
     // entity.create event.
     const createEntity = (options = {}) => {
       const details = {
-        entity: { uuid: 'e' }
+        entity: { uuid: 'e' },
+        source: {}
       };
       if (options.submission === true) {
-        testData.extendedSubmissions.createPast(1);
-        details.submissionCreate = testData.extendedAudits
-          .createPast(1, { action: 'submission.create' })
-          .last();
+        const submission = testData.extendedSubmissions.createPast(1).last();
+        details.source = { submission: { ...submission, xmlFormId: 'f' } };
       }
       testData.extendedAudits.createPast(1, {
         action: 'entity.create',
@@ -218,7 +224,7 @@ describe('EntityFeedEntry', () => {
       testData.extendedEntityVersions.createPast(1);
       testData.extendedAudits.createPast(1, {
         action: 'entity.update.version',
-        details: {}
+        details: { source: {} }
       });
     });
 
@@ -250,21 +256,21 @@ describe('EntityFeedEntry', () => {
 
   describe('entity.update.version (via submission) audit event', () => {
     const updateEntityFromSubmission = ({ deleted = false, ...options } = {}) => {
-      const details = {
-        entity: { uuid: 'e' }
-      };
-
-      details.submissionCreate = testData.extendedAudits
-        .createPast(1, {
-          action: 'submission.create',
-          details: { instanceId: 'x' }
-        })
-        .last();
-
-      const submission = testData.extendedSubmissions
+      // create the submission this event is based on
+      const fullSubmission = testData.extendedSubmissions
         .createPast(1, { instanceId: 's', ...options })
         .last();
 
+      // return either full or partial submission info depending on if submission was deleted
+      const submission = (deleted)
+        ? { instanceId: fullSubmission.instanceId, submitter: fullSubmission.submitter, createdAt: fullSubmission.createdAt }
+        : { ...fullSubmission, xmlFormId: 'f' }; // full submission augmented with form id
+
+      const details = {
+        entity: { uuid: 'e' },
+        source: { submission } // source would also have `event` but that is not used in this component
+      };
+
       testData.extendedEntityVersions.createPast(1);
       const audit = testData.extendedAudits
         .createPast(1, {
@@ -275,7 +281,7 @@ describe('EntityFeedEntry', () => {
 
       return {
         entry: audit,
-        submission: deleted ? null : { ...submission, xmlFormId: 'f' }
+        submission
       };
     };
 
@@ -308,7 +314,7 @@ describe('EntityFeedEntry', () => {
     it('shows the correct text with deleted submission instance id', () => {
       const component = mountComponent({ props: updateEntityFromSubmission({ deleted: true }) });
       const text = component.get('.feed-entry-title .title').text();
-      text.should.equal('Data updated by (deleted Submission x)');
+      text.should.equal('Data updated by (deleted Submission s)');
     });
 
     it('does not link to the deleted submission', () => {

test/components/entity/show.spec.js

@@ -86,7 +86,7 @@ describe('EntityShow', () => {
           });
           testData.extendedAudits.createPast(1, {
             action: 'entity.update.version',
-            details: {}
+            details: { source: {} }
           });
           return testData.standardEntities.last();
         })
@@ -151,7 +151,7 @@ describe('EntityShow', () => {
           });
           testData.extendedAudits.createPast(1, {
             action: 'entity.update.version',
-            details: {}
+            details: { source: {} }
           });
 
           return testData.standardEntities.last();