Skip to content

Commit 3d665e9

Browse files
jonatansbergfranky19
authored andcommitted
Removed members-forward route (TryGhost#27040)
closes https://linear.app/ghost/issue/BER-3506/rework-feature-flagging-for-release Reworked `membersForward` to gate React rendering on `/members` instead of changing sidebar URLs. Added React support for `/members/import`, kept Ember fallback routes, prevented duplicate Ember rendering under the flag, and restored missing members access checks.
1 parent a4037a1 commit 3d665e9

47 files changed

Lines changed: 720 additions & 500 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/admin/src/layout/app-sidebar/member-sidebar-views.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,20 @@ function isMemberSidebarView(view: SharedView): view is MemberSidebarView {
1515
}
1616

1717
function getMemberViewUrl(filter: string) {
18-
return `members-forward?${new URLSearchParams({filter}).toString()}`;
18+
return `members?${new URLSearchParams({filter}).toString()}`;
1919
}
2020

21-
function isMemberViewActive(currentSearch: string, filter: string) {
21+
function isMemberViewActive(pathname: string, currentSearch: string, filter: string) {
22+
if (pathname !== '/members') {
23+
return false;
24+
}
25+
2226
return new URLSearchParams(currentSearch).get('filter') === filter;
2327
}
2428

2529
export function useMemberSidebarViews() {
2630
const location = useLocation();
2731
const sharedViews = useSharedViews('members');
28-
const isOnMembersForward = location.pathname === '/members-forward';
2932

3033
return useMemo<NavSavedView[]>(() => {
3134
return sharedViews
@@ -34,7 +37,7 @@ export function useMemberSidebarViews() {
3437
key: `${view.name}:${view.filter.filter}`,
3538
name: view.name,
3639
to: getMemberViewUrl(view.filter.filter),
37-
isActive: isOnMembersForward && isMemberViewActive(location.search, view.filter.filter)
40+
isActive: isMemberViewActive(location.pathname, location.search, view.filter.filter)
3841
}));
39-
}, [isOnMembersForward, location.search, sharedViews]);
42+
}, [location.pathname, location.search, sharedViews]);
4043
}

apps/admin/src/layout/app-sidebar/nav-content.helpers.test.ts

Lines changed: 0 additions & 55 deletions
This file was deleted.

apps/admin/src/layout/app-sidebar/nav-content.helpers.ts

Lines changed: 0 additions & 32 deletions
This file was deleted.

apps/admin/src/layout/app-sidebar/nav-content.tsx

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import React from "react"
22

