-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat(bulk-import): use react query to fetch repositories #23
feat(bulk-import): use react query to fetch repositories #23
Conversation
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.
It works great. From my side we can merge it this way already.
Some small feedback anyway:
...paces/bulk-import/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesForm.tsx
Outdated
Show resolved
Hide resolved
}); | ||
setOpenDrawer(false); | ||
if (!mutationCreate.isError) { |
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 would expect that you need await mutate to see the error here. Also then I'm unsure if this error is automatically updated before the component is re-rendered. Did you tested the error case?
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.
The error scenario that I tested, the mutationCreate.isError
returns the boolean value without the await
workspaces/bulk-import/plugins/bulk-import/src/hooks/useAddedRepositories.ts
Outdated
Show resolved
Hide resolved
workspaces/bulk-import/plugins/bulk-import/src/hooks/useRepositories.ts
Outdated
Show resolved
Hide resolved
5c0f97b
to
54e98c1
Compare
54e98c1
to
bfffab3
Compare
bfffab3
to
bc9d9cf
Compare
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.
Thanks for all the effort that goes into this PR!!
Lgtm!
/lgtm
/approve
Hey, I just made a Pull Request!
Resolves:
https://issues.redhat.com/browse/RHIDP-4035
✔️ Checklist