From 688fa02f0b5a8a23d4212ce4590d53083541665a Mon Sep 17 00:00:00 2001
From: Kathleen Tuite <ktuite@getodk.org>
Date: Thu, 3 Oct 2024 13:39:52 -0700
Subject: [PATCH] Fix baseVersion parsing when create=true

---
 lib/data/entity.js               |  7 +++--
 test/unit/data/entity.js         | 20 ++++++------
 test/unit/model/frames/entity.js | 53 ++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/lib/data/entity.js b/lib/data/entity.js
index dbb2baba2..f26f900f9 100644
--- a/lib/data/entity.js
+++ b/lib/data/entity.js
@@ -19,7 +19,7 @@ const { PartialPipe } = require('../util/stream');
 const Problem = require('../util/problem');
 const { submissionXmlToFieldStream } = require('./submission');
 const { nextUrlFor, getServiceRoot, jsonDataFooter, extractPaging } = require('../util/odata');
-const { sanitizeOdataIdentifier, blankStringToNull } = require('../util/util');
+const { sanitizeOdataIdentifier, blankStringToNull, isBlank } = require('../util/util');
 
 const odataToColumnMap = new Map([
   ['__system/createdAt', 'entities.createdAt'],
@@ -61,12 +61,15 @@ const extractBaseVersionFromSubmission = (entity, options = { update: true }) =>
   if (update) {
     if (!baseVersion)
       throw Problem.user.missingParameter({ field: 'baseVersion' });
-
+  }
+  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;
 };
 
 const extractBranchIdFromSubmission = (entity) => {
diff --git a/test/unit/data/entity.js b/test/unit/data/entity.js
index cebe68f48..47ab73d84 100644
--- a/test/unit/data/entity.js
+++ b/test/unit/data/entity.js
@@ -135,17 +135,9 @@ describe('extracting and validating entities', () => {
         extractBaseVersionFromSubmission(entity, { update: true }).should.equal(99);
       });
 
-      it('not return base version if create is true because it is not relevant', () => {
+      it('should extract base version even if create is true and update is not', () => {
         const entity = { system: { baseVersion: '99' } };
-        should.not.exist(extractBaseVersionFromSubmission(entity, { create: true }));
-      });
-
-      it('not complain if baseVersion is missing when update is false and create is true', () => {
-        const entity = { system: { update: '1', create: '1' } };
-        // the create/update values do not get pulled from the entity system data here
-        // but rather from the branch in the code that decides whether a create
-        // or update is currently being attempted.
-        should.not.exist(extractBaseVersionFromSubmission(entity, { create: true }));
+        extractBaseVersionFromSubmission(entity, { create: true }).should.equal(99);
       });
 
       it('should complain if baseVersion is missing when update is true', () => {
@@ -157,6 +149,14 @@ describe('extracting and validating entities', () => {
         });
       });
 
+      it('not complain if baseVersion is missing when update is false and create is true', () => {
+        const entity = { system: { } };
+        // the create/update values do not get pulled from the entity system data here
+        // but rather from the branch in the code that decides whether a create
+        // or update is currently being attempted.
+        should.not.exist(extractBaseVersionFromSubmission(entity, { create: true }));
+      });
+
       it('should complain if baseVersion not an integer', () => {
         const entity = { system: { update: '1', baseVersion: 'ten' } };
         assert.throws(() => { extractBaseVersionFromSubmission(entity); }, (err) => {
diff --git a/test/unit/model/frames/entity.js b/test/unit/model/frames/entity.js
index 5a16f5f1f..f75d2708a 100644
--- a/test/unit/model/frames/entity.js
+++ b/test/unit/model/frames/entity.js
@@ -1,5 +1,7 @@
 const appRoot = require('app-root-path');
 const { Entity } = require(appRoot + '/lib/model/frames');
+const assert = require('assert');
+
 
 describe('entity', () => {
   describe('fromParseEntityData', () => {
@@ -23,6 +25,57 @@ describe('entity', () => {
       }));
       partial.aux.should.have.property('dataset', 'people');
     });
+
+    describe('baseVersion', () => {
+      it('should parse successfully for empty baseVersion, create: true', () => {
+        const partial = Entity.fromParseEntityData({
+          system: {
+            label: 'label',
+            id: 'uuid:12345678-1234-4123-8234-abcd56789abc',
+            create: '1',
+            baseVersion: '',
+            dataset: 'people'
+          },
+          data: { field: 'value' }
+        },
+        { create: true });
+        partial.aux.def.should.not.have.property('baseVersion');
+      });
+
+      it('should return baseVersion even when create: true', () => {
+        const partial = Entity.fromParseEntityData({
+          system: {
+            label: 'label',
+            id: 'uuid:12345678-1234-4123-8234-abcd56789abc',
+            create: '1',
+            baseVersion: '73',
+            dataset: 'people'
+          },
+          data: { field: 'value' }
+        },
+        { create: true });
+        partial.aux.def.baseVersion.should.equal(73);
+      });
+
+      it('should complain about missing baseVersion when update: true', () => {
+        const entity = {
+          system: {
+            label: 'label',
+            id: 'uuid:12345678-1234-4123-8234-abcd56789abc',
+            create: '1',
+            baseVersion: '',
+            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 baseVersion missing.');
+          return true;
+        });
+      });
+    });
   });
 });