Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ export const modalPaths: {[key: string]: ModalName} = {
'theme/install': 'DesignAndThemeModal', // this is a special route, because it can install a theme directly from the Ghost Marketplace
'navigation/edit': 'NavigationModal',
'staff/invite': 'InviteUserModal',
'staff/:slug': 'UserDetailModal',
'staff/:slug/social-links': 'UserDetailModal',
'staff/:slug/email-notifications': 'UserDetailModal',
'staff/:slug/edit': 'UserDetailModal',
'staff/:slug': 'UserDetailModal',
'portal/edit': 'PortalModal',
'tiers/add': 'TierDetailModal',
'tiers/:id': 'TierDetailModal',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,17 @@ export interface UserDetailProps {
}

const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
const {updateRoute} = useRouting();
const {updateRoute, route} = useRouting();

const getTabFromPath = (path: string): string => {
const lastSegment = path.split('/').pop() || '';

if (lastSegment === 'social-links' || lastSegment === 'email-notifications') {
return lastSegment;
}

return 'profile';
};
Comment on lines +167 to +175
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n --type=ts --type=tsx "staff/.*edit" -g '!*.test.*'

Repository: TryGhost/Ghost

Length of output: 85


🏁 Script executed:

# Also check for any route definitions and references to the edit route
rg -n "edit.*route\|route.*edit" apps/admin-x-settings/src --type=ts --type=tsx

Repository: TryGhost/Ghost

Length of output: 85


🏁 Script executed:

# Check the settings-router.tsx file to see the context of line 14
cat -n apps/admin-x-settings/src/components/settings/general/settings-router.tsx | head -20

Repository: TryGhost/Ghost

Length of output: 163


🏁 Script executed:

# Search for any navigation to /staff/:slug/edit pattern
rg -n "navigate.*edit\|href.*edit" apps/admin-x-settings/src --type=ts --type=tsx -A 2

Repository: TryGhost/Ghost

Length of output: 85


🏁 Script executed:

# Find the correct paths for these files
fd -type f "settings-router\|user-detail-modal" --extension ts --extension tsx

Repository: TryGhost/Ghost

Length of output: 228


🏁 Script executed:

# Search for staff and edit references more broadly
rg -n "staff.*edit" -A 2 -B 2 --max-count=20

Repository: TryGhost/Ghost

Length of output: 3115


🏁 Script executed:

# Find all route definitions with staff
rg -n "staff.*:slug" -A 2 -B 2

Repository: TryGhost/Ghost

Length of output: 1367


🏁 Script executed:

# Search for getTabFromPath function usage
rg -n "getTabFromPath" -A 3 -B 3

Repository: TryGhost/Ghost

Length of output: 1751


🏁 Script executed:

# Search for navigation or links to the edit route
rg -n "staff.*edit\|/edit" apps/admin-x-settings/src/components/settings/general --max-count=30

Repository: TryGhost/Ghost

Length of output: 40


🏁 Script executed:

# Check if there are any comments or documentation about the edit route
rg -n "edit.*route\|TODO.*edit\|FIXME" apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx

Repository: TryGhost/Ghost

Length of output: 40


🏁 Script executed:

# Search for all uses of updateRoute that might navigate to edit
rg -n "updateRoute.*edit" apps/admin-x-settings/src --max-count=20

Repository: TryGhost/Ghost

Length of output: 1648


🏁 Script executed:

# Double-check by searching the entire codebase for any navigation to staff/:slug/edit
rg -n "staff/.*edit" --type-list | head -5

Repository: TryGhost/Ghost

Length of output: 447


🏁 Script executed:

# Search all TypeScript/JS files for staff edit navigation
rg "staff.*edit" -g '*.ts' -g '*.tsx' -g '*.js' --no-heading

Repository: TryGhost/Ghost

Length of output: 819


🏁 Script executed:

# Check if staff/:slug/edit is referenced in any test files or config
rg "staff.*:slug.*edit"

Repository: TryGhost/Ghost

Length of output: 167


🏁 Script executed:

# Check when the staff/:slug/edit route was added and if there are any comments
git log --oneline -p apps/admin-x-settings/src/components/providers/settings-router.tsx | grep -A 5 -B 5 "staff/:slug/edit" | head -20

Repository: TryGhost/Ghost

Length of output: 628


🏁 Script executed:

# See the full user-detail-modal.tsx to understand the complete picture
wc -l apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx

Repository: TryGhost/Ghost

Length of output: 137


🏁 Script executed:

# Check what the base 'staff/:slug' route resolves to
cat apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx | grep -A 10 "const initialTab"

Repository: TryGhost/Ghost

Length of output: 413


Remove the unused /staff/:slug/edit route or document its purpose.

The route 'staff/:slug/edit': 'UserDetailModal' defined in settings-router.tsx is never navigated to by the application. The getTabFromPath function doesn't recognize 'edit' as a segment, causing it to default to 'profile', but more importantly, no code in the codebase ever generates navigation to this URL. The handleTabChange function navigates to either the base staff/{slug} (for profile tab) or staff/{slug}/{tab} (for other tabs), but never includes an 'edit' segment. This route appears to be dead code and should either be removed or its purpose documented if it exists for backwards compatibility.

🤖 Prompt for AI Agents
In apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx
around lines 167 to 175, the codebase exposes a dead route 'staff/:slug/edit'
(settings-router.tsx) that is never navigated to and isn't recognized by
getTabFromPath (so it falls back to 'profile'); either remove the unused route
from settings-router.tsx to eliminate dead code, or if it must be retained for
backwards compatibility, document its purpose in that router and update
getTabFromPath and handleTabChange to recognize and route to the 'edit' segment
(add 'edit' to the recognized tab list and ensure navigation generates
staff/{slug}/edit where appropriate), plus update any tests/comments
accordingly.

const {ownerUser} = useStaffUsers();
const {currentUser} = useGlobalData();
const handleError = useHandleError();
Expand Down Expand Up @@ -402,7 +412,15 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {

const suspendedText = formState.status === 'inactive' ? ' (Suspended)' : '';

const [selectedTab, setSelectedTab] = useState<string>('profile');
const initialTab = getTabFromPath(route);
const [selectedTab, setSelectedTab] = useState<string>(initialTab);

const handleTabChange = (newTabId: string) => {
const urlSegment = newTabId === 'profile' ? '' : `/${newTabId}`;

updateRoute(`staff/${user.slug}${urlSegment}`);
setSelectedTab(newTabId);
};
Comment on lines +415 to +423
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Browser back/forward navigation won't update the selected tab.

The selectedTab state is initialized from initialTab only on mount. When the route changes via browser navigation (back/forward), the component re-renders and initialTab is recomputed, but selectedTab retains its previous value, causing the UI to show the wrong tab.

Apply this fix to synchronize selectedTab with route changes:

     const initialTab = getTabFromPath(route);
     const [selectedTab, setSelectedTab] = useState<string>(initialTab);
+
+    // Sync selectedTab with route changes from browser navigation
+    useEffect(() => {
+        const tabFromRoute = getTabFromPath(route);
+        setSelectedTab(tabFromRoute);
+    }, [route]);

     const handleTabChange = (newTabId: string) => {

You'll need to add useEffect to the React import on line 4:

-import React, {useCallback, useState} from 'react';
+import React, {useCallback, useEffect, useState} from 'react';

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx
around lines 415 to 423, selectedTab is only initialized from initialTab on
mount so browser back/forward route changes don't update the UI; add useEffect
(and import it from React on line 4) that watches initialTab (or route) and
calls setSelectedTab(initialTab) whenever it changes, ensuring the selectedTab
state stays synchronized with the current route; keep the existing
handleTabChange logic intact.


return (
<Modal
Expand Down Expand Up @@ -546,7 +564,7 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
contents: <EmailNotificationsTab setUserData={setUserData} user={formState} />
}
]}
onTabChange={setSelectedTab}
onTabChange={handleTabChange}
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class CommentsServiceEmails {
accentColor: this.settingsCache.get('accent_color'),
fromEmail: this.notificationFromAddress,
toEmail: to,
staffUrl: this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${author.get('slug')}`)
staffUrl: this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${author.get('slug')}/email-notifications`)
};

