[O2B-1621] Associate eor reasons to user by user id and not name

docs/data-model.md

@@ -101,7 +101,7 @@ Concerning the **Update mode** of the fields:
 | `description`      | Other information on the reason               | `Run stopped due to faulty detector` | AT COE                 | `description`    | Insert          |
 | `reason_type_id`   | Id of the general reason type belonging to    | '1'                                  | AT COE                 | `reason_type_id` | Insert          |
 | `run_id`           | RUN id for which the reason was added         | `500540`                             | AT COE                 | `run_id`         | Insert          |
-| `last_edited_name` | Name of the person who last edited the fields | `Anonymous`, `Jan Janssen`           | When fields are edited | `id`             | Update          |
+| `last_edited_by_user_id` | Id (in `users` table) of the user who last edited the fields | `1`, `2`         | When fields are edited | `id`             | Update          |
 | `created_at`       | When the entity is created                    |                                      | AT COE                 | `created_at`     | Insert          |
 | `updated_at`       | When entity is edited                         |                                      | When fields are edited | `updated_at`     | Update          |
 

lib/database/adapters/EorReasonAdapter.js

@@ -24,6 +24,11 @@ class EorReasonAdapter {
          */
         this.reasonTypeAdapter = null;
 
+        /**
+         * @type {UserAdapter|null}
+         */
+        this.userAdapter = null;
+
         this.toEntity = this.toEntity.bind(this);
         this.toDatabase = this.toDatabase.bind(this);
     }
