Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: revert select all #10208

Merged
merged 9 commits into from
Nov 20, 2024
Merged

chore: revert select all #10208

merged 9 commits into from
Nov 20, 2024

Conversation

hkang1
Copy link
Contributor

@hkang1 hkang1 commented Nov 18, 2024

Ticket

N/A

Description

Reverting Select-All feature for the experiment list page. The select-all feature works with the frontend sending a custom query to the backend which parses the query and performs the action. This capability expands the net to be able to capture the entire matching set of experiments. Previously there was a bug which allowed an incorrect query to cause deletion of all experiments. For the purpose of winding down the codebase, we are removing this for stability reasons.

Test Plan

Basic idea of ensuring the behavior reverts back to how select all used to behave, which was that the select all chose all the experiments on a given page. So if users see experiments #26~#50 on page 2, it Select All would select experiments 26-50. However the selections are cumulative. So if the user navigates to a different page and select more, it would include new selections plus the original selection, BUT it only applies the API bulk action to the new selections.

  • ensure select all behaves as it used to and described above
  • ensure the batch API calls match the selected experiments

Checklist

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

@cla-bot cla-bot bot added the cla-signed label Nov 18, 2024
Copy link

netlify bot commented Nov 18, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit 9b7d857
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/673d5c321aad82000866673a
😎 Deploy Preview https://deploy-preview-10208--determined-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@hkang1 hkang1 force-pushed the revert-select-all branch 4 times, most recently from cc45518 to f477465 Compare November 19, 2024 18:18
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 28.36305% with 442 lines in your changes missing coverage. Please review.

Project coverage is 53.67%. Comparing base (258455d) to head (9b7d857).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
webui/react/src/pages/FlatRuns/FlatRuns.tsx 11.17% 159 Missing ⚠️
...bui/react/src/pages/F_ExpList/F_ExperimentList.tsx 0.00% 151 Missing and 1 partial ⚠️
webui/react/src/components/Searches/Searches.tsx 32.82% 88 Missing ⚠️
webui/react/src/components/TableActionBar.tsx 53.84% 24 Missing ⚠️
webui/react/src/components/ExperimentMoveModal.tsx 60.60% 13 Missing ⚠️
...eact/src/components/ExperimentTensorBoardModal.tsx 71.42% 4 Missing ⚠️
...i/react/src/pages/FlatRuns/FlatRunActionButton.tsx 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10208      +/-   ##
==========================================
- Coverage   53.88%   53.67%   -0.21%     
==========================================
  Files        1257     1255       -2     
  Lines      156507   155708     -799     
  Branches     3653     3617      -36     
==========================================
- Hits        84335    83582     -753     
+ Misses      72039    71993      -46     
  Partials      133      133              
Flag Coverage Δ
backend 45.81% <ø> (-0.25%) ⬇️
harness 67.37% <ø> (ø)
web 54.45% <28.36%> (-0.31%) ⬇️

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

Files with missing lines Coverage Δ
.../react/src/components/ComparisonView.test.mock.tsx 99.30% <ø> (-0.02%) ⬇️
webui/react/src/components/ComparisonView.tsx 67.72% <100.00%> (+0.58%) ⬆️
.../react/src/components/ExperimentActionDropdown.tsx 79.79% <ø> (-0.06%) ⬇️
...bui/react/src/components/ExperimentCreateModal.tsx 86.50% <100.00%> (ø)
webui/react/src/components/LoadableCount.tsx 100.00% <100.00%> (+7.44%) ⬆️
webui/react/src/components/RunActionDropdown.tsx 95.59% <100.00%> (-0.02%) ⬇️
...components/RunFilterInterstitialModalComponent.tsx 97.85% <100.00%> (+0.07%) ⬆️
...ages/ExperimentDetails/ExperimentDetailsHeader.tsx 74.32% <ø> (-0.04%) ⬇️
webui/react/src/pages/ExperimentList.tsx 0.00% <ø> (ø)
...ebui/react/src/pages/FlatRuns/FlatRunMoveModal.tsx 85.85% <100.00%> (+1.07%) ⬆️
... and 11 more

... and 6 files with indirect coverage changes

---- 🚨 Try these New Features:

@hkang1 hkang1 marked this pull request as ready for review November 19, 2024 21:12
@hkang1 hkang1 requested review from a team as code owners November 19, 2024 21:12
Copy link
Contributor

@gt2345 gt2345 left a comment

Choose a reason for hiding this comment

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

Commits look safe, and functionalities work as expected

@hkang1 hkang1 enabled auto-merge (squash) November 20, 2024 02:05
@hkang1 hkang1 disabled auto-merge November 20, 2024 14:02
@hkang1 hkang1 merged commit d189416 into main Nov 20, 2024
76 of 89 checks passed
@hkang1 hkang1 deleted the revert-select-all branch November 20, 2024 14:02
Copy link

The backport to release-0.38.0 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release-0.38.0 release-0.38.0
# Navigate to the new working tree
cd .worktrees/backport-release-0.38.0
# Create a new branch
git switch --create backport-10208-to-release-0.38.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d1894166a4e28f6f33c465571a8becf55cfab042
# Push it to GitHub
git push --set-upstream origin backport-10208-to-release-0.38.0
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release-0.38.0

Then, create a pull request where the base branch is release-0.38.0 and the compare/head branch is backport-10208-to-release-0.38.0.

djanicekpach pushed a commit that referenced this pull request Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants