fix(ui): derive the API path prefix from the browser location - #5547
Draft
sysadmind wants to merge 3 commits into
Draft
fix(ui): derive the API path prefix from the browser location#5547sysadmind wants to merge 3 commits into
sysadmind wants to merge 3 commits into
Conversation
`createQueryFn` builds every Alertmanager API request the Mantine UI makes and had no test coverage. Export it and exercise it against an ephemeral `node:http` server rather than a mocked `fetch`, so the assertions cover real request URLs, real status codes, and real JSON parsing failures. Covers the success and error envelopes, non-envelope payloads, non-OK responses with and without a JSON content type, malformed JSON, unreachable servers, aborted signals, query parameter encoding including repeated keys, and the response-time callback. Signed-off-by: Joe Adams <github@joeadams.io>
The Mantine UI hardcoded an empty path prefix, so every API request was issued against an absolute `/api/v2/...` URL. The application is served at `<route-prefix>/ui/`, which means a deployment behind `--web.route-prefix` or a reverse proxy requested `/api/v2/...` instead of `<route-prefix>/api/v2/...` and received a 404 for every query. Derive the prefix from `window.location.pathname` by anchoring on the `/ui` mount point, and expose it through a settings context so the query hooks no longer read module-level state. The search runs right to left and matches whole path segments, so a route prefix that itself contains `/ui` is preserved and a sibling such as `/uiassets` is not mistaken for the mount point. The derivation is independent of the client-side routes, so adding a page does not require a corresponding change here. Signed-off-by: Joe Adams <github@joeadams.io>
Add tests for the path prefix derivation, the settings context, and the `useAPIQuery` / `useSuspenseAPIQuery` hooks. The hook tests reuse the live-server harness and point the application at the ephemeral server through the settings context, which also covers the route-prefix regression this branch fixes. Signed-off-by: Joe Adams <github@joeadams.io>
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.
Depends on #5546. Should merge after that PR. In draft until that merges or the direction changes.
The Mantine UI hardcodes
pathPrefix = ''indata/api.ts, so all API requests go to an absolute/api/v2/.... Since #5502 the app is served at<route-prefix>/ui/, so any deployment using--web.route-prefixor a reverse proxy subpath requests/api/v2/...rather than<route-prefix>/api/v2/...and gets a 404 on every query.This derives the prefix from the browser location, following the approach Prometheus uses in
settingsSlice.ts, and moves it behind auseSettings()context rather than module-level state. Anchoring on the/uimount, scanning right to left, and matching whole path segments means a route prefix that itself contains/ui(--web.route-prefix=/ui) or a page path (--web.route-prefix=/alerts) still resolves correctly.Unlike the Prometheus implementation, this does not enumerate the client-side routes, so adding a page requires no change to it. Deep routes such as
/ui/silence/:idalready resolve, so it also does not need revisiting when the app moves offHashRouter.Pull Request Checklist
Please check all the applicable boxes.
Which user-facing changes does this PR introduce?