fix(settings): don't crash on a fresh load before permissions resolve
On a hard navigation or deep link, usePermissions() starts out empty, which filters every settings nav group down to nothing. allItems is then [], so `allItems.find(...) ?? allItems[0]` yields undefined and `<activeItem.icon>` threw -- sometimes into the error boundary, sometimes racing past it. Reproduced 6+ times across the webhooks/moderation/slideshow/security/events tabs; in-app SPA navigation never hit it. Extend the file's existing early-return to `isLoading || permissionsLoading`. activeTab lives in useState seeded from ?tab= at mount, independent of the gate, so deep links still land on the right tab once permissions arrive. Also null-guard activeItem before the section heading: a role holding zero settings-tab permissions crashes identically even after permissions finish loading, which the loading gate alone does not cover. Refs testplan REPORT.md #11 (Part 3, J.08).
This commit is contained in:
@@ -170,7 +170,7 @@ export const SettingsPage: React.FC = () => {
|
|||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
const [searchParams, setSearchParams] = useSearchParams();
|
const [searchParams, setSearchParams] = useSearchParams();
|
||||||
const { flags, isLoading: flagsLoading } = useFeatureFlags();
|
const { flags, isLoading: flagsLoading } = useFeatureFlags();
|
||||||
const { hasAnyPermission } = usePermissions();
|
const { hasAnyPermission, isLoading: permissionsLoading } = usePermissions();
|
||||||
|
|
||||||
// Read ?tab=… on mount; default to Features per the redesign.
|
// Read ?tab=… on mount; default to Features per the redesign.
|
||||||
const initialTab: TabType = isValidTab(searchParams.get('tab'))
|
const initialTab: TabType = isValidTab(searchParams.get('tab'))
|
||||||
@@ -288,7 +288,11 @@ export const SettingsPage: React.FC = () => {
|
|||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [flagsLoading, activeTab, flags.quotes, flags.bills, flags.contracts, flags.reminderEmails, flags.accounting, flags.whatsapp, flags.slideshow]);
|
}, [flagsLoading, activeTab, flags.quotes, flags.bills, flags.contracts, flags.reminderEmails, flags.accounting, flags.whatsapp, flags.slideshow]);
|
||||||
|
|
||||||
if (isLoading) {
|
// Wait for the permissions context too: on a fresh/hard mount it starts out
|
||||||
|
// empty, which filters every nav group down to nothing and left `activeItem`
|
||||||
|
// undefined below (QA J.08 crash). `activeTab` is held in state, so a
|
||||||
|
// deep-linked ?tab= still lands on the right tab once permissions arrive.
|
||||||
|
if (isLoading || permissionsLoading) {
|
||||||
return (
|
return (
|
||||||
<div className="flex items-center justify-center min-h-[400px]">
|
<div className="flex items-center justify-center min-h-[400px]">
|
||||||
<Loading size="lg" text={t('settings.loadingSettings')} />
|
<Loading size="lg" text={t('settings.loadingSettings')} />
|
||||||
@@ -483,7 +487,7 @@ export const SettingsPage: React.FC = () => {
|
|||||||
</aside>
|
</aside>
|
||||||
|
|
||||||
<div className="min-w-0">
|
<div className="min-w-0">
|
||||||
{showSectionHeading && (
|
{showSectionHeading && activeItem && (
|
||||||
<div className="mb-4 lg:mb-6 pb-3 border-b border-neutral-200 dark:border-neutral-700">
|
<div className="mb-4 lg:mb-6 pb-3 border-b border-neutral-200 dark:border-neutral-700">
|
||||||
<div className="flex items-center gap-2">
|
<div className="flex items-center gap-2">
|
||||||
{/* Section heading icon stays neutral so the Settings
|
{/* Section heading icon stays neutral so the Settings
|
||||||
|
|||||||
@@ -0,0 +1,110 @@
|
|||||||
|
/**
|
||||||
|
* Settings crashed on a fresh/hard load of any non-default tab (QA J.08).
|
||||||
|
*
|
||||||
|
* The nav groups are permission-filtered, so before PermissionsContext has
|
||||||
|
* resolved every group filters to empty, `allItems[0]` is undefined, and the
|
||||||
|
* section heading's `<activeItem.icon />` throws. In-app SPA navigation never
|
||||||
|
* hit it because the context was already warm.
|
||||||
|
*/
|
||||||
|
import React from 'react';
|
||||||
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
|
import { render, screen } from '@testing-library/react';
|
||||||
|
import { MemoryRouter } from 'react-router-dom';
|
||||||
|
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||||
|
|
||||||
|
vi.mock('react-i18next', async () => {
|
||||||
|
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
useTranslation: () => ({ t: (k: string, fb?: unknown) => (typeof fb === 'string' ? fb : k) }),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
const flagsState = { flags: {} as Record<string, boolean>, isLoading: false };
|
||||||
|
vi.mock('../../../contexts/FeatureFlagsContext', () => ({
|
||||||
|
useFeatureFlags: () => flagsState,
|
||||||
|
useFeatureEnabled: () => false,
|
||||||
|
}));
|
||||||
|
|
||||||
|
const permissionsState = { hasAnyPermission: (_: string[]) => true, isLoading: false };
|
||||||
|
vi.mock('../../../contexts/PermissionsContext', () => ({
|
||||||
|
usePermissions: () => permissionsState,
|
||||||
|
}));
|
||||||
|
|
||||||
|
// The settings barrel pulls in every tab; stub it down to the shell's needs.
|
||||||
|
vi.mock('../../../features/settings', () => {
|
||||||
|
const Stub = () => null;
|
||||||
|
return {
|
||||||
|
useSettingsState: () => ({ isLoading: false }),
|
||||||
|
FeaturesTab: Stub,
|
||||||
|
GeneralTab: Stub,
|
||||||
|
EventsTab: Stub,
|
||||||
|
StatusTab: Stub,
|
||||||
|
SecurityTab: Stub,
|
||||||
|
ImageSecurityTab: Stub,
|
||||||
|
CategoriesTab: Stub,
|
||||||
|
AnalyticsTab: Stub,
|
||||||
|
ModerationTab: Stub,
|
||||||
|
StylingTab: Stub,
|
||||||
|
SEOTab: Stub,
|
||||||
|
ThumbnailsTab: Stub,
|
||||||
|
DownloadsTab: Stub,
|
||||||
|
ApiTokensTab: Stub,
|
||||||
|
WebhooksTab: Stub,
|
||||||
|
AccountingTab: Stub,
|
||||||
|
WhatsAppTab: Stub,
|
||||||
|
SsoTab: Stub,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.mock('../EmailConfigPage', () => ({ EmailConfigPage: () => null }));
|
||||||
|
vi.mock('../BrandingPage', () => ({ BrandingPage: () => null }));
|
||||||
|
vi.mock('../EventTypesPage', () => ({ EventTypesPage: () => null }));
|
||||||
|
vi.mock('../SlideshowSettingsPage', () => ({ SlideshowSettingsPage: () => null }));
|
||||||
|
vi.mock('../BackupManagement', () => ({ BackupManagement: () => null }));
|
||||||
|
vi.mock('../CMSPage', () => ({ CMSPage: () => null }));
|
||||||
|
vi.mock('../settings/SettingsBusinessProfilePage', () => ({ SettingsBusinessProfilePage: () => null }));
|
||||||
|
vi.mock('../settings/CrmSettingsPage', () => ({ CrmSettingsPage: () => null }));
|
||||||
|
vi.mock('../settings/ReminderTemplatesPage', () => ({ ReminderTemplatesPage: () => null }));
|
||||||
|
vi.mock('../contracts/BlockLibraryPage', () => ({ BlockLibraryPage: () => null }));
|
||||||
|
|
||||||
|
import { SettingsPage } from '../SettingsPage';
|
||||||
|
|
||||||
|
function renderAt(tab: string) {
|
||||||
|
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||||
|
return render(
|
||||||
|
<QueryClientProvider client={qc}>
|
||||||
|
<MemoryRouter initialEntries={[`/admin/settings?tab=${tab}`]}>
|
||||||
|
<SettingsPage />
|
||||||
|
</MemoryRouter>
|
||||||
|
</QueryClientProvider>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('SettingsPage fresh-mount permission race (QA J.08)', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
permissionsState.hasAnyPermission = () => true;
|
||||||
|
permissionsState.isLoading = false;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not crash on a deep-linked tab while permissions are still loading', () => {
|
||||||
|
permissionsState.isLoading = true;
|
||||||
|
permissionsState.hasAnyPermission = () => false;
|
||||||
|
|
||||||
|
expect(() => renderAt('webhooks')).not.toThrow();
|
||||||
|
expect(screen.getByText('settings.loadingSettings')).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still lands on the deep-linked tab once permissions arrive', () => {
|
||||||
|
renderAt('webhooks');
|
||||||
|
|
||||||
|
expect(screen.getByRole('heading', { level: 2, name: 'Webhooks' })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not crash when the role has no settings tab permissions at all', () => {
|
||||||
|
permissionsState.hasAnyPermission = () => false;
|
||||||
|
|
||||||
|
expect(() => renderAt('webhooks')).not.toThrow();
|
||||||
|
expect(screen.getByText('settings.title')).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user