33
import {SidebarGroup, SidebarGroupContent, SidebarMenu, SidebarMenuBadge} from "@tryghost/shade/components"
44
import {formatNumber, LucideIcon} from "@tryghost/shade/utils"
5-
import { useLocation } from "@tryghost/admin-x-framework";
65
import { useCurrentUser } from "@tryghost/admin-x-framework/api/current-user";
76
import { canManageMembers, canManageTags } from "@tryghost/admin-x-framework/api/users";
87
import { NavMenuItem } from "./nav-menu-item";
@@ -11,7 +10,6 @@ import { useNavigationExpanded } from "./hooks/use-navigation-preferences";
1110
import { NavCustomViews } from "./nav-custom-views";
1211
import { NavMemberViews } from "./nav-member-views";
1312
import { useMemberSidebarViews } from "./member-sidebar-views";
14-
import { getMembersNavActiveRoutes, isMembersNavActive } from "./nav-content.helpers";
1513
import { useCustomSidebarViews } from "./use-custom-sidebar-views";
1614
import { useEmberRouting } from "@/ember-bridge";
1715
import { useFeatureFlag } from "@/hooks/use-feature-flag";
@@ -72,12 +70,12 @@ function NavContent({ ...props }: React.ComponentProps<typeof SidebarGroup>) {
7270
const [savedMembersExpanded, setMembersExpanded] = useNavigationExpanded('members');
7371
const postCustomViews = useCustomSidebarViews('posts');
7472
const memberViews = useMemberSidebarViews();
75-
const hasMemberViews = memberViews.length > 0;
76-
const location = useLocation();
7773
const memberCount = useMemberCount();
7874
const routing = useEmberRouting();
7975
const commentModerationEnabled = useFeatureFlag('commentModeration');
8076
const membersForwardEnabled = useFeatureFlag('membersForward');
77+
const visibleMemberViews = membersForwardEnabled ? memberViews : [];
78+
const hasMemberViews = visibleMemberViews.length > 0;
8179

8280
const showTags = currentUser && canManageTags(currentUser);
8381
const showMembers = currentUser && canManageMembers(currentUser);
@@ -86,20 +84,14 @@ function NavContent({ ...props }: React.ComponentProps<typeof SidebarGroup>) {
8684
const isPublishedPostsRouteActive = routing.isRouteActive('posts', {type: 'published'});
8785
const hasActivePostChild = isDraftPostsRouteActive || isScheduledPostsRouteActive || isPublishedPostsRouteActive || postCustomViews.some(view => view.isActive);
8886
const postsExpanded = savedPostsExpanded;
89-
const isOnMembersForward = location.pathname === '/members-forward';
90-
const hasActiveMemberView = isOnMembersForward && memberViews.some(view => view.isActive);
87+
const hasActiveMemberChild = visibleMemberViews.some(view => view.isActive);
9188
const membersExpanded = savedMembersExpanded;
92-
const membersNavActive = isMembersNavActive({
93-
membersForwardEnabled,
94-
isOnMembersForward,
95-
hasActiveMemberView,
96-
isMembersExpanded: membersExpanded,
97-
isLegacyMembersRouteActive: routing.isRouteActive(getMembersNavActiveRoutes())
98-
});
89+
const isMembersBaseRouteActive = routing.isRouteActive(['members', 'member', 'member.new', 'members-activity']);
9990
const postsRoute = routing.getRouteUrl('posts');
10091
const isPostsRouteActive = routing.isRouteActive('posts');
10192
const postsNavActive = isPostsRouteActive || (!postsExpanded && hasActivePostChild);
102-
const membersRoute = membersForwardEnabled ? 'members-forward' : routing.getRouteUrl('members');
93+
const membersNavActive = (isMembersBaseRouteActive && !hasActiveMemberChild) || (!membersExpanded && hasActiveMemberChild);
94+
const membersRoute = routing.getRouteUrl('members');
10395

10496
return (
10597
<SidebarGroup {...props}>
@@ -176,7 +168,7 @@ function NavContent({ ...props }: React.ComponentProps<typeof SidebarGroup>) {
176168

177169
{showMembers && (
178170
<>
179-
{membersForwardEnabled && hasMemberViews ? (
171+
{hasMemberViews ? (
180172
<NavMenuItem.Collapsible
181173
expanded={membersExpanded}
182174
id="members-submenu"
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import {Outlet} from "@tryghost/admin-x-framework";
2+
import {EmberFallback} from "./ember-bridge";
3+
import {useFeatureFlag} from "./hooks/use-feature-flag";
4+
5+
export function MembersRouteGate() {
6+
const membersForwardEnabled = useFeatureFlag("membersForward");
7+
8+
if (!membersForwardEnabled) {
9+
return <EmberFallback />;
10+
}
11+
12+
return <Outlet />;
13+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import {render, screen} from '@testing-library/react';
2+
import React from 'react';
3+
import {beforeEach, describe, expect, it, vi} from 'vitest';
4+
import {MembersRoute} from './members-route';
5+
6+
const {mockCanManageMembers, mockUseCurrentUser} = vi.hoisted(() => ({
7+
mockCanManageMembers: vi.fn(),
8+
mockUseCurrentUser: vi.fn()
9+
}));
10+
11+
vi.mock('@tryghost/admin-x-framework', () => ({
12+
Navigate: ({replace, to}: {replace?: boolean; to: string}) => React.createElement('div', {
13+
'data-replace': String(Boolean(replace)),
14+
'data-testid': 'navigate',
15+
'data-to': to
16+
})
17+
}));
18+
19+
vi.mock('@tryghost/admin-x-framework/api/current-user', () => ({
20+
useCurrentUser: mockUseCurrentUser
21+
}));
22+
23+
vi.mock('@tryghost/admin-x-framework/api/users', () => ({
24+
canManageMembers: mockCanManageMembers
25+
}));
26+
27+
vi.mock('./members-route-gate', () => ({
28+
MembersRouteGate: () => React.createElement('div', {'data-testid': 'members-route-gate'})
29+
}));
30+
31+
describe('MembersRoute', () => {
32+
beforeEach(() => {
33+
mockCanManageMembers.mockReturnValue(true);
34+
mockUseCurrentUser.mockReturnValue({
35+
data: {
36+
id: '1',
37+
roles: [{name: 'Administrator'}]
38+
},
39+
isError: false,
40+
isLoading: false
41+
});
42+
});
43+
44+
it('renders the members route gate for authorized users', () => {
45+
render(<MembersRoute />);
46+
47+
expect(screen.getByTestId('members-route-gate')).toBeInTheDocument();
48+
});
49+
50+
it('redirects users without member permissions to home', () => {
51+
mockCanManageMembers.mockReturnValue(false);
52+
53+
render(<MembersRoute />);
54+
55+
expect(screen.getByTestId('navigate')).toHaveAttribute('data-to', '/');
56+
expect(screen.getByTestId('navigate')).toHaveAttribute('data-replace', 'true');
57+
});
58+
59+
it('renders nothing while the current user is still loading', () => {
60+
mockUseCurrentUser.mockReturnValue({
61+
data: undefined,
62+
isError: false,
63+
isLoading: true
64+
});
65+
66+
const {container} = render(<MembersRoute />);
67+
68+
expect(container).toBeEmptyDOMElement();
69+
});
70+
71+
it('redirects to home when the current user is unavailable after loading', () => {
72+
mockUseCurrentUser.mockReturnValue({
73+
data: undefined,
74+
isError: false,
75+
isLoading: false
76+
});
77+
78+
render(<MembersRoute />);
79+
80+
expect(screen.getByTestId('navigate')).toHaveAttribute('data-to', '/');
81+
});
82+
});

apps/admin/src/members-route.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import {Navigate} from "@tryghost/admin-x-framework";
2+
import {useCurrentUser} from "@tryghost/admin-x-framework/api/current-user";
3+
import {canManageMembers} from "@tryghost/admin-x-framework/api/users";
4+
import {MembersRouteGate} from "./members-route-gate";
5+
6+
export function MembersRoute() {
7+
const {data: currentUser, isError, isLoading} = useCurrentUser();
8+
9+
if (!currentUser) {
10+
if (isError || !isLoading) {
11+
return <Navigate replace to="/" />;
12+
}
13+
14+
return null;
15+
}
16+
17+
if (!canManageMembers(currentUser)) {
18+
return <Navigate replace to="/" />;
19+
}
20+
21+
return <MembersRouteGate />;
22+
}

apps/admin/src/routes.tsx

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import MyProfileRedirect from "./my-profile-redirect";
1515
// Ember
1616
import { EmberFallback, ForceUpgradeGuard } from "./ember-bridge";
1717
import type { RouteHandle } from "./ember-bridge";
18+
import { MembersRoute } from "./members-route";
1819

1920
import { NotFound } from "./not-found";
2021

@@ -40,7 +41,8 @@ const EMBER_ROUTES: string[] = [
4041
"/tags/new",
4142
"/explore/*",
4243
"/migrate/*",
43-
"/members/*",
44+
"/members/new",
45+
"/members/:member_id",
4446
"/members-activity",
4547
"/designsandbox",
4648
"/mentions",
@@ -53,6 +55,33 @@ const emberFallbackRoutes: RouteObject[] = EMBER_ROUTES.map(path => ({
5355
Component: EmberFallback,
5456
handle: emberFallbackHandle,
5557
}));
58+
59+
const membersRoute: RouteObject = {
60+
path: "/members",
61+
element: <MembersRoute />,
62+
handle: emberFallbackHandle,
63+
children: [
64+
{
65+
index: true,
66+
lazy: lazyComponent(() => import("@tryghost/posts/src/views/members/members"))
67+
},
68+
{
69+
path: "import",
70+
lazy: lazyComponent(() => import("@tryghost/posts/src/views/members/members"))
71+
}
72+
]
73+
};
74+
75+
const membersForwardRedirectRoute: RouteObject = {
76+
path: "/members-forward",
77+
// TODO: Remove once the legacy Ember members list is deleted.
78+
handle: emberFallbackHandle,
79+
loader: ({request}) => {
80+
const url = new URL(request.url);
81+
return redirect(`/members${url.search}`);
82+
}
83+
};
84+
5685
export const routes: RouteObject[] = [
5786
{
5887
// ForceUpgradeGuard wraps all routes to redirect to /pro when in force upgrade mode.
@@ -69,6 +98,8 @@ export const routes: RouteObject[] = [
6998
Component: EmberFallback,
7099
handle: emberFallbackHandle,
71100
},
101+
membersRoute,
102+
membersForwardRedirectRoute,
72103
{
73104
element: (
74105
<PostsAppContextProvider value={{ fromAnalytics: true }}>

0 commit comments

Comments
 (0)