[O2B-1552] Move ad hoc filters inside runs overviews model to filtering model#2106
Conversation
* Filtering by stableBeamsStart and stableBeamsEnd has been added to LHC Fills overview page * lhcFills endpoint & DTO validation modified and testing added for the aforementioned changes --------- Co-authored-by: GuustMetz <guust.metz@cern.ch> Co-authored-by: Guust <metzguust@gmail.com>
* Replaced the two-query pattern with a single queryBuilder in GetAllEnvironmentsUseCase. The previous approach was redundant following Sequelize performance improvements; furthermore, the original implementation's logic was flawed which resulted in the pagination bug.
NarrowsProjects
changed the title
…unsOverviewModel-to-filteringModel
…unsOverviewModel-to-filteringModel
…cNotBadFraction name
…unsOverviewModel-to-filteringModel
…y to only notify when notBadFraction is not empty.
| * | ||
| * @return {boolean} true if filter is stable beams only | ||
| */ | ||
| isToggled() { |
There was a problem hiding this comment.
Could this be a getter? isToggled represents a state rather than an action, so I think it would be more natural, what do you think?
There was a problem hiding this comment.
It makes sense, especially since there is already precedence for this with how FilterModels have isEmpty as a getter
| * | ||
| * @param {RadioSelectionModel} selectionModel the a selectionmodel | ||
| * @param {string} filterName the name of the filter | ||
| * @return {vnode} A number of radiobuttons corresponding with the selection options |
There was a problem hiding this comment.
radiobuttons think you forgot a space.
There was a problem hiding this comment.
Good catch
There was a problem hiding this comment.
Could you rename this to match the new name of the filter?
There was a problem hiding this comment.
I have noticed that this dataPassIds change does not actually work as it is testing relatively here and not checking the parent, as demonstrated by the detectorsQcNotBadFraction using the helpers.
It would be good to rectify this and also add a test in the api.
| */ | ||
| export const toggleFilter = (toggleFilterModel, name, id, radioButtonMode = false) => { | ||
| if (radioButtonMode) { | ||
| return h('.form-group-header.flex-row.w-100', [ |
There was a problem hiding this comment.
Given we are creating toggleFilter, which is definitely a nice abstraction, I looked at the classes here and form-group-header I don't think should be used. Neither in toggleFilter.js or in radioButtonFilter.js. From what I can tell it was put into combinationOperatorChoiceComponent.js to add some spacing between its radio buttons and then (in all current cases) a subsequent selection dropdown. But in the toggleFilter.js and radioButtonFilter.js these do not have any following elements and this is then excess margin.
2 participants
I have a JIRA ticket
Notable changes for users:
Notable changes for developers:
Filter restructuring
ddflpFilter,dcsFilter, andepnFilterhave been moved to thefilteringModel.detectorsQc[_id][notBadFraction]filters have been moved to a new model:MultiCompositeFilterModel.gaq[notBadFraction]now uses its own filter model with unique normalization logic:GaqFilterModel.New filter models
GaqFilterModelgaq[notBadFraction].ToggleFilterModelmcReproducibleAsNotBadhas been converted to use this model.MultiCompositeFilterModelShared filter behavior
mcReproducibleAsNotBad:ToggleFilterModel.GaqFilterModelMultiCompositeFilterModelNew components
RadioButtonFilterToggleFilter(component + model)ToggleFilterModel.Renaming
detectorsQc[_id][notBadFraction]→detectorsQcNotBadFraction[_id]filterModel.Implementation note
This approach avoids:
NumericalComparisonFilterModelsolely for normalization, orMultiCompositeFilterModelwithin anotherMultiCompositeFilterModelto achieve automatic normalization.Changes made to the database: