Skip to content

Commit

Permalink
responding to code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@ktuite
ktuite committed Nov 13, 2024
1 parent 7020876 commit 16f844e
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 14 deletions.
12 changes: 5 additions & 7 deletions lib/model/query/entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ const _lockEntity = (exec, uuid) => {
////////////////////////////////////////////////////////////////////////////////
// WRAPPER FUNCTIONS FOR CREATING AND UPDATING ENTITIES

const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing = false) => async ({ Audits, Entities }) => {
const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent, forceOutOfOrderProcessing) => 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') ||
Expand Down Expand Up @@ -254,7 +254,7 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss
// Extra flags
// - forceOutOfOrderProcessing: entity was in backlog and was force-processed, which affects base version calculation
// - createSubAsUpdate: submission was meant to *create* entity (and should be parsed as such) but is applied as an update
const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing = false, createSubAsUpdate = false) => async ({ Audits, Entities }) => {
const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing, createSubAsUpdate = false) => async ({ Audits, Entities }) => {
if (!(event.action === 'submission.create'
|| event.action === 'submission.update.version'
|| event.action === 'submission.reprocess'))
Expand Down Expand Up @@ -367,11 +367,9 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss

// Used by _updateVerison to figure out the intended base version in Central
// based on the branchId, trunkVersion, and baseVersion in the submission
const _computeBaseVersion = (eventId, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing = false, createSubAsUpdate = false) => async ({ Entities }) => {
const _computeBaseVersion = (eventId, dataset, clientEntity, submissionDef, forceOutOfOrderProcessing, createSubAsUpdate) => async ({ Entities }) => {
if (createSubAsUpdate) {
// if no baseVersion is specified but we are updating and trying to find the base version
// we are probably in the special case of force-apply create-as-update. get the latest version.

// We are in the special case of force-apply create-as-update. get the latest version.
const latestEntity = await Entities.getById(dataset.id, clientEntity.uuid)
.then(getOrReject(Problem.user.entityNotFound({ entityUuid: clientEntity.uuid, datasetName: dataset.name })));
return latestEntity.aux.currentVersion;
Expand Down Expand Up @@ -527,7 +525,7 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Audits, Dataset
// There was a problem creating the entity
// If it is a uuid collision, check if the entity was created via an update
// in which case its ok to apply this create as an update
if (err.problemCode === 409.3) {
if (err.problemCode === 409.3 && err.problemDetails?.fields[0] === 'uuid') {
const rootDef = await Entities.getDef(dataset.id, entityData.system.id, new QueryOptions({ root: true })).then(o => o.orNull());
if (rootDef && rootDef.aux.source.forceProcessed) {
maybeEntity = await Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, forceOutOfOrderProcessing, true);
Expand Down
61 changes: 54 additions & 7 deletions test/integration/api/offline-entities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1129,18 +1129,12 @@ describe('Offline Entities', () => {
body.currentVersion.branchBaseVersion.should.equal(1);
should.not.exist(body.currentVersion.baseVersion);
should.not.exist(body.currentVersion.trunkVersion);
should(body.conflict).equal(null); // conflict should be null after update-as-create
});

backlogCount = await container.oneFirst(sql`select count(*) from entity_submission_backlog`);
backlogCount.should.equal(0);

await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`)
.expect(200)
.then(({ body }) => {
body.currentVersion.data.should.eql({ status: 'checked in' });
should(body.conflict).equal(null); // conflict should be null after update-as-create
});

// First submission creates the entity, but this will be processed as an update
await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
.send(testData.instances.offlineEntity.two)
Expand All @@ -1155,6 +1149,59 @@ describe('Offline Entities', () => {
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.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);
should.not.exist(body.currentVersion.trunkVersion);
should.not.exist(body.currentVersion.branchId);
});
}));

it('should verify that the create-as-update submission was parsed as a create even when applied as an update', testOfflineEntities(async (service, container) => {
const asAlice = await service.login('alice');
const branchId = uuid();

// Send first submission, which is an update that will be applied as a create
// Removing extra fields of the submission to demonstrate a simpler update with missing fields
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-update')
.replace('baseVersion=""', 'baseVersion="1"')
.replace('<status>new</status>', '<status>checked in</status>')
.replace('<label>Megan (20)</label>', '')
.replace('<age>20</age>', '')
.replace('<name>Megan</name>', '')
)
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);
await container.Entities.processBacklog(true);

// First submission creates the entity, but it should trigger an entity-processing error
// because there is no label
await asAlice.post('/v1/projects/1/forms/offlineEntity/submissions')
.send(testData.instances.offlineEntity.two
.replace('<label>Megan (20)</label>', '')
)
.set('Content-Type', 'application/xml')
.expect(200);

await exhaust(container);

await asAlice.get(`/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789ddd`)
.expect(200)
.then(({ body }) => {
body.currentVersion.version.should.equal(1);
});

await asAlice.get('/v1/projects/1/forms/offlineEntity/submissions/two/audits')
.expect(200)
.then(({ body }) => {
body[0].action.should.equal('entity.error');
body[0].details.errorMessage.should.eql('Required parameter label missing.');
});
}));

Expand Down

0 comments on commit 16f844e

Please sign in to comment.