🐛 Fixed staff profile URL not updating after changing a user's slug

no ref

- 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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: sagzy

apps/admin-x-framework/src/providers/routing-provider.tsx

@@ -21,6 +21,7 @@ export type ExternalLink = {
 export type InternalLink = {
     isExternal?: false;
     route: string;
+    replace?: boolean;
 }
 
 export type RoutingModalProps = {
@@ -125,13 +126,17 @@ export const RoutingProvider: React.FC<RoutingProviderProps> = ({basePath, modal
         }
 
         const newPath = options.route.replace(/^\//, '');
+        const newHash = newPath ? `/${basePath}/${newPath}` : `/${basePath}`;
 
         if (newPath === route) {
             // No change
-        } else if (newPath) {
-            window.location.hash = `/${basePath}/${newPath}`;
+        } else if (options.replace) {
+            // Replace the current history entry so back/refresh use the new
+            // path, then fire hashchange manually since replaceState doesn't
+            window.history.replaceState(window.history.state, '', `#${newHash}`);
+            window.dispatchEvent(new HashChangeEvent('hashchange'));
         } else {
-            window.location.hash = `/${basePath}`;
+            window.location.hash = newHash;
         }
 
         eventTarget.dispatchEvent(new CustomEvent('routeChange', {detail: {newPath, oldPath: route}}));

apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx

@@ -1,7 +1,7 @@
 import EmailNotificationsTab from './users/email-notifications-tab';
 import NiceModal, {useModal} from '@ebay/nice-modal-react';
 import ProfileTab from './users/profile-tab';
-import React, {useCallback, useState} from 'react';
+import React, {useCallback, useEffect, useRef, useState} from 'react';
 import SocialLinksTab from './users/social-links-tab';
 import clsx from 'clsx';
 import usePinturaEditor from '../../../hooks/use-pintura-editor';
@@ -95,7 +95,18 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
             }, {});
         },
         onSave: async (values) => {
-            await updateUser?.(values);
+            const response = await updateUser?.(values);
+
+            // The server may have sanitized the submitted slug
+            const savedSlug = response?.users?.[0]?.slug;
+
+            if (savedSlug && savedSlug !== user.slug) {
+                // Keep the URL in sync with the new slug, replacing the
+                // history entry so refresh and back button still work
+                const tab = getTabFromPath(route);
+                const urlSegment = tab === 'profile' ? '' : `/${tab}`;
+                updateRoute({route: `staff/${savedSlug}${urlSegment}`, replace: true});
+            }
         },
         onSaveError: handleError
     });
@@ -482,21 +493,51 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
 
 const UserDetailModal: React.FC<RoutingModalProps> = ({params}) => {
     const {currentUser} = useGlobalData();
+    const {updateRoute} = useRouting();
 
     // Skip API call if it's the current user (we already have their data)
     const isCurrentUser = currentUser.slug === params?.slug;
 
     // Fetch user by slug if it's not the current user
-    const {data: fetchedUserData} = useGetUserBySlug(
+    const {data: fetchedUserData, isError} = useGetUserBySlug(
         params?.slug || '',
         {enabled: !isCurrentUser && !!params?.slug}
     );
 
     // Use current user data or fetched user data
     const user = isCurrentUser ? currentUser : fetchedUserData?.users?.[0];
 
+    // Keep showing the last loaded user while a refetch is in flight, e.g.
+    // when a slug change updates the URL and triggers a fetch by the new slug
+    const lastUserRef = useRef<User | undefined>(undefined);
     if (user) {
-        return <UserDetailModalContent user={user} />;
+        lastUserRef.current = user;
+    }
+    const displayUser = user || lastUserRef.current;
+
+    const notFoundSlug = (!displayUser && !!params?.slug && (isError || fetchedUserData !== undefined)) ? params.slug : null;
+    const notFoundHandledRef = useRef<string | null>(null);
+
+    useEffect(() => {
+        if (!notFoundSlug || notFoundHandledRef.current === notFoundSlug) {
+            return;
+        }
+        notFoundHandledRef.current = notFoundSlug;
+
+        showToast({
+            type: 'error',
+            message: 'User not found'
+        });
+
+        if (canAccessSettings(currentUser)) {
+            updateRoute('staff');
+        } else {
+            updateRoute({isExternal: true, route: ''});
+        }
+    }, [notFoundSlug, currentUser, updateRoute]);
+
+    if (displayUser) {
+        return <UserDetailModalContent user={displayUser} />;
     } else {
         return null;
     }

apps/admin-x-settings/test/acceptance/general/users/profile.test.ts

@@ -932,3 +932,98 @@ test.describe('User profile', async () => {
         });
     });
 });
