Skip to content
This repository has been archived by the owner on Jan 12, 2023. It is now read-only.

Bug 1947519: Track migrate/cutover request state locally to the button instead of at the plans page level #537

Merged
merged 1 commit into from Apr 9, 2021

Conversation

mturley
Copy link
Collaborator

@mturley mturley commented Apr 8, 2021

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1947519

Currently when clicking the Start, Restart or Cutover buttons on a plan, all the action buttons for all plans turn grey while the CR is being created/mutated. This is because there is one query hook at the plans page level for all plans, since we display any errors with these requests at the top of the plans page. This is confusing because greying out all the buttons on unrelated plans looks weird, and because it isn't apparent that the action is working.

This PR factors out a MigrateOrCutoverButton component which contains its own useCreateMigrationMutation and useSetCutoverMutation hooks, allowing us to track that request state at a per-plan level. To achieve the centralized error container we had before, a React Portal is used which allows the error child of the button component to be rendered at the top of the plans page. Also, instead of greying out the button during the request, it is replaced by a spinner to be more clear that something is happening.

Along with #531, this should hopefully clear up any confusion during the transition while a migration is being started.

Although we can't start migrations in mock mode, the spinner behavior and error reporting can still be tested there.

@mturley mturley requested a review from a team April 8, 2021 17:46
@github-actions
Copy link

github-actions bot commented Apr 8, 2021

valid bug 1947519

@konveyor-preview-bot
Copy link

🚀 Deployed Preview: http://konveyor-forklift-ui-pr-537-preview.surge.sh

Compare with current main branch: http://konveyor-forklift-ui-preview.surge.sh

@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #537 (30668ad) into main (e089e6a) will decrease coverage by 0.00%.
The diff coverage is 31.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #537      +/-   ##
==========================================
- Coverage   59.99%   59.98%   -0.01%     
==========================================
  Files         134      135       +1     
  Lines        4339     4353      +14     
  Branches     1080     1081       +1     
==========================================
+ Hits         2603     2611       +8     
- Misses       1711     1717       +6     
  Partials       25       25              
Impacted Files Coverage Δ
...rc/app/Plans/components/MigrateOrCutoverButton.tsx 30.00% <30.00%> (ø)
src/app/Plans/components/PlansTable.tsx 17.46% <33.33%> (+1.40%) ⬆️
src/app/Plans/PlansPage.tsx 52.63% <50.00%> (+6.79%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e089e6a...30668ad. Read the comment docs.

Copy link

@vconzola vconzola left a comment

Choose a reason for hiding this comment

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

@mturley I really like this change. Looks great.

Copy link
Contributor

@ibolton336 ibolton336 left a comment

Choose a reason for hiding this comment

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

VISACK

@mturley mturley merged commit 7742096 into kubev2v:main Apr 9, 2021
@mturley mturley deleted the bz1947519-grey-migrate-buttons branch April 9, 2021 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants