From d0f068071b883b7c56ddc9410226f9f95569d988 Mon Sep 17 00:00:00 2001
From: Kathleen Tuite <ktuite@getodk.org>
Date: Tue, 3 Oct 2023 11:48:56 -0700
Subject: [PATCH] moving conditional, adding validation

---
 lib/model/query/entities.js | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js
index cee87c713..29febc081 100644
--- a/lib/model/query/entities.js
+++ b/lib/model/query/entities.js
@@ -133,6 +133,12 @@ createVersion.audit.withResult = true;
 // Processing submission events to create and update entities
 
 const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent) => async ({ Audits, Entities }) => {
+  // If dataset requires approval on submission to create an entity and this event is not
+  // an approval event, then don't create an entity
+  if ((dataset.approvalRequired && event.details.reviewState !== 'approved') ||
+      (!dataset.approvalRequired && event.action === 'submission.update')) // don't process submission if approval is not required and submission metadata is updated
+    return null;
+
   const partial = await Entity.fromParseEntityData(entityData);
 
   const sourceDetails = { submission: { instanceId: submissionDef.instanceId }, parentEventId: parentEvent ? parentEvent.id : undefined };
@@ -150,13 +156,15 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss
 };
 
 const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event) => async ({ Audits, Entities }) => {
+  // Get client version of entity
+  const clientEntity = await Entity.fromParseEntityData(entityData); // validation happens here
 
   // Get version of entity on the server
-  const serverEntity = await Entities.getById(dataset.id, entityData.system.id).then(o => o.get());
+  const serverEntity = await Entities.getById(dataset.id, clientEntity.uuid).then(o => o.get());
 
   // merge data
-  const mergedData = mergeRight(serverEntity.aux.currentVersion.data, entityData.data);
-  const mergedLabel = entityData.system.label || serverEntity.aux.currentVersion.label;
+  const mergedData = mergeRight(serverEntity.aux.currentVersion.data, clientEntity.def.data);
+  const mergedLabel = clientEntity.def.label || serverEntity.aux.currentVersion.label;
 
   // make some kind of source object
   const sourceDetails = {
@@ -173,8 +181,8 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
   const entity = await Entities.createVersion(dataset, partial, submissionDef, serverEntity.aux.currentVersion.version + 1, sourceId);
   return Audits.log({ id: event.actorId }, 'entity.update.version', { acteeId: dataset.acteeId },
     {
-      entityId: entity.id, // Added in v2023.3 and backfilled
-      entityDefId: entity.aux.currentVersion.id, // Added in v2023.3 and backfilled
+      entityId: entity.id,
+      entityDefId: entity.aux.currentVersion.id,
       entity: { uuid: entity.uuid, dataset: dataset.name },
       submissionId,
       submissionDefId
@@ -218,27 +226,18 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Datasets, Entit
 
   const entityData = await parseSubmissionXml(fields, submissionDef.xml);
 
-  // If dataset requires approval on submission to create an entity and this event is not
-  // an approval event, then don't create an entity
-  //
-  // We can't simply use submissionDefId to trace back to dataset and find out if approval
-  // is required because in future there can be multiple entities in a single submission.
-  // So we have to rely on dataset name from the xml to fetch the dataset.approvalRequired flag.
+  // We have to look up the dataset based on the name in the XML
   if (!entityData.system.dataset || entityData.system.dataset.trim() === '') {
     throw Problem.user.missingParameter({ field: 'dataset' });
   }
-
   const dataset = (await Datasets.get(form.get().projectId, entityData.system.dataset, true))
     .orThrow(Problem.user.datasetNotFound({ datasetName: entityData.system.dataset }));
 
-  if ((dataset.approvalRequired && event.details.reviewState !== 'approved') ||
-      (!dataset.approvalRequired && event.action === 'submission.update')) // don't process submission if approval is not required and submission metadata is updated
-    return null;
-
-  // If create is not true (either 1 or true) then we don't need to process further
+  // Create entity
   if (entityData.system.create === '1' || entityData.system.create === 'true')
     return Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent);
 
+  // Or update entity
   if (entityData.system.update === '1' || entityData.system.update === 'true')
     return Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event);