Skip to content

[O2B-1555] Add normalize setter functions to all classes that extend filter model and selection model#2143

Merged
graduta merged 14 commits into
mainfrom
impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel
May 26, 2026
Merged

[O2B-1555] Add normalize setter functions to all classes that extend filter model and selection model#2143
graduta merged 14 commits into
mainfrom
impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel

Conversation

@NarrowsProjects
Copy link
Copy Markdown
Collaborator

@NarrowsProjects NarrowsProjects commented Apr 23, 2026

I have a JIRA ticket

  • branch and/or PR name(s) include(s) JIRA ID
  • issue has "Fix version" assigned
  • issue "Status" is set to "In review"
  • PR labels are selected

Notable changes for users:

  • Setting filters will now display them in the url

Notable changes for developers:

  • All filters now have a normalized setter that will set the values of the filters based on the output of the normalize getter in query parameter form.

Changes made to the database:

  • N/A

@NarrowsProjects NarrowsProjects self-assigned this Apr 23, 2026
@NarrowsProjects NarrowsProjects added frontend javascript Pull requests that update Javascript code labels Apr 23, 2026
@NarrowsProjects NarrowsProjects changed the base branch from main to feature/O2B-1556/Configure-FilteringModel-observability-to-set-active-filters-to-the-url April 23, 2026 15:10
@NarrowsProjects NarrowsProjects changed the title Impov/o2 b 1555/add normalize setter functions to all classes that extend filter model and selection model [O2B-1555] Add normalize setter functions to all classes that extend filter model and selection model Apr 23, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 0% with 85 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.89%. Comparing base (2497966) to head (7ec70c2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...blic/components/common/selection/SelectionModel.js 0.00% 15 Missing ⚠️
...onents/Filters/RunsFilter/MagnetsFilteringModel.js 0.00% 12 Missing ⚠️
...s/common/filters/NumericalComparisonFilterModel.js 0.00% 6 Missing ⚠️
.../Filters/common/filters/ProcessedTextInputModel.js 0.00% 5 Missing ⚠️
...ponents/Filters/RunsFilter/EorReasonFilterModel.js 0.00% 4 Missing ⚠️
.../Filters/RunsFilter/MultiCompositionFilterModel.js 0.00% 4 Missing ⚠️
...blic/views/Runs/ActiveColumns/runsActiveColumns.js 0.00% 4 Missing ⚠️
...ponents/Filters/RunsFilter/DetectorsFilterModel.js 0.00% 3 Missing ⚠️
...ic/components/Filters/RunsFilter/GaqFilterModel.js 0.00% 3 Missing ⚠️
...nts/Filters/RunsFilter/RunDefinitionFilterModel.js 0.00% 3 Missing ⚠️
... and 15 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2143      +/-   ##
==========================================
- Coverage   45.93%   45.89%   -0.05%     
==========================================
  Files        1036     1035       -1     
  Lines       17150    17167      +17     
  Branches     3115     3124       +9     
==========================================
  Hits         7878     7878              
- Misses       9272     9289      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@NarrowsProjects NarrowsProjects force-pushed the impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel branch 11 times, most recently from e540f27 to edb626f Compare April 27, 2026 07:04
@NarrowsProjects NarrowsProjects force-pushed the feature/O2B-1556/Configure-FilteringModel-observability-to-set-active-filters-to-the-url branch from 4a30d27 to ae6c287 Compare April 27, 2026 07:17
@NarrowsProjects NarrowsProjects force-pushed the impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel branch 2 times, most recently from 88cc4f4 to 49ed030 Compare April 27, 2026 07:36
Base automatically changed from feature/O2B-1556/Configure-FilteringModel-observability-to-set-active-filters-to-the-url to main May 5, 2026 10:02
@NarrowsProjects NarrowsProjects force-pushed the impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel branch 4 times, most recently from bc70145 to beca3c1 Compare May 7, 2026 17:51
…ownModel as submodels to extend it instead
@NarrowsProjects NarrowsProjects force-pushed the impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel branch from beca3c1 to 319bb4d Compare May 11, 2026 06:50
Comment thread lib/public/components/common/selection/SelectionModel.js Outdated
*
* @return {string}
*/
set normalized(value) {
Copy link
Copy Markdown
Collaborator

@isaachilly isaachilly May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if value is null or undefined? Could value ever be null or undefined? Maybe not in UI but maybe if passed in URL? What do you think? Goes for all the setters.

Copy link
Copy Markdown
Collaborator Author

@NarrowsProjects NarrowsProjects May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since queryRouter omits any values that are undefined when computing its params field, the normalized values derived from params will always be usable.

As a result, if a value is passed via URL (which is why the setters exist), neither value nor its equivalent can end up being undefined.

Copy link
Copy Markdown
Collaborator Author

@NarrowsProjects NarrowsProjects May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, while undefined can't really reach the setters, empty strings can

Comment thread lib/public/components/common/selection/SelectionModel.js
Comment thread lib/public/components/Filters/RunsFilter/MagnetsFilteringModel.js Outdated
Comment thread lib/public/components/Filters/RunsFilter/MagnetsFilteringModel.js
Comment thread lib/public/components/common/selection/SelectionModel.js Outdated
@NarrowsProjects NarrowsProjects force-pushed the impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel branch from f314b72 to 1443e47 Compare May 19, 2026 09:34
Copy link
Copy Markdown
Member

@graduta graduta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice effort, seems to be in the right direction. I wanted to ask if you had the opportunity to look at tests or update them? More specifically, given the refactor, what was your testing strategy to ensure consistency across changes?

* Accounts for the options being either RemoteData or an array.
*
* @return {SelectionDropdownModel} the dropdown model
* @param {object}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete docs

Comment thread lib/public/components/Filters/RunsFilter/MagnetsFilteringModel.js
Comment thread lib/public/components/common/selection/SelectionModel.js Outdated
Comment thread lib/public/components/Filters/common/TagFilterModel.js Outdated
@NarrowsProjects
Copy link
Copy Markdown
Collaborator Author

NarrowsProjects commented May 26, 2026

Nice effort, seems to be in the right direction. I wanted to ask if you had the opportunity to look at tests or update them? More specifically, given the refactor, what was your testing strategy to ensure consistency across changes?

The refactors were all in models that already had tests that checked the effectiveness.
The only thing of note that changed was how the runTypes filter had the return type of it's normalized function changed.

However, according to the the back-end code. And the test that checks the effectiveness of the runTypes filter on the front-end, this change did not make a difference.

So, I'm not quite sure which tests would be updated.

@NarrowsProjects NarrowsProjects requested a review from graduta May 26, 2026 11:02
…s-to-all-classes-that-extend-FilterModel-and-SelectionModel
@graduta graduta merged commit 469ef0f into main May 26, 2026
39 of 42 checks passed
@graduta graduta deleted the impov/O2B-1555/Add-normalize-setter-functions-to-all-classes-that-extend-FilterModel-and-SelectionModel branch May 26, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend javascript Pull requests that update Javascript code

Development

Successfully merging this pull request may close these issues.

3 participants