- Notifications
You must be signed in to change notification settings - Fork927
fix: display error when fetching OAuth2 provider apps#11713
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Looks good! Just had one question about how the error was getting passed in
| @@ -28,12 +29,14 @@ type OAuth2AppsSettingsProps = { | |||
| apps?: TypesGen.OAuth2ProviderApp[]; | |||
| isEntitled: boolean; | |||
| isLoading: boolean; | |||
| error?: unknown; | |||
ParkreinerJan 22, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Should this be optional? Feels like making a dev always pass a value in (even if it's hopefullyundefined 99% of the time) would help make sure we don't forget our errors as the code gets updated over time
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.
Good point, I just assumed it could be undefined from React Query but I did not actually check. 😛
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.
Ah I see, it can benull but notundefined. Fixing.
This was pulled out of#11609. I am trying to break out parts of that PR so we can review and merge it more easily.