feat(frontend): render backend-declared plugin metadata - #567
Conversation
|
@claude-review |
|
Claude finished @hamza-56's task in 3m 8s —— View job Review: render backend-declared plugin metadata
Solid, well-scoped refactor. The frontend genuinely stops being a second source of truth for display metadata — Things done well
Suggestions (non-blocking)Medium — Low — unmapped icon names fail silently. Low — test coverage gap on behavioral changes. The two highest-regression-risk changes have no test: dashboard filtering on Notes
No blocking issues. Nice consolidation. |
eb50e7b to
8f4dead
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
7cbe193 to
446c764
Compare
abdulrafey1
left a comment
abdulrafey1
left a comment
There was a problem hiding this comment.
Few small comments:
useEnabledPlugins and the provider's enabledPlugins state are now dead, and they are the frontend-side copy this PR set out to retire. Files: frontend/lib/plugins/context.tsx:27, 146, 318-325
446c764 to
b35b777
Compare
|
Addressed @abdulrafey1's review comment about This comment was written with the assistance of an LLM (Claude). |
The frontend kept its own copy of every plugin's display name, description, icon, and sidebar config, drifting from the backend (#565, item 4 of #372). - PluginDefinition shrinks to what only the frontend can own: the plugin name and the component loaders. Display metadata comes from the user-plugins API (display, sidebar, has_frontend), typed via the generated OpenAPI schema. - Sidebar, dashboard cards, settings list, and plugin pages read the backend-declared metadata; backend-only plugins (canvas, open-edx) now show a proper name and description in settings. - Icons resolve from declared lucide names via a small registry; plugins register custom brand icons (slack) under their name. - google-drive added to generateStaticParams (was missing).
The AppSidebar test came from main mocking useEnabledPlugins, but the sidebar now reads userPlugins via usePluginContext, so the mock left the hook undefined and every case failed to render.
The hand-written interface duplicated UserPluginResponse and had already drifted from it: config was typed Record<string, string> though the backend stores JSONB, and has_frontend was optional though the response always carries it (weakening the dashboard filter). Typing config values as unknown makes the JSONB reality visible at the read sites, so add lib/plugins/config helpers to narrow there instead of asserting: configString for a single text field, stringConfigOf for the generic editor that renders every field as an input.
sidebarItemsFrom filtered on enabled && sidebar, so a backend plugin declaring a SidebarEntry without a frontend app still got a nav link, which 404s under output: export. The registry lookup used to be this guard before the list moved to backend metadata. A type-guard filter also lets TS narrow sidebar as non-null through the sort, removing the unreachable ?? DEFAULT_SIDEBAR_ORDER fallback (order is required in the generated schema).
Before the user-plugins fetch resolves, userPlugins is empty, so isEnabled is false and displayName falls back to the slug. The plugin page and PluginRenderer rendered a 'not enabled'/'disabled' verdict with the slug in it on every load. Render a spinner while the context is loading instead.
useEnabledPlugins and the provider's enabledPlugins state had no consumers left; they were the frontend-side registry projection this branch set out to retire.
The hardcoded list was a third place a plugin name had to be written before its page existed under output: export; the registry already holds exactly the set of renderable plugin pages.
Every other consumer imports displayNameOf from @/lib/plugins; the dashboard page reached into the leaf modules.
…onent An unmapped lucide name silently degraded to the icon-less fallback, invisible to a plugin author who declared the icon backend-side. Outside production the resolver now logs which name is missing and where to register it.
Main centralized frontend unit tests under frontend/tests/ while this branch was in flight; this test was added co-located and the relocation on main could not know about it.
0425dad to
c10dcfd
Compare
generateStaticParams in the plugin page (a server component) now imports from the @/lib/plugins barrel, which re-exports this hooks module; without the directive the static-export build fails.
2 participants
Part of: Plugin identity and display metadata are split across disconnected backend and frontend registries
Second PR of the sequence proposed in #372, stacked on #566 (hooks + API exposure). The drift gate follows in #568.
What
The frontend stops keeping its own copy of plugin display metadata and renders what the backend declares:
PluginDefinitionshrinks to the plugin name plus component loaders, and display name, description, icon, and sidebar config come from the user-plugins API.Changes
PluginDefinitionshrinks to what only the frontend can own (name,loadComponent,loadSettingsComponent);displayName,description,icon,category, and the sidebar fields are removedUserPluginStatecarriesdisplay,sidebar, andhas_frontend, typed from the generated OpenAPI schemalib/plugins/icons.ts; plugins register custom brand icons under their name (slack)generateStaticParams(was missing)How to Test
cd frontend && bun run test(new suites:lib/tests/plugin-metadata.test.ts,lib/tests/plugin-icons.test.ts,components/settings/ListItem.test.tsx)Notes
getSidebarPlugins,getPluginsByCategory,getCorePlugins,searchPlugins,useSidebarPlugins(all unused outside the registry).This PR description was written with the assistance of an LLM (Claude).