Skip to content

Commit bcdc536

Browse files
committed
Changed members routes to use feature-gated rendering
ref https://linear.app/ghost/issue/BER-3506/rework-feature-flagging-for-release This keeps the members list and import flow on the stable /members routes, preserves the Ember fallback behind the flag, and aligns the e2e suite layout around members and members-legacy.
1 parent 46e314a commit bcdc536

31 files changed

Lines changed: 564 additions & 245 deletions
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// @vitest-environment jsdom
2+
3+
import {describe, expect, it, vi} from 'vitest';
4+
import {renderHook} from '@testing-library/react';
5+
import {type SharedView} from './shared-views';
6+
import {useMemberSidebarViews} from './member-sidebar-views';
7+
8+
const {mockUseLocation, mockUseSharedViews} = vi.hoisted(() => ({
9+
mockUseLocation: vi.fn(),
10+
mockUseSharedViews: vi.fn<(route?: string) => SharedView[]>()
11+
}));
12+
13+
vi.mock('@tryghost/admin-x-framework', () => ({
14+
useLocation: mockUseLocation
15+
}));
16+
17+
vi.mock('./shared-views', () => ({
18+
useSharedViews: mockUseSharedViews
19+
}));
20+
21+
describe('useMemberSidebarViews', () => {
22+
it('builds saved member view URLs on the members route', () => {
23+
mockUseLocation.mockReturnValue({
24+
pathname: '/members',
25+
search: '?filter=status%3Afree'
26+
});
27+
mockUseSharedViews.mockReturnValue([
28+
{
29+
name: 'Free members',
30+
route: 'members',
31+
filter: {filter: 'status:free'}
32+
}
33+
]);
34+
35+
const {result} = renderHook(() => useMemberSidebarViews());
36+
37+
expect(result.current).toEqual([
38+
{
39+
key: 'Free members:status:free',
40+
name: 'Free members',
41+
to: 'members?filter=status%3Afree',
42+
isActive: true
43+
}
44+
]);
45+
});
46+
47+
it('does not mark saved member views active off the base members route', () => {
48+
mockUseLocation.mockReturnValue({
49+
pathname: '/members/import',
50+
search: '?filter=status%3Afree'
51+
});
52+
mockUseSharedViews.mockReturnValue([
53+
{
54+
name: 'Free members',
55+
route: 'members',
56+
filter: {filter: 'status:free'}
57+
}
58+
]);
59+
60+
const {result} = renderHook(() => useMemberSidebarViews());
61+
62+
expect(result.current[0]).toMatchObject({
63+
to: 'members?filter=status%3Afree',
64+
isActive: false
65+
});
66+
});
67+
});

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ 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

2121
function isMemberViewActive(currentSearch: string, filter: string) {
@@ -25,7 +25,7 @@ function isMemberViewActive(currentSearch: string, filter: string) {
2525
export function useMemberSidebarViews() {
2626
const location = useLocation();
2727
const sharedViews = useSharedViews('members');
28-
const isOnMembersForward = location.pathname === '/members-forward';
28+
const isOnMembersListRoute = location.pathname === '/members';
2929

3030
return useMemo<NavSavedView[]>(() => {
3131
return sharedViews
@@ -34,7 +34,7 @@ export function useMemberSidebarViews() {
3434
key: `${view.name}:${view.filter.filter}`,
3535
name: view.name,
3636
to: getMemberViewUrl(view.filter.filter),
37-
isActive: isOnMembersForward && isMemberViewActive(location.search, view.filter.filter)
37+
isActive: isOnMembersListRoute && isMemberViewActive(location.search, view.filter.filter)
3838
}));
39-
}, [isOnMembersForward, location.search, sharedViews]);
39+
}, [isOnMembersListRoute, location.search, sharedViews]);
4040
}

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@ import {describe, expect, it} from 'vitest';
22
import {getMembersNavActiveRoutes, isMembersNavActive} from './nav-content.helpers';
33

