feat(#358): Paket 5 — statische /contacts-Routen entfernt nach bewiesenem PluginRouteRenderer (nested Routes, :id-Matching, useParams), Contacts-Seiten in STATIC_COMPONENT_MAP (ARCH-019)
This commit is contained in:
+1
-1
@@ -10,7 +10,7 @@
|
||||
2. **AppShell ×4 + Router ×2 Mock-Fixes** — ✅ erledigt (2026-08-28): AppShell war bereits grün (useCurrentUser-Mock existierte in Zeile 9; Plan-Eintrag veraltet, bewiesen: Lauf 23:00 'Tests 2 failed (6)' = nur Router). Router.test.tsx gefixt: QueryClientProvider + Mocks useCurrentUser/useUserPermissions → **2/2 passed (2.03s)**
|
||||
3. **ContactEditModal File-Level** — ✅ erledigt (2026-08-28): Geister-Test nach §10 gelöscht. Ursache bewiesen: ContactEditModal.tsx (349 Z.) wurde in db4701b (BUG-080/082) gelöscht, Test blieb → Vitest 'Failed to resolve import' (1 failed, no tests). Ersatz ContactEditForm.tsx lebt und wird von ContactsList/ContactDetailPage genutzt
|
||||
4. **custom_field_definitions generisch machen** — ✅ erledigt (2026-08-29): W4b-Muster komplett angewendet. Route: 422-Entity-Validierung + 403-Owner-Modul-Check (contacts:read etc.) auf create/list/update/delete, ACL-Loch gefixt (delete übergab user_id nie → 500 für alle Nicht-Admins), PermissionError→403-Handler. Shape-Fix: Backend lieferte Array, alle 6 Frontend-Consumer lesen `data.items` → CustomFields-UI zeigte seit jeher leer; jetzt `{items,total}` (CustomFieldDefinitionListResponse). entity_permission_service: Plural-Ableitungs-Fix ('workflow'→workflows:read statt Phantom contacts:read; 'address'→addresses:read via +es), unregister_entity_model räumt ENTITY_PLUGIN_OWNERS mit auf (Lifecycle-Leak), zentrale Helper validate_entity_type/check_entity_read_permission — saved_filters+saved_views Duplikate entfernt (Aliase, Call-Sites unverändert). Neue Suite tests/test_custom_field_definitions.py **13 Tests**: Rot bewiesen 10 failed/4 passed → Grün **25 passed** (13 cfd + 12 saved_filters-Regression), Permission-Suiten 22/22, custom_fields+lifecycle 13/13, ruff exit=0, create_app OK. 3er-Kombi-Failures (custom_fields+entity_registry+lifecycle) per Stash als identischer Vorbestand bewiesen (clean HEAD: gleiche 7 Failures — Suite-Isolation, kein Zusammenhang mit Änderung). api-documentation.md ergänzt (4 Endpoints)
|
||||
5. **Sidebar /contacts statische Route entfernen** — routes/index.tsx Zeile 250; erst PluginRouteRenderer beweisen (Kritikpunkt 21: Production-Build + Reload-Test), dann entfernen
|
||||
5. **Sidebar /contacts statische Route entfernen** — ✅ erledigt (2026-08-29): Kritikpunkt 21 zuerst erfüllt (Renderer bewiesen, DANN entfernt). PluginRouteRenderer komplett neu: verschachtelte `<Routes>` statt manueller find()-Logik — vorher bewiesen kaputt: ':id'-Patterns konnten NIE matchen (Detail-Deep-Link /contacts/abc hätte die Liste gerendert), '/contacts/dedup' wäre auf die Liste gefallen, useParams() blieb im catch-all leer. Rot 4 failed → **Grün 9/9 Tests** (inkl. :id-Match, useParams={"id":"abc-123"}-Beweis, dedup-schlägt-:id-Spezifität). STATIC_COMPONENT_MAP um 3 Contacts-Seiten ergänzt (ARCH-019: Production-Build kann Runtime-Fallback nicht laden; Named-Exports explizit, Pages haben kein default). Statische Routen /contacts, /contacts/:id, /contacts/dedup aus index.tsx entfernt + 3 ungenutzte lazy-Imports; /trash (contacts:read) und /guest/contacts unberührt. Gates: tsc OK · **Production-Build BUILD_EXIT=0 (2.76s) mit frischen Chunks ContactsList-DZ-WOyL2.js/ContactDetailPage-LZBUypcB.js/DedupMerge-H6U7AYKS.js (02:04)** · Renderer 9/9 · routePermissions 6/6 · Router 2/2 · Sidebar-Nav-Quelle bewiesen: Sidebar.tsx Z.97 flatMap menu_items (Manifest, unberührt). AppShell-Solo/Combo-Worker-Crash = dokumentierter Vorbestand (heute 3× vor Änderung reproduziert)
|
||||
6. **Kontakt-Model ins ContactsPlugin** — groß (~30 Import-Stellen, Alembic-Kette), bewusst zurückgestellt
|
||||
7. **Phase L: Dokumente-Generator** — PLATFORM_ROADMAP.md 'Phase L' (L1-L5, ~9-15 Tage), user-abgestimmt, Basis: report_generator-Plugin
|
||||
|
||||
|
||||
@@ -125,6 +125,9 @@ const STATIC_COMPONENT_MAP: Record<string, LazyComponentFactory> = {
|
||||
'@/pages/AutomationSettings': () => import('@/pages/AutomationSettings').then(normalizeModule),
|
||||
'@/pages/Calendar': () => import('@/pages/Calendar').then(normalizeModule),
|
||||
'@/pages/Communication': () => import('@/pages/Communication').then(normalizeModule),
|
||||
'@/pages/ContactDetailPage': () => import('@/pages/ContactDetailPage').then((m) => ({ default: m.ContactDetailPage })),
|
||||
'@/pages/ContactsList': () => import('@/pages/ContactsList').then((m) => ({ default: m.ContactsListPage })),
|
||||
'@/pages/DedupMergePage': () => import('@/pages/DedupMerge').then((m) => ({ default: m.DedupMergePage })),
|
||||
'@/pages/Dms': () => import('@/pages/Dms').then(normalizeModule),
|
||||
'@/pages/DmsTrash': () => import('@/pages/DmsTrash').then(normalizeModule),
|
||||
'@/pages/GlobalSearchResults': () => import('@/pages/GlobalSearchResults').then(normalizeModule),
|
||||
|
||||
@@ -1,16 +1,22 @@
|
||||
import { useMemo } from 'react';
|
||||
import { useLocation } from 'react-router-dom';
|
||||
import { useLocation, Route, Routes } from 'react-router-dom';
|
||||
import { Loader2 } from 'lucide-react';
|
||||
import { usePluginStore } from '@/store/pluginStore';
|
||||
import { ProtectedRoute } from '@/components/common/ProtectedRoute';
|
||||
import { PluginPage } from './PluginLoader';
|
||||
|
||||
/**
|
||||
* PluginRouteRenderer — catch-all route handler that checks the current URL
|
||||
* against all plugin page_routes from the plugin store.
|
||||
* PluginRouteRenderer — catch-all route handler that renders the plugin
|
||||
* page_routes from the plugin store via real react-router <Routes>.
|
||||
*
|
||||
* If a matching route is found, it renders the plugin's page component.
|
||||
* Otherwise it renders a simple "Not Found" message.
|
||||
* Paket 5 / Kritikpunkt 21: the previous manual find() (exact + prefix
|
||||
* match) could neither match `:id` patterns nor pass route params to pages
|
||||
* (`useParams()` stayed empty inside the catch-all `*` route), and a prefix
|
||||
* match let `/contacts` swallow `/contacts/dedup` and detail URLs. Nested
|
||||
* <Routes> give us router-native matching for free:
|
||||
* - `:param` segments match and populate useParams()
|
||||
* - static segments beat dynamic ones (`/contacts/dedup` beats `/contacts/:id`)
|
||||
* - no accidental prefix swallowing — unknown paths fall through to 404
|
||||
*
|
||||
* This component is intended to be used as the last child of the protected
|
||||
* route group in routes/index.tsx:
|
||||
@@ -21,45 +27,20 @@ export function PluginRouteRenderer() {
|
||||
const manifests = usePluginStore((s) => s.manifests);
|
||||
const loaded = usePluginStore((s) => s.loaded);
|
||||
|
||||
const routes = useMemo(
|
||||
() => manifests
|
||||
.flatMap((m) => m.page_routes)
|
||||
.sort((a, b) => a.order - b.order),
|
||||
// Flatten manifests → route entries with their owning plugin attached.
|
||||
// Attaching the name here (instead of a later find()) stays correct even
|
||||
// if two plugins declare overlapping paths.
|
||||
const entries = useMemo(
|
||||
() =>
|
||||
manifests.flatMap((m) =>
|
||||
m.page_routes.map((r) => ({
|
||||
route: r,
|
||||
pluginName: m.display_name || m.name || r.path,
|
||||
}))
|
||||
),
|
||||
[manifests]
|
||||
);
|
||||
|
||||
// Find the first matching route (exact match or prefix match for nested routes)
|
||||
const matchedRoute = routes.find((r) => {
|
||||
// Exact match
|
||||
if (location.pathname === r.path) return true;
|
||||
// Prefix match for nested routes (e.g. /calendar/settings matches /calendar)
|
||||
if (r.path !== '/' && location.pathname.startsWith(r.path + '/')) return true;
|
||||
return false;
|
||||
});
|
||||
|
||||
if (matchedRoute) {
|
||||
// Find the plugin name for display
|
||||
const plugin = manifests.find((m) =>
|
||||
m.page_routes.some((pr) => pr.path === matchedRoute.path)
|
||||
);
|
||||
const pluginName = plugin?.display_name || plugin?.name || matchedRoute.path;
|
||||
|
||||
const page = (
|
||||
<PluginPage
|
||||
component={matchedRoute.component}
|
||||
pluginName={pluginName}
|
||||
/>
|
||||
);
|
||||
|
||||
// ARCH-006: enforce the manifest permission like the static routes do.
|
||||
// Empty permission = any authenticated user (route is already inside the
|
||||
// protected route group).
|
||||
if (matchedRoute.permission) {
|
||||
return <ProtectedRoute permission={matchedRoute.permission}>{page}</ProtectedRoute>;
|
||||
}
|
||||
return page;
|
||||
}
|
||||
|
||||
// If manifests haven't loaded yet, show a spinner (not null/blank)
|
||||
if (!loaded) {
|
||||
return (
|
||||
@@ -69,12 +50,43 @@ export function PluginRouteRenderer() {
|
||||
);
|
||||
}
|
||||
|
||||
// No plugin route matched — show a simple not-found
|
||||
// No plugin routes at all — nothing to match against
|
||||
if (entries.length === 0) {
|
||||
return <PluginRouteNotFound pathname={location.pathname} />;
|
||||
}
|
||||
|
||||
return (
|
||||
<Routes>
|
||||
{entries.map(({ route, pluginName }) => (
|
||||
<Route
|
||||
key={route.path}
|
||||
path={route.path}
|
||||
element={
|
||||
route.permission ? (
|
||||
// ARCH-006: enforce the manifest permission like the static
|
||||
// routes do. Empty permission = any authenticated user (the
|
||||
// route is already inside the protected route group).
|
||||
<ProtectedRoute permission={route.permission}>
|
||||
<PluginPage component={route.component} pluginName={pluginName} />
|
||||
</ProtectedRoute>
|
||||
) : (
|
||||
<PluginPage component={route.component} pluginName={pluginName} />
|
||||
)
|
||||
}
|
||||
/>
|
||||
))}
|
||||
{/* Unknown paths fall through to not-found */}
|
||||
<Route path="*" element={<PluginRouteNotFound pathname={location.pathname} />} />
|
||||
</Routes>
|
||||
);
|
||||
}
|
||||
|
||||
function PluginRouteNotFound({ pathname }: { pathname: string }) {
|
||||
return (
|
||||
<div className="flex flex-col items-center justify-center min-h-[50vh] text-secondary-500">
|
||||
<h2 className="text-2xl font-semibold mb-2">Page Not Found</h2>
|
||||
<p className="text-sm">
|
||||
The page <code className="bg-secondary-100 px-1 rounded">{location.pathname}</code> was not found.
|
||||
The page <code className="bg-secondary-100 px-1 rounded">{pathname}</code> was not found.
|
||||
</p>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -1,18 +1,29 @@
|
||||
import React from 'react';
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { MemoryRouter } from 'react-router-dom';
|
||||
import { MemoryRouter, useParams } from 'react-router-dom';
|
||||
import { PluginRouteRenderer } from '../PluginRouteRenderer';
|
||||
import { usePluginStore } from '@/store/pluginStore';
|
||||
import { useAuthStore } from '@/store/authStore';
|
||||
import type { PluginUiManifest } from '@/store/pluginStore';
|
||||
|
||||
// Mock PluginPage to avoid lazy loading issues in tests
|
||||
// Mock PluginPage to avoid lazy loading issues in tests.
|
||||
// Renders useParams() as data-params so tests can prove that route params
|
||||
// (e.g. :id) actually reach the plugin page (Paket 5 / Kritikpunkt 21).
|
||||
vi.mock('../PluginLoader', () => ({
|
||||
PluginPage: ({ component, pluginName }: { component: string; pluginName: string }) => (
|
||||
<div data-testid="plugin-page" data-component={component} data-plugin={pluginName}>
|
||||
Plugin: {pluginName}
|
||||
</div>
|
||||
),
|
||||
PluginPage: ({ component, pluginName }: { component: string; pluginName: string }) => {
|
||||
const params = useParams();
|
||||
return (
|
||||
<div
|
||||
data-testid="plugin-page"
|
||||
data-component={component}
|
||||
data-plugin={pluginName}
|
||||
data-params={JSON.stringify(params)}
|
||||
>
|
||||
Plugin: {pluginName}
|
||||
</div>
|
||||
);
|
||||
},
|
||||
}));
|
||||
|
||||
const mockManifests: PluginUiManifest[] = [
|
||||
@@ -121,3 +132,91 @@ describe('PluginRouteRenderer', () => {
|
||||
expect(screen.getByText('Plugin: Calendar')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe('PluginRouteRenderer pattern matching (Paket 5 / Kritikpunkt 21)', () => {
|
||||
// Contacts manifest mirrors the real backend contribution (plugin.py
|
||||
// page_routes): list, :id detail and dedup — the routes Paket 5 wants to
|
||||
// remove from the static router.
|
||||
const contactsManifest: PluginUiManifest = {
|
||||
name: 'contacts',
|
||||
display_name: 'Contacts',
|
||||
version: '1.1.0',
|
||||
is_core: true,
|
||||
menu_items: [],
|
||||
page_routes: [
|
||||
{ path: '/contacts', component: '@/pages/ContactsList', parent: '', protected: true, permission: 'contacts:read', order: 100 },
|
||||
{ path: '/contacts/:id', component: '@/pages/ContactDetailPage', parent: '/contacts', protected: true, permission: 'contacts:read', order: 110 },
|
||||
{ path: '/contacts/dedup', component: '@/pages/DedupMergePage', parent: '/contacts', protected: true, permission: 'contacts:read', order: 120 },
|
||||
],
|
||||
detail_tabs: [],
|
||||
settings_pages: [],
|
||||
dashboard_widgets: [],
|
||||
custom_fields: [],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
usePluginStore.getState().reset();
|
||||
usePluginStore.setState({ manifests: [contactsManifest], loaded: true });
|
||||
// The manifest routes carry permission='contacts:read' → the renderer
|
||||
// wraps them in ProtectedRoute. Grant access as a system admin so the
|
||||
// tests verify matching, not authorization.
|
||||
useAuthStore.setState({
|
||||
isAuthenticated: true,
|
||||
user: {
|
||||
id: '1',
|
||||
email: 'admin@test.de',
|
||||
first_name: 'Admin',
|
||||
last_name: 'Test',
|
||||
role: 'admin',
|
||||
avatar_url: null,
|
||||
is_system_admin: true,
|
||||
permissions: ['contacts:read'],
|
||||
tenants: [{ id: 't1', name: 'T', slug: 't' }],
|
||||
} as never,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
useAuthStore.setState({ isAuthenticated: false, user: null });
|
||||
});
|
||||
|
||||
it('renders the list page for /contacts', () => {
|
||||
render(
|
||||
<MemoryRouter initialEntries={['/contacts']}>
|
||||
<PluginRouteRenderer />
|
||||
</MemoryRouter>
|
||||
);
|
||||
const page = screen.getByTestId('plugin-page');
|
||||
expect(page).toHaveAttribute('data-component', '@/pages/ContactsList');
|
||||
});
|
||||
|
||||
it('renders the DETAIL page for /contacts/:id (not the list via prefix)', () => {
|
||||
render(
|
||||
<MemoryRouter initialEntries={['/contacts/abc-123']}>
|
||||
<PluginRouteRenderer />
|
||||
</MemoryRouter>
|
||||
);
|
||||
const page = screen.getByTestId('plugin-page');
|
||||
expect(page).toHaveAttribute('data-component', '@/pages/ContactDetailPage');
|
||||
});
|
||||
|
||||
it('passes the :id param to the plugin page (useParams works)', () => {
|
||||
render(
|
||||
<MemoryRouter initialEntries={['/contacts/abc-123']}>
|
||||
<PluginRouteRenderer />
|
||||
</MemoryRouter>
|
||||
);
|
||||
const page = screen.getByTestId('plugin-page');
|
||||
expect(page).toHaveAttribute('data-params', '{"id":"abc-123"}');
|
||||
});
|
||||
|
||||
it('renders the DEDUP page for /contacts/dedup (static beats :id)', () => {
|
||||
render(
|
||||
<MemoryRouter initialEntries={['/contacts/dedup']}>
|
||||
<PluginRouteRenderer />
|
||||
</MemoryRouter>
|
||||
);
|
||||
const page = screen.getByTestId('plugin-page');
|
||||
expect(page).toHaveAttribute('data-component', '@/pages/DedupMergePage');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,8 +16,6 @@ import { ErrorBoundary } from '@/components/common/ErrorBoundary';
|
||||
|
||||
// Lazy-loaded pages (code-splitting)
|
||||
const DashboardPage = React.lazy(() => import('@/pages/Dashboard').then(m => ({ default: m.DashboardPage })));
|
||||
const ContactsListPage = React.lazy(() => import('@/pages/ContactsList').then(m => ({ default: m.ContactsListPage })));
|
||||
const ContactDetailPage = React.lazy(() => import('@/pages/ContactDetailPage').then(m => ({ default: m.ContactDetailPage })));
|
||||
const AuditLogPage = React.lazy(() => import('@/pages/AuditLog').then(m => ({ default: m.AuditLogPage })));
|
||||
const GlobalSearchResultsPage = React.lazy(() => import('@/pages/GlobalSearchResults').then(m => ({ default: m.GlobalSearchResultsPage })));
|
||||
const SettingsPage = React.lazy(() => import('@/pages/Settings').then(m => ({ default: m.SettingsPage })));
|
||||
@@ -57,7 +55,6 @@ const ReportsPage = React.lazy(() => import('@/pages/Reports').then(m => ({ defa
|
||||
const TasksPage = React.lazy(() => import('@/pages/Tasks').then(m => ({ default: m.TasksPage })));
|
||||
const CommunicationPage = React.lazy(() => import('@/pages/Communication').then(m => ({ default: m.CommunicationPage })));
|
||||
const WorkflowsPage = React.lazy(() => import('@/pages/Workflows').then(m => ({ default: m.WorkflowsPage })));
|
||||
const DedupMergePage = React.lazy(() => import('@/pages/DedupMerge').then(m => ({ default: m.DedupMergePage })));
|
||||
const ImportExportPage = React.lazy(() => import('@/pages/ImportExport').then(m => ({ default: m.ImportExportPage })));
|
||||
const TagsPage = React.lazy(() => import('@/pages/Tags').then(m => ({ default: m.TagsPage })));
|
||||
const CustomFieldsPage = React.lazy(() => import('@/pages/CustomFields').then(m => ({ default: m.CustomFieldsPage })));
|
||||
@@ -242,8 +239,6 @@ const router = createBrowserRouter([
|
||||
children: [
|
||||
{ path: '/', element: <Navigate to="/start" replace /> },
|
||||
{ path: '/dashboard', element: withSuspense(<DashboardPage />) },
|
||||
{ path: '/contacts', element: <PermissionRoute permission="contacts:read">{withSuspense(<ContactsListPage />)}</PermissionRoute> },
|
||||
{ path: '/contacts/:id', element: <PermissionRoute permission="contacts:read">{withSuspense(<ContactDetailPage />)}</PermissionRoute> },
|
||||
{ path: '/audit-log', element: <PermissionRoute permission="audit:read">{withSuspense(<AuditLogPage />)}</PermissionRoute> },
|
||||
{ path: '/search', element: withSuspense(<GlobalSearchResultsPage />) },
|
||||
{ path: '/calendar', element: <PermissionRoute permission="calendar:read">{withSuspense(<CalendarPage />)}</PermissionRoute> },
|
||||
@@ -257,7 +252,6 @@ const router = createBrowserRouter([
|
||||
{ path: '/tasks', element: <PermissionRoute permission="tasks:read">{withSuspense(<TasksPage />)}</PermissionRoute> },
|
||||
{ path: '/communication', element: <PermissionRoute permission="comm:read">{withSuspense(<CommunicationPage />)}</PermissionRoute> },
|
||||
{ path: '/workflows', element: <PermissionRoute permission="workflows:read">{withSuspense(<WorkflowsPage />)}</PermissionRoute> },
|
||||
{ path: '/contacts/dedup', element: <PermissionRoute permission="contacts:read">{withSuspense(<DedupMergePage />)}</PermissionRoute> },
|
||||
{ path: '/import-export', element: <PermissionRoute permission="import_export:read">{withSuspense(<ImportExportPage />)}</PermissionRoute> },
|
||||
{ path: 'tags', element: <PermissionRoute permission="tags:read">{withSuspense(<TagsPage />)}</PermissionRoute> },
|
||||
{ path: '/api-docs', element: <PermissionRoute permission="settings:read">{withSuspense(<ApiDocsPage />)}</PermissionRoute> },
|
||||
|
||||
Reference in New Issue
Block a user