🐛 Fixed staff profile URL not updating after changing a user's slug#29041
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Sequence Diagram(s)Diagrams are included in the hidden review stack artifact above. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 10s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-f... |
✅ Succeeded | 2m 39s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 36s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 5s | View ↗ |
nx run ghost:build:assets |
✅ Succeeded | 2s | View ↗ |
nx run-many -t lint -p @tryghost/admin-x-framew... |
✅ Succeeded | 1s | View ↗ |
nx run ghost:build:tsc |
✅ Succeeded | 6s | View ↗ |
nx run ghost-admin:test |
✅ Succeeded | 1s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-02 14:11:24 UTC
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6128b2a7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/admin-x-settings/test/acceptance/general/users/profile.test.ts (1)
1014-1028: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the not-found path after a profile is already retained.
This starts from a cold unknown URL, so
lastUserRefis empty. Add a case that opens a valid staff modal first, then navigates to an unknown/outdated slug and asserts the toast + staff-list redirect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin-x-settings/test/acceptance/general/users/profile.test.ts` around lines 1014 - 1028, The current test only covers a cold start with an unknown slug, so it never exercises the retained-profile not-found path. Update the acceptance test in the user profile suite to first open a valid staff member through the existing profile/modal flow, then navigate to an unknown or outdated slug and assert the “User not found” toast, redirect back to the staff list, and that the detail modal closes; use the existing profile navigation helpers and the user-detail-modal assertions to keep the scenario aligned with the retained state behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx`:
- Around line 516-518: The fallback in displayUser is letting
lastUserRef.current mask a later failed lookup, so notFoundSlug never becomes
truthy for an unknown or outdated slug. Update the logic in user-detail-modal’s
displayUser/notFoundSlug flow to only use the retained last user while the
current fetch is still pending, and stop falling back once fetchedUserData has
resolved with an error or a completed no-user result so the 404 redirect can
trigger.
---
Nitpick comments:
In `@apps/admin-x-settings/test/acceptance/general/users/profile.test.ts`:
- Around line 1014-1028: The current test only covers a cold start with an
unknown slug, so it never exercises the retained-profile not-found path. Update
the acceptance test in the user profile suite to first open a valid staff member
through the existing profile/modal flow, then navigate to an unknown or outdated
slug and assert the “User not found” toast, redirect back to the staff list, and
that the detail modal closes; use the existing profile navigation helpers and
the user-detail-modal assertions to keep the scenario aligned with the retained
state behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f63a41ac-8aae-43df-b71d-8d7339a20207
📒 Files selected for processing (3)
apps/admin-x-framework/src/providers/routing-provider.tsxapps/admin-x-settings/src/components/settings/general/user-detail-modal.tsxapps/admin-x-settings/test/acceptance/general/users/profile.test.ts
sagzy
force-pushed
the
fix-staff-slug-url-forwarding
branch
from
929945b to
9bb273f
Compare
ref https://linear.app/ghost/issue/ONC-1873 - the old Ember staff screen rewrote the browser URL (via replaceState) when a user's slug was saved, so refresh/back/bookmarks kept working; this behaviour was lost when staff management moved to the React settings app, leaving the address bar pointing at a dead slug - changing your own slug was even worse: the modal compares the current user's slug against the URL param, so after saving, the profile modal silently unmounted - added a `replace` option to the (legacy) routing provider's updateRoute so the modal can swap the history entry without adding a new one, mirroring the old Ember behaviour - the user detail modal now updates the route to the server-returned slug after a save, keeps showing the last loaded user while the refetch by new slug is in flight, and shows a "User not found" toast (navigating back to the staff list) instead of rendering nothing when a stale/unknown slug URL is opened
sagzy
force-pushed
the
fix-staff-slug-url-forwarding
branch
from
95ff0ce to
dc1284f
Compare
2 participants
ref https://linear.app/ghost/issue/ONC-1873
Problem
A customer reported that the admin URL for a staff user's profile used to follow slug changes automatically, but no longer does.
The old Ember staff screen rewrote the browser URL via
replaceStatewhenever a user's slug was saved, so refresh, back button and bookmarks kept working after a rename. That behaviour was lost when staff management moved to the React settings app (#18740):#/settings/staff/old-slug— a dead URL on the next refresh, which rendered a silent blank (the modal component returnsnullwhen the slug lookup fails).currentUser.slugagainst the URL param. Saving updates the shared current-user cache immediately, the comparison flips, the fetch by the (now stale) URL slug finds nothing, and the profile modal silently unmounts right after hitting Save.Solution
replaceoption to the legacy routing provider'supdateRoute, implemented withhistory.replaceState+ a synthetichashchange, mirroring the old Ember behaviour so a slug rename swaps the history entry instead of pushing a dead one.Testing