44
describe('getMembersNavActiveRoutes', () => {
5-
it('always includes members-forward alongside the legacy members routes', () => {
5+
it('returns only the members routes that are still Ember-owned', () => {
66
expect(getMembersNavActiveRoutes()).toEqual([
7-
'members-forward',
87
'members',
98
'member',
109
'member.new'
@@ -13,10 +12,10 @@ describe('getMembersNavActiveRoutes', () => {
1312
});
1413

1514
describe('isMembersNavActive', () => {
16-
it('uses the legacy route active state when members forward is disabled', () => {
15+
it('uses the legacy route active state when the feature flag is disabled', () => {
1716
expect(isMembersNavActive({
1817
membersForwardEnabled: false,
19-
isOnMembersForward: false,
18+
isOnMembersRoute: false,
2019
hasActiveMemberView: false,
2120
isMembersExpanded: false,
2221
isLegacyMembersRouteActive: true
@@ -26,7 +25,7 @@ describe('isMembersNavActive', () => {
2625
it('marks the base Members link active when a saved member view is active but collapsed', () => {
2726
expect(isMembersNavActive({
2827
membersForwardEnabled: true,
29-
isOnMembersForward: true,
28+
isOnMembersRoute: true,
3029
hasActiveMemberView: true,
3130
isMembersExpanded: false,
3231
isLegacyMembersRouteActive: false
@@ -36,17 +35,17 @@ describe('isMembersNavActive', () => {
3635
it('marks the base Members link inactive when a saved member view is active and expanded', () => {
3736
expect(isMembersNavActive({
3837
membersForwardEnabled: true,
39-
isOnMembersForward: true,
38+
isOnMembersRoute: true,
4039
hasActiveMemberView: true,
4140
isMembersExpanded: true,
4241
isLegacyMembersRouteActive: false
4342
})).toBe(false);
4443
});
4544

46-
it('falls back to the base Members link when no saved member view is active', () => {
45+
it('marks the base Members link active on React-owned members routes when no saved member view is active', () => {
4746
expect(isMembersNavActive({
4847
membersForwardEnabled: true,
49-
isOnMembersForward: true,
48+
isOnMembersRoute: true,
5049
hasActiveMemberView: false,
5150
isMembersExpanded: false,
5251
isLegacyMembersRouteActive: false

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
export function getMembersNavActiveRoutes(): string[] {
2-
// TODO: Remove members-forward once the membersForward flag and legacy route split are gone.
3-
return ['members-forward', 'members', 'member', 'member.new'];
2+
return ['members', 'member', 'member.new'];
43
}
54

65
export function isMembersNavActive({
76
membersForwardEnabled,
8-
isOnMembersForward,
7+
isOnMembersRoute,
98
hasActiveMemberView,
109
isMembersExpanded,
1110
isLegacyMembersRouteActive
1211
}: {
1312
membersForwardEnabled: boolean;
14-
isOnMembersForward: boolean;
13+
isOnMembersRoute: boolean;
1514
hasActiveMemberView: boolean;
1615
isMembersExpanded: boolean;
1716
isLegacyMembersRouteActive: boolean;
@@ -20,7 +19,7 @@ export function isMembersNavActive({
2019
return isLegacyMembersRouteActive;
2120
}
2221

23-
if (isOnMembersForward) {
22+
if (isOnMembersRoute) {
2423
if (!hasActiveMemberView) {
2524
return true;
2625
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,20 @@ function NavContent({ ...props }: React.ComponentProps<typeof SidebarGroup>) {
9292
const isPublishedPostsRouteActive = routing.isRouteActive('posts', {type: 'published'});
9393
const hasActivePostChild = isDraftPostsRouteActive || isScheduledPostsRouteActive || isPublishedPostsRouteActive || postCustomViews.some(view => view.isActive);
9494
const postsExpanded = savedPostsExpanded;
95-
const isOnMembersForward = location.pathname === '/members-forward';
96-
const hasActiveMemberView = isOnMembersForward && memberViews.some(view => view.isActive);
95+
const isOnMembersRoute = location.pathname === '/members' || location.pathname === '/members/import';
96+
const hasActiveMemberView = memberViews.some(view => view.isActive);
9797
const membersExpanded = savedMembersExpanded;
9898
const membersNavActive = isMembersNavActive({
9999
membersForwardEnabled,
100-
isOnMembersForward,
100+
isOnMembersRoute,
101101
hasActiveMemberView,
102102
isMembersExpanded: membersExpanded,
103103
isLegacyMembersRouteActive: routing.isRouteActive(getMembersNavActiveRoutes())
104104
});
105105
const postsRoute = routing.getRouteUrl('posts');
106106
const isPostsRouteActive = routing.isRouteActive('posts');
107107
const postsNavActive = isPostsRouteActive || (!postsExpanded && hasActivePostChild);
108-
const membersRoute = membersForwardEnabled ? 'members-forward' : routing.getRouteUrl('members');
108+
const membersRoute = routing.getRouteUrl('members');
109109

110110
return (
111111
<SidebarGroup {...props}>
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+
}

apps/admin/src/routes.tsx

Lines changed: 29 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 { MembersRouteGate } from "./members-route-gate";
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,30 @@ const emberFallbackRoutes: RouteObject[] = EMBER_ROUTES.map(path => ({
5355
Component: EmberFallback,
5456
handle: emberFallbackHandle,
5557
}));
58+
59+
const membersRoute: RouteObject = {
60+
path: "/members",
61+
element: <MembersRouteGate />,
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: () => redirect("/members")
80+
};
81+
5682
export const routes: RouteObject[] = [
5783
{
5884
// ForceUpgradeGuard wraps all routes to redirect to /pro when in force upgrade mode.
@@ -69,6 +95,8 @@ export const routes: RouteObject[] = [
6995
Component: EmberFallback,
7096
handle: emberFallbackHandle,
7197
},
98+
membersRoute,
99+
membersForwardRedirectRoute,
72100
{
73101
element: (
74102
<PostsAppContextProvider value={{ fromAnalytics: true }}>

apps/posts/src/routes.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,6 @@ export const routes: RouteObject[] = [
6969
path: 'comments',
7070
lazy: lazyComponent(() => import('@views/comments/comments'))
7171
},
72-
{
73-
path: 'members-forward',
74-
lazy: lazyComponent(() => import('@views/members/members'))
75-
},
7672

7773
// Error handling
7874
{

apps/posts/src/views/members/components/bulk-action-modals/import-members-modal.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {CompleteStep, ErrorStep, InitStep, MappingStep, ProcessingStep} from './import-members/components';
22
import {Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle, cn} from '@tryghost/shade';
3+
import {type ImportResponse} from './import-members/state';
34
import {MembersFieldMapping, detectFieldTypes} from './import-members/mapping';
45
import {buildImportResponse} from './import-members/upload';
56
import {createInitialImportState, importReducer} from './import-members/reducer';
@@ -11,13 +12,15 @@ import {useLabelPicker} from '@src/hooks/use-label-picker';
1112
interface ImportMembersModalProps {
1213
open: boolean;
1314
onOpenChange: (open: boolean) => void;
14-
onComplete?: () => void;
15+
onComplete?: (importResponse?: ImportResponse) => void;
16+
onClose?: (importResponse?: ImportResponse) => void;
1517
}
1618

1719
export function ImportMembersModal({
1820
open,
1921
onOpenChange,
20-
onComplete
22+
onComplete,
23+
onClose
2124
}: ImportMembersModalProps) {
2225
const [state, dispatch] = useReducer(importReducer, undefined, createInitialImportState);
2326
const errorCsvUrlRef = useRef<string | null>(null);
@@ -28,9 +31,11 @@ export function ImportMembersModal({
2831
});
2932

3033
const revokeErrorCsvUrl = useCallback(() => {
31-
if (errorCsvUrlRef.current) {
34+
if (errorCsvUrlRef.current && typeof URL.revokeObjectURL === 'function') {
3235
URL.revokeObjectURL(errorCsvUrlRef.current);
3336
errorCsvUrlRef.current = null;
37+
} else if (errorCsvUrlRef.current) {
38+
errorCsvUrlRef.current = null;
3439
}
3540
}, []);
3641

@@ -50,10 +55,12 @@ export function ImportMembersModal({
5055
return;
5156
}
5257
if (!isOpen) {
58+
const importResponse = state.importResponse ?? undefined;
5359
reset();
60+
onClose?.(importResponse);
5461
}
5562
onOpenChange(isOpen);
56-
}, [onOpenChange, reset, state.status]);
63+
}, [onClose, onOpenChange, reset, state.importResponse, state.status]);
5764

5865
useEffect(() => {
5966
if (!state.file) {
@@ -257,7 +264,7 @@ export function ImportMembersModal({
257264
type: 'UPLOAD_COMPLETE',
258265
importResponse
259266
});
260-
onComplete?.();
267+
onComplete?.(importResponse);
261268
} catch {
262269
dispatch({
263270
type: 'UPLOAD_ERROR',

apps/posts/src/views/members/components/bulk-action-modals/import-members/state.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,18 @@ import {MembersFieldMapping} from './mapping';
22

33
export type ImportStatus = 'INIT' | 'MAPPING' | 'UPLOADING' | 'PROCESSING' | 'COMPLETE' | 'ERROR';
44

5+
export interface ImportLabel {
6+
name: string;
7+
slug: string;
8+
}
9+
510
export interface ImportResponse {
611
importedCount: number;
712
errorCount: number;
813
errorCsvUrl: string;
914
errorCsvName: string;
1015
errorList: Array<{message: string; count: number}>;
16+
importLabel?: ImportLabel;
1117
}
1218

1319
export interface ImportState {

0 commit comments

Comments
 (0)