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

fix: Searches view fixes (ET-297) #9487

Merged
merged 5 commits into from
Jun 11, 2024
Merged

fix: Searches view fixes (ET-297) #9487

merged 5 commits into from
Jun 11, 2024

Conversation

johnkim-det
Copy link
Contributor

@johnkim-det johnkim-det commented Jun 7, 2024

Ticket

ET-297

Description

Addresses feedback from Searches View acceptance meeting and other issues

  • Change filter criteria from "numTrials > 1" to "searcherType != single"
  • Rename "Trials" tab to "Runs"
  • On Search Runs, remove "Searcher Type", "Searcher Metric", and "Searcher Metric Value" from default columns
    • Make sure Search Runs for a project and the Project Runs view have separate settings (for visible columns, filters, etc)
  • Fix bug with back button from Search Runs
    • This was an issue in FlatRuns related to the handling of the page param. Changing pagination in FlatRuns to match how its handled in F_ExperimentList to avoid this issue

Test Plan

  • Make sure Project -> Searches tab only shows experiments with "single" Searcher Type. Can show experiments with only 1 trial.
  • Click the id for a search to view the Search Details page. Tab should show "Runs" instead of "Trials"
  • Search Details' Runs tab should not show "Searcher Type", "Searcher Metric", and "Searcher Metric Value" as default columns (can reset columns via Columns menu to verify)
  • On Search Details' Runs tab, make sure there are enough runs to span multiple pages. Change the page number. Refresh the app. Page number should still be the changed one.
  • Hitting the back button from the Search Details' page should navigate to the previous page as expected

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.

@johnkim-det johnkim-det added the feature-flagged The changes in this PR are behind a feature flag label Jun 7, 2024
@johnkim-det johnkim-det requested a review from a team as a code owner June 7, 2024 18:22
@cla-bot cla-bot bot added the cla-signed label Jun 7, 2024
@johnkim-det johnkim-det requested review from EmilyBonar and removed request for keita-determined June 7, 2024 18:22
Copy link

netlify bot commented Jun 7, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit cad6e8e
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/6668a6fc92d3d60008114527
😎 Deploy Preview https://deploy-preview-9487--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.

Copy link

codecov bot commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 0% with 54 lines in your changes missing coverage. Please review.

Project coverage is 43.78%. Comparing base (a45aa1e) to head (cad6e8e).
Report is 23 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9487      +/-   ##
==========================================
- Coverage   51.06%   43.78%   -7.29%     
==========================================
  Files         747      423     -324     
  Lines      111167    71203   -39964     
  Branches     2778     2779       +1     
==========================================
- Hits        56772    31176   -25596     
+ Misses      54220    39852   -14368     
  Partials      175      175              
Flag Coverage Δ
harness ?
web 44.12% <0.00%> (-0.01%) ⬇️

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

Files Coverage Δ
...ebui/react/src/pages/FlatRuns/FlatRuns.settings.ts 0.00% <0.00%> (ø)
webui/react/src/components/Searches/Searches.tsx 0.00% <0.00%> (ø)
webui/react/src/pages/FlatRuns/columns.ts 0.00% <0.00%> (ø)
webui/react/src/pages/SearchDetails.tsx 0.00% <0.00%> (ø)
webui/react/src/pages/FlatRuns/FlatRuns.tsx 0.00% <0.00%> (ø)

... and 324 files with indirect coverage changes

Copy link
Contributor

@EmilyBonar EmilyBonar left a comment

Choose a reason for hiding this comment

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

Looks good!

webui/react/src/pages/FlatRuns/columns.ts Outdated Show resolved Hide resolved
@johnkim-det johnkim-det merged commit e138267 into main Jun 11, 2024
82 of 97 checks passed
@johnkim-det johnkim-det deleted the ET-297 branch June 11, 2024 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed feature-flagged The changes in this PR are behind a feature flag
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants