[O2B-1505] LHCfills beam duration filter #2035
Conversation
Houwie7000
added
frontend
backend
javascript
|
Git borked, it seems that it included changes from the previous task in this one... |
Houwie7000
force-pushed
the
feature/O2B-1503/lhcfills-fill-numbers-filter
branch
from
56e5c6a to
847d092
Compare
Houwie7000
force-pushed
the
feature/O2B-1505/lhcfills-beam-duration-filter
branch
from
23dbb7b to
11c1f9d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2035 +/- ##
==========================================
- Coverage 45.43% 45.35% -0.08%
==========================================
Files 1028 1031 +3
Lines 17136 17181 +45
Branches 3120 3127 +7
==========================================
+ Hits 7785 7792 +7
- Misses 9351 9389 +38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This reverts commit 695622b.
…ith the query/reset logic. Just set the value afterwards
Houwie7000
force-pushed
the
feature/O2B-1503/lhcfills-fill-numbers-filter
branch
from
847d092 to
3110c29
Compare
Houwie7000
force-pushed
the
feature/O2B-1503/lhcfills-fill-numbers-filter
branch
from
768e86f to
1e3f503
Compare
Houwie7000
force-pushed
the
feature/O2B-1505/lhcfills-beam-duration-filter
branch
from
7f0d22c to
7628bca
Compare
| fillNumbers: Joi.string().trim().custom(validateRange).messages({ | ||
| 'any.invalid': '{{#message}}', | ||
| }), | ||
| beamDuration: Joi.string().trim().min(8).max(8).custom(validateTime).messages({ |
There was a problem hiding this comment.
Could add here a pattern to match the timestamp format to clean up validate function.
lib/utilities/validateTime.js
Outdated
| */ | ||
| export const validateTime = (value, helpers) => { | ||
| const timeSectionsString = value.split(':'); | ||
| let timeSeconds = 0; |
There was a problem hiding this comment.
I think the powerValue iterating through is overcomplicating the matter.
I suggest that with the pattern matching previously done and the fact you know where the hours, minutes and seconds will be, just extract them like:
const [hoursStr, minutesStr, secondsStr] = value.split(':');
const hours = Number(hoursStr);
and just hardcode the time multipliers return hours * 3600 + minutes * 60 + seconds.
I think this will simplify the function and make it more readable.
| } | ||
| } | ||
| // Beam duration filter and corresponding operator. | ||
| if (beamDuration !== null && beamDuration !== undefined && beamDurationOperator) { |
There was a problem hiding this comment.
This is quite an overloaded statement. Consider taking out duration.
duration = beamDuration === 0 ? null : beamDuration
| isAnyFilterActive() { | ||
| return this._filteringModel.isAnyFilterActive(); | ||
| return this._filteringModel.isAnyFilterActive() | ||
| || this._beamDurationOperator !== defaultBeamDurationOperator; |
There was a problem hiding this comment.
If operator is changed but the duration doesn’t actually have a value I think it is okay to leave it out of reset filters.
This is same functionality as runs duration filter.
| this._beamDurationOperator = defaultBeamDurationOperator; | ||
|
|
||
| this._filteringModel.observe(() => this._applyFilters(true)); | ||
| this._filteringModel.visualChange$.bubbleTo(this); |
There was a problem hiding this comment.
Adding debounce here is good practice.
| /** | ||
| * Beam duration operator setter | ||
| */ | ||
| setBeamDurationOperator(beamDurationOperator) { |
There was a problem hiding this comment.
Could be worth viewing how numericalComparisonFilter works and making one for text.
…ture/O2B-1505/lhcfills-beam-duration-filter
|
Fixing broken tests WIP..... |
I have a JIRA ticket
Notable changes for users:
Notable changes for developers:
Changes made to the database: