diff --git a/apps/admin-x-framework/src/api/members.ts b/apps/admin-x-framework/src/api/members.ts index 038cb9bfdb5..92545401e78 100644 --- a/apps/admin-x-framework/src/api/members.ts +++ b/apps/admin-x-framework/src/api/members.ts @@ -1,5 +1,7 @@ -import {InfiniteData} from '@tanstack/react-query'; +import {InfiniteData, useQueryClient} from '@tanstack/react-query'; +import {useEffect} from 'react'; import {Meta, createInfiniteQuery, createMutation, createQuery, createQueryWithId} from '../utils/api/hooks'; +import {apiUrl} from '../utils/api/fetch-api'; export type MemberLabel = { id: string; @@ -103,12 +105,29 @@ export interface MembersResponseType { } const dataType = 'MembersResponseType'; +const membersPath = '/members/'; + +const memberCountSearchParams = {limit: '1'} as const; + +export const getMemberCountQueryKey = () => [dataType, apiUrl(membersPath, memberCountSearchParams)] as const; export const useBrowseMembers = createQuery({ dataType, - path: '/members/' + path: membersPath }); +const useBrowseMemberCount = createQuery({ + dataType, + path: membersPath, + defaultSearchParams: memberCountSearchParams +}); + +export function useMemberCount() { + const {data} = useBrowseMemberCount(); + + return data?.meta?.pagination.total; +} + export type NewMember = { email: string; name?: string | null; @@ -123,6 +142,70 @@ export const useAddMember = createMutation({ invalidateQueries: {dataType} }); +export type ImportMembersImportLabel = { + name: string; + slug: string; +} | null; + +export type ImportMembersAcceptedResponseType = { + meta: { + originalImportSize: number; + stats?: undefined; + import_label?: ImportMembersImportLabel; + }; +}; + +export type ImportMembersCompleteResponseType = { + meta: { + originalImportSize?: number; + stats: { + imported: number; + invalid?: Array & {error: string}>; + }; + import_label?: ImportMembersImportLabel; + }; +}; + +export type ImportMembersResponseType = ImportMembersAcceptedResponseType | ImportMembersCompleteResponseType; + +export function isImportMembersAcceptedResponse(response: ImportMembersResponseType): response is ImportMembersAcceptedResponseType { + return typeof response.meta?.originalImportSize === 'number' && response.meta.stats === undefined; +} + +export function isImportMembersCompleteResponse(response: ImportMembersResponseType): response is ImportMembersCompleteResponseType { + return typeof response.meta?.stats?.imported === 'number'; +} + +export type ImportMembersPayload = { + file: File; + labels?: string[]; + mapping?: Record; +}; + +function buildImportMembersFormData({file, labels = [], mapping = {}}: ImportMembersPayload) { + const formData = new FormData(); + formData.append('membersfile', file); + + for (const label of labels) { + formData.append('labels', label); + } + + for (const [key, val] of Object.entries(mapping)) { + if (val) { + formData.append(`mapping[${key}]`, val); + } + } + + return formData; +} + +export const useImportMembers = createMutation({ + method: 'POST', + path: () => '/members/upload/', + body: buildImportMembersFormData, + invalidateQueries: {dataType} +}); + export const getMember = createQueryWithId({ dataType, path: id => `/members/${id}/` @@ -160,9 +243,9 @@ export interface MembersInfiniteResponseType extends MembersResponseType { isEnd: boolean; } -export const useBrowseMembersInfinite = createInfiniteQuery({ +const useBrowseMembersInfiniteQuery = createInfiniteQuery({ dataType, - path: '/members/', + path: membersPath, defaultSearchParams: { include: 'labels,tiers', limit: '100', @@ -190,6 +273,51 @@ export const useBrowseMembersInfinite = createInfiniteQuery[0]; +type BrowseMembersInfiniteResult = ReturnType; + +function hasMemberFilterOrSearch(searchParams?: Record) { + return Boolean(searchParams?.filter || searchParams?.search); +} + +export function useBrowseMembersInfinite(options: BrowseMembersInfiniteOptions = {}): BrowseMembersInfiniteResult { + const queryClient = useQueryClient(); + const result = useBrowseMembersInfiniteQuery(options); + const responseTotalMembers = result.data?.meta?.pagination?.total; + + useEffect(() => { + if (hasMemberFilterOrSearch(options.searchParams) || result.isError || result.isPlaceholderData || result.isPreviousData || typeof responseTotalMembers !== 'number') { + return; + } + + const memberCountQueryKey = getMemberCountQueryKey(); + const memberCountQueryState = queryClient.getQueryState(memberCountQueryKey); + const memberCountData = memberCountQueryState?.data; + const pagination = memberCountData?.meta?.pagination; + + if (!memberCountData || !pagination || pagination.total === responseTotalMembers) { + return; + } + + if (memberCountQueryState.dataUpdatedAt > result.dataUpdatedAt) { + return; + } + + queryClient.setQueryData(memberCountQueryKey, { + ...memberCountData, + meta: { + ...memberCountData.meta, + pagination: { + ...pagination, + total: responseTotalMembers + } + } + }); + }, [options.searchParams, queryClient, responseTotalMembers, result.dataUpdatedAt, result.isError, result.isPlaceholderData, result.isPreviousData]); + + return result; +} + // Bulk operations export interface BulkEditAction { type: 'addLabel' | 'removeLabel' | 'unsubscribe'; diff --git a/apps/admin-x-framework/test/unit/api/members.test.tsx b/apps/admin-x-framework/test/unit/api/members.test.tsx new file mode 100644 index 00000000000..1b884c83992 --- /dev/null +++ b/apps/admin-x-framework/test/unit/api/members.test.tsx @@ -0,0 +1,270 @@ +import {act, waitFor} from '@testing-library/react'; +import {describe, expect, it, vi} from 'vitest'; +import {currentUserQueryKey} from '../../../src/api/current-user'; +import {createTestQueryClient, renderHookWithProviders} from '../../../src/test/test-utils'; +import {getMemberCountQueryKey, useAddMember, useBrowseMembersInfinite, useBulkDeleteMembers, useImportMembers, useMemberCount} from '../../../src/api/members'; +import type {MembersResponseType} from '../../../src/api/members'; +import {withMockFetch} from '../../utils/mock-fetch'; + +const memberCountKey = getMemberCountQueryKey(); + +function membersResponse(total: number, limit: number | 'all' = 1): MembersResponseType { + return { + members: [], + meta: {pagination: {page: 1, limit, pages: 1, total, next: null, prev: null}} + }; +} + +function seedMemberCount(queryClient: ReturnType, total = 10, options?: {updatedAt?: number}) { + queryClient.setQueryData(memberCountKey, membersResponse(total), options); +} + +function createQueryClientWithCurrentUser() { + const queryClient = createTestQueryClient(); + + queryClient.setQueryDefaults(currentUserQueryKey, {staleTime: Infinity}); + queryClient.setQueryDefaults(memberCountKey, {cacheTime: Infinity}); + queryClient.setQueryData(currentUserQueryKey, { + users: [{ + id: 'user-1', + name: 'Test User', + email: 'test@example.com', + roles: [] + }] + }); + + return queryClient; +} + +async function browseMembers({ + queryClient, + searchParams, + total = 83427 +}: { + queryClient: ReturnType; + searchParams?: Record; + total?: number; +}) { + await withMockFetch({ + json: membersResponse(total, 100) + }, async () => { + const {result} = renderHookWithProviders(() => useBrowseMembersInfinite({ + ...(searchParams ? {searchParams} : {}) + }), {queryClient}); + + await waitFor(() => { + expect(result.current.isSuccess).toBe(true); + }); + }); +} + +describe('members api', () => { + function expectQueryInvalidation(queryClient: ReturnType, dataType: string, isInvalidated: boolean) { + const queries = queryClient.getQueryCache().getAll().filter(q => q.queryKey[0] === dataType); + expect(queries.length).toBeGreaterThan(0); + expect(queries.every(q => q.state.isInvalidated === isInvalidated)).toBe(true); + } + + it('invalidates member queries after adding a member', async () => { + const queryClient = createTestQueryClient(); + + seedMemberCount(queryClient); + + await withMockFetch({ + json: { + members: [{ + email: 'jamie@example.com' + }] + } + }, async () => { + const {result} = renderHookWithProviders(() => useAddMember(), {queryClient}); + + await act(async () => { + await result.current.mutateAsync({email: 'jamie@example.com'}); + }); + + await waitFor(() => { + expectQueryInvalidation(queryClient, 'MembersResponseType', true); + }); + }); + }); + + it('invalidates member queries after importing members', async () => { + const queryClient = createTestQueryClient(); + const onInvalidate = vi.fn(); + const unrelatedKey = ['PostsResponseType', 'http://localhost:3000/ghost/api/admin/posts/']; + const file = new File(['email\njamie@example.com'], 'members.csv', {type: 'text/csv'}); + + seedMemberCount(queryClient); + queryClient.setQueryData(unrelatedKey, {posts: []}); + + await withMockFetch({ + json: { + meta: { + stats: { + imported: 1, + invalid: [] + } + } + } + }, async (mock) => { + const {result} = renderHookWithProviders(() => useImportMembers(), { + frameworkProps: {onInvalidate}, + queryClient + }); + + await act(async () => { + await result.current.mutateAsync({ + file, + labels: ['VIP'], + mapping: {email: 'Email'} + }); + }); + + expect(mock.calls[0][0]).toBe('http://localhost:3000/ghost/api/admin/members/upload/'); + expect(mock.calls[0][1].method).toBe('POST'); + expect(mock.calls[0][1].body).toBeInstanceOf(FormData); + expect(mock.calls[0][1].body.get('membersfile')).toBe(file); + expect(mock.calls[0][1].body.get('labels')).toBe('VIP'); + expect(mock.calls[0][1].body.get('mapping[email]')).toBe('Email'); + expect(mock.calls[0][1].headers).not.toHaveProperty('content-type'); + await waitFor(() => { + expectQueryInvalidation(queryClient, 'MembersResponseType', true); + expectQueryInvalidation(queryClient, 'PostsResponseType', false); + }); + expect(onInvalidate).toHaveBeenCalledWith('MembersResponseType'); + }); + }); + + it('invalidates member queries after accepting a background member import', async () => { + const queryClient = createTestQueryClient(); + const file = new File(['email\njamie@example.com'], 'members.csv', {type: 'text/csv'}); + + seedMemberCount(queryClient); + + await withMockFetch({ + status: 202, + json: { + meta: { + originalImportSize: 501 + } + } + }, async () => { + const {result} = renderHookWithProviders(() => useImportMembers(), {queryClient}); + + await act(async () => { + await result.current.mutateAsync({file}); + }); + + await waitFor(() => { + expectQueryInvalidation(queryClient, 'MembersResponseType', true); + }); + }); + }); + + it('invalidates member queries after bulk deleting members', async () => { + const queryClient = createTestQueryClient(); + + seedMemberCount(queryClient); + + await withMockFetch({ + json: { + meta: { + stats: { + successful: 3, + unsuccessful: 0 + } + } + } + }, async () => { + const {result} = renderHookWithProviders(() => useBulkDeleteMembers(), {queryClient}); + + await act(async () => { + await result.current.mutateAsync({all: true}); + }); + + await waitFor(() => { + expectQueryInvalidation(queryClient, 'MembersResponseType', true); + }); + }); + }); + + it('fetches the member count with the dedicated count hook', async () => { + const queryClient = createQueryClientWithCurrentUser(); + + await withMockFetch({ + json: membersResponse(102466) + }, async (mockFetch) => { + const {result} = renderHookWithProviders(() => useMemberCount(), {queryClient}); + + await waitFor(() => { + expect(result.current).toBe(102466); + }); + + expect(mockFetch.calls[0][0].toString()).toBe('http://localhost:3000/ghost/api/admin/members/?limit=1'); + }); + }); + + it('syncs the sidebar member count from an unfiltered members list query', async () => { + const queryClient = createQueryClientWithCurrentUser(); + const memberDetailKey = ['MembersResponseType', 'http://localhost:3000/ghost/api/admin/members/member-1/']; + + queryClient.setQueryDefaults(memberDetailKey, {cacheTime: Infinity}); + seedMemberCount(queryClient, 102466); + + queryClient.setQueryData(memberDetailKey, { + members: [{id: 'member-1'}] + }); + + await browseMembers({queryClient}); + + expect(queryClient.getQueryData(memberCountKey)?.meta?.pagination.total).toBe(83427); + expect(queryClient.getQueryData(memberDetailKey)?.members).toEqual([{id: 'member-1'}]); + }); + + it('does not replace a newer sidebar count cache entry with older members list data', async () => { + const queryClient = createQueryClientWithCurrentUser(); + + seedMemberCount(queryClient, 102466, {updatedAt: Date.now() + 60_000}); + + await browseMembers({queryClient}); + + expect(queryClient.getQueryData(memberCountKey)?.meta?.pagination.total).toBe(102466); + }); + + it('does not sync the sidebar member count from a filtered members list query', async () => { + const queryClient = createQueryClientWithCurrentUser(); + + seedMemberCount(queryClient, 102466); + + await browseMembers({ + queryClient, + searchParams: {filter: 'status:paid', limit: '100', order: 'created_at desc'}, + total: 12 + }); + + expect(queryClient.getQueryData(memberCountKey)?.meta?.pagination.total).toBe(102466); + }); + + it('does not sync the sidebar member count from a searched members list query', async () => { + const queryClient = createQueryClientWithCurrentUser(); + + seedMemberCount(queryClient, 102466); + + await browseMembers({ + queryClient, + searchParams: {limit: '100', order: 'created_at desc', search: 'jamie'}, + total: 12 + }); + + expect(queryClient.getQueryData(memberCountKey)?.meta?.pagination.total).toBe(102466); + }); + + it('does not create a sidebar member count query when one is absent', async () => { + const queryClient = createQueryClientWithCurrentUser(); + + await browseMembers({queryClient}); + + expect(queryClient.getQueryData(memberCountKey)).toBeUndefined(); + }); +}); diff --git a/apps/admin/src/ember-bridge/ember-bridge.test.tsx b/apps/admin/src/ember-bridge/ember-bridge.test.tsx index 195a33f6b0c..76bcd2baed4 100644 --- a/apps/admin/src/ember-bridge/ember-bridge.test.tsx +++ b/apps/admin/src/ember-bridge/ember-bridge.test.tsx @@ -4,6 +4,7 @@ import { describe, expect, beforeEach, afterEach, vi, test as baseTest } from "v import { queryClientFixtures, type TestWrapperComponent } from "@test-utils/fixtures/query-client"; import type { QueryClient } from "@tanstack/react-query"; import type { StateBridge, StateBridgeEventMap } from "./ember-bridge"; +import { getMemberCountQueryKey } from "@tryghost/admin-x-framework/api/members"; const queryTest = baseTest.extend<{ queryClient: QueryClient; @@ -114,6 +115,41 @@ describe('useEmberDataSync', () => { unmount(); }); + queryTest('invalidates the sidebar member count query for Ember member changes', async ({ queryClient, wrapper }) => { + const mock = createMockStateBridge(); + window.EmberBridge = { state: mock.stateBridge }; + const sidebarMemberCountKey = getMemberCountQueryKey(); + const postsKey = ['PostsResponseType', '/posts']; + + queryClient.setQueryDefaults(sidebarMemberCountKey, {cacheTime: Infinity}); + queryClient.setQueryDefaults(postsKey, {cacheTime: Infinity}); + queryClient.setQueryData(sidebarMemberCountKey, { + members: [], + meta: {pagination: {page: 1, limit: 1, pages: 1, total: 102466, next: null, prev: null}} + }); + queryClient.setQueryData(postsKey, { posts: [] }); + + renderHook(() => useEmberDataSync(), { wrapper }); + + await waitFor(() => { + expect(mock.onSpy).toHaveBeenCalledWith('emberDataChange', expect.any(Function)); + }); + + act(() => { + mock.emit('emberDataChange', { + operation: 'delete', + modelName: 'member', + id: 'member-1', + data: null, + }); + }); + + await waitFor(() => { + expect(queryClient.getQueryState(sidebarMemberCountKey)?.isInvalidated).toBe(true); + expect(queryClient.getQueryState(postsKey)?.isInvalidated).toBe(false); + }); + }); + queryTest('ignores unmapped Ember models', async ({ queryClient, wrapper }) => { const mock = createMockStateBridge(); window.EmberBridge = { state: mock.stateBridge }; diff --git a/apps/admin/src/layout/app-sidebar/hooks/use-member-count.ts b/apps/admin/src/layout/app-sidebar/hooks/use-member-count.ts deleted file mode 100644 index 0e04ac88775..00000000000 --- a/apps/admin/src/layout/app-sidebar/hooks/use-member-count.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { useBrowseMembers } from "@tryghost/admin-x-framework/api/members"; - -/** - * Hook to fetch the total member count efficiently. - * Only fetches pagination metadata (limit=1) to minimize API overhead. - * Automatically invalidates when members are created/updated/deleted in Ember. - */ -export function useMemberCount() { - const { data: membersData } = useBrowseMembers({ - searchParams: { limit: '1' } - }); - - return membersData?.meta?.pagination.total; -} - diff --git a/apps/admin/src/layout/app-sidebar/nav-content.tsx b/apps/admin/src/layout/app-sidebar/nav-content.tsx index a6a561eaeb8..305e296e2ab 100644 --- a/apps/admin/src/layout/app-sidebar/nav-content.tsx +++ b/apps/admin/src/layout/app-sidebar/nav-content.tsx @@ -3,10 +3,10 @@ import React from "react" import {SidebarGroup, SidebarGroupContent, SidebarMenu, SidebarMenuBadge} from "@tryghost/shade/components" import {formatNumber, LucideIcon} from "@tryghost/shade/utils" import { useCurrentUser } from "@tryghost/admin-x-framework/api/current-user"; +import { useMemberCount } from "@tryghost/admin-x-framework/api/members"; import {getSettingValue, useBrowseSettings} from "@tryghost/admin-x-framework/api/settings"; import { canManageMembers, canManageTags } from "@tryghost/admin-x-framework/api/users"; import { NavMenuItem } from "./nav-menu-item"; -import { useMemberCount } from "./hooks/use-member-count"; import { useNavigationExpanded } from "./hooks/use-navigation-preferences"; import { NavCustomViews } from "./nav-custom-views"; import { NavMemberViews } from "./nav-member-views"; diff --git a/apps/posts/src/views/members/components/bulk-action-modals/import-members-modal.tsx b/apps/posts/src/views/members/components/bulk-action-modals/import-members-modal.tsx index b6cba587bec..6e40cbc0659 100644 --- a/apps/posts/src/views/members/components/bulk-action-modals/import-members-modal.tsx +++ b/apps/posts/src/views/members/components/bulk-action-modals/import-members-modal.tsx @@ -1,11 +1,12 @@ import {CompleteStep, ErrorStep, InitStep, MappingStep, ProcessingStep} from './import-members/components'; import {Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle} from '@tryghost/shade/components'; +import {HostLimitError, JSONError, RequestEntityTooLargeError, ValidationError} from '@tryghost/admin-x-framework/errors'; import {type ImportResponse} from './import-members/state'; import {MembersFieldMapping, detectFieldTypes, getFieldMappings} from './import-members/mapping'; import {buildImportResponse} from './import-members/upload'; import {cn} from '@tryghost/shade/utils'; import {createInitialImportState, importReducer} from './import-members/reducer'; -import {getGhostPaths} from '@tryghost/admin-x-framework/helpers'; +import {isImportMembersAcceptedResponse, isImportMembersCompleteResponse, useImportMembers} from '@tryghost/admin-x-framework/api/members'; import {parseCSV} from './import-members/csv'; import {useBrowseConfig} from '@tryghost/admin-x-framework/api/config'; import {useCallback, useEffect, useMemo, useReducer, useRef} from 'react'; @@ -27,6 +28,7 @@ export function ImportMembersModal({ const [state, dispatch] = useReducer(importReducer, undefined, createInitialImportState); const errorCsvUrlRef = useRef(null); const {data: configData} = useBrowseConfig(); + const {mutateAsync: importMembers} = useImportMembers(); const importMemberTier = configData?.config?.labs?.importMemberTier === true; const giftSubscriptionsEnabled = configData?.config?.labs?.giftSubscriptions === true; const fieldMappings = useMemo(() => getFieldMappings({importMemberTier, giftSubscriptionsEnabled}), [importMemberTier, giftSubscriptionsEnabled]); @@ -185,45 +187,43 @@ export function ImportMembersModal({ dispatch({type: 'UPLOAD_START'}); - const formData = new FormData(); - formData.append('membersfile', state.file); - - // Convert slugs to names for the upload API + const selectedLabelNames: string[] = []; for (const slug of state.selectedLabelSlugs) { const label = labelPicker.labels.find(l => l.slug === slug); if (label) { - formData.append('labels', label.name); - } - } - - if (state.mapping) { - const mappingJSON = state.mapping.toJSON(); - for (const [key, val] of Object.entries(mappingJSON)) { - if (val) { - formData.append(`mapping[${key}]`, val); - } + selectedLabelNames.push(label.name); } } try { - const {apiRoot} = getGhostPaths(); - const response = await fetch(`${apiRoot}/members/upload/`, { - method: 'POST', - body: formData, - credentials: 'include', - mode: 'cors', - headers: { - 'app-pragma': 'no-cache' - } + const importData = await importMembers({ + file: state.file, + labels: selectedLabelNames, + mapping: state.mapping?.toJSON() }); - if (response.status === 202) { + if (isImportMembersCompleteResponse(importData)) { + const importResponse = buildImportResponse(importData); + revokeErrorCsvUrl(); + errorCsvUrlRef.current = importResponse.errorCsvUrl; + + dispatch({ + type: 'UPLOAD_COMPLETE', + importResponse + }); + onComplete?.(importResponse); + return; + } + + if (isImportMembersAcceptedResponse(importData)) { dispatch({type: 'UPLOAD_ACCEPTED'}); onComplete?.(); return; } - if (response.status === 413) { + throw new Error('Unexpected members import response'); + } catch (error) { + if (error instanceof RequestEntityTooLargeError) { dispatch({ type: 'UPLOAD_ERROR', errorMessage: 'The file you uploaded was larger than the maximum file size your server allows.' @@ -231,47 +231,27 @@ export function ImportMembersModal({ return; } - if (!response.ok) { - const data = await response.json(); - const err = data?.errors?.[0]; - - if (err?.type === 'HostLimitError' && err?.code === 'EMAIL_VERIFICATION_NEEDED') { - dispatch({ - type: 'UPLOAD_ERROR', - errorMessage: err.message, - errorHeader: 'Woah there cowboy, that\'s a big list', - showTryAgainButton: false - }); - onComplete?.(); - return; - } - - if (err?.type === 'DataImportError' || err?.type === 'ValidationError') { - dispatch({ - type: 'UPLOAD_ERROR', - errorMessage: err.message - }); - return; - } + const apiError = error instanceof JSONError ? error.data?.errors?.[0] : null; + if (error instanceof HostLimitError && apiError?.code === 'EMAIL_VERIFICATION_NEEDED') { dispatch({ type: 'UPLOAD_ERROR', - errorMessage: 'An unexpected error occurred, please try again' + errorMessage: apiError.message, + errorHeader: 'Woah there cowboy, that\'s a big list', + showTryAgainButton: false }); + onComplete?.(); return; } - const importData = await response.json(); - const importResponse = buildImportResponse(importData); - revokeErrorCsvUrl(); - errorCsvUrlRef.current = importResponse.errorCsvUrl; + if (error instanceof ValidationError || apiError?.type === 'DataImportError') { + dispatch({ + type: 'UPLOAD_ERROR', + errorMessage: apiError?.message || (error instanceof Error ? error.message : 'An unexpected error occurred, please try again') + }); + return; + } - dispatch({ - type: 'UPLOAD_COMPLETE', - importResponse - }); - onComplete?.(importResponse); - } catch { dispatch({ type: 'UPLOAD_ERROR', errorMessage: 'An unexpected error occurred, please try again' @@ -283,6 +263,7 @@ export function ImportMembersModal({ state.mappingError, state.selectedLabelSlugs, labelPicker.labels, + importMembers, revokeErrorCsvUrl, onComplete ]); diff --git a/apps/posts/src/views/members/components/bulk-action-modals/import-members/upload.ts b/apps/posts/src/views/members/components/bulk-action-modals/import-members/upload.ts index c85e1ca1f32..ecbe4a55d64 100644 --- a/apps/posts/src/views/members/components/bulk-action-modals/import-members/upload.ts +++ b/apps/posts/src/views/members/components/bulk-action-modals/import-members/upload.ts @@ -1,24 +1,10 @@ import moment from 'moment-timezone'; +import {type ImportMembersCompleteResponseType} from '@tryghost/admin-x-framework/api/members'; import {ImportResponse} from './state'; import {formatImportError} from './mapping'; import {unparseErrorCSV} from './csv'; -type InvalidMemberRow = Record & {error: string}; - -type UploadApiResponse = { - meta: { - stats: { - imported: number; - invalid: InvalidMemberRow[]; - }; - import_label?: { - name: string; - slug: string; - }; - }; -}; - -export function buildImportResponse(importData: UploadApiResponse): ImportResponse { +export function buildImportResponse(importData: ImportMembersCompleteResponseType): ImportResponse { const importedCount = importData.meta.stats.imported; const erroredMembers = importData.meta.stats.invalid || []; const errorCount = erroredMembers.length; @@ -51,6 +37,6 @@ export function buildImportResponse(importData: UploadApiResponse): ImportRespon errorCsvUrl, errorCsvName, errorList: Object.values(errorListMap), - importLabel: importData.meta.import_label + importLabel: importData.meta.import_label || undefined }; } diff --git a/apps/posts/test/unit/views/members/import-members/modal.test.tsx b/apps/posts/test/unit/views/members/import-members/modal.test.tsx index 68f9c6bcdf7..4ed5c0133bf 100644 --- a/apps/posts/test/unit/views/members/import-members/modal.test.tsx +++ b/apps/posts/test/unit/views/members/import-members/modal.test.tsx @@ -1,3 +1,4 @@ +import {HostLimitError, JSONError, RequestEntityTooLargeError, ValidationError} from '@tryghost/admin-x-framework/errors'; import {ImportMembersModal} from '@src/views/members/components/bulk-action-modals/import-members-modal'; import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; import {fireEvent, render, screen, waitFor} from '@testing-library/react'; @@ -15,9 +16,19 @@ vi.mock('@tryghost/admin-x-framework/api/config', () => ({ }) })); -vi.mock('@tryghost/admin-x-framework/helpers', () => ({ - getGhostPaths: () => ({ - apiRoot: '/ghost/api/admin' +const {mockImportMembers} = vi.hoisted(() => ({ + mockImportMembers: vi.fn() +})); + +vi.mock('@tryghost/admin-x-framework/api/members', () => ({ + isImportMembersAcceptedResponse: (response: {meta?: {originalImportSize?: number; stats?: unknown}}) => ( + typeof response.meta?.originalImportSize === 'number' && response.meta.stats === undefined + ), + isImportMembersCompleteResponse: (response: {meta?: {stats?: {imported?: number}}}) => ( + typeof response.meta?.stats?.imported === 'number' + ), + useImportMembers: () => ({ + mutateAsync: mockImportMembers }) })); @@ -44,6 +55,52 @@ function createFile(name: string, type: string, contents = 'content') { }; } +function createApiError(type: string, message: string, code = '') { + return { + errors: [{ + code, + context: null, + details: null, + ghostErrorCode: null, + help: '', + id: 'error-id', + message, + property: null, + type + }] + }; +} + +async function uploadCsv() { + const dropTarget = screen.getByRole('button', {name: /select or drop a csv file/i}); + const csvFile = createFile('members.csv', 'text/csv', 'email,name\nmember@example.com,Member'); + + fireEvent.drop(dropTarget, { + dataTransfer: { + files: [csvFile], + items: [{ + kind: 'file', + type: csvFile.type, + getAsFile: () => csvFile + }], + types: ['Files'] + } + }); + + const importButton = await screen.findByRole('button', {name: /import 1 member/i}); + fireEvent.click(importButton); +} + +function renderModal(props: Partial[0]> = {}) { + return render( + {}} + {...props} + /> + ); +} + class MockFileReader { static LOADING = 1; onload: ((event: {target: {result: string}}) => void) | null = null; @@ -74,6 +131,12 @@ describe('ImportMembersModal', () => { mockCsvContents = 'email,name\nmember@example.com,Member'; vi.stubGlobal('FileReader', MockFileReader); vi.stubGlobal('fetch', vi.fn(async () => new Response(null, {status: 202}))); + mockImportMembers.mockReset(); + mockImportMembers.mockResolvedValue({ + meta: { + originalImportSize: 1 + } + }); Object.defineProperty(URL, 'createObjectURL', { configurable: true, writable: true, @@ -101,50 +164,129 @@ describe('ImportMembersModal', () => { }); }); + it('uploads members through the members API mutation so member queries are invalidated', async () => { + renderModal(); + + await uploadCsv(); + + await waitFor(() => { + expect(mockImportMembers).toHaveBeenCalledTimes(1); + }); + + expect(mockImportMembers.mock.calls[0][0]).toEqual(expect.objectContaining({ + file: expect.objectContaining({name: 'members.csv'}), + mapping: expect.objectContaining({ + email: 'email', + name: 'name' + }) + })); + expect(fetch).not.toHaveBeenCalled(); + }); + it('calls onComplete when the upload response is accepted for background processing', async () => { const onComplete = vi.fn(); - render( - {}} - /> - ); + renderModal({onComplete}); - const dropTarget = screen.getByRole('button', {name: /select or drop a csv file/i}); - const csvFile = createFile('members.csv', 'text/csv', 'email,name\nmember@example.com,Member'); + await uploadCsv(); - fireEvent.drop(dropTarget, { - dataTransfer: { - files: [csvFile], - items: [{ - kind: 'file', - type: csvFile.type, - getAsFile: () => csvFile - }], - types: ['Files'] + await waitFor(() => { + expect(onComplete).toHaveBeenCalledTimes(1); + }); + + expect(screen.getByRole('heading', {name: /import in progress/i})).toBeInTheDocument(); + }); + + it('shows the import result when the upload completes inline', async () => { + const onComplete = vi.fn(); + mockImportMembers.mockResolvedValueOnce({ + meta: { + stats: { + imported: 1, + invalid: [] + }, + import_label: { + name: 'Test Import', + slug: 'test-import' + } } }); - const importButton = await screen.findByRole('button', {name: /import 1 member/i}); - fireEvent.click(importButton); + renderModal({onComplete}); + + await uploadCsv(); await waitFor(() => { - expect(onComplete).toHaveBeenCalledTimes(1); + expect(onComplete).toHaveBeenCalledWith(expect.objectContaining({ + importedCount: 1, + errorCount: 0, + errorCsvUrl: 'blob:mock/0' + })); }); - expect(screen.getByRole('heading', {name: /import in progress/i})).toBeInTheDocument(); + expect(screen.getByRole('heading', {name: /import complete/i})).toBeInTheDocument(); + }); + + it('shows the file size error when the upload is too large', async () => { + mockImportMembers.mockRejectedValueOnce(new RequestEntityTooLargeError(new Response(null, {status: 413}), 'too large')); + + renderModal(); + + await uploadCsv(); + + expect(await screen.findByText('The file you uploaded was larger than the maximum file size your server allows.')).toBeInTheDocument(); + }); + + it('shows the email verification host limit error without retrying', async () => { + const onComplete = vi.fn(); + const errorData = createApiError('HostLimitError', 'Please verify your email before importing this many members.', 'EMAIL_VERIFICATION_NEEDED'); + mockImportMembers.mockRejectedValueOnce(new HostLimitError(new Response(JSON.stringify(errorData), {status: 403}), errorData)); + + renderModal({onComplete}); + + await uploadCsv(); + + expect(await screen.findByRole('heading', {name: /woah there cowboy/i})).toBeInTheDocument(); + expect(screen.getByText('Please verify your email before importing this many members.')).toBeInTheDocument(); + expect(screen.queryByRole('button', {name: /try again/i})).not.toBeInTheDocument(); + expect(onComplete).toHaveBeenCalledTimes(1); + }); + + it('shows validation errors returned by the upload API', async () => { + const errorData = createApiError('ValidationError', 'Please map Email to one of the fields in the CSV.'); + mockImportMembers.mockRejectedValueOnce(new ValidationError(new Response(JSON.stringify(errorData), {status: 422}), errorData)); + + renderModal(); + + await uploadCsv(); + + expect(await screen.findByText('Please map Email to one of the fields in the CSV.')).toBeInTheDocument(); + }); + + it('shows data import errors returned by the upload API', async () => { + const errorData = createApiError('DataImportError', 'Some rows could not be imported.'); + mockImportMembers.mockRejectedValueOnce(new JSONError(new Response(JSON.stringify(errorData), {status: 422}), errorData)); + + renderModal(); + + await uploadCsv(); + + expect(await screen.findByText('Some rows could not be imported.')).toBeInTheDocument(); + }); + + it('shows a generic error for unexpected successful upload responses', async () => { + mockImportMembers.mockResolvedValueOnce({meta: {}} as never); + + renderModal(); + + await uploadCsv(); + + expect(await screen.findByText('An unexpected error occurred, please try again')).toBeInTheDocument(); }); it('shows tier as a mapped field when importMemberTier is enabled', async () => { mockCsvContents = 'email,import_tier\nmember@example.com,Gold'; - render( - {}} - /> - ); + renderModal(); const dropTarget = screen.getByRole('button', {name: /select or drop a csv file/i}); const csvFile = createFile('members.csv', 'text/csv', mockCsvContents);