-
Notifications
You must be signed in to change notification settings - Fork 357
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: display archived column for runs and searches #9987
Conversation
This adds the archived column to the columns api. It also updates the flat runs view to handle showing the archived state of runs and ensures the archived definition matches the show archived behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9987 +/- ##
==========================================
- Coverage 54.51% 54.51% -0.01%
==========================================
Files 1256 1256
Lines 156695 156720 +25
Branches 3603 3605 +2
==========================================
+ Hits 85426 85430 +4
- Misses 71136 71157 +21
Partials 133 133
Flags with carried forward coverage won't be shown. Click here to find out more.
|
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backend LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Web side of things looks good. Backend seems fine.
Column: "archived", | ||
DisplayName: "Archived", | ||
Location: projectv1.LocationType_LOCATION_TYPE_RUN, | ||
Type: projectv1.ColumnType_COLUMN_TYPE_UNSPECIFIED, // no boolean type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: would it be possible to add a boolean type in this pr, or is that out of scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's handle it separately because parentArchived
in runs + metadata is also affected by that change.
@@ -60,9 +61,9 @@ const EXCLUDED_SEARCH_DEFAULT_COLUMNS: RunColumn[] = [ | |||
|
|||
export type RunColumn = (typeof runColumns)[number]; | |||
|
|||
export const defaultRunColumns: RunColumn[] = [...runColumns]; | |||
export const defaultRunColumns: RunColumn[] = runColumns.slice(0, -1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: might be worth explicitly filtering out "archived" to avoid issues if the array gets reordered
Ticket
ET-304
Description
This adds the archived column to the columns api. It also updates the flat runs view to handle showing the archived state of runs and ensures the archived definition matches the show archived behavior.
Test Plan
When viewing the column list for the new experiment list, the searches view, and the runs view, the user should see the "archived" column in the general tab.
Checklist
docs/release-notes/
See Release Note for details.