fix: Bulk Action bug

[johnkim-det]

Ticket

ET-237

Description

Removes boolean selectAll and dependent logic from BulkAction/BatchAction handling.

Test Plan

• [ ] Select multiple rows from the Experiment List
• [ ] Any action selected from the "Actions" menu that appears should function as expected. Action should not happen on more or less items than what's selected

Checklist

• [ ] Changes have been manually QA'd
• [ ] User-facing API changes need the "User-facing API Change" label.
• [ ] Release notes should be added as a separate file under docs/release-notes/. See Release Note for details.
• [ ] Licenses should be included for new code which was copied and/or modified from any external code.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 19.62025% with 254 lines in your changes are missing coverage. Please review.

Project coverage is 44.68%. Comparing base (aea83df) to head (c97f2c1). Report is 24 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9255      +/-   ##
==========================================
+ Coverage   44.66%   44.68%   +0.02%     
==========================================
  Files        1273     1273              
  Lines      155822   155830       +8     
  Branches     2439     2438       -1     
==========================================
+ Hits        69591    69629      +38     
+ Misses      85992    85962      -30     
  Partials      239      239              

Flag	Coverage Δ
backend	41.75% <53.08%> (+0.01%)	:arrow_up:
harness	64.21% <0.00%> (-0.02%)	:arrow_down:
web	35.12% <8.40%> (+0.04%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
master/internal/api_experiment.go	56.74% <100.00%> (+0.51%)	:arrow_up:
master/internal/api_runs.go	73.61% <100.00%> (ø)
master/internal/core.go	4.10% <ø> (ø)
webui/react/src/components/UserSettings.tsx	85.71% <100.00%> (+2.20%)	:arrow_up:
...ages/ExperimentDetails/ExperimentDetailsHeader.tsx	74.96% <100.00%> (+0.03%)	:arrow_up:
.../react/src/components/ExperimentActionDropdown.tsx	0.00% <0.00%> (ø)
webui/react/src/components/ExperimentMoveModal.tsx	66.07% <50.00%> (-0.30%)	:arrow_down:
...react/src/components/ExperimentRetainLogsModal.tsx	59.21% <75.00%> (+0.15%)	:arrow_up:
webui/react/src/services/types.ts	0.00% <0.00%> (ø)
webui/react/src/pages/ExperimentList.tsx	0.00% <0.00%> (ø)
... and 8 more

... and 3 files with indirect coverage changes

[netlify[bot]]

✅ Deploy Preview for determined-ui ready!

Name	Link
🔨 Latest commit	c97f2c12067b711b685b3fe4b16d6aa3aa3c36ca
🔍 Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/6633c15a53d22600083b17ca
😎 Deploy Preview	https://deploy-preview-9255--determined-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[corban-beaird]

It looks like the selection count isn't updating properly, the bulk action is properly scope to only the visible experiments, but we're telling users they have 22 out of 2 experiments selected.

https://github.com/determined-ai/determined/assets/25434156/eb7d6512-8b4b-4dd4-affd-9aab521823d9

It also leads to some confusion because it looks like we've selected things, but we don't allow the option to do anything with them. To clarify the problem is not with the lack of an ability to perform actions on the "ghost" selections, but with the ghost selections.

It's also a tad quirky since now we make it seem like they can perform action across multiple pages, but we're not really giving them an option to do so.

[corban-beaird]

@keita-determined

qq:

BulkAction requests to include the relevant project id

where do we supposed to see the projectId? i didnt see it in archive API

We're leaving this out as a requirement this is handled by exhaustively listing out the experiment ids.

@johnkim-det However, the more I'm thinking about this, in the event that an experiment is no longer in the currently open project, then we'd like to make sure don't perform an action on it unintentionally. Assuming another user moved an experiment at the same time and their request was handled first.

So yes, we want to include the project_id, sorry for any confusion!

[johnkim-det]

addressing the last review by separately fetching experiments by id, for the selected ids. if this approach is acceptable, I think it does address the problem of batch actions working with selections across pages.

[corban-beaird]

Looks like there's a failure in tests/cluster/test_workspace_org.py::test_workspace_org that isn't occurring on main. There are also some timeouts in other tests, are those flakes?

@MikhailKardash I was able to replicate tests/cluster/test_workspace_org.py::test_workspace_org on my local if I'm running a licensed instance on latest main w/RBAC enabled; however, the test passes on both this branch & main when it's not.

The issue I'm seeing is that the determined user doesn't have view permission on the Uncategorized workspace when RBAC is enabled which is being assumed. **Edit: I was able to reproduce the workspace_org issue outside of the RBAC enabled devcluster, working to resolve now *** Resolved, the bulk action move hadn't account for single experiment request like the others, corrected.

The timeout failures should now be resolved, the batch kill request had not been correctly adjusted originally; it should be now.

[johnkim-det]

my understanding is that infinite scroll should be disabled for now, but ideally want to bring it back if we can find a solution that works well with row selection. I can create a ticket for that so it's captured and we make sure to have that discussion.

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @corban-beaird on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla.

After we approve your CLA, we will update the contributors list (private) and comment @cla-bot[bot] check to rerun the check.

[corban-beaird]

@cla-bot[bot] check

[cla-bot[bot]]

The cla-bot has been summoned, and re-checked this pull request!