Repository navigation
fix: show "not found" for deleted servers, nodes and users - #175
Merged
Merged
Conversation
Opening a server that had been deleted showed "Something went wrong" with Laravel's "No query results for model [Convoy\Models\Server]": the backend sent that message as the 404's body and the server routers put any error's message on a generic error screen. Admin node and user pages were worse: their loaders swallowed the error and returned null, and the routers then read node.name / user.name and broke. Backend: a renderable on NotFoundHttpException (Laravel has converted the ModelNotFoundException by then) answers JSON requests with "This <model> could not be found. It may have been deleted." Frontend: a 404 now shows the existing NotFound screen -- "Server not found" / "Node not found" / "User not found" -- with Go Back to the list. Server routers check isNotFoundError() on the fetch; the node and user loaders return a LoadFailure instead of null, which their routers render through LoadFailureScreen before touching the record. Other errors still use the generic error screen, now with a readable message. Tested with Pest (the API message for a missing server and node) and a Playwright spec opening a missing server (client and admin), node and user; all four fail on the old code.
NotFound didn't pass ScreenBlock's center, so inside a page it sat at the left edge, unlike the other status screens. The full-page 404 is centred by its own layout and looks the same.
# Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opening a server that had been deleted showed "Something went wrong" with Laravel's
No query results for model [Convoy\Models\Server]. Deleted nodes and users in the admin area broke the page outright.Changes
NotFoundHttpException; by that point Laravel has already converted theModelNotFoundException.NotFoundscreen ("Server not found", with Go Back to the server list). Other errors still use the generic error screen, now with a readable message.null, and the routers then readnode.name/user.nameand crashed. The loaders now return aLoadFailure, which the routers render as "Node not found" / "User not found" (or the real error) before touching the record.Testing