Migrate entity.create.error audit action to entity.error (#1027)

* Migrate entity.create.error to entity.error

* Change entity update uuid not found to be more specific

* Updated entity actions in api docs

docs/api.yaml

@@ -814,7 +814,10 @@ 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 processing a Submission to create or update an Entity.
+    * `entity.update.version` when an Entity is updated.
+    * `entity.update.resolve` when an Entity conflict is resolved.
+    * `entity.delete` when an Entity is deleted.
     * `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.
@@ -6831,7 +6834,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

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 };

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 (

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.update.resolve', '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.update.resolve', '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.update.resolve', 'entity.delete')`;
+    return sql`action in ('entity.create', 'entity.error', 'entity.update.version', 'entity.update.resolve', 'entity.delete')`;
 
   return sql`action=${action}`;
 };

lib/model/query/entities.js

@@ -13,7 +13,6 @@ const { equals, extender, unjoiner, page, markDeleted } = require('../../util/db
 const { map, mergeRight, pickAll } = require('ramda');
 const { blankStringToNull, construct } = require('../../util/util');
 const { QueryOptions } = require('../../util/db');
-const { getOrNotFound } = require('../../util/promise');
 const { odataFilter } = require('../../data/odata-filter');
 const { odataToColumnMap, parseSubmissionXml, getDiffProp, ConflictType } = require('../../data/entity');
 const { isTrue } = require('../../util/http');
@@ -177,7 +176,8 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
   const clientEntity = await Entity.fromParseEntityData(entityData); // validation happens here
 
   // Get version of entity on the server
-  const serverEntity = await Entities.getById(dataset.id, clientEntity.uuid).then(getOrNotFound);
+  const serverEntity = (await Entities.getById(dataset.id, clientEntity.uuid))
+    .orThrow(Problem.user.entityNotFound({ entityUuid: clientEntity.uuid, datasetName: dataset.name }));
 
   let { conflict } = serverEntity;
   let conflictingProperties; // Maybe we don't need to persist this??? just compute at the read time
@@ -298,7 +298,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,

lib/util/problem.js

@@ -157,6 +157,9 @@ const problems = {
     // dataset in submission does not exist
     datasetNotFound: problem(404.7, ({ datasetName }) => `The dataset (${datasetName}) specified in the submission does not exist.`),
 
+    // entity in submission does not exist
+    entityNotFound: problem(404.8, ({ entityUuid, datasetName }) => `The entity with UUID (${entityUuid}) specified in the submission does not exist in the dataset (${datasetName}).`),
+
     // { allowed: [ acceptable formats ], got }
     unacceptableFormat: problem(406.1, ({ allowed }) => `Requested format not acceptable; this resource allows: ${allowed.join()}.`),
 

test/integration/api/audits.js

@@ -414,7 +414,7 @@ describe('/audits', () => {
             'entity.update.resolve',
             'entity.update.version',
             'entity.update.version',
-            'entity.create.error',
+            'entity.error',
             'entity.create'
           ]);
         });

test/integration/api/datasets.js

@@ -3149,7 +3149,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/);

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'));

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.');
@@ -574,12 +574,12 @@ describe('worker: entity', () => {
         should.exist(subEvent.processed);
         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());
+        // the entity processing error should be logged
+        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.');
-        event.details.problem.problemCode.should.equal(404.1);
+        event.details.problem.problemCode.should.equal(404.8);
+        event.details.errorMessage.should.equal('The entity with UUID (12345678-1234-4123-8234-123456789abc) specified in the submission does not exist in the dataset (people).');
       }));
 
       it('should fail for other constraint errors like dataset name does not exist', testService(async (service, container) => {
@@ -614,8 +614,8 @@ describe('worker: entity', () => {
         should.exist(subEvent.processed);
         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());
+        // the entity processing error should be logged
+        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);
     }));