Skip to content

Commit 303f8c5

Browse files
martinboulaismartinboulais
committed
Code review
1 parent a54431e commit 303f8c5

13 files changed

+90
-34
lines changed

‎lib/public/components/qcFlags/getRunQcExclusionReason.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* @license
3+
* Copyright CERN and copyright holders of ALICE O2. This software is
4+
* distributed under the terms of the GNU General Public License v3 (GPL
5+
* Version 3), copied verbatim in the file "COPYING".
6+
*
7+
* See http://alice-o2.web.cern.ch/license for full licensing information.
8+
*
9+
* In applying this license CERN does not waive the privileges and immunities
10+
* granted to it by virtue of its status as an Intergovernmental Organization
11+
* or submit itself to any jurisdiction.
12+
*/
13+
14+
import { RunQualities } from '../../domain/enums/RunQualities.js';
15+
import { RUN_TAGS_DISABLING_QC } from './isRunNotSubjectToQc.js';
16+
17+
/**
18+
* Return the reason why a run is not subject to QC
19+
*
20+
* @param {Run} run a run
21+
* @return {string} reason
22+
*/
23+
export const getRunQcExclusionReason = ({ runNumber, runQuality, tags }) => {
24+
const reasons = [];
25+
if (runQuality === RunQualities.BAD) {
26+
reasons.push(`Quality of run ${runNumber} was changed to ${RunQualities.BAD} so it is no more subject to QC`);
27+
}
28+
if (tags.find(({ text }) => RUN_TAGS_DISABLING_QC.includes(text))) {
29+
reasons.push(`Run ${runNumber} was tagged as not for physics so it is no more subject to QC`);
30+
}
31+
return reasons.join('; ');
32+
};

‎lib/public/components/qcFlags/isRunSubjectToQc.js renamed to ‎lib/public/components/qcFlags/isRunNotSubjectToQc.js

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { RunQualities } from '../../domain/enums/RunQualities.js';
1717
/**
1818
* Array of names of tags which prevent run to be subject to QC
1919
*/
20-
const RUN_TAGS_DISABLING_QC = ['Not for physics'];
20+
export const RUN_TAGS_DISABLING_QC = ['Not for physics'];
2121

2222
/**
2323
* States whether run is subject to QC
@@ -28,19 +28,3 @@ const RUN_TAGS_DISABLING_QC = ['Not for physics'];
2828
export const isRunNotSubjectToQc = ({ runQuality, tags }) => runQuality === RunQualities.BAD
2929
|| tags.find(({ text }) => RUN_TAGS_DISABLING_QC.includes(text));
3030

31-
/**
32-
* Get reason why a run is not subject to QC
33-
*
34-
* @param {Run} run a run
35-
* @return {string} reason
36-
*/
37-
export const getRunNotSubjectToQcReason = ({ runNumber, runQuality, tags }) => {
38-
const reasons = [];
39-
if (runQuality === RunQualities.BAD) {
40-
reasons.push(`Quality of run ${runNumber} was changed to ${RunQualities.BAD} so it is no more subject to QC`);
41-
}
42-
if (tags.find(({ text }) => RUN_TAGS_DISABLING_QC.includes(text))) {
43-
reasons.push(`Run ${runNumber} was tagged as not for physics so it is no more subject to QC`);
44-
}
45-
return reasons.join('; ');
46-
};

‎lib/public/components/qcFlags/qcFlagsPagesButtons.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@
1313

1414
import { frontLink } from '../common/navigation/frontLink.js';
1515
import { tooltip } from '../common/popover/tooltip.js';
16-
import { getRunNotSubjectToQcReason, isRunNotSubjectToQc } from './isRunSubjectToQc.js';
16+
import { isRunNotSubjectToQc } from './isRunNotSubjectToQc.js';
1717
import { h, iconPlus, iconWarning } from '/js/src/index.js';
18+
import { getRunQcExclusionReason } from './getRunQcExclusionReason.js';
1819

