chore: run bulk action interstitial component #9390
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ashtonG
requested review from
keita-determined
and removed request for
thiagodallacqua-hpe
| @@ -6,7 +6,7 @@ import { useCallback, useEffect } from 'react'; | |||
| export type onInterstitialCloseActionType = (reason: 'ok' | 'close' | 'failed') => void; | |||
|
|
|||
| interface Props<T> { | |||
| onCloseAction: (reason: 'ok' | 'close' | 'failed') => void; | |||
| onCloseAction: onInterstitialCloseActionType; | |||
There was a problem hiding this comment.
oops, nice improvement!
webui/react/src/components/RunFilterInterstitialModalComponent.test.tsx
Outdated
Show resolved
Hide resolved
webui/react/src/components/RunFilterInterstitialModalComponent.test.tsx
Outdated
Show resolved
Hide resolved
| const { lastCall } = vi.mocked(searchRuns).mock; | ||
| const filterFormSetString = lastCall?.[0].filter; | ||
| expect(filterFormSetString).toBeDefined(); | ||
| const filterFormSet = JSON.parse(filterFormSetString || ''); |
There was a problem hiding this comment.
can we add type here? prob FilterFormSetWithoutId?
| const onCloseAction = vi.fn(); | ||
| userEvent.setup(); | ||
|
|
||
| const view = render( |
There was a problem hiding this comment.
super nit: can we get rid of view?
| ); | ||
|
|
||
| return ( | ||
| <InterstitialModal.Component |
There was a problem hiding this comment.
qq: this does not show the modal like the one in the ticket, right? will the modal be implemented in this PR?
| export interface Props { | ||
| projectId?: number; | ||
| selection: SelectionType; | ||
| filterFormSet: FilterFormSetWithoutId; |
There was a problem hiding this comment.
FormSet doesn't have a function to return FilterFormSetWithoutId directly. can we use FilterFormSet because it's easier to pass it? but that will affect testing data...
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/actions-flat-run-table #9390 +/- ##
==================================================================
+ Coverage 48.61% 49.28% +0.66%
==================================================================
Files 1235 1238 +3
Lines 159575 159804 +229
Branches 2810 2851 +41
==================================================================
+ Hits 77574 78754 +1180
+ Misses 81827 80878 -949
+ Partials 174 172 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ashtonG
force-pushed
the
feat/ET-94/bulk-action-interstitial
branch
from
9aa0224
to
42c5b29
Compare
Merged
4 tasks
ashtonG
force-pushed
the
feat/ET-94/bulk-action-interstitial
branch
from
42c5b29
to
bc9d056
Compare
ashtonG
force-pushed
the
feat/ET-94/bulk-action-interstitial
branch
from
bc9d056
to
368958d
Compare
keita-determined
force-pushed
the
feature/actions-flat-run-table
branch
from
763becb
to
6919563
Compare
this adds an interstitial component for run bulk actions. when opened, it runs a search given the current selection and filters and determines whether or not the selection contains any runs that are part of a search, returning the results to the opener. as a side effect, we pull out the `mergeAbortControllers` helper from `trialDetailsLogs` while also adding test coverage to that helper.
Co-authored-by: Keita Fish <[email protected]>
ashtonG
force-pushed
the
feat/ET-94/bulk-action-interstitial
branch
from
a7226a0
to
31a4aec
Compare
keita-determined
approved these changes
May 30, 2024
There was a problem hiding this comment.
LGTM
The usage of this is
- call
RunFilterInterstitialModalComponentfirst - get the close reason
- if statement
- show
RunMoveWarningModalComponentwhenhas_search_runsis returned - show move modal when
no_search_runsis returned without showingRunMoveWarningModalComponent - show error message when
failedis retuend
- show
is that correct?
|
i think we should show the move modal first -- that way we defer the check until after the user confirms the details of the move. |
|
got it. if its ready to get merged, could you merge it to work on the move action? |
ashtonG
merged commit faf66e2
into
feature/actions-flat-run-table
81 of 94 checks passed
keita-determined
added a commit
that referenced
this pull request
Jun 3, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 4, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 5, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 7, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 10, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 11, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 13, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 14, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 14, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 17, 2024
Co-authored-by: Keita Fish <[email protected]>
keita-determined
added a commit
that referenced
this pull request
Jun 24, 2024
Co-authored-by: Keita Fish <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket
ET-94
Description
this adds an interstitial component for run bulk actions. when opened, it runs a search given the current selection and filters and determines whether or not the selection contains any runs that are part of a search, returning the results to the opener. as a side effect, we pull out the
mergeAbortControllershelper fromtrialDetailsLogswhile also adding test coverage to that helper.Test Plan
Component is not currently user-facing. Unit tests should comprehensively check functionality.
Checklist
docs/release-notes/.See Release Note for details.