[O2B-1366] Fixed user starting/stopping runs not being properly extracted

lib/server/kafka/AliEcsSynchronizer.js

@@ -74,17 +74,27 @@
         this.ecsRunConsumer = new AliEcsEventMessagesConsumer(kafkaClient, RUN_CONSUMER_GROUP, RUN_TOPICS);
         this.ecsRunConsumer.onMessageReceived(async (eventMessage) => {
             const { timestamp, runEvent: { environmentId, runNumber, state, transition, lastRequestUser } } = eventMessage;
-            const { externalId: externalUserId } = lastRequestUser ?? {};
+            const { externalId: externalUserId, name: userName } = lastRequestUser ?? {};
 
             if (state === 'CONFIGURED' && transition === 'START_ACTIVITY') {
                 runService
-                    .createOrUpdate(runNumber, environmentId, { timeO2Start: timestamp.toNumber() }, { userO2Start: { externalUserId } })
+                    .createOrUpdate(
+                        runNumber,
+                        environmentId,
+                        { timeO2Start: timestamp.toNumber() },
+                        { userO2Start: { userIdentifier: { externalUserId }, name: userName } },
+                    )
                     .catch((error) => this._logger.errorMessage(`Failed to save run start for ${runNumber}: ${error.message}`));
             }
 
             if (state === 'RUNNING' && transition === 'STOP_ACTIVITY') {
                 runService
-                    .createOrUpdate(runNumber, environmentId, { timeO2End: timestamp.toNumber() }, { userO2Stop: { externalUserId } })
+                    .createOrUpdate(
+                        runNumber,
+                        environmentId,
+                        { timeO2End: timestamp.toNumber() },
+                        { userO2Stop: { userIdentifier: { externalUserId }, name: userName } },
+                    )
                     .catch((error) => this._logger.errorMessage(`Failed to save run end for ${runNumber}: ${error.message}`));
             }
         });

lib/server/services/run/RunService.js

@@ -45,6 +45,12 @@ const { flpRoleService } = require('../flp/FlpRoleService.js');
  * @property {number} [runId] the id of the run, ignored if runNumber is present
  */
 
