Skip to content

Commit

Permalink
[O2B-536] Improve runs overview run definition filter
Browse files Browse the repository at this point in the history
  • Loading branch information
martinboulais committed Feb 3, 2025
1 parent 2f6964b commit a527733
Show file tree
Hide file tree
Showing 11 changed files with 288 additions and 123 deletions.
119 changes: 119 additions & 0 deletions lib/database/seeders/20200508094502-logs.js

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { RUN_DEFINITIONS, RunDefinition } from '../../../domain/enums/RunDefinition.js';
import { SelectionFilterModel } from '../common/filters/SelectionFilterModel.js';

/**
* Run definition filter model
*/
export class RunDefinitionFilterModel extends SelectionFilterModel {
/**
* Constructor
*/
constructor() {
super({ availableOptions: RUN_DEFINITIONS.map((definition) => ({ value: definition })) });

Check warning on line 12 in lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js#L11-L12

Added lines #L11 - L12 were not covered by tests
}

/**
* Returns true if the current filter is physics only
*
* @return {boolean} true if filter is physics only
*/
isPhysicsOnly() {
const selectedOptions = this._selectionModel.selected;
return selectedOptions.length === 1 && selectedOptions[0] === RunDefinition.Physics;

Check warning on line 22 in lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js#L20-L22

Added lines #L20 - L22 were not covered by tests
}

/**
* Sets the current filter to physics only
*
* @return {void}
*/
setPhysicsOnly() {
if (!this.isPhysicsOnly()) {
this._selectionModel.selectedOptions = [];
this._selectionModel.select(RunDefinition.Physics);
this.notify();

Check warning on line 34 in lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js#L30-L34

Added lines #L30 - L34 were not covered by tests
}
}

/**
* Empty the current filter
*
* @return {void}
*/
setEmpty() {
if (!this.isEmpty) {
this.reset();
this.notify();

Check warning on line 46 in lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/RunsFilter/RunDefinitionFilterModel.js#L43-L46

Added lines #L43 - L46 were not covered by tests
}
}
}
31 changes: 0 additions & 31 deletions lib/public/components/Filters/RunsFilter/definitionFilter.js

This file was deleted.

25 changes: 25 additions & 0 deletions lib/public/components/Filters/RunsFilter/runDefinitionFilter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* @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.
*/

import { checkboxes } from '../common/filters/checkboxFilter.js';

/**
* Renders a list of checkboxes that lets the user look for runs with specific definition
*
* @param {RunDefinitionFilterModel} runDefinitionFilterModel run definition filter model
* @return {Component} the filter
*/
export const runDefinitionFilter = (runDefinitionFilterModel) => checkboxes(

Check warning on line 22 in lib/public/components/Filters/RunsFilter/runDefinitionFilter.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/RunsFilter/runDefinitionFilter.js#L22

Added line #L22 was not covered by tests
runDefinitionFilterModel.selectionModel,
{ selector: 'run-definition' },
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/**
* @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.
*/
import { FilterModel } from '../FilterModel.js';
import { SelectionModel } from '../../../common/selection/SelectionModel.js';

/**
* Filter model based on a selection model
*/
export class SelectionFilterModel extends FilterModel {
/**
* Constructor
*
* @param {object} [configuration] the selection filter configuration
* @param {SelectionOption[]} [configuration.availableOptions=[]] the list of available options
*/
constructor(configuration) {
super();

Check warning on line 27 in lib/public/components/Filters/common/filters/SelectionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/common/filters/SelectionFilterModel.js#L26-L27

Added lines #L26 - L27 were not covered by tests

this._selectionModel = new SelectionModel({ availableOptions: configuration.availableOptions });
this._selectionModel.bubbleTo(this);

Check warning on line 30 in lib/public/components/Filters/common/filters/SelectionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/common/filters/SelectionFilterModel.js#L29-L30

Added lines #L29 - L30 were not covered by tests
}

// eslint-disable-next-line valid-jsdoc
/**
* @inheritDoc
*/
reset() {
this._selectionModel.reset();

Check warning on line 38 in lib/public/components/Filters/common/filters/SelectionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/common/filters/SelectionFilterModel.js#L37-L38

Added lines #L37 - L38 were not covered by tests
}

// eslint-disable-next-line valid-jsdoc
/**
* @inheritDoc
*/
get isEmpty() {
return this._selectionModel.isEmpty;

Check warning on line 46 in lib/public/components/Filters/common/filters/SelectionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/common/filters/SelectionFilterModel.js#L45-L46

Added lines #L45 - L46 were not covered by tests
}

// eslint-disable-next-line valid-jsdoc
/**
* @inheritDoc
*/
get normalized() {
return this._selectionModel.selected.join(',');

Check warning on line 54 in lib/public/components/Filters/common/filters/SelectionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/common/filters/SelectionFilterModel.js#L53-L54

Added lines #L53 - L54 were not covered by tests
}

/**
* Return the underlying selection model
*
* @return {SelectionModel} the underlying selection model
*/
get selectionModel() {
return this._selectionModel;

Check warning on line 63 in lib/public/components/Filters/common/filters/SelectionFilterModel.js

View check run for this annotation

Codecov / codecov/patch

lib/public/components/Filters/common/filters/SelectionFilterModel.js#L62-L63

Added lines #L62 - L63 were not covered by tests
}
}
1 change: 0 additions & 1 deletion lib/public/components/common/selection/SelectionModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,6 @@ export class SelectionModel extends Observable {
*/
set selectedOptions(selected) {
this._selectedOptions = selected;
this.notify();
}

/**
Expand Down
11 changes: 9 additions & 2 deletions lib/public/views/Runs/ActiveColumns/runsActiveColumns.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import nEpnsFilter from '../../../components/Filters/RunsFilter/nEpns.js';
import { triggerValueFilter } from '../../../components/Filters/RunsFilter/triggerValueFilter.js';
import lhcPeriodsFilter from '../../../components/Filters/RunsFilter/lhcPeriod.js';
import { formatRunType } from '../../../utilities/formatting/formatRunType.js';
import definitionFilter from '../../../components/Filters/RunsFilter/definitionFilter.js';
import { runDefinitionFilter } from '../../../components/Filters/RunsFilter/runDefinitionFilter.js';
import { profiles } from '../../../components/common/table/profiles.js';
import { formatDuration } from '../../../utilities/formatting/formatDuration.mjs';
import { formatRunStart } from '../format/formatRunStart.js';
Expand Down Expand Up @@ -326,7 +326,14 @@ export const runsActiveColumns = {
}
return h('.flex-column.items-start', lines);
},
filter: definitionFilter,

/**
* Run definition filter component
*
* @param {RunsOverviewModel} runsOverviewModel the runs overview model
* @return {Component} the filter component
*/
filter: (runsOverviewModel) => runDefinitionFilter(runsOverviewModel.filteringModel.get('definitions')),

Check warning on line 336 in lib/public/views/Runs/ActiveColumns/runsActiveColumns.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Runs/ActiveColumns/runsActiveColumns.js#L336

Added line #L336 was not covered by tests
},
runDuration: {
name: 'Duration',
Expand Down
63 changes: 2 additions & 61 deletions lib/public/views/Runs/Overview/RunsOverviewModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { eorReasonTypeProvider } from '../../../services/eorReason/eorReasonType
import { runTypesProvider } from '../../../services/runTypes/runTypesProvider.js';
import { TimeRangeFilterModel } from '../../../components/Filters/RunsFilter/TimeRangeFilter.js';
import { RawTextFilterModel } from '../../../components/Filters/common/filters/RawTextFilterModel.js';
import { RunDefinitionFilterModel } from '../../../components/Filters/RunsFilter/RunDefinitionFilterModel.js';

/**
* Model representing handlers for runs page
Expand Down Expand Up @@ -59,6 +60,7 @@ export class RunsOverviewModel extends OverviewPageModel {
),
o2start: new TimeRangeFilterModel(),
o2end: new TimeRangeFilterModel(),
definitions: new RunDefinitionFilterModel(),
runTypes: new RunTypesFilterModel(runTypesProvider.items$),
eorReason: new EorReasonFilterModel(eorReasonTypeProvider.items$),
magnets: new MagnetsFilteringModel(),
Expand Down Expand Up @@ -166,8 +168,6 @@ export class RunsOverviewModel extends OverviewPageModel {
resetFiltering(fetch = true) {
this._filteringModel.reset();

this._runDefinitionFilter = [];

this._fillNumbersFilter = '';

this._runDurationFilter = null;
Expand Down Expand Up @@ -205,7 +205,6 @@ export class RunsOverviewModel extends OverviewPageModel {
*/
isAnyFilterActive() {
return this._filteringModel.isAnyFilterActive()
|| this._runDefinitionFilter.length > 0
|| this._fillNumbersFilter !== ''
|| this._runDurationFilter !== null
|| this._lhcPeriodsFilter !== null
Expand Down Expand Up @@ -253,61 +252,6 @@ export class RunsOverviewModel extends OverviewPageModel {
this.notify();
}

/**
* States if the given definition is currently in the run definition filter, and it's the only one
*
* @param {string} definition the run definition to look for
* @return {boolean} true if the definition is the only one currently applied
*/
isDefinitionOnlyOneInFilter(definition) {
return this._runDefinitionFilter.length === 1 && this._runDefinitionFilter[0] === definition;
}

/**
* States if the given definition is currently in the run definition filter
*
* @param {string} definition the run definition to look for
* @return {boolean} true if the definition is included in the filter
*/
isDefinitionInFilter(definition) {
return this._runDefinitionFilter.includes(definition);
}

/**
* Add a given definition in the current run definition filter if it is not already present, then refresh runs list
*
* @param {string} definition the run definition to add
* @return {void}
*/
addDefinitionFilter(definition) {
if (!this.isDefinitionInFilter(definition)) {
this._runDefinitionFilter.push(definition);
this._applyFilters();
}
}

/**
* Remove a given definition from the current run definition filter if it is in it (else do nothing) then refresh runs list
*
* @param {string} definition the definition to add
* @return {void}
*/
removeDefinitionFilter(definition) {
this._runDefinitionFilter = this._runDefinitionFilter.filter((existingDefinition) => definition !== existingDefinition);
this._applyFilters();
}

/**
* Set the list of definition to be used as the current run definition filter
*
* @param {string[]} definitions the new definition filter
* @return {void}
*/
setDefinitionFilter(definitions) {
this._runDefinitionFilter = [...definitions];
this._applyFilters();
}

/**
* Return the currently applied fill number filter
*
Expand Down Expand Up @@ -634,9 +578,6 @@ export class RunsOverviewModel extends OverviewPageModel {
*/
_getFilterQueryParams() {
return {
...this._runDefinitionFilter.length > 0 && {
'filter[definitions]': this._runDefinitionFilter.join(','),
},
...this._fillNumbersFilter && {
'filter[fillNumbers]': this._fillNumbersFilter,
},
Expand Down
14 changes: 7 additions & 7 deletions lib/public/views/Runs/Overview/RunsOverviewPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { runsActiveColumns } from '../ActiveColumns/runsActiveColumns.js';
import { table } from '../../../components/common/table/table.js';
import { runNumbersFilter } from '../../../components/Filters/RunsFilter/runNumbersFilter.js';
import { switchInput } from '../../../components/common/form/switchInput.js';
import { RunDefinition } from '../../../domain/enums/RunDefinition.js';

const TABLEROW_HEIGHT = 59;
// Estimate of the navbar and pagination elements height total; Needs to be updated in case of changes;
Expand All @@ -29,14 +28,15 @@ const PAGE_USED_HEIGHT = 215;
/**
* Display a toggle switch to display physics runs only
*
* @param {RunsOverviewModel} runsOverviewModel the model of the runs overview
* @param {RunDefinitionFilterModel} runDefinitionFilterModel the run definition filter model
* @returns {Component} the toggle switch
*/
export const togglePhysicsOnlyFilter = (runsOverviewModel) => {
const isPhysicsOnly = runsOverviewModel.isDefinitionOnlyOneInFilter(RunDefinition.Physics);
export const togglePhysicsOnlyFilter = (runDefinitionFilterModel) => {
const isPhysicsOnly = runDefinitionFilterModel.isPhysicsOnly();

Check warning on line 35 in lib/public/views/Runs/Overview/RunsOverviewPage.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Runs/Overview/RunsOverviewPage.js#L34-L35

Added lines #L34 - L35 were not covered by tests
const onChange = isPhysicsOnly
? () => runsOverviewModel.setDefinitionFilter([])
: () => runsOverviewModel.setDefinitionFilter([RunDefinition.Physics]);
? () => runDefinitionFilterModel.setEmpty()
: () => runDefinitionFilterModel.setPhysicsOnly();

Check warning on line 38 in lib/public/views/Runs/Overview/RunsOverviewPage.js

View check run for this annotation

Codecov / codecov/patch

lib/public/views/Runs/Overview/RunsOverviewPage.js#L37-L38

Added lines #L37 - L38 were not covered by tests

return switchInput(isPhysicsOnly, onChange, { labelAfter: 'PHYSICS ONLY' });
};

Expand All @@ -55,7 +55,7 @@ export const RunsOverviewPage = ({ runs: { overviewModel: runsOverviewModel }, m
h('.flex-row.header-container.g2.pv2', [
filtersPanelPopover(runsOverviewModel, runsActiveColumns),
h('.pl2#runOverviewFilter', runNumbersFilter(runsOverviewModel.filteringModel.get('runNumbers'))),
togglePhysicsOnlyFilter(runsOverviewModel),
togglePhysicsOnlyFilter(runsOverviewModel.filteringModel.get('definitions')),
exportRunsTriggerAndModal(runsOverviewModel, modalModel),
]),
h('.flex-column.w-100', [
Expand Down
17 changes: 4 additions & 13 deletions test/public/logs/detail.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,8 @@ module.exports = () => {
const context = browser.defaultBrowserContext();
context.overridePermissions(url, ['clipboard-read', 'clipboard-write', 'clipboard-sanitized-write']);

await goToPage(page, 'log-detail', { queryParameters: { id: 119 } });

// Expect the button to be there. Log 117 should be a parent to 119.
const log117CopyBtn = await page.waitForSelector('#copy-117');

await log117CopyBtn.click();
await pressElement(page, '#copy-117');

// The url has log 119, but the clipboard should have the url for 117.
const actualClipboardContents = await page.evaluate(() => navigator.clipboard.readText());
Expand All @@ -92,15 +88,10 @@ module.exports = () => {
});

it('should display feedback to the user when the copy link button is clicked', async () => {
await goToPage(page, 'log-detail', { queryParameters: { id: 119 } });

// Expect the button to be there. Log 117 should be a parent to 119.
const log117CopyBtn = await page.waitForSelector('#copy-117');

// Expect the text before the click to be different after
await expectInnerText(page, '#copy-117', 'Copy Link');
await log117CopyBtn.click();
await expectInnerText(page, '#copy-117', 'Copied!');
await expectInnerText(page, '#copy-118', 'Copy Link');
await pressElement(page, '#copy-118');
await expectInnerText(page, '#copy-118', 'Copied!');
});

it('should successfully expand opened log when displaying a log tree', async () => {
Expand Down
Loading

0 comments on commit a527733

Please sign in to comment.