determined
determined copied to clipboard
fix: actually pass search filters to experiment actions
Description
This fixes an issue where bulk experiment actions did not work when using select all. The endpoint used a different, incompatible filter format that did not apply the proper filters to the selection. We introduce the new filter as a new parameter (for backwards compatibility's sake) and apply it to all actions.
Test Plan
one
- On an experiment list page, select 2 experiments, click the actions menu and then click "archive"
- the selected experiments should be archived (if the current view does not show archived experiments, they should be hidden)
- in the filters menu, select "Show archived", then click the selection column header, then click "select all"
- click the actions menu and select "unarchive"
- confirm the action
- The experiment list should show the same experiments regardless of the "show archived" filter state
two
- submit an unmanaged experiment to a project with 20 or more experiments in it with a name beginning with the letter a
- to the same project, submit a managed experiment with a name beginning with the letter z
- on an experiment list of that project, disable infinite pagination
- on page one of the experiment list, select the unmanaged experiment
- on the last page of the experiment list, select the managed experiment
- The ui should show that you have two experiments selected
- click the actions menu and select "pause"
- confirm the action
- There should be a popup saying that one experiment was paused
Checklist
- [x] Changes have been manually QA'd
- [x] User-facing API changes need the "User-facing API Change" label.
- [x] Release notes should be added as a separate file under
docs/release-notes/
. See Release Note for details. - [x] Licenses should be included for new code which was copied and/or modified from any external code.
Ticket
ET-33
Deploy Preview for determined-ui canceled.
Name | Link |
---|---|
Latest commit | 76f65e2abdb5475649ceaab4aeeeaee6fb1d0e33 |
Latest deploy log | https://app.netlify.com/sites/determined-ui/deploys/662ad65daae7b00008784347 |
Codecov Report
Attention: Patch coverage is 16.19537%
with 326 lines
in your changes are missing coverage. Please review.
Project coverage is 44.69%. Comparing base (
1e87778
) to head (76f65e2
). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #9083 +/- ##
==========================================
- Coverage 44.71% 44.69% -0.03%
==========================================
Files 1270 1270
Lines 155328 155438 +110
Branches 2436 2438 +2
==========================================
+ Hits 69461 69474 +13
- Misses 85631 85728 +97
Partials 236 236
Flag | Coverage Δ | |
---|---|---|
backend | 41.62% <21.83%> (-0.03%) |
:arrow_down: |
harness | 64.19% <14.28%> (-0.09%) |
:arrow_down: |
web | 35.21% <10.34%> (-0.01%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Files | Coverage Δ | |
---|---|---|
...omponents/FilterForm/components/FilterFormStore.ts | 93.68% <100.00%> (ø) |
|
master/internal/api_experiment.go | 56.34% <96.55%> (+0.11%) |
:arrow_up: |
master/internal/api_runs.go | 70.20% <80.00%> (ø) |
|
master/internal/filter/experiment_filter.go | 95.14% <83.33%> (ø) |
|
webui/react/src/components/Searches/Searches.tsx | 0.00% <0.00%> (ø) |
|
webui/react/src/components/ExperimentMoveModal.tsx | 67.28% <50.00%> (+0.91%) |
:arrow_up: |
...react/src/components/ExperimentRetainLogsModal.tsx | 60.13% <60.00%> (+1.07%) |
:arrow_up: |
webui/react/src/services/types.ts | 0.00% <0.00%> (ø) |
|
master/internal/api_project.go | 24.16% <0.00%> (ø) |
|
...eact/src/components/ExperimentTensorBoardModal.tsx | 0.00% <0.00%> (ø) |
|
... and 5 more |
@keita-determined updated the pr with a fix for the selection spanning across pages and added a test case that should handle most edge cases.
I'm not entirely clear on why we need to maintain the old filter parameter and struct for these bulk endpoints, do we have other things using these outside of our WebUI?
For the scope of this ticket from a triage perspective, I can understand why we'd leave both filter parameters in; however, for the sake of code hygiene, I'd like to come back and re-evaluate how we approach this problem, by at least removing the need for one or the other parameter, and I'd be curious if we could get to a position where we could eliminate the need for specifying the filters entirely (only allowing actions to be done on a per page level).
Additionally, our bulk select feature feels odd. We provide options to select the first 5, 10, 25, or all experiments; however, our page sizes are restricted to 20, 40, and 80. Not a single option allows selecting an entire page. Why would our bulk select not first select everything in the page and then give the object to select everything in the given project with the filters applied.
closing -- we can circle back to this later.