diff --git a/PROGRESS.md b/PROGRESS.md index 8f6da35..f5d624c 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -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 `` 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 diff --git a/frontend/src/components/plugins/PluginLoader.tsx b/frontend/src/components/plugins/PluginLoader.tsx index f60a9e8..a1d2101 100644 --- a/frontend/src/components/plugins/PluginLoader.tsx +++ b/frontend/src/components/plugins/PluginLoader.tsx @@ -125,6 +125,9 @@ const STATIC_COMPONENT_MAP: Record = { '@/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), diff --git a/frontend/src/components/plugins/PluginRouteRenderer.tsx b/frontend/src/components/plugins/PluginRouteRenderer.tsx index 72b5be9..147154a 100644 --- a/frontend/src/components/plugins/PluginRouteRenderer.tsx +++ b/frontend/src/components/plugins/PluginRouteRenderer.tsx @@ -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 . * - * 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 + * 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 = ( - - ); - - // 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 {page}; - } - 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 ; + } + + return ( + + {entries.map(({ route, pluginName }) => ( + + + + ) : ( + + ) + } + /> + ))} + {/* Unknown paths fall through to not-found */} + } /> + + ); +} + +function PluginRouteNotFound({ pathname }: { pathname: string }) { return (

Page Not Found

- The page {location.pathname} was not found. + The page {pathname} was not found.

); diff --git a/frontend/src/components/plugins/__tests__/PluginRouteRenderer.test.tsx b/frontend/src/components/plugins/__tests__/PluginRouteRenderer.test.tsx index 8bd61d3..9cbc8a4 100644 --- a/frontend/src/components/plugins/__tests__/PluginRouteRenderer.test.tsx +++ b/frontend/src/components/plugins/__tests__/PluginRouteRenderer.test.tsx @@ -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 }) => ( -
- Plugin: {pluginName} -
- ), + PluginPage: ({ component, pluginName }: { component: string; pluginName: string }) => { + const params = useParams(); + return ( +
+ Plugin: {pluginName} +
+ ); + }, })); 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( + + + + ); + 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( + + + + ); + 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( + + + + ); + 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( + + + + ); + const page = screen.getByTestId('plugin-page'); + expect(page).toHaveAttribute('data-component', '@/pages/DedupMergePage'); + }); +}); diff --git a/frontend/src/routes/index.tsx b/frontend/src/routes/index.tsx index 30593bf..7c3f63f 100644 --- a/frontend/src/routes/index.tsx +++ b/frontend/src/routes/index.tsx @@ -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: }, { path: '/dashboard', element: withSuspense() }, - { path: '/contacts', element: {withSuspense()} }, - { path: '/contacts/:id', element: {withSuspense()} }, { path: '/audit-log', element: {withSuspense()} }, { path: '/search', element: withSuspense() }, { path: '/calendar', element: {withSuspense()} }, @@ -257,7 +252,6 @@ const router = createBrowserRouter([ { path: '/tasks', element: {withSuspense()} }, { path: '/communication', element: {withSuspense()} }, { path: '/workflows', element: {withSuspense()} }, - { path: '/contacts/dedup', element: {withSuspense()} }, { path: '/import-export', element: {withSuspense()} }, { path: 'tags', element: {withSuspense()} }, { path: '/api-docs', element: {withSuspense()} },