Skip to content

Commit

Permalink
Migrate entity.create.error to entity.error
Browse files Browse the repository at this point in the history
  • Loading branch information
ktuite committed Oct 13, 2023
1 parent bab9ef9 commit 7b2588c
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 28 deletions.
4 changes: 2 additions & 2 deletions docs/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ tags:
* `dataset.update` when a Dataset is updated.
* `dataset.update.publish` when a Dataset is published.
* `entity.create` when an Entity is created.
* `entity.create.error` when there is an error during entity creation process.
* `entity.error` when there is an error during entity creation process.
* `config.set` when a system configuration is set.
* `analytics` when a Usage Report is attempted.
* Deprecated: `backup` when a backup operation is attempted for Google Drive backups.
Expand Down Expand Up @@ -6819,7 +6819,7 @@ paths:
description: |-
Currently, the only updatable _metadata_ on a Submission is its `reviewState`. To update the submission _data_ itself, please see [Updating Submission data](/central-api-submission-management/#updating-submission-data).

Starting with Version 2022.3, changing the `reviewState` of a Submission to `approved` can create an Entity in a Dataset if the corresponding Form maps Dataset Properties to Form Fields. If an Entity is created successfully then an `entity.create` event is logged in Audit logs, else `entity.create.error` is logged.
Starting with Version 2022.3, changing the `reviewState` of a Submission to `approved` can create an Entity in a Dataset if the corresponding Form maps Dataset Properties to Form Fields. If an Entity is created successfully then an `entity.create` event is logged in Audit logs, else `entity.error` is logged.
operationId: Updating Submission metadata
parameters:
- name: projectId
Expand Down
19 changes: 19 additions & 0 deletions lib/model/migrations/20231013-01-change-entity-error-action.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2023 ODK Central Developers
// See the NOTICE file at the top-level directory of this distribution and at
// https://github.com/getodk/central-backend/blob/master/NOTICE.
// This file is part of ODK Central. It is subject to the license terms in
// the LICENSE file found in the top-level directory of this distribution and at
// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central,
// including this file, may be copied, modified, propagated, or distributed
// except according to the terms contained in the LICENSE file.

const up = async (db) => {
await db.raw('UPDATE audits SET "action" = \'entity.error\' WHERE "action" = \'entity.create.error\'');
};

const down = async (db) => {
// will set any error back to create error, which isn't necessarily right
await db.raw('UPDATE audits SET "action" = \'entity.create.error\' WHERE "action" = \'entity.error\'');
};

module.exports = { up, down };
2 changes: 1 addition & 1 deletion lib/model/query/analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ FROM datasets ds
JOIN submissions s ON CAST((a.details ->> 'submissionId'::TEXT) AS integer) = s.id
JOIN submission_defs sd ON sd."submissionId" = s.id AND sd."current"
JOIN dataset_form_defs dfd ON sd."formDefId" = dfd."formDefId"
WHERE a."action" = 'entity.create.error'
WHERE a."action" = 'entity.error'
GROUP BY dfd."datasetId"
) AS errors ON ds.id = errors."datasetId"
LEFT JOIN (
Expand Down
4 changes: 2 additions & 2 deletions lib/model/query/audits.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const actionCondition = (action) => {
// The backup action was logged by a backup script that has been removed.
// Even though the script has been removed, the audit log entries it logged
// have not, so we should continue to exclude those.
return sql`action not in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'backup', 'analytics')`;
return sql`action not in ('entity.create', 'entity.error', 'entity.update.version', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'backup', 'analytics')`;
else if (action === 'user')
return sql`action in ('user.create', 'user.update', 'user.delete', 'user.assignment.create', 'user.assignment.delete', 'user.session.create')`;
else if (action === 'field_key')
Expand All @@ -52,7 +52,7 @@ const actionCondition = (action) => {
else if (action === 'dataset')
return sql`action in ('dataset.create', 'dataset.update')`;
else if (action === 'entity')
return sql`action in ('entity.create', 'entity.create.error', 'entity.update.version', 'entity.delete')`;
return sql`action in ('entity.create', 'entity.error', 'entity.update.version', 'entity.delete')`;

return sql`action=${action}`;
};
Expand Down
2 changes: 1 addition & 1 deletion lib/model/query/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ const processSubmissionEvent = (event, parentEvent) => (container) =>
// so this probably isn't even an issue.
// The JSON.stringify appears to strip out error.stack so that doesn't make it to the
// log details even for our Problems.
container.Audits.log({ id: event.actorId }, 'entity.create.error', null,
container.Audits.log({ id: event.actorId }, 'entity.error', null,
{ submissionId: event.details.submissionId,
submissionDefId: event.details.submissionDefId,
errorMessage: err.message,
Expand Down
2 changes: 1 addition & 1 deletion test/integration/api/audits.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ describe('/audits', () => {
body.map(a => a.action).should.eql([
'entity.delete',
'entity.update.version',
'entity.create.error',
'entity.error',
'entity.create'
]);
});
Expand Down
2 changes: 1 addition & 1 deletion test/integration/api/datasets.js
Original file line number Diff line number Diff line change
Expand Up @@ -3129,7 +3129,7 @@ describe('datasets and entities', () => {
body.length.should.be.eql(1);
});

const entityErrors = await container.Audits.get(new QueryOptions({ args: { action: 'entity.create.error' } }));
const entityErrors = await container.Audits.get(new QueryOptions({ args: { action: 'entity.error' } }));

entityErrors.length.should.be.eql(1);
entityErrors[0].details.errorMessage.should.match(/Required parameter label missing/);
Expand Down
4 changes: 2 additions & 2 deletions test/integration/other/analytics-queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ describe('analytics task queries', function () {
await exhaust(container);

// let's set date of entity errors to long time ago
await container.run(sql`UPDATE audits SET "loggedAt" = '1999-1-1' WHERE action = 'entity.create.error'`);
await container.run(sql`UPDATE audits SET "loggedAt" = '1999-1-1' WHERE action = 'entity.error'`);

await submitToForm(service, 'alice', 1, 'simpleEntity', testData.instances.simpleEntity.three.replace(/bbb/, 'xxx'));
await asAlice.patch('/v1/projects/1/forms/simpleEntity/submissions/three').send({ reviewState: 'approved' });
Expand Down Expand Up @@ -1344,7 +1344,7 @@ describe('analytics task queries', function () {
await exhaust(container);

// Make the error ancient
await container.run(sql`UPDATE audits SET "loggedAt" = '1999-1-1' WHERE action = 'entity.create.error'`);
await container.run(sql`UPDATE audits SET "loggedAt" = '1999-1-1' WHERE action = 'entity.error'`);

// Create new Submission that will cause entity creation error
await submitToForm(service, 'alice', 1, 'simpleEntity', testData.instances.simpleEntity.three.replace(/bbb|three/g, 'xxx'));
Expand Down
36 changes: 18 additions & 18 deletions test/integration/worker/entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('worker: entity', () => {

// There should be no entity events logged.
const createEvent = await container.Audits.getLatestByAction('entity.create');
const errorEvent = await container.Audits.getLatestByAction('entity.create.error');
const errorEvent = await container.Audits.getLatestByAction('entity.error');
createEvent.isEmpty().should.equal(true);
errorEvent.isEmpty().should.equal(true);
}));
Expand Down Expand Up @@ -68,7 +68,7 @@ describe('worker: entity', () => {

// There should be no entity events logged.
const createEvent = await container.Audits.getLatestByAction('entity.create');
const errorEvent = await container.Audits.getLatestByAction('entity.create.error');
const errorEvent = await container.Audits.getLatestByAction('entity.error');
createEvent.isEmpty().should.equal(true);
errorEvent.isEmpty().should.equal(true);
}));
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('worker: entity', () => {

// There should be no entity events logged.
const createEvent = await container.Audits.getLatestByAction('entity.create');
const errorEvent = await container.Audits.getLatestByAction('entity.create.error');
const errorEvent = await container.Audits.getLatestByAction('entity.error');
createEvent.isEmpty().should.equal(true);
errorEvent.isEmpty().should.equal(true);
}));
Expand Down Expand Up @@ -138,7 +138,7 @@ describe('worker: entity', () => {
firstApproveEvent.id.should.not.equal(secondApproveEvent.id);

// there should be no log of an entity-creation error
const errorEvent = await container.Audits.getLatestByAction('entity.create.error');
const errorEvent = await container.Audits.getLatestByAction('entity.error');
errorEvent.isEmpty().should.be.true();
}));

Expand Down Expand Up @@ -179,7 +179,7 @@ describe('worker: entity', () => {
should.exist(secondApproveEvent.processed);

// there should be no log of an entity-creation error
const errorEvent = await container.Audits.getLatestByAction('entity.create.error');
const errorEvent = await container.Audits.getLatestByAction('entity.error');
errorEvent.isEmpty().should.be.true();
}));

Expand Down Expand Up @@ -317,7 +317,7 @@ describe('worker: entity', () => {
const createEvent = await container.Audits.getLatestByAction('entity.create');
createEvent.isEmpty().should.be.true();

const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(updateEvent.details.submissionId);
event.details.errorMessage.should.equal('Invalid input data type: expected (uuid) to be (valid UUID)');
Expand Down Expand Up @@ -345,7 +345,7 @@ describe('worker: entity', () => {
const createEvent = await container.Audits.getLatestByAction('entity.create');
createEvent.isEmpty().should.be.true();

const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(updateEvent.details.submissionId);
event.details.errorMessage.should.equal('Required parameter dataset missing.');
Expand Down Expand Up @@ -394,7 +394,7 @@ describe('worker: entity', () => {
updateEvent.failures.should.equal(0);

// the entity creation error should be logged
const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(updateEvent.details.submissionId);
event.details.errorMessage.should.equal('A resource already exists with uuid value(s) of 12345678-1234-4123-8234-123456789abc.');
Expand Down Expand Up @@ -423,7 +423,7 @@ describe('worker: entity', () => {
updateEvent.failures.should.equal(0);

// the entity creation error should be logged
const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(updateEvent.details.submissionId);
event.details.problem.problemCode.should.equal(404.7);
Expand All @@ -442,7 +442,7 @@ describe('worker: entity', () => {
updateEvent2.failures.should.equal(0);

// the entity creation error should be logged
const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
should.exist(event);
// The error in this case is not one of our Problems but an error thrown by slonik
// from passing in some broken (undefined/missing) value for submissionDefId.
Expand All @@ -454,7 +454,7 @@ describe('worker: entity', () => {
});

describe('should catch problems updating entity', () => {
// TODO: these errors are getting logged as entity.create.error audit events
// TODO: these errors are getting logged as entity.error audit events
describe('validation errors', () => {
it('should fail because UUID is invalid', testService(async (service, container) => {
const asAlice = await service.login('alice');
Expand Down Expand Up @@ -491,7 +491,7 @@ describe('worker: entity', () => {
const updateEvent = await container.Audits.getLatestByAction('entity.update');
updateEvent.isEmpty().should.be.true();

const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(subEvent.details.submissionId);
event.details.errorMessage.should.equal('Invalid input data type: expected (uuid) to be (valid UUID)');
Expand Down Expand Up @@ -533,7 +533,7 @@ describe('worker: entity', () => {
const udpateEvent = await container.Audits.getLatestByAction('entity.update');
udpateEvent.isEmpty().should.be.true();

const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(subEvent.details.submissionId);
event.details.errorMessage.should.equal('Required parameter dataset missing.');
Expand Down Expand Up @@ -575,7 +575,7 @@ describe('worker: entity', () => {
subEvent.failures.should.equal(0);

// the entity creation error should be logged
const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(subEvent.details.submissionId);
event.details.errorMessage.should.equal('Could not find the resource you were looking for.');
Expand Down Expand Up @@ -615,7 +615,7 @@ describe('worker: entity', () => {
subEvent.failures.should.equal(0);

// the entity creation error should be logged
const event = await container.Audits.getLatestByAction('entity.create.error').then((o) => o.get());
const event = await container.Audits.getLatestByAction('entity.error').then((o) => o.get());
event.actorId.should.equal(5); // Alice
event.details.submissionId.should.equal(subEvent.details.submissionId);
event.details.problem.problemCode.should.equal(404.7);
Expand Down Expand Up @@ -803,7 +803,7 @@ describe('worker: entity', () => {
.expect(200)
.then(({ body }) => body.length.should.be.eql(1));

const errors = await container.Audits.get(new QueryOptions({ args: { action: 'entity.create.error' } }));
const errors = await container.Audits.get(new QueryOptions({ args: { action: 'entity.error' } }));

errors.should.be.empty();

Expand Down Expand Up @@ -841,7 +841,7 @@ describe('worker: entity', () => {
await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc')
.expect(404);

const errors = await container.Audits.get(new QueryOptions({ args: { action: 'entity.create.error' } }));
const errors = await container.Audits.get(new QueryOptions({ args: { action: 'entity.error' } }));

errors.should.be.empty();

Expand Down Expand Up @@ -1057,7 +1057,7 @@ describe('worker: entity', () => {

// There should be no entity update events logged.
const createEvent = await container.Audits.getLatestByAction('entity.update.version');
const errorEvent = await container.Audits.getLatestByAction('entity.create.error');
const errorEvent = await container.Audits.getLatestByAction('entity.error');
createEvent.isEmpty().should.equal(true);
errorEvent.isEmpty().should.equal(true);
}));
Expand Down

0 comments on commit 7b2588c

Please sign in to comment.