Skip to content

Commit 07907d6

Browse files
gradutagraduta
authored
[OGUI-1854] Notify user that a new run has started (#3293)
* fixes a bug in which 2 web sockets were sent on run start. This was not distinguishing on transition status * fixes a bug in which the web socket handler was not being triggered * fixes a bug in which multiple calls were made on WS trigger on page reset and filter update
1 parent 1aed786 commit 07907d6

10 files changed

Lines changed: 83 additions & 41 deletions

File tree

‎QualityControl/common/library/enums/transition.enum.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,20 @@
1818
* @readonly
1919
*/
2020
export const Transition = Object.freeze({
21+
NULL: 'NULL', // custom QCG value for no transition
2122
START_ACTIVITY: 'START_ACTIVITY',
2223
STOP_ACTIVITY: 'STOP_ACTIVITY',
2324
});
25+
26+
/**
27+
* Enumeration for different statuses of a transitions as per:
28+
* @link https://github.com/AliceO2Group/Control/blob/master/common/protos/events.proto#L35
29+
*/
30+
export const TransitionStatus = Object.freeze({
31+
NULL: 'NULL',
32+
STARTED: 'STARTED',
33+
ONGOING: 'ONGOING',
34+
DONE_OK: 'DONE_OK',
35+
DONE_ERROR: 'DONE_ERROR',
36+
DONE_TIMEOUT: 'DONE_TIMEOUT',
37+
});

‎QualityControl/lib/services/RunModeService.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import { LogManager, WebSocketMessage } from '@aliceo2/web-ui';
1616
import { EmitterKeys } from '../../common/library/enums/emitterKeys.enum.js';
17-
import { Transition } from '../../common/library/enums/transition.enum.js';
17+
import { Transition, TransitionStatus } from '../../common/library/enums/transition.enum.js';
1818
import { RunStatus } from '../../common/library/runStatus.enum.js';
1919
import { parseObjects } from '../../common/library/qcObject/utils.js';
2020
import QCObjectDto from '../dtos/QCObjectDto.js';
@@ -138,10 +138,11 @@ export class RunModeService {
138138
* @param {object} runEvent - Object containing runNumber and transition type.
139139
* @param {number} runEvent.runNumber - The run number associated with the event.
140140
* @param {string} runEvent.transition - The transition type (e.g., 'START_ACTIVITY', 'STOP_ACTIVITY').
141+
* @param {string} runEvent.transitionStatus - The status of the transition (e.g., 'DONE_OK').
141142
* @returns {Promise<void>}
142143
*/
143-
async _onRunTrackEvent({ runNumber, transition }) {
144-
if (transition === Transition.START_ACTIVITY) {
144+
async _onRunTrackEvent({ runNumber, transition = Transition.NULL, transitionStatus = TransitionStatus.NULL }) {
145+
if (transition === Transition.START_ACTIVITY && transitionStatus === TransitionStatus.DONE_OK) {
145146
await this._initializeRunData(runNumber);
146147

147148
const wsMessage = new WebSocketMessage();

‎QualityControl/lib/services/external/AliEcsSynchronizer.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ import { ServiceStatus } from '../../../common/library/enums/Status/serviceStatu
1818
const LOG_FACILITY = `${process.env.npm_config_log_label ?? 'qcg'}/ecs-synchronizer`;
1919
const RUN_TOPICS = ['aliecs.run'];
2020

21+
/**
22+
* @type {RunEvent}
23+
* @property {number} runNumber - The run number associated with the event.
24+
* @property {Transition} transition - The type of transition (e.g., START_ACTIVITY, END_ACTIVITY).
25+
* @property {TransitionStatus} transitionStatus - The status of the transition (e.g., DONE_OK, DONE_ERROR).
26+
*/
27+
2128
/**
2229
* Service for processing events sent via Kafka from AliECS with proto objects
2330
*/
@@ -73,6 +80,9 @@ export class AliEcsSynchronizer {
7380
* @returns {void}
7481
*/
7582
async _onRunMessage(eventMessage) {
83+
/**
84+
* @param {RunEvent} - eventMessage - message received on run topic
85+
*/
7686
const { runEvent, timestamp } = eventMessage;
7787
if (!runEvent) {
7888
this._logger.warnMessage('Received run message on run topic without runEvent field');
@@ -82,9 +92,10 @@ export class AliEcsSynchronizer {
8292
} else if (!runEvent.transition) {
8393
this._logger.warnMessage('Received run message on run topic without runEvent.transition field');
8494
} else {
85-
const { runNumber, transition } = runEvent;
95+
const { runNumber, transition, transitionStatus } = runEvent;
8696
this._eventEmitter.emit(EmitterKeys.RUN_TRACK, {
8797
runNumber,
98+
transitionStatus,
8899
transition,
89100
timestamp: timestamp.toNumber(),
90101
});

‎QualityControl/public/common/notifications/model/NotificationRunStartModel.js‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export default class NotificationRunStartModel extends Observable {
3434

3535
this.model.ws.addListener('command', (message) => {
3636
if (message.command === `${EmitterKeys.RUN_TRACK}:${Transition.START_ACTIVITY}`) {
37-
this._handleWSRunTrack.bind(this, message.payload);
37+
this._handleWSRunTrack(message.payload);
3838
}
3939
});
4040
}
@@ -82,21 +82,14 @@ export default class NotificationRunStartModel extends Observable {
8282
}
8383

8484
showNativeBrowserNotification({
85-
title: `RUN ${runNumber ?? 'unknown'} has started`,
85+
title: `RUN ${runNumber ?? 'unknown'} has started. Click here to enter RunMode`,
8686
onclick: () => {
87-
// On notification click we always navigate to the `objectTree` page.
88-
// Additionally, we view the run using the given `runNumber`.
8987
this.model.router.go(`?page=objectTree&RunNumber=${runNumber}`);
9088

91-
// If RunMode is not activated, we should enable it
9289
const { isRunModeActivated } = this.model.filterModel;
9390
if (!isRunModeActivated) {
9491
this.model.filterModel.activateRunsMode(this.model.filterModel.getPageTargetModel());
9592
}
96-
97-
// We select the given `runNumber` in RunMode.
98-
// We do not have to set the parameter in the URL, as this is already achieved on navigation.
99-
this.model.filterModel.setFilterValue('RunNumber', runNumber?.toString());
10093
},
10194
});
10295
}

‎QualityControl/test/lib/services/RunModeService.test.js‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import sinon from 'sinon';
1919
import { RunModeService } from '../../../lib/services/RunModeService.js';
2020
import { RunStatus } from '../../../common/library/runStatus.enum.js';
2121
import { EmitterKeys } from '../../../common/library/enums/emitterKeys.enum.js';
22-
import { Transition } from '../../../common/library/enums/transition.enum.js';
22+
import { Transition, TransitionStatus } from '../../../common/library/enums/transition.enum.js';
2323
import { delayAndCheck } from '../../testUtils/delay.js';
2424
import { WebSocketMessage } from '@aliceo2/web-ui';
2525

@@ -152,7 +152,7 @@ export const runModeServiceTestSuite = async () => {
152152

153153
suite('_onRunTrackEvent - test suite', () => {
154154
test('should correctly parse event to RUN_TRACK and update ongoing runs map', async () => {
155-
const runEvent = { runNumber: 1234, transition: 'START_ACTIVITY' };
155+
const runEvent = { runNumber: 1234, transition: 'START_ACTIVITY', transitionStatus: TransitionStatus.DONE_OK };
156156
runModeService._dataService.getObjectsLatestVersionList = sinon.stub().resolves([{ path: '/path/from/event' }]);
157157

158158
await runModeService._onRunTrackEvent(runEvent);
@@ -164,7 +164,9 @@ export const runModeServiceTestSuite = async () => {
164164
});
165165

166166
test('should listen to events on RUN_TRACK and update ongoing runs map', async () => {
167-
const runEvent = { runNumber: 1234, transition: Transition.START_ACTIVITY };
167+
const runEvent = {
168+
runNumber: 1234, transition: Transition.START_ACTIVITY, transitionStatus: TransitionStatus.DONE_OK,
169+
};
168170
runModeService._dataService.getObjectsLatestVersionList = sinon.stub().resolves([{ path: '/path/from/event' }]);
169171

170172
eventEmitter.emit(EmitterKeys.RUN_TRACK, runEvent);
@@ -177,7 +179,9 @@ export const runModeServiceTestSuite = async () => {
177179

178180
test('should listen to events on RUN_TRACK and broadcast to websocket', async () => {
179181
const runNumber = 1234;
180-
const runEvent = { runNumber, transition: Transition.START_ACTIVITY };
182+
const runEvent = {
183+
runNumber, transition: Transition.START_ACTIVITY, transitionStatus: TransitionStatus.DONE_OK,
184+
};
181185
runModeService._dataService.getObjectsLatestVersionList = sinon.stub().resolves([{ path: '/path/from/event' }]);
182186

183187
eventEmitter.emit(EmitterKeys.RUN_TRACK, runEvent);
@@ -194,7 +198,9 @@ export const runModeServiceTestSuite = async () => {
194198
});
195199

196200
test('should remove run from ongoing runs map on STOP_ACTIVITY event', async () => {
197-
const runEventStop = { runNumber: 5678, transition: Transition.STOP_ACTIVITY };
201+
const runEventStop = {
202+
runNumber: 5678, transition: Transition.STOP_ACTIVITY, transitionStatus: TransitionStatus.DONE_OK,
203+
};
198204
runModeService._ongoingRuns.set(runEventStop.runNumber, [{ path: '/some/path' }]);
199205

200206
eventEmitter.emit(EmitterKeys.RUN_TRACK, runEventStop);

‎QualityControl/test/lib/services/external/AliEcsSynchronizer.test.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { ok, deepStrictEqual } from 'node:assert';
1616
import { test, beforeEach, afterEach } from 'node:test';
1717
import { stub, restore } from 'sinon';
1818
import { AliEcsSynchronizer } from '../../../../lib/services/external/AliEcsSynchronizer.js';
19-
import { Transition } from '../../../../common/library/enums/transition.enum.js';
19+
import { Transition, TransitionStatus } from '../../../../common/library/enums/transition.enum.js';
2020
import { EmitterKeys } from '../../../../common/library/enums/emitterKeys.enum.js';
2121

2222
export const aliecsSynchronizerTestSuite = async () => {
@@ -50,13 +50,14 @@ export const aliecsSynchronizerTestSuite = async () => {
5050
test('should emit a run track event when a valid run message is received', () => {
5151
const runNumber = 123;
5252
const transition = Transition.START_ACTIVITY;
53+
const transitionStatus = TransitionStatus.DONE_OK;
5354
const fixedTimestamp = Date.now();
5455
const timestamp = { toNumber: () => fixedTimestamp };
55-
aliecsSynchronizer._onRunMessage({ runEvent: { runNumber, transition }, timestamp });
56+
aliecsSynchronizer._onRunMessage({ runEvent: { runNumber, transition, transitionStatus }, timestamp });
5657
ok(eventEmitterMock.emit.called);
5758
deepStrictEqual(eventEmitterMock.emit.firstCall.args[0], EmitterKeys.RUN_TRACK);
5859
deepStrictEqual(eventEmitterMock.emit.firstCall.args[1], {
59-
runNumber, transition, timestamp: timestamp.toNumber(),
60+
runNumber, transition, transitionStatus, timestamp: timestamp.toNumber(),
6061
});
6162
});
6263
};

‎QualityControl/test/public/components/profileHeader.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { getLocalStorageAsJson } from '../../testUtils/localStorage.js';
1919
import { IntegratedServices } from '../../../common/library/enums/Status/integratedServices.enum.js';
2020
import { ServiceStatus } from '../../../common/library/enums/Status/serviceStatus.enum.js';
2121
import { integratedServiceInterceptor } from '../../testUtils/interceptors/integratedServiceInterceptor.js';
22+
import { ONGOING_RUN_NUMBER } from '../../setup/mockKafkaEvents.js';
2223

2324
/**
2425
* Performs a series of automated tests on the layoutList page using Puppeteer.
@@ -199,8 +200,6 @@ export const profileHeaderTests = async (url, page, timeout = 1000, testParent)
199200
});
200201

201202
await testParent.test('should enable RunMode when browser notification is clicked', { timeout }, async () => {
202-
const RUN_NUMBER = 1234;
203-
204203
/*
205204
* Kafka must be enabled for the browser notification feature to function correctly.
206205
* We intercept the request and return a SUCCESS state of the kafka service.
@@ -261,6 +260,7 @@ export const profileHeaderTests = async (url, page, timeout = 1000, testParent)
261260
window.Notification = MockNotification;
262261
});
263262

263+
const RUN_NUMBER = ONGOING_RUN_NUMBER;
264264
// Trigger native browser notification by simulating websocket message
265265
await page.evaluate(
266266
(wsMessage) => window.model.notificationRunStartModel._handleWSRunTrack(wsMessage),

‎QualityControl/test/public/features/runMode.test.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,17 @@
1212
*/
1313
/* eslint-disable @stylistic/js/max-len */
1414

15-
import { strictEqual, ok } from 'node:assert';
15+
import { strictEqual, ok, deepStrictEqual } from 'node:assert';
1616
import { delay } from '../../testUtils/delay.js';
1717
import { IntegratedServices } from '../../../common/library/enums/Status/integratedServices.enum.js';
1818
import { ServiceStatus } from '../../../common/library/enums/Status/serviceStatus.enum.js';
1919
import { integratedServiceInterceptor } from '../../testUtils/interceptors/integratedServiceInterceptor.js';
20+
import { ONGOING_RUN_NUMBER } from '../../setup/mockKafkaEvents.js';
2021

2122
// If using nock for HTTP mocking (uncomment if available)
2223
// import nock from 'nock';
2324
export const runModeTests = async (url, page, timeout = 5000, testParent) => {
24-
const mockedTestRunNumber = 500001;
25+
const mockedTestRunNumber = ONGOING_RUN_NUMBER;
2526
let countOngoingRunsCalls = 0;
2627
let countRunStatusCalls = 0;
2728
let expectCountRunStatusCalls = 0;
@@ -185,10 +186,7 @@ export const runModeTests = async (url, page, timeout = 5000, testParent) => {
185186
.filter((value) => value !== '');
186187
});
187188

188-
ok(availableOptions.length > 0, 'Should have ongoing runs available in selector');
189-
['500001', '500002', '500003'].forEach((run) => {
190-
ok(availableOptions.includes(run), `Should include mock run ${run}`);
191-
});
189+
deepStrictEqual(availableOptions, ['500001', '500002', '500003'], 'Ongoing runs selector should have correct options');
192190
});
193191

194192
await testParent.test('should automatically select first run and update URL', { timeout }, async () => {

‎QualityControl/test/setup/mockKafkaEvents.js‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,25 @@
1313
*/
1414

1515
import { EmitterKeys } from '../../common/library/enums/emitterKeys.enum.js';
16-
import { Transition } from '../../common/library/enums/transition.enum.js';
16+
import { Transition, TransitionStatus } from '../../common/library/enums/transition.enum.js';
17+
18+
export const ONGOING_RUN_NUMBER = 500001;
19+
export const ONGOING_RUNS_LIST = [ONGOING_RUN_NUMBER, 500002, 500003];
1720

1821
/**
1922
* Mock Kafka events for testing purposes
2023
* @param {EventEmitter} eventEmitter - Event emitter to emit mock events
2124
*/
2225
export const setupMockKafkaEvents = (eventEmitter) => {
23-
// Simulate some ongoing runs being started
24-
const mockOngoingRuns = ['500001', '500002', '500003'];
25-
2626
// Emit START_ACTIVITY events for mock runs after a short delay
2727
setTimeout(() => {
28-
mockOngoingRuns.forEach((runNumber) => {
28+
ONGOING_RUNS_LIST.forEach((runNumber) => {
2929
eventEmitter.emit(EmitterKeys.RUN_TRACK, {
3030
runNumber: parseInt(runNumber, 10),
3131
transition: Transition.START_ACTIVITY,
32+
transitionStatus: TransitionStatus.DONE_OK,
3233
timestamp: Date.now(),
3334
});
3435
});
35-
}, 100);
36-
37-
return mockOngoingRuns;
36+
}, 200);
3837
};

‎QualityControl/test/setup/testSetupForBkp.js‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import nock from 'nock';
1616
import { config } from '../config.js';
1717
import { BKP_MOCK_DATA } from './seeders/bkp-mock-data.js';
1818
import { GET_BKP_GUI_STATUS_PATH } from '../../lib/services/BookkeepingService.js';
19+
import { ONGOING_RUN_NUMBER } from './mockKafkaEvents.js';
1920

2021
const BKP_URL = `${config.bookkeeping.url}`;
2122
const TOKEN_PATH = `?token=${config.bookkeeping.token}`;
@@ -160,25 +161,43 @@ export const initializeNockForBkp = () => {
160161
},
161162
});
162163
nock(BKP_URL)
163-
.get(`/api/runs/500001${TOKEN_PATH}`)
164+
.get(`/api/runs/${ONGOING_RUN_NUMBER}${TOKEN_PATH}`)
164165
.reply(200, {
165166
data: {
166167
timeO2End: null,
167168
},
168169
})
169-
.get(`/api/runs/500001${TOKEN_PATH}`)
170+
.get(`/api/runs/${ONGOING_RUN_NUMBER}${TOKEN_PATH}`)
170171
.reply(200, {
171172
data: {
172173
timeO2End: null,
173174
},
174175
})
175-
.get(`/api/runs/500001${TOKEN_PATH}`)
176+
.get(`/api/runs/${ONGOING_RUN_NUMBER}${TOKEN_PATH}`)
177+
.reply(200, {
178+
data: {
179+
timeO2End: null,
180+
},
181+
})
182+
.get(`/api/runs/${ONGOING_RUN_NUMBER}${TOKEN_PATH}`)
183+
.reply(200, {
184+
data: {
185+
timeO2End: null,
186+
},
187+
})
188+
.get(`/api/runs/${ONGOING_RUN_NUMBER}${TOKEN_PATH}`)
189+
.reply(200, {
190+
data: {
191+
timeO2End: null,
192+
},
193+
})
194+
.get(`/api/runs/${ONGOING_RUN_NUMBER}${TOKEN_PATH}`)
176195
.reply(200, {
177196
data: {
178197
timeO2End: '2023-12-01T10:30:00Z',
179198
},
180199
})
181-
.get(`/api/runs/500001${TOKEN_PATH}`)
200+
.get(`/api/runs/${ONGOING_RUN_NUMBER}${TOKEN_PATH}`)
182201
.reply(200, {
183202
data: {
184203
timeO2End: '2023-12-01T10:30:00Z',

0 commit comments

Comments
 (0)