From 232a4765b998e2945860f3892d1f0144735b0852 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Fri, 6 Dec 2024 17:43:15 -0800 Subject: [PATCH] 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 | 6 +- lib/model/query/entities.js | 17 +++- test/integration/api/offline-entities.js | 99 +++++++++++++++++++++++- 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/lib/data/entity.js b/lib/data/entity.js index 5dd20e068..529a34bd8 100644 --- a/lib/data/entity.js +++ b/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 }); diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 1606ba0bb..9d20c7321 100644 --- a/lib/model/query/entities.js +++ b/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 }), { diff --git a/test/integration/api/offline-entities.js b/test/integration/api/offline-entities.js index 6ed99defb..7a132e020 100644 --- a/test/integration/api/offline-entities.js +++ b/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('new', 'checked in') + .replace('20', '22') + .replace('Megan', '') + .replace('', '') + ) + .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() {