-
Notifications
You must be signed in to change notification settings - Fork 140
fix: display errors when adding a new repo #1120
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1120 +/- ##
==========================================
+ Coverage 82.83% 82.84% +0.01%
==========================================
Files 66 66
Lines 2784 2787 +3
Branches 334 334
==========================================
+ Hits 2306 2309 +3
Misses 431 431
Partials 47 47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
@andypols Looks great, thanks for the contribution! 🚀
Our E2E tests are pretty barebones so any improvements here are much appreciated! I just have some minor things to point out - will likely open a few issues for these!
Signed-off-by: Juan Escalada <[email protected]>
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.
Needs a rebase, there are a handful of conflicting/similar changes in main.
I think there are some other cases where the errors aren't displayed that would benefit from similar changes, such as cancelling a push in the dashboard when you are not allowed. That's no doubt a separate issue ;-)
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.
LGTM - couple of minor comments.
The data-testid tags might be worth noting in the testing guide @jescalada is working on in #1147
Co-authored-by: Kris West <[email protected]> Signed-off-by: Andy Pols <[email protected]>
Summary
This PR fixes a bug where the client silently ignored server-side errors when adding a new repository. As a result, users were led to believe the repository was added successfully, only to see it disappear upon refreshing the page.
Root Cause
The
NewRepo.tsx
component (src/ui/views/RepoList/Components/NewRepo.tsx
) always closed the modal, regardless of whether the server request succeeded or failed.Fix
Display server errors returned by addRepo in
src/ui/services/repo.js
.Extended Cypress tests to:
Notes
Adding unit tests for the React components is desirable but considered out of scope for this PR.