code fixes

getodk · Oct 8, 2024 · 0412663 · 0412663
1 parent 73ad6c4
commit 0412663
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 13 deletions.
diff --git a/lib/data/entity.js b/lib/data/entity.js
@@ -56,20 +56,18 @@ const extractLabelFromSubmission = (entity, options = { create: true }) => {
 
 // Input: object representing entity, parsed from submission XML
 const extractBaseVersionFromSubmission = (entity, options = { update: true }) => {
-  const { update } = options;
+  const { update = false } = options;
   const { baseVersion } = entity.system;
-  if (update) {
-    if (!baseVersion)
+  if (isBlank(baseVersion)) {
+    if (update)
       throw Problem.user.missingParameter({ field: 'baseVersion' });
+    return null;
   }
-  if (!isBlank(baseVersion)) {
-    // Check type and parseInt in both create and update case
-    if (!/^\d+$/.test(baseVersion))
-      throw Problem.user.invalidDataTypeOfParameter({ field: 'baseVersion', expected: 'integer' });
 
-    return parseInt(entity.system.baseVersion, 10);
-  }
-  return null;
+  if (!/^\d+$/.test(baseVersion))
+    throw Problem.user.invalidDataTypeOfParameter({ field: 'baseVersion', expected: 'integer' });
+
+  return parseInt(entity.system.baseVersion, 10);
 };
 
 const extractBranchIdFromSubmission = (entity) => {

diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js
@@ -486,11 +486,11 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset
       maybeEntity = await Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing);
     } catch (err) {
       const attemptCreate = (entityData.system.create === '1' || entityData.system.create === 'true') || forceOutOfOrderProcessing;
-      // The two cases we attempt to create after a failed update:
+      // The two types of errors for which we attempt to create after a failed update:
       // 1. entity not found
       // 2. baseVersion is empty and failed to parse in the update case
       // This second one is a special case related to issue c#727
-      if ((err.problemCode === 404.8 || err.problemCode === 400.2) && attemptCreate) {
+      if ((err.problemCode === 404.8 || (err.problemCode === 400.2 && err.problemDetails.field === 'baseVersion')) && attemptCreate) {
         maybeEntity = await Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing);
       } else {
         throw (err);

diff --git a/test/unit/model/frames/entity.js b/test/unit/model/frames/entity.js
@@ -26,6 +26,26 @@ describe('entity', () => {
       partial.aux.should.have.property('dataset', 'people');
     });
 
+    it('should throw 400.2 for other problems like missing branchId when trunkVersion is present', () => {
+      const entity = {
+        system: {
+          label: 'label',
+          id: 'uuid:12345678-1234-4123-8234-abcd56789abc',
+          update: '1',
+          trunkVersion: '1',
+          baseVersion: '3',
+          dataset: 'people'
+        },
+        data: { field: 'value' }
+      };
+
+      assert.throws(() => { Entity.fromParseEntityData(entity, { update: true }); }, (err) => {
+        err.problemCode.should.equal(400.2);
+        err.message.should.equal('Required parameter branchId missing.');
+        return true;
+      });
+    });
+
     describe('baseVersion', () => {
       it('should parse successfully for empty baseVersion, create: true', () => {
         const partial = Entity.fromParseEntityData({
@@ -62,7 +82,7 @@ describe('entity', () => {
           system: {
             label: 'label',
             id: 'uuid:12345678-1234-4123-8234-abcd56789abc',
-            create: '1',
+            update: '1',
             baseVersion: '',
             dataset: 'people'
           },