Dashboard: Fix plugin dashboard save as button #55197
Conversation
lpskdl
requested review from
joshhunt,
axelavargas and
ivanortegaalba
and removed request for
a team
lpskdl
added
backport v9.1.x
| @@ -126,7 +126,7 @@ export const SaveDashboardDrawer = ({ dashboard, onDismiss, onSaveSuccess, isCop | |||
| ); | |||
| }; | |||
|
|
|||
| if (state.error && isFetchError(state.error)) { | |||
| if (state.error && isFetchError(state.error) && !state.error.isHandled) { | |||
| ) | ||
| ); | ||
|
|
||
| return { rerender, mockDashboard, store }; |
There was a problem hiding this comment.
setup() is doing too many things: creating a dashboard mock and returning it, configuring store and returning it, and rendering a component and returning it. I think it would be better to have mockDashboard as a part of a single describe() or even inside a single test whenever it's needed.
The configuration of the store can also just happen as a separate thing once for the whole test module.
| mockPost.mockClear(); | ||
| }); | ||
|
|
||
| it("renders SaveDashboardErrorProxy if there's an error and it not yet handled", async () => { |
There was a problem hiding this comment.
Since we are never actually looking for the name SaveDashboardErrorProxy in the test and it will have to be changed here if the component is changed, maybe it's better to just say "renders a corresponding modal if there's an error..."
| expect(screen.getByRole('button', { name: /save as/i })).toBeInTheDocument(); | ||
| expect(screen.getByRole('button', { name: /overwrite/i })).toBeInTheDocument(); | ||
|
|
||
| rerender( |
There was a problem hiding this comment.
From here down it's confusing what is happening, as it seems to be a different test: "renders a corresponding Save new modal if there's an error that has already been handled".
lpskdl
force-pushed
the
lpskdl/54735
branch
from
cbdface
to
470cc86
Compare
lpskdl
force-pushed
the
lpskdl/54735
branch
from
470cc86
to
50f365f
Compare
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/34483 |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-55197-to-v9.1.x origin/v9.1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 383602a850b9c546fece58c40bd158e6e45e6484
# Push it to GitHub
git push --set-upstream origin backport-55197-to-v9.1.x
git switch main
# Remove the local backport branch
git branch -D backport-55197-to-v9.1.xThen, create a pull request where the |
* render SaveDashboardErrorProxy only when error is not yet handled * improve dashbaoard drawer test (cherry picked from commit 383602a)
What this PR does / why we need it:
Render the SaveDashboardErrorProxy component only when error is not yet handled.
Screen.Recording.2022-09-14.at.22.13.32.mov
Which issue(s) this PR fixes:
Fixes #54735