Skip to content

Commit

Permalink
Refactor Entities.createVersion (#1013)
Browse files Browse the repository at this point in the history
* Refactor Entities.createVersion

* moving conditional, adding validation

* Remove extra subdef.id check
  • Loading branch information
ktuite authored Oct 5, 2023
1 parent c870db9 commit 4b8549e
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 76 deletions.
2 changes: 1 addition & 1 deletion lib/model/frames/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Entity extends Frame.define(
});
const { uuid, label } = entityData.system;
const { data } = entityData;
return new Entity.Partial({ uuid, datasetId: dataset.id }, {
return new Entity.Partial({ uuid, datasetId: dataset.id, id: oldEntity?.id }, {
def: new Entity.Def({ data, label }),
dataset: dataset.name
});
Expand Down
165 changes: 91 additions & 74 deletions lib/model/query/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ const { isTrue } = require('../../util/http');
const Problem = require('../../util/problem');
const { runSequentially } = require('../../util/promise');


/////////////////////////////////////////////////////////////////////////////////
// ENTITY DEF SOURCES

const createSource = (details = null, subDefId = null, eventId = null) => ({ one }) => {
const type = (subDefId) ? 'submission' : 'api';
return one(sql`insert into entity_def_sources ("type", "submissionDefId", "auditId", "details")
values (${type}, ${subDefId}, ${eventId}, ${JSON.stringify(details)})
returning id`)
.then((row) => row.id);
};

////////////////////////////////////////////////////////////////////////////////
// ENTITY CREATE

Expand All @@ -37,15 +49,13 @@ const nextval = sql`nextval(pg_get_serial_sequence('entities', 'id'))`;
const createNew = (dataset, partial, subDef, sourceId, userAgentIn) => ({ one, context }) => {
let creatorId;
let userAgent;
let subDefId;

// Set creatorId and userAgent from submission def if it exists
if (subDef != null && subDef.id != null)
({ id: subDefId, submitterId: creatorId, userAgent } = subDef);
else {
if (subDef != null) {
({ submitterId: creatorId, userAgent } = subDef);
} else {
creatorId = context.auth.actor.map((actor) => actor.id).orNull();
userAgent = blankStringToNull(userAgentIn);
subDefId = null;
}

const json = JSON.stringify(partial.def.data);
Expand All @@ -60,17 +70,13 @@ select ins.*, def.id as "entityDefId" from ins, def;`)
new Entity(entityData, {
currentVersion: new Entity.Def({
id: entityDefId,
entityId: entityData.id,
submissionDefId: subDefId,
data: partial.def.data,
label: partial.def.label
entityId: entityData.id
})
}));
};


createNew.audit = (newEntity, dataset, partial, subDef) => (log) => {
if (!subDef)
if (!subDef) // entities created from submissions are logged elsewhere
return log('entity.create', dataset, {
entityId: newEntity.id, // Added in v2023.3 and backfilled
entityDefId: newEntity.aux.currentVersion.id, // Added in v2023.3 and backfilled
Expand All @@ -80,20 +86,59 @@ createNew.audit = (newEntity, dataset, partial, subDef) => (log) => {
createNew.audit.withResult = true;


// Creates a source entry for entities created through audit events and submissions
const createSource = (details = null, subDefId = null, eventId = null) => ({ one }) => {
const type = (subDefId) ? 'submission' : 'api';
return one(sql`insert into entity_def_sources ("type", "submissionDefId", "auditId", "details")
values (${type}, ${subDefId}, ${eventId}, ${JSON.stringify(details)})
returning id`)
.then((row) => row.id);
////////////////////////////////////////////////////////////////////////////////
// ENTITY UPDATE

const createVersion = (dataset, partial, subDef, version, sourceId, userAgentIn = null) => ({ context, one }) => {
let creatorId;
let userAgent;

// Set creatorId and userAgent from submission def if it exists
if (subDef != null) {
({ submitterId: creatorId, userAgent } = subDef);
} else {
creatorId = context.auth.actor.map((actor) => actor.id).orNull();
userAgent = blankStringToNull(userAgentIn);
}

const json = JSON.stringify(partial.def.data);

const _unjoiner = unjoiner(Entity, Entity.Def.into('currentVersion'));

return one(sql`
with def as (${_defInsert(partial.id, false, creatorId, userAgent, partial.def.label, json, version, sourceId)}),
upd as (update entity_defs set current=false where entity_defs."entityId" = ${partial.id}),
entities as (update entities set "updatedAt"=clock_timestamp()
where "uuid"=${partial.uuid}
returning *)
select ${_unjoiner.fields} from def as entity_defs
join entities on entity_defs."entityId" = entities.id
`)
.then(_unjoiner);
};

createVersion.audit = (updatedEntity, dataset, partial, subDef) => (log) => {
if (!subDef) // entities updated from submissions are logged elsewhere
return log('entity.update.version', dataset, {
entityId: updatedEntity.id,
entityDefId: updatedEntity.aux.currentVersion.id,
entity: { uuid: updatedEntity.uuid, dataset: dataset.name }
});
};
createVersion.audit.withResult = true;


//
// Entity event processing
//

const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent) => async ({ Entities, Audits }) => {
/////////////////////////////////////////////////////////////////////////
// Processing submission events to create and update entities

const _createEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent) => 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') ||
(!dataset.approvalRequired && event.action === 'submission.update')) // don't process submission if approval is not required and submission metadata is updated
return null;

const partial = await Entity.fromParseEntityData(entityData);

const sourceDetails = { submission: { instanceId: submissionDef.instanceId }, parentEventId: parentEvent ? parentEvent.id : undefined };
Expand All @@ -110,24 +155,38 @@ const _createEntity = (dataset, entityData, submissionId, submissionDef, submiss
});
};

const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event) => async ({ Entities }) => {
const _updateEntity = (dataset, entityData, submissionId, submissionDef, submissionDefId, event) => async ({ Audits, Entities }) => {
// Get client version of entity
const clientEntity = await Entity.fromParseEntityData(entityData); // validation happens here

// Get version of entity on the server
const serverEntity = await Entities.getById(dataset.id, entityData.system.id).then(o => o.get());
const serverEntity = await Entities.getById(dataset.id, clientEntity.uuid).then(o => o.get());

// merge data
const mergedData = mergeRight(serverEntity.aux.currentVersion.data, entityData.data);
const mergedLabel = entityData.system.label || serverEntity.aux.currentVersion.label;
const mergedData = mergeRight(serverEntity.aux.currentVersion.data, clientEntity.def.data);
const mergedLabel = clientEntity.def.label || serverEntity.aux.currentVersion.label;

// make some kind of source object
const sourceDetails = {
submission: { instanceId: submissionDef.instanceId }
};
const sourceId = await Entities.createSource(sourceDetails, submissionDefId, event.id);
const partial = new Entity.Partial(serverEntity, {
def: new Entity.Def({
data: mergedData,
label: mergedLabel
})
});

// TODO: createVersion infers creator id from context.auth, shoving the submitter id in as another arg
// should be refactored
return Entities.createVersion(dataset, serverEntity, mergedData, mergedLabel, serverEntity.aux.currentVersion.version + 1, sourceId, null, submissionDef.submitterId);
const entity = await Entities.createVersion(dataset, partial, submissionDef, serverEntity.aux.currentVersion.version + 1, sourceId);
return Audits.log({ id: event.actorId }, 'entity.update.version', { acteeId: dataset.acteeId },
{
entityId: entity.id,
entityDefId: entity.aux.currentVersion.id,
entity: { uuid: entity.uuid, dataset: dataset.name },
submissionId,
submissionDefId
});
};

// Entrypoint to where submissions (a specific version) become entities
Expand Down Expand Up @@ -167,27 +226,18 @@ const _processSubmissionEvent = (event, parentEvent) => async ({ Datasets, Entit

const entityData = await parseSubmissionXml(fields, submissionDef.xml);

// If dataset requires approval on submission to create an entity and this event is not
// an approval event, then don't create an entity
//
// We can't simply use submissionDefId to trace back to dataset and find out if approval
// is required because in future there can be multiple entities in a single submission.
// So we have to rely on dataset name from the xml to fetch the dataset.approvalRequired flag.
// We have to look up the dataset based on the name in the XML
if (!entityData.system.dataset || entityData.system.dataset.trim() === '') {
throw Problem.user.missingParameter({ field: 'dataset' });
}

const dataset = (await Datasets.get(form.get().projectId, entityData.system.dataset, true))
.orThrow(Problem.user.datasetNotFound({ datasetName: entityData.system.dataset }));

if ((dataset.approvalRequired && event.details.reviewState !== 'approved') ||
(!dataset.approvalRequired && event.action === 'submission.update')) // don't process submission if approval is not required and submission metadata is updated
return null;

// If create is not true (either 1 or true) then we don't need to process further
// Create entity
if (entityData.system.create === '1' || entityData.system.create === 'true')
return Entities._createEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event, parentEvent);

// Or update entity
if (entityData.system.update === '1' || entityData.system.update === 'true')
return Entities._updateEntity(dataset, entityData, submissionId, submissionDef, submissionDefId, event);

Expand Down Expand Up @@ -220,39 +270,6 @@ const createEntitiesFromPendingSubmissions = (submissionEvents, parentEvent) =>





////////////////////////////////////////////////////////////////////////////////
// UPDATING ENTITIES

const createVersion = (dataset, entity, data, label, version, sourceId, userAgentIn = null, submitterId = null) => ({ context, one }) => {
// dataset is passed in so the audit log can use its actee id
const creatorId = (context.auth ? context.auth.actor.map((actor) => actor.id).orNull() : submitterId);
const userAgent = blankStringToNull(userAgentIn);
const json = JSON.stringify(data);

const _unjoiner = unjoiner(Entity, Entity.Def.into('currentVersion'));

return one(sql`
with def as (${_defInsert(entity.id, false, creatorId, userAgent, label, json, version, sourceId)}),
upd as (update entity_defs set current=false where entity_defs."entityId" = ${entity.id}),
entities as (update entities set "updatedAt"=clock_timestamp()
where "uuid"=${entity.uuid}
returning *)
select ${_unjoiner.fields} from def as entity_defs
join entities on entity_defs."entityId" = entities.id
`)
.then(_unjoiner);
};

createVersion.audit = (newEntity, dataset, entity) => (log) =>
log('entity.update.version', dataset, {
entityId: entity.id,
entityDefId: newEntity.aux.currentVersion.id,
entity: { uuid: newEntity.uuid, dataset: dataset.name }
});
createVersion.audit.withResult = true;

////////////////////////////////////////////////////////////////////////////////
// GETTING ENTITIES

Expand Down
2 changes: 1 addition & 1 deletion lib/resources/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ module.exports = (service, endpoint) => {

const sourceId = await Entities.createSource();

return Entities.createVersion(dataset, entity, partial.def.data, partial.def.label, serverEntityVersion + 1, sourceId, userAgent);
return Entities.createVersion(dataset, partial, null, serverEntityVersion + 1, sourceId, userAgent);
}));

service.delete('/projects/:projectId/datasets/:name/entities/:uuid', endpoint(async ({ Datasets, Entities }, { auth, params, queryOptions }) => {
Expand Down
8 changes: 8 additions & 0 deletions test/integration/worker/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,14 @@ describe('worker: entity', () => {
person.currentVersion.version.should.equal(2);
});

await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/audits')
.expect(200)
.then(({ body: logs }) => {
logs[0].should.be.an.Audit();
logs[0].action.should.be.eql('entity.update.version');
logs[0].actor.displayName.should.be.eql('Alice');
});

// update again
await asAlice.post('/v1/projects/1/forms/updateEntity/submissions')
.send(testData.instances.updateEntity.one
Expand Down

0 comments on commit 4b8549e

Please sign in to comment.