diff --git a/docs/api.yaml b/docs/api.yaml index e366d61ba..2f14415fe 100644 --- a/docs/api.yaml +++ b/docs/api.yaml @@ -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. @@ -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 diff --git a/lib/model/migrations/20231013-01-change-entity-error-action.js b/lib/model/migrations/20231013-01-change-entity-error-action.js new file mode 100644 index 000000000..e52fbd6e8 --- /dev/null +++ b/lib/model/migrations/20231013-01-change-entity-error-action.js @@ -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 }; diff --git a/lib/model/query/analytics.js b/lib/model/query/analytics.js index cb92d921a..ecdb678db 100644 --- a/lib/model/query/analytics.js +++ b/lib/model/query/analytics.js @@ -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 ( diff --git a/lib/model/query/audits.js b/lib/model/query/audits.js index 24ae80cb8..8e961cc20 100644 --- a/lib/model/query/audits.js +++ b/lib/model/query/audits.js @@ -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') @@ -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}`; }; diff --git a/lib/model/query/entities.js b/lib/model/query/entities.js index 37827431c..0358b4d9d 100644 --- a/lib/model/query/entities.js +++ b/lib/model/query/entities.js @@ -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, diff --git a/test/integration/api/audits.js b/test/integration/api/audits.js index 5131cfe78..0cb8b70d6 100644 --- a/test/integration/api/audits.js +++ b/test/integration/api/audits.js @@ -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' ]); }); diff --git a/test/integration/api/datasets.js b/test/integration/api/datasets.js index d582e359b..ae23640a7 100644 --- a/test/integration/api/datasets.js +++ b/test/integration/api/datasets.js @@ -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/); diff --git a/test/integration/other/analytics-queries.js b/test/integration/other/analytics-queries.js index 344be5f1f..05a3c4f2f 100644 --- a/test/integration/other/analytics-queries.js +++ b/test/integration/other/analytics-queries.js @@ -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' }); @@ -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')); diff --git a/test/integration/worker/entity.js b/test/integration/worker/entity.js index 1cd92e37f..3e8bd2ae5 100644 --- a/test/integration/worker/entity.js +++ b/test/integration/worker/entity.js @@ -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); })); @@ -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); })); @@ -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); })); @@ -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(); })); @@ -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(); })); @@ -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)'); @@ -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.'); @@ -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.'); @@ -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); @@ -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. @@ -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'); @@ -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)'); @@ -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.'); @@ -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.'); @@ -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); @@ -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(); @@ -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(); @@ -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); }));