+
+test.describe('Staff profile URLs', async () => {
+    test('Updates the URL and keeps the modal open when changing another user\'s slug', async ({page}) => {
+        const userToEdit = responseFixtures.users.users.find(user => user.email === 'administrator@test.com')!;
+
+        await mockApi({page, requests: {
+            ...globalDataRequests,
+            getUserBySlug: {method: 'GET', path: `/users/slug/${userToEdit.slug}/?include=roles`, response: {
+                users: [userToEdit]
+            }},
+            getUserByNewSlug: {method: 'GET', path: '/users/slug/new-admin/?include=roles', response: {
+                users: [{...userToEdit, slug: 'new-admin'}]
+            }},
+            browseUsers: {method: 'GET', path: '/users/?limit=100&include=roles', response: responseFixtures.users},
+            editUser: {method: 'PUT', path: `/users/${userToEdit.id}/?include=roles`, response: {
+                users: [{...userToEdit, slug: 'new-admin'}]
+            }}
+        }});
+
+        await page.goto('/');
+
+        const section = page.getByTestId('users');
+        const activeTab = section.locator('[role=tabpanel]:not(.hidden)');
+
+        await section.getByRole('tab', {name: 'Administrators'}).click();
+
+        const listItem = activeTab.getByTestId('user-list-item').last();
+        await listItem.hover();
+        await listItem.getByRole('button', {name: 'Edit'}).click();
+
+        const modal = page.getByTestId('user-detail-modal');
+
+        await expect(page).toHaveURL(new RegExp(`#/settings/staff/${userToEdit.slug}$`));
+
+        await modal.getByLabel('Slug').fill('new-admin');
+        await modal.getByRole('button', {name: 'Save'}).click();
+
+        await expect(modal.getByRole('button', {name: 'Saved'})).toBeVisible();
+
+        // The URL follows the new slug and the modal stays open
+        await expect(page).toHaveURL(/#\/settings\/staff\/new-admin$/);
+        await expect(modal).toBeVisible();
+
+        // Refreshing the updated URL loads the same profile
+        await page.reload();
+        await expect(page.getByTestId('user-detail-modal')).toBeVisible();
+        await expect(page).toHaveURL(/#\/settings\/staff\/new-admin$/);
+    });
+
+    test('Updates the URL and keeps the modal open when changing your own slug', async ({page}) => {
+        const ownerUser = responseFixtures.users.users.find(user => user.email === 'owner@test.com')!;
+
+        await mockApi({page, requests: {
+            ...globalDataRequests,
+            browseUsers: {method: 'GET', path: '/users/?limit=100&include=roles', response: responseFixtures.users},
+            editUser: {method: 'PUT', path: `/users/${ownerUser.id}/?include=roles`, response: {
+                users: [{...ownerUser, slug: 'renamed-owner'}]
+            }}
+        }});
+
+        await page.goto('/');
+
+        const section = page.getByTestId('users');
+
+        const listItem = section.getByTestId('owner-user').last();
+        await listItem.hover();
+        await listItem.getByRole('button', {name: 'View profile'}).click();
+
+        const modal = page.getByTestId('user-detail-modal');
+
+        await modal.getByLabel('Slug').fill('renamed-owner');
+        await modal.getByRole('button', {name: 'Save'}).click();
+
+        await expect(modal.getByRole('button', {name: 'Saved'})).toBeVisible();
+
+        await expect(page).toHaveURL(/#\/settings\/staff\/renamed-owner$/);
+        await expect(modal).toBeVisible();
+    });
+
+    test('Shows an error and returns to the staff list when the user is not found', async ({page}) => {
+        await mockApi({page, requests: {
+            ...globalDataRequests,
+            browseUsers: {method: 'GET', path: '/users/?limit=100&include=roles', response: responseFixtures.users},
+            getUserBySlug: {method: 'GET', path: '/users/slug/unknown-user/?include=roles', responseStatus: 404, response: {
+                errors: [{type: 'NotFoundError', message: 'Resource not found error, cannot read user.'}]
+            }}
+        }});
+
+        await page.goto('/#/settings/staff/unknown-user');
+
+        await expect(page.getByTestId('toast-error').filter({hasText: 'User not found'})).toBeVisible();
+        await expect(page).toHaveURL(/#\/settings\/staff$/);
+        await expect(page.getByTestId('user-detail-modal')).not.toBeVisible();
+    });
+});