-
Notifications
You must be signed in to change notification settings - Fork 886
fix(UI): workspace restart button stops build before starting a new one #7301
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
Conversation
* Refactor primary buttons * refactor(site): Always show the main actions * Remove tests that are testes on Storybook * Fix tests * Fix keys * added restart btn --------- Co-authored-by: BrunoQuaresma <bruno_nonato_quaresma@hotmail.com>
} | ||
}, | ||
}) | ||
} |
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.
Originally, I wanted to use an implementation closer to the following, which react query's docs suggest is possible:
const requestStopWorkspace = useMutation({ mutationFn: stopWorkspace })
const requestStartWorkspace = useMutation({ mutationFn: startWorkspace })
const restartWorkspace = async () => {
try {
await requestStopWorkspace.mutateAsync(workspace.id)
await waitForStop(requestStopWorkspace.data)
await requestStartWorkspace.mutateAsync({
workspaceId,
templateVersionId,
})
} catch (error) {
...
}
}
but mutateAsync
does not seem to be working and the queries are not treated asynchronously. @BrunoQuaresma is this something you've run into before?
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.
I put a comment above, I think you don't need to have each API call into a "query". You can have a single function to restart the workspace and wrap this function into a query.
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.
Hmmm that's a good idea. Let me try it!
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.
@BrunoQuaresma Woo that's a lot better! Thanks!
Thank you for fixing this so quickly :) |
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.
Works as expected with AWS template (stops workspace then starts)
Resolves #6241 and replaces the reverted #7268.
I've added a test to ensure the restart button hook is called appropriately but React Query makes it difficult to test side effects so I'm effectively mocking the
stopWorkspace
api call and not thestartWorkspace
api call. I will take a second look at this during a quieter day, but for the time being, smoke testing would be appreciated!