const {
Expand Down Expand Up @@ -160,7 +160,7 @@ class CommentsServiceEmails {
accentColor: this.settingsCache.get('accent_color'),
fromEmail: this.notificationFromAddress,
toEmail: to,
staffUrl: this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${owner.get('slug')}`)
staffUrl: this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${owner.get('slug')}/email-notifications`)
};

const {html, text} = await this.commentsServiceEmailRenderer.renderEmailTemplate('report', templateData);
Expand Down
12 changes: 6 additions & 6 deletions ghost/core/core/server/services/staff/StaffServiceEmails.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class StaffServiceEmails {
attributionTitle = 'Homepage';
}

let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}`);
let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}/email-notifications`);

const templateData = {
memberData,
Expand Down Expand Up @@ -94,7 +94,7 @@ class StaffServiceEmails {
attributionTitle = 'Homepage';
}

let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}`);
let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}/email-notifications`);

const templateData = {
memberData,
Expand Down Expand Up @@ -149,7 +149,7 @@ class StaffServiceEmails {
cancellationReason: subscription.cancellationReason || ''
};

let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}`);
let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}/email-notifications`);

const templateData = {
memberData,
Expand Down Expand Up @@ -182,7 +182,7 @@ class StaffServiceEmails {
* @param {string} recipient.slug
*/
async getSharedData(recipient) {
let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${recipient.slug}`);
let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${recipient.slug}/email-notifications`);

return {
siteTitle: this.settingsCache.get('title'),
Expand Down Expand Up @@ -227,7 +227,7 @@ class StaffServiceEmails {
for (const user of users) {
const to = user.email;

let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}`);
let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}/email-notifications`);

const templateData = {
siteTitle: this.settingsCache.get('title'),
Expand Down Expand Up @@ -282,7 +282,7 @@ class StaffServiceEmails {
for (const user of users) {
const to = user.email;

let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}`);
let staffUrl = this.urlUtils.urlJoin(this.urlUtils.urlFor('admin', true), '#', `/settings/staff/${user.slug}/email-notifications`);

const templateData = {
siteTitle: this.settingsCache.get('title'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,7 @@ exports[`Webmentions (receiving) can display page title in notification email 1:
</tr>
<tr>
<td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 14px; vertical-align: top; padding-top: 2px\\">
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 11px; color: #738A94; font-weight: normal; margin: 0; margin-bottom: 2px;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs\\" style=\\"text-decoration: underline; color: #738A94; font-size: 11px;\\">here</a>.</p>
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 11px; color: #738A94; font-weight: normal; margin: 0; margin-bottom: 2px;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs/email-notifications\\" style=\\"text-decoration: underline; color: #738A94; font-size: 11px;\\">here</a>.</p>
</td>
</tr>

Expand Down Expand Up @@ -2466,7 +2466,7 @@ exports[`Webmentions (receiving) can display post title in notification email 1:
</tr>
<tr>
<td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 14px; vertical-align: top; padding-top: 2px\\">
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 11px; color: #738A94; font-weight: normal; margin: 0; margin-bottom: 2px;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs\\" style=\\"text-decoration: underline; color: #738A94; font-size: 11px;\\">here</a>.</p>
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 11px; color: #738A94; font-weight: normal; margin: 0; margin-bottom: 2px;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs/email-notifications\\" style=\\"text-decoration: underline; color: #738A94; font-size: 11px;\\">here</a>.</p>
</td>
</tr>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ table.body a.small {
</tr>
<tr>
<td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 12px; vertical-align: top;\\">
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 12px; color: #7C8B9A; font-weight: normal; margin: 0;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs\\" style=\\"text-decoration: underline; color: #7C8B9A; font-size: 12px;\\">here</a>.</p>
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 12px; color: #7C8B9A; font-weight: normal; margin: 0;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs/email-notifications\\" style=\\"text-decoration: underline; color: #7C8B9A; font-size: 12px;\\">here</a>.</p>
</td>
</tr>

Expand Down Expand Up @@ -229,7 +229,7 @@ You have been recommended by Other Ghost Site.
---

Sent to [email protected] from 127.0.0.1.
If you would no longer like to receive these notifications you can adjust your settings at http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs.
If you would no longer like to receive these notifications you can adjust your settings at http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs/email-notifications.
"
`;

Expand Down Expand Up @@ -446,7 +446,7 @@ table.body a.small {
</tr>
<tr>
<td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 12px; vertical-align: top;\\">
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 12px; color: #7C8B9A; font-weight: normal; margin: 0;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs\\" style=\\"text-decoration: underline; color: #7C8B9A; font-size: 12px;\\">here</a>.</p>
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 12px; color: #7C8B9A; font-weight: normal; margin: 0;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs/email-notifications\\" style=\\"text-decoration: underline; color: #7C8B9A; font-size: 12px;\\">here</a>.</p>
</td>
</tr>

Expand Down Expand Up @@ -669,7 +669,7 @@ table.body a.small {
</tr>
<tr>
<td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 12px; vertical-align: top;\\">
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 12px; color: #7C8B9A; font-weight: normal; margin: 0;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/smith-wellingsworth\\" style=\\"text-decoration: underline; color: #7C8B9A; font-size: 12px;\\">here</a>.</p>
<p class=\\"small\\" style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; line-height: 18px; font-size: 12px; color: #7C8B9A; font-weight: normal; margin: 0;\\">Don’t want to receive these emails? Manage your preferences <a class=\\"small\\" href=\\"http://127.0.0.1:2369/ghost/#/settings/staff/smith-wellingsworth/email-notifications\\" style=\\"text-decoration: underline; color: #7C8B9A; font-size: 12px;\\">here</a>.</p>
</td>
</tr>

Expand Down Expand Up @@ -700,7 +700,7 @@ You have been recommended by Other Ghost Site.
---

Sent to [email protected] from 127.0.0.1.
If you would no longer like to receive these notifications you can adjust your settings at http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs.
If you would no longer like to receive these notifications you can adjust your settings at http://127.0.0.1:2369/ghost/#/settings/staff/joe-bloggs/email-notifications.
"
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function testCommonMailData({mailStub, getEmailAlertUsersStub}) {

// Has staff admin url
mailStub.calledWith(
sinon.match.has('html', sinon.match('https://admin.ghost.example/#/settings/staff/ghost'))
sinon.match.has('html', sinon.match('https://admin.ghost.example/#/settings/staff/ghost/email-notifications'))
).should.be.true();
}

Expand Down
Loading