@@ -33,11 +38,11 @@ class EorReasonAdapter {
      * @param {SequelizeEorReason} databaseObject Object to convert.
      * @returns {EorReason} Converted entity object.
      */
-    toEntity({ id, description, runId, reasonTypeId, reasonType, lastEditedName, createdAt, updatedAt }) {
+    toEntity({ id, description, runId, reasonTypeId, reasonType, lastEditedBy, createdAt, updatedAt }) {
         const entityObject = {
             id,
             description,
-            lastEditedName,
+            lastEditedBy: lastEditedBy ? this.userAdapter.toNameOnly(lastEditedBy) : null,
             reasonTypeId,
             runId,
             createdAt: new Date(createdAt).getTime(),
@@ -62,7 +67,7 @@ class EorReasonAdapter {
         return {
             id: entityObject.id,
             description: entityObject.description,
-            lastEditedName: entityObject.lastEditedName,
+            lastEditedByUserId: entityObject.lastEditedByUserId,
             reasonTypeId: entityObject.reasonTypeId,
             runId: entityObject.runId,
         };

lib/database/adapters/index.js

@@ -105,6 +105,7 @@ environmentAdapter.environmentHistoryItemAdapter = environmentHistoryItemAdapter
 environmentAdapter.runAdapter = runAdapter;
 
 eorReasonAdapter.reasonTypeAdapter = reasonTypeAdapter;
+eorReasonAdapter.userAdapter = userAdapter;
 
 flpRoleAdapter.runAdapter = runAdapter;
 

lib/database/migrations/v1/20261001100000-eor-reasons-last-edited-by-user-id.js

@@ -0,0 +1,56 @@
+/*
+ *  @license
+ *  Copyright CERN and copyright holders of ALICE O2. This software is
+ *  distributed under the terms of the GNU General Public License v3 (GPL
+ *  Version 3), copied verbatim in the file "COPYING".
+ *
+ *  See http://alice-o2.web.cern.ch/license for full licensing information.
+ *
+ *  In applying this license CERN does not waive the privileges and immunities
+ *  granted to it by virtue of its status as an Intergovernmental Organization
+ *  or submit itself to any jurisdiction.
+ */
+
+'use strict';
+
+/** @type {import('sequelize-cli').Migration} */
+module.exports = {
+    up: async (queryInterface, Sequelize) => queryInterface.sequelize.transaction(async (transaction) => {
+        await queryInterface.addColumn('eor_reasons', 'last_edited_by_user_id', {
+            type: Sequelize.INTEGER,
+            allowNull: true,
+            references: {
+                model: 'users',
+                key: 'id',
+            },
+            onUpdate: 'CASCADE',
+            onDelete: 'SET NULL',
+        }, { transaction });
+
+        // Link existing EoR reasons to the user matching the stored name (ambiguous names resolve to the oldest user)
+        await queryInterface.sequelize.query(
+            `UPDATE eor_reasons e
+            SET e.last_edited_by_user_id = (SELECT MIN(u.id) FROM users u WHERE u.name = e.last_edited_name)
+            WHERE e.last_edited_name IS NOT NULL`,
+            { transaction },
+        );
+
+        await queryInterface.removeColumn('eor_reasons', 'last_edited_name', { transaction });
+    }),
+
+    down: async (queryInterface, Sequelize) => queryInterface.sequelize.transaction(async (transaction) => {
+        await queryInterface.addColumn('eor_reasons', 'last_edited_name', {
+            type: Sequelize.STRING,
+            allowNull: true,
+        }, { transaction });
+
+        await queryInterface.sequelize.query(
+            `UPDATE eor_reasons e
+            INNER JOIN users u ON u.id = e.last_edited_by_user_id
+            SET e.last_edited_name = u.name`,
+            { transaction },
+        );
+
+        await queryInterface.removeColumn('eor_reasons', 'last_edited_by_user_id', { transaction });
+    }),
+};

lib/database/models/eorreason.js

@@ -24,8 +24,9 @@ module.exports = (sequelize) => {
         description: {
             type: Sequelize.STRING,
         },
-        lastEditedName: {
-            type: Sequelize.STRING,
+        lastEditedByUserId: {
+            type: Sequelize.INTEGER,
+            allowNull: true,
         },
         reasonTypeId: {
             type: Sequelize.INTEGER,
@@ -38,6 +39,7 @@ module.exports = (sequelize) => {
     EorReason.associate = (models) => {
         EorReason.belongsTo(models.Run);
         EorReason.belongsTo(models.ReasonType, { as: 'reasonType' });
+        EorReason.belongsTo(models.User, { as: 'lastEditedBy', foreignKey: 'lastEditedByUserId' });
     };
 
     return EorReason;

lib/database/models/typedefs/SequelizeEorReason.js

@@ -16,7 +16,8 @@
  *
  * @property {number} id
  * @property {string|null} description
- * @property {string|null} lastEditedName
+ * @property {number|null} lastEditedByUserId
+ * @property {SequelizeUser|null} [lastEditedBy]
  * @property {number} reasonTypeId
  * @property {number} runId
  * @property {string} createdAt

lib/database/seeders/20220513153907-eor-reason.js

@@ -20,29 +20,29 @@ module.exports = {
                 queryInterface.bulkInsert('eor_reasons', [
                     {
                         description: 'Some Reason other than selected',
-                        last_edited_name: 'Anonymous',
+                        last_edited_by_user_id: 3,
                         run_id: 1,
                         reason_type_id: 1,
                         created_at: new Date('2022-08-09'),
                         updated_at: new Date('2022-08-10 15:00:00'),
                     },
                     {
                         description: 'Some Reason other than selected plus one',
-                        last_edited_name: 'Anonymous',
+                        last_edited_by_user_id: 3,
                         reason_type_id: 2,
                         run_id: 1,
                         created_at: new Date('2022-08-09'),
                         updated_at: new Date('2022-08-10 05:00:00'),
                     },
                     {
-                        last_edited_name: 'Anonymous',
+                        last_edited_by_user_id: 3,
                         reason_type_id: 3,
                         run_id: 56,
                         created_at: new Date('2021-08-09'),
                         updated_at: new Date('2021-08-10 15:00:00'),
                     },
                     {
-                        last_edited_name: 'Anonymous',
+                        last_edited_by_user_id: 3,
                         reason_type_id: 1,
                         run_id: 56,
                         created_at: new Date('2021-07-09'),

lib/domain/entities/EorReason.js

@@ -16,7 +16,7 @@
  *
  * @property {number} id
  * @property {string|null} description
- * @property {string|null} lastEditedName
+ * @property {{name: string}|null} lastEditedBy the name of the user who last edited the EoR reason
  * @property {number} reasonTypeId
  * @property {number} runId
  * @property {number} [createdAt]

lib/public/views/Runs/Details/RunPatch.js

@@ -9,7 +9,7 @@ import { RunQualities } from '../../../domain/enums/RunQualities.js';
  * @property {string} category
  * @property {string} title
  * @property {string} description
- * @property {string|null} [lastEditedName]
+ * @property {{name: string}|null} [lastEditedBy]
  */
 
 /**
@@ -76,7 +76,7 @@ export class RunPatch extends Observable {
         }
 
         if (this._eorReasons.length !== this._run.eorReasons.length || this._eorReasons.some(({ id }) => id === undefined)) {
-            // Strip lastEditedName — the server's EorReasonDto only accepts id, reasonTypeId, and description
+            // Strip lastEditedBy — the server's EorReasonDto only accepts id, reasonTypeId, and description
             ret.eorReasons = this._eorReasons.map(({ id, reasonTypeId, description }) => ({ id, reasonTypeId, description }));
         }
 
@@ -128,11 +128,11 @@ export class RunPatch extends Observable {
         } = this._run || {};
 
         this._runQuality = runQuality;
-        this._eorReasons = eorReasons.map(({ id, description, reasonTypeId, lastEditedName }) => ({
+        this._eorReasons = eorReasons.map(({ id, description, reasonTypeId, lastEditedBy }) => ({
             id,
             description,
             reasonTypeId,
-            lastEditedName,
+            lastEditedBy,
         }));
         this._tags = tags.map(({ text }) => text);
 

lib/public/views/Runs/format/editRunEorReasons.js

@@ -94,7 +94,7 @@ export const editRunEorReasons = (runDetailsModel) => {
                  */
                 runDetailsModel.runPatch.eorReasons.length > 0
                     ? runDetailsModel.runPatch.eorReasons.map((eorReason) => {
-                        const { reasonTypeId, description, lastEditedName } = eorReason;
+                        const { reasonTypeId, description, lastEditedBy } = eorReason;
                         const { category = '-', title } = eorReasonTypes.find((eorReasonType) => eorReasonType.id === reasonTypeId) || {};
                         const titleString = title ? ` - ${title}` : '';
                         const descriptionString = description ? ` - ${description}` : '';
@@ -110,7 +110,7 @@ export const editRunEorReasons = (runDetailsModel) => {
                                     }, iconTrash()),
                                     h('.w-wrapped', `${category} ${titleString} ${descriptionString}`),
                                 ]),
-                                h('.w-wrapped', lastEditedName || null),
+                                h('.w-wrapped', lastEditedBy?.name || null),
                             ],
                         );
                     })

lib/public/views/Runs/format/formatRunEorReason.js

@@ -16,21 +16,21 @@ import { tooltip } from '../../../../components/common/popover/tooltip.js';
 import { formatEorReason } from './formatEorReason.mjs';
 
 /**
- * Display the given EoR reason as a vnode component with lastEditedName tooltip
+ * Display the given EoR reason as a vnode component with a tooltip containing the name of its last editor
  *
  * @param {Partial<{
  *   category: string,
  *   title: string,
  *   description: string,
- *   lastEditedName: string,
+ *   lastEditedBy: {name: string}|null,
  * }>} eorReason the EoR reason to display
  * @return {VNode} the vnode component
  */
 export const formatRunEorReason = (eorReason) => {
-    const { lastEditedName } = eorReason;
+    const lastEditorName = eorReason.lastEditedBy?.name;
     const reasonText = formatEorReason(eorReason);
     return h('.w-100.flex-row.justify-between', [
         h('', reasonText),
-        lastEditedName ? tooltip(h('.w-wrapped', lastEditedName), 'Last edited by') : null,
+        lastEditorName ? tooltip(h('.w-wrapped', lastEditorName), 'Last edited by') : null,
     ]);
 };

lib/server/controllers/runs.controller.js

@@ -241,12 +241,11 @@ const updateRun = async (request, response) => {
         return;
     }
 
-    const { result: run, error } = await new UpdateRunUseCase().execute(value);
-
-    if (error) {
-        response.status(Number(error.status)).json({ errors: [error] });
-    } else {
+    try {
+        const run = await new UpdateRunUseCase().execute(value);
         response.status(201).json({ data: runToHttpView(run) });
+    } catch (error) {
+        updateExpressResponseFromNativeError(response, error);
     }
 };
 
@@ -265,12 +264,11 @@ const updateRunByRunNumber = async (request, response) => {
         return;
     }
 
-    const { result: run, error } = await new UpdateRunUseCase().execute(value);
-
-    if (error) {
-        response.status(Number(error.status)).json({ errors: [error] });
-    } else {
+    try {
+        const run = await new UpdateRunUseCase().execute(value);
         response.status(200).json({ data: runToHttpView(run) });
+    } catch (error) {
+        updateExpressResponseFromNativeError(response, error);
     }
 };
 

lib/server/services/run/RunService.js

@@ -384,7 +384,7 @@ class RunService {
 
             // Update EOR reasons if they are provided
             if (eorReasons) {
-                await updateEorReasonsOnRun(run.id, run.runNumber, user?.name, eorReasons, transaction);
+                await updateEorReasonsOnRun(run.id, run.runNumber, user, eorReasons, transaction);
             }
 
             // Update detector qualities if they are provided
@@ -434,7 +434,13 @@ class RunService {
             queryBuilder.include('runType');
         }
         if (relations.eorReasons) {
-            queryBuilder.include({ association: 'eorReasons', include: { model: ReasonType, as: 'reasonType' } });
+            queryBuilder.include({
+                association: 'eorReasons',
+                include: [
+                    { model: ReasonType, as: 'reasonType' },
+                    { association: 'lastEditedBy', attributes: ['name'] },
+                ],
+            });
         }
         if (relations.flpRoles) {
             queryBuilder.include('flpRoles');
@@ -570,13 +576,13 @@ class RunService {
  *
  * @param {number} runId - id of the run that is due to be modified
  * @param {number} runNumber - run number of the run that is due to be modified
- * @param {string} userName - name of the user editing the EOR reasons
+ * @param {SequelizeUser|null} user - the user editing the EOR reasons, null if the change is not done by a user (e.g. automatic EoR reasons)
  * @param {EorReasonPatch[]} eorReasonsPatches - full list of EoR reasons to apply on the run (any existing EoR reason not in the list will be
  *                                              removed)
  * @param {import('sequelize').Transaction} [transaction] optional transaction in which operations must be wrapped
  * @returns {Promise<undefined|Error>} - promise on result of db queries
  */
-const updateEorReasonsOnRun = async (runId, runNumber, userName, eorReasonsPatches, transaction) => {
+const updateEorReasonsOnRun = async (runId, runNumber, user, eorReasonsPatches, transaction) => {
     const reasonTypes = await ReasonTypeRepository.findAll();
     const idsOfReasonTypesToLog = [];
 
@@ -628,7 +634,7 @@ const updateEorReasonsOnRun = async (runId, runNumber, userName, eorReasonsPatch
         if (id) {
             toKeepEorReasonsIds.push(id);
         } else {
-            newEorReasons.push({ runId, reasonTypeId, description, lastEditedName: userName });
+            newEorReasons.push({ runId, reasonTypeId, description, lastEditedByUserId: user?.id ?? null });
 
             if (idsOfReasonTypesToLog.includes(reasonTypeId)) {
                 needLoggingForEorReason = true;
@@ -642,7 +648,7 @@ const updateEorReasonsOnRun = async (runId, runNumber, userName, eorReasonsPatch
     if (needLoggingForEorReason) {
         await logEorReasonChange(
             runNumber,
-            userName,
+            user?.name,
             eorReasons.map(({ reasonTypeId, description }) => {
                 const eorReasonType = reasonTypesMap.get(reasonTypeId) ?? {};
                 return { category: eorReasonType.category, title: eorReasonType.title, description };

lib/usecases/run/GetRunUseCase.js

@@ -47,10 +47,16 @@ class GetRunUseCase {
             .include({
                 model: EorReason,
                 as: 'eorReasons',
-                include: {
-                    model: ReasonType,
-                    as: 'reasonType',
-                },
+                include: [
+                    {
+                        model: ReasonType,
+                        as: 'reasonType',
+                    },
+                    {
+                        association: 'lastEditedBy',
+                        attributes: ['name'],
+                    },
+                ],
             })
             .include('lhcFill')
             .include('lhcPeriod')