1920
/**
2021
* Render link to QC flag creation page
@@ -65,7 +66,7 @@ export const qcFlagCreationPanelLink = (
6566
Loading: () => null,
6667
Failure: () => tooltip(h('.f3', iconWarning()), 'An error has occurred. Unable to determine whether run is subject for QC'),
6768
Success: (run) => isRunNotSubjectToQc(run)
68-
? h('button.btn.btn-primary', { disabled: true, title: getRunNotSubjectToQcReason(run) }, content)
69+
? h('button.btn.btn-primary', { disabled: true, title: getRunQcExclusionReason(run) }, content)
6970
: h('div.flex-row.align-center', [
7071
frontLink(
7172
content,

‎lib/public/services/run/runsProvider.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ export class RunsProvider {
1111
* @return {Promise<Run[]>} the found runs
1212
*/
1313
async getByRunNumbers(runNumbers) {
14-
const runNumbersFilter = runNumbers.length > 1
15-
? runNumbers.join(',')
16-
: `${runNumbers[0]},${runNumbers[0]}`; // Send duplicate run number if one to avoid partial run number matching
14+
if (runNumbers.length === 0) {
15+
return [];
16+
}
17+
18+
const runNumbersFilter = runNumbers.length === 1
19+
? `${runNumbers[0]},${runNumbers[0]}` // Send duplicate run number if one to avoid partial run number matching
20+
: runNumbers.join(',');
1721

1822
const { data: runs } = await getRemoteData(`/api/runs/?filter[runNumbers]=${runNumbersFilter}`);
1923
return runs;

‎lib/public/views/QcFlags/Create/qcFlagCreationComponent.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ import { timeRangeInput } from '../../../components/common/form/inputs/TimeRange
2121
import { formatRunStart } from '../../Runs/format/formatRunStart.js';
2222
import { formatRunEnd } from '../../Runs/format/formatRunEnd.js';
2323
import errorAlert from '../../../components/common/errorAlert.js';
24-
import { getRunNotSubjectToQcReason, isRunNotSubjectToQc } from '../../../components/qcFlags/isRunSubjectToQc.js';
24+
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunNotSubjectToQc.js';
2525
import { frontLink } from '../../../components/common/navigation/frontLink.js';
26+
import { getRunQcExclusionReason } from '../../../components/qcFlags/getRunQcExclusionReason.js';
2627

2728
/**
2829
* Create table with selected detectors and runs information
@@ -150,7 +151,14 @@ const qcFlagCreationForm = (creationModel) => {
150151
export const qcFlagCreationComponent = (qcFlagCreationModel) => {
151152
const { runs } = qcFlagCreationModel;
152153

153-
return runs.every(isRunNotSubjectToQc)
154-
? runs.map((run) => errorAlert([{ title: getRunNotSubjectToQcReason(run) }]))
155-
: qcFlagCreationForm(qcFlagCreationModel);
154+
const errors = [];
155+
for (const run of runs) {
156+
if (isRunNotSubjectToQc(run)) {
157+
errors.push({ title: getRunQcExclusionReason(run) });
158+
}
159+
}
160+
console.log(runs);
161+
return errors.length === 0
162+
? qcFlagCreationForm(qcFlagCreationModel)
163+
: errorAlert(errors);
156164
};

‎lib/public/views/Runs/ActiveColumns/runDetectorsAsyncQcActiveColumns.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@
1313
import { h, iconBan, iconX, RemoteData } from '/js/src/index.js';
1414
import { qcFlagCreationPanelLink, qcFlagOverviewPanelLink } from '../../../components/qcFlags/qcFlagsPagesButtons.js';
1515
import { tooltip } from '../../../components/common/popover/tooltip.js';
16-
import { getRunNotSubjectToQcReason, isRunNotSubjectToQc } from '../../../components/qcFlags/isRunSubjectToQc.js';
16+
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunNotSubjectToQc.js';
1717
import { getQcSummaryDisplay } from './getQcSummaryDisplay.js';
18+
import { getRunQcExclusionReason } from '../../../components/qcFlags/getRunQcExclusionReason.js';
1819

1920
/**
2021
* Render QC summary for given run and detector
@@ -33,7 +34,7 @@ const runDetectorAsyncQualityDisplay = ({ dataPassId, simulationPassId }, run, d
3334
let qcSummaryDisplay = getQcSummaryDisplay(qcSummary[runNumber][dplDetectorId]);
3435

3536
qcSummaryDisplay = isRunNotSubjectToQc(run)
36-
? tooltip(qcSummaryDisplay, getRunNotSubjectToQcReason(run))
37+
? tooltip(qcSummaryDisplay, getRunQcExclusionReason(run))
3738
: qcSummaryDisplay;
3839

3940
return qcFlagOverviewPanelLink(
@@ -94,9 +95,11 @@ export const createRunDetectorsAsyncQcActiveColumns = (
9495
},
9596
);
9697

98+
const runNotSubjectToQc = isRunNotSubjectToQc(run);
99+
97100
if (!qcSummary?.[runNumber]?.[dplDetectorId]) { // If there is no QC flag assigned
98-
return isRunNotSubjectToQc(run)
99-
? h('.text-center', tooltip(iconX(), getRunNotSubjectToQcReason(run)))
101+
return runNotSubjectToQc
102+
? h('.text-center', tooltip(iconX(), getRunQcExclusionReason(run)))
100103
: h('.flex-row.items-center.gc1', [
101104
checkbox,
102105
qcFlagCreationPanelLink(
@@ -116,7 +119,7 @@ export const createRunDetectorsAsyncQcActiveColumns = (
116119
}
117120

118121
return h('.flex-row.items-center.gc1', [
119-
checkbox,
122+
!runNotSubjectToQc && checkbox,
120123
runDetectorAsyncQualityDisplay(
121124
{ dataPassId, simulationPassId },
122125
run,

‎lib/public/views/Runs/RunDetectorsSelectionModel.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ export class RunDetectorsSelectionModel extends Observable {
1717
this._selectedRunDetectors = new Map();
1818
}
1919

20+
/**
21+
* Clear the selection
22+
*
23+
* @return {void}
24+
*/
25+
reset() {
26+
this._selectedRunDetectors = new Map();
27+
}
28+
2029
/**
2130
* States if a given run & detector is currently selected for grouped actions
2231
*

‎lib/public/views/Runs/RunPerDataPass/RunsPerDataPassOverviewModel.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ export class RunsPerDataPassOverviewModel extends FixedPdpBeamTypeRunsOverviewMo
167167
if (!this._dataPassId) {
168168
return;
169169
}
170+
171+
this._runDetectorsSelectionModel.reset();
172+
170173
this._fetchQcSummary();
171174
this._fetchGaqSummary();
172175
await this._fetchDataPass();

‎lib/public/views/Runs/RunPerDataPass/RunsPerDataPassOverviewPage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { inelasticInteractionRateActiveColumnsForProtonProton } from '../ActiveC
2626
import { filtersPanelPopover } from '../../../components/Filters/common/filtersPanelPopover.js';
2727
import runNumberFilter from '../../../components/Filters/RunsFilter/runNumber.js';
2828
import { qcSummaryLegendTooltip } from '../../../components/qcFlags/qcSummaryLegendTooltip.js';
29-
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunSubjectToQc.js';
29+
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunNotSubjectToQc.js';
3030
import { frontLink } from '../../../components/common/navigation/frontLink.js';
3131
import { getQcSummaryDisplay } from '../ActiveColumns/getQcSummaryDisplay.js';
3232
import errorAlert from '../../../components/common/errorAlert.js';

‎lib/public/views/Runs/RunPerPeriod/RunsPerLhcPeriodOverviewPage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { runsActiveColumns } from '../ActiveColumns/runsActiveColumns.js';
2020
import { runDetectorsQualitiesActiveColumns } from '../ActiveColumns/runDetectorsQualitiesActiveColumns.js';
2121
import { inelasticInteractionRateActiveColumnsForProtonProton } from '../ActiveColumns/inelasticInteractionRateActiveColumnsForProtonProton.js';
2222
import { inelasticInteractionRateActiveColumnsForPbPb } from '../ActiveColumns/inelasticInteractionRateActiveColumnsForPbPb.js';
23-
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunSubjectToQc.js';
23+
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunNotSubjectToQc.js';
2424
import { runDetectorsSyncQcActiveColumns } from '../ActiveColumns/runDetectorsSyncQcActiveColumns.js';
2525
import { tabbedPanelComponent } from '../../../components/TabbedPanel/tabbedPanelComponent.js';
2626
import { RUNS_PER_LHC_PERIOD_PANELS_KEYS } from './RunsPerLhcPeriodOverviewModel.js';

‎lib/public/views/Runs/RunsPerSimulationPass/RunsPerSimulationPassOverviewModel.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ export class RunsPerSimulationPassOverviewModel extends RunsOverviewModel {
7474
* @inheritdoc
7575
*/
7676
async load() {
77+
this._runDetectorsSelectionModel.reset();
78+
7779
this._fetchQcSummary();
7880
this._fetchSimulationPass();
7981
super.load();

‎lib/public/views/Runs/RunsPerSimulationPass/RunsPerSimulationPassOverviewPage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { createRunDetectorsAsyncQcActiveColumns } from '../ActiveColumns/runDete
2424
import { inelasticInteractionRateActiveColumnsForPbPb } from '../ActiveColumns/inelasticInteractionRateActiveColumnsForPbPb.js';
2525
import { inelasticInteractionRateActiveColumnsForProtonProton } from '../ActiveColumns/inelasticInteractionRateActiveColumnsForProtonProton.js';
2626
import { qcSummaryLegendTooltip } from '../../../components/qcFlags/qcSummaryLegendTooltip.js';
27-
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunSubjectToQc.js';
27+
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunNotSubjectToQc.js';
2828
import { PdpBeamType } from '../../../domain/enums/PdpBeamType.js';
2929
import { frontLink } from '../../../components/common/navigation/frontLink.js';
3030

‎test/public/qcFlags/forDataPassCreation.test.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,16 @@ module.exports = () => {
201201

202202
await expectInnerText(page, '.alert.alert-danger', 'Quality of run 2 was changed to bad so it is no more subject to QC');
203203
await page.waitForSelector('input', { hidden: true });
204+
205+
await goToPage(page, 'qc-flag-creation-for-data-pass', {
206+
queryParameters: {
207+
dataPassId: 2,
208+
runNumberDetectorsMap: '2:1;55:7',
209+
},
210+
});
211+
212+
await expectInnerText(page, '.alert.alert-danger', 'Quality of run 2 was changed to bad so it is no more subject to QC');
213+
await page.waitForSelector('input', { hidden: true });
204214
});
205215

206216
it('should allow multiple runs and detectors to be selected', async () => {

0 commit comments

Comments
 (0)