-
Notifications
You must be signed in to change notification settings - Fork 5
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
Show error messages from backend on API failure #524
Show error messages from backend on API failure #524
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.
I think this makes sense although I am still unsure of the design for this. The toasts generated by this are not easy to read.
Also we need to decide what to do with the general messages such as the one found here:
https://github.com/ServiceNow/azimuth/pull/524/files#diff-eab2d4424b736fc0a65af85697bd0abf297013425f52e684f71463ddcf22e16dL60
b13a570
to
3fdf3a0
Compare
591532d
to
c3aa758
Compare
06c6bc2
to
672873f
Compare
@@ -139,7 +139,7 @@ describe("PerturbationTestingPreview with Failure response", () => { | |||
// expected error message | |||
expect( | |||
screen.getByText( | |||
"Something went wrong fetching behavioral testing summary" | |||
/Something went wrong fetching behavioral testing summary/i |
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.
Any reason for making the regexps case insensitive (i
)? If not, I suggest removing it
/Something went wrong fetching behavioral testing summary/i | |
/Something went wrong fetching behavioral testing summary/ |
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.
Done in 357d66b
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.
Okay, tests were failing so I added regex but i is not required here. Thank you for removing 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.
Super! 👍
Resolve #
Description:
Checklist:
You should check all boxes before the PR is ready. If a box does not apply, check it to acknowledge it.
ran
pre-commit run --all-files
at the end.our users.
README
files and our wiki for any big design decisions, if relevant.