+/**
+ * @typedef GetOrCreateUserPayload object used to uniquely identify a user or create one if it do not exists yet
+ * @property {UserIdentifier} userIdentifier the unique user identifier
+ * @property {string|null} name the name of the user to create
+ */
+
 /**
  * @typedef RunRelationsToInclude object specifying which run relations should be fetched alongside the run
  * @property {boolean} [tags] if true, related tags will be fetched alongside the run
@@ -207,8 +213,8 @@ class RunService {
      * @param {string} environmentId the id of the environment containing the run
      * @param {Partial<Run>} [partialRun] optional runs properties to define
      * @param {Object} [relations] run relations to create/update
-     * @param {UserIdentifier} [relations.userO2Start] if not null, the identifier of the user that started the run
-     * @param {UserIdentifier} [relations.userO2Stop] if not null, the identifier of the user that stopped the run
+     * @param {GetOrCreateUserPayload} [relations.userO2Start] if not null, the identifier of the user that started the run
+     * @param {GetOrCreateUserPayload} [relations.userO2Stop] if not null, the identifier of the user that stopped the run
      * @return {Promise<Run>} resolves with the run
      */
     createOrUpdate(runNumber, environmentId, partialRun, relations) {
@@ -261,8 +267,8 @@ class RunService {
      * @param {Object} [relations={}] the run's relations
      * @param {string|null} [relations.runTypeName=null] if not null, the name of the created run's type
      * @param {string|null} [relations.lhcPeriodName=null] if not null, the name of lhc period for which the run should be assigned
-     * @param {UserIdentifier|null} [relations.userO2Start=null] if not null, the identifier of the user that started the run
-     * @param {UserIdentifier|null} [relations.userO2Stop=null] if not null, the identifier of the user that stopped the run
+     * @param {GetOrCreateUserPayload|null} [relations.userO2Start=null] if not null, the identifier of the user that started the run
+     * @param {GetOrCreateUserPayload|null} [relations.userO2Stop=null] if not null, the identifier of the user that stopped the run
      * @return {Promise<Run>} resolve with the created run instance
      */
     async create(newRun, relations) {
@@ -285,10 +291,10 @@ class RunService {
 
             // Update the users for the run
             if (userO2Start) {
-                newRun.userIdO2Start = (await getOrCreateUser(userO2Start))?.id;
+                newRun.userIdO2Start = (await userService.getOrCreate(userO2Start.userIdentifier, userO2Start.name)).id;
             }
             if (userO2Stop) {
-                newRun.userIdO2Stop = (await getOrCreateUser(userO2Stop))?.id;
+                newRun.userIdO2Stop = (await userService.getOrCreate(userO2Stop.userIdentifier, userO2Stop.name)).id;
             }
 
             const runId = await createRun(runAdapter.toDatabase(newRun), { detectors, runType });
@@ -322,8 +328,8 @@ class RunService {
      * @param {UserIdentifier} [payload.relations.userIdentifier] if not null, the identifier of the user requesting the run update
      * @param {{detectorId: number, quality: string}[]} [payload.relations.detectorsQualities] an optional list representing the new quality of
      *     the run's detector (the run must be related to the given detector, the detectors not in this list keep their original quality)
-     * @param {UserIdentifier|null} [payload.relations.userO2Start=null] if not null, the identifier of the user that started the run
-     * @param {UserIdentifier|null} [payload.relations.userO2Stop=null] if not null, the identifier of the user that stopped the run
+     * @param {GetOrCreateUserPayload|null} [payload.relations.userO2Start=null] if not null, the identifier of the user that started the run
+     * @param {GetOrCreateUserPayload|null} [payload.relations.userO2Stop=null] if not null, the identifier of the user that stopped the run
      * @param {string} [payload.relations.lhcPeriodName] if not null, the name of lhc period for which the run should be assigned
      * @return {Promise<Run>} resolve with the resulting run
      */
@@ -354,10 +360,10 @@ class RunService {
 
         // Update the users for the run
         if (userO2Start) {
-            runPatch.userIdO2Start = (await getOrCreateUser(userO2Start))?.id;
+            runPatch.userIdO2Start = (await userService.getOrCreate(userO2Start.userIdentifier, userO2Start.name)).id;
         }
         if (userO2Stop) {
-            runPatch.userIdO2Stop = (await getOrCreateUser(userO2Stop))?.id;
+            runPatch.userIdO2Stop = (await userService.getOrCreate(userO2Stop.userIdentifier, userO2Stop.name)).id;
         }
 
         await dataSource.transaction(async (transaction) => {
@@ -625,22 +631,6 @@ const updateEorReasonsOnRun = async (runId, runNumber, userName, eorReasonsPatch
     }
 };
 
-/**
- * Method to process a user for the run
- *
- * @param {object|null} userData - user object, either userO2Start or userO2Stop
- * @param {number} [userData.externalId] - external identifier of the user
- * @param {string|null} [userData.name] - name of the user or empty
- * @returns {Promise<User|null>} - returns the id of the user or null if the userData is null
- */
-const getOrCreateUser = (userData) => {
-    if (userData?.externalId) {
-        // Create the user if it does not yet exist
-        return userService.getOrCreate({ externalUserId: userData.externalId }, userData.name);
-    }
-    return null;
-};
-
 exports.RunService = RunService;
 
 exports.runService = new RunService();

lib/server/services/user/UserService.js

@@ -30,16 +30,15 @@ class UserService {
      *
      * @param {UserIdentifier} identifier the identifier of the user to find
      * @param {string|null} name - username of the user
-     * @returns {User} - found user | newly created user
+     * @returns {Promise<User>} - found user | newly created user
      */
     async getOrCreate(identifier, name) {
         const foundUser = await getUser(identifier);
 
         // If the user does not exist, create it as a new user
         if (!foundUser) {
             const { externalUserId } = identifier;
-            const user = await createOrUpdateUser({ personid: externalUserId, name });
-            return user ? userAdapter.toEntity(user) : null;
+            return userAdapter.toEntity(await createOrUpdateUser({ personid: externalUserId, name }));
         }
 
         return userAdapter.toEntity(foundUser);

test/lib/server/services/run/RunService.test.js

@@ -502,7 +502,7 @@ module.exports = () => {
     it('should successfully create run with only userO2Start', async () => {
         const runNumber = ++lastRunNumber;
         const userO2Start = {
-            externalId: 1000,
+            userIdentifier: { externalUserId: 1000 },
             name: 'test',
         };
 
@@ -525,7 +525,7 @@ module.exports = () => {
     it('should successfully create run with only userO2Stop', async () => {
         const runNumber = ++lastRunNumber;
         const userO2Stop = {
-            externalId: 1001,
+            userIdentifier: { externalUserId: 1001 },
             name: 'test',
         };
 
@@ -548,11 +548,11 @@ module.exports = () => {
     it('should successfully create run with userO2Start and userO2Stop', async () => {
         const runNumber = ++lastRunNumber;
         const userO2Start = {
-            externalId: 1002,
+            userIdentifier: { externalUserId: 1002 },
             name: 'test',
         };
         const userO2Stop = {
-            externalId: 1003,
+            userIdentifier: { externalUserId: 1003 },
             name: 'test',
         };
 
@@ -577,7 +577,7 @@ module.exports = () => {
     it('should successfully update run with only userO2Start', async () => {
         const runNumber = 1;
         const userO2Start = {
-            externalId: 1000,
+            userIdentifier: { externalUserId: 1000 },
             name: 'test',
         };
 
@@ -598,7 +598,7 @@ module.exports = () => {
     it('should successfully update run with only userO2Stop', async () => {
         const runNumber = 1;
         const userO2Stop = {
-            externalId: 1000,
+            userIdentifier: { externalUserId: 1000 },
             name: 'test',
         };
 
@@ -619,11 +619,11 @@ module.exports = () => {
     it('should successfully update run with userO2Start and userO2Stop', async () => {
         const runNumber = 1;
         const userO2Start = {
-            externalId: 1002,
+            userIdentifier: { externalUserId: 1002 },
             name: 'test',
         };
         const userO2Stop = {
-            externalId: 1003,
+            userIdentifier: { externalUserId: 1003 },
             name: 'test',
         };
 
@@ -684,7 +684,7 @@ module.exports = () => {
         expect(levelsCombinations).have.all.deep.members([{ l3Level: 20003, dipoleLevel: 0 }, { l3Level: 30003, dipoleLevel: 0 }]);
     });
 
-    it('should successfully extract informations from environment configuration when creating a run', async () => {
+    it('should successfully extract information from environment configuration when creating a run', async () => {
         const timeO2Start = new Date('2019-08-09 20:01:00');
         const timeTrgStart = new Date('2019-08-09 20:02:00');
         const timeTrgEnd = new Date('2019-08-09 20:03:00');
@@ -693,7 +693,7 @@ module.exports = () => {
             1234,
             'CmCvjNbg',
             { timeO2Start, timeTrgStart, timeTrgEnd, timeO2End },
-            { externalUserId: 1 },
+            { userO2Start: { userIdentifier: { externalUserId: 1 } }, userO2Stop: { userIdentifier: { externalUserId: 456 } } },
         );
 
         expect(run.triggerValue).to.equal('CTP');
@@ -716,5 +716,7 @@ module.exports = () => {
         expect(run.nFlps).to.equal(3);
         expect(run.nEpns).to.equal(2);
         expect(run.readoutCfgUri).to.equal('file:///local/replay/2024-04-17-pp-650khz-synt-4tf/readout-replay-24g-dd40.cfg');
+        expect(run.userIdO2Start).to.equal(1);
+        expect(run.userIdO2Stop).to.equal(2);
     });
 };