Log create-as-update as a conflict (#1319)

* Test demonstrating bug from QA

* refine test

* mark create-as-update as conflict at level of entity version

* Improvements based on discussion

* fixed a test and added a soft conflict test

lib/data/entity.js

@@ -500,8 +500,12 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => {
     if (v.version > 1) { // v.root is false here - can use either
       const baseNotContiguousWithTrunk = v.branchId != null &&
         branches.get(v.branchId).lastContiguousWithTrunk < v.baseVersion;
+
+      // check if it's a create applied as an update, which is also a conflict
+      const createAsUpdate = def.aux.source?.details?.action === 'create';
+
       const conflict = v.version !== (v.baseVersion + 1) ||
-        baseNotContiguousWithTrunk;
+        baseNotContiguousWithTrunk || createAsUpdate;
 
       v.baseDiff = getDiffProp(v.dataReceived, { ...defs[v.baseVersion - 1].data, label: defs[v.baseVersion - 1].label });
 

lib/model/query/entities.js

@@ -317,7 +317,17 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
       conflict = conflictingProperties.length > 0 ? ConflictType.HARD : ConflictType.SOFT;
     }
   } else if (createSubAsUpdate) {
-    conflict = ConflictType.SOFT;
+    const versionDiff = getDiffProp(serverEntity.aux.currentVersion.data, clientEntity.def.dataReceived);
+    const diffData = pickAll(versionDiff, serverEntity.aux.currentVersion.data);
+
+    if (serverEntity.aux.currentVersion.label !== clientEntity.def.label)
+      diffData.label = serverEntity.aux.currentVersion.label;
+
+    conflictingProperties = Object.keys(clientEntity.def.dataReceived).filter(key => key in diffData && clientEntity.def.dataReceived[key] !== diffData[key]);
+
+    if (conflict !== ConflictType.HARD) { // We don't want to downgrade conflict here
+      conflict = conflictingProperties.length > 0 ? ConflictType.HARD : ConflictType.SOFT;
+    }
   } else {
     // This may still be a soft conflict if the new version is not contiguous with this branch's trunk version
     const interrupted = await Entities._interruptedBranch(serverEntity.id, clientEntity);
@@ -332,7 +342,10 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
 
   // make some kind of source object
   const sourceDetails = {
-    submission: { instanceId: submissionDef.instanceId }
+    submission: {
+      instanceId: submissionDef.instanceId,
+    },
+    ...createSubAsUpdate ? { action: 'create' } : {}
   };
   const sourceId = await Entities.createSource(sourceDetails, submissionDefId, event.id, forceOutOfOrderProcessing);
   const partial = new Entity.Partial(serverEntity.with({ conflict }), {

test/integration/api/offline-entities.js

@@ -1198,7 +1198,7 @@ describe('Offline Entities', () => {
         .then(({ body }) => {
           body.currentVersion.version.should.equal(2);
           body.currentVersion.data.should.eql({ age: '20', status: 'new', first_name: 'Megan' });
-          body.conflict.should.equal('soft'); // this should be marked as a soft conflict
+          body.conflict.should.equal('hard'); // this should be marked as a soft conflict
           body.currentVersion.baseVersion.should.equal(1); // baseVersion is set, but normally the baseVersion of an entity-create is null
           // the rest of these are null like a normal entity-create
           should.not.exist(body.currentVersion.branchBaseVersion);
@@ -1714,6 +1714,103 @@ describe('Offline Entities', () => {
           should(entity.conflict).equal(null);
         });
     }));
+
+    it('should show proper conflict info on create applied as update', testOfflineEntities(async (service, container) => {
+      // Issue c#815
+      const asAlice = await service.login('alice');
+      const branchId = uuid();
+
+      // Send update
+      await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
+        .send(testData.instances.offlineEntity.two
+          .replace('create="1"', 'update="1"')
+          .replace('branchId=""', `branchId="${branchId}"`)
+          .replace('two', 'two-update1')
+          .replace('baseVersion=""', 'baseVersion="1"')
+          .replace('<status>new</status>', '<status>checked in</status>')
+          .replace('<age>20</age>', '<age>22</age>')
+          .replace('<name>Megan</name>', '')
+          .replace('<label>Megan (20)</label>', '')
+        )
+        .set('Content-Type', 'application/xml')
+        .expect(200);
+
+      await exhaust(container);
+
+      // Check backlog
+      const backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`);
+      backlogCount.should.equal(1);
+
+      await container.Entities.processBacklog(true);
+
+      // Send registration
+      await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
+        .send(testData.instances.offlineEntity.two)
+        .set('Content-Type', 'application/xml')
+        .expect(200);
+
+      await exhaust(container);
+
+      // Check that entity as a whole is a conflict
+      await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd')
+        .then(({ body }) => {
+          body.conflict.should.equal('hard');
+        });
+
+      // Check versions
+      await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd/versions')
+        .expect(200)
+        .then(({ body }) => {
+          body.map(v => v.conflict).should.eql([null, 'hard']);
+          body[1].conflictingProperties.should.eql([ 'status', 'age', 'label' ]);
+        });
+    }));
+
+    it('should show proper conflict info on create applied as update that is only a soft conflict', testOfflineEntities(async (service, container) => {
+      const asAlice = await service.login('alice');
+      const branchId = uuid();
+
+      // Send update
+      await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
+        .send(testData.instances.offlineEntity.two
+          .replace('create="1"', 'update="1"')
+          .replace('branchId=""', `branchId="${branchId}"`)
+          .replace('two', 'two-update1')
+          .replace('baseVersion=""', 'baseVersion="1"')
+        )
+        .set('Content-Type', 'application/xml')
+        .expect(200);
+
+      await exhaust(container);
+
+      // Check backlog
+      const backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`);
+      backlogCount.should.equal(1);
+
+      await container.Entities.processBacklog(true);
+
+      // Send registration
+      await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
+        .send(testData.instances.offlineEntity.two)
+        .set('Content-Type', 'application/xml')
+        .expect(200);
+
+      await exhaust(container);
+
+      // Check that entity as a whole is a soft conflict
+      await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd')
+        .then(({ body }) => {
+          body.conflict.should.equal('soft');
+        });
+
+      // Check versions
+      await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd/versions')
+        .expect(200)
+        .then(({ body }) => {
+          body.map(v => v.conflict).should.eql([null, 'soft']);
+          body[1].conflictingProperties.should.eql([]);
+        });
+    }));
   });
 
   describe('locking an entity while processing a related submission', function() {