feat(integrations): 5 built-in JS integration registrations#210
feat(integrations): 5 built-in JS integration registrations#210rubenvdlinde wants to merge 17 commits into
Conversation
… PR 9 — part 1)
Mirrors OpenRegister's built-in PHP `IntegrationProvider`s on the JS
side. Each descriptor maps a built-in onto its existing
`CnObjectSidebar` tab plus a compact widget for dashboard / detail
surfaces, with `referenceType === id` for the AD-18 crossover.
id order group tab widget
------------ ----- ----- ---------------- ----------------------
files 1 core CnFilesTab CnFilesCard
notes 2 core CnNotesTab CnNotesCard (adapter)
tags 3 core CnTagsTab CnTagsCard
tasks 4 core CnTasksTab CnTasksCard (adapter)
audit-trail 5 core CnAuditTrailTab CnAuditTrailCard
id / order / icon / group match the PHP providers exactly so PHP + JS
stay in lockstep. `CnNotesCard` / `CnTasksCard` predate the registry
and take `registerId` / `schemaId` props, so a thin adapter component
in the builtin module renames `register` / `schema` (the convention
the surface components forward) — no change to those existing cards.
`registerBuiltinIntegrations(registry?)` registers all five onto a
registry (default: the singleton) and is what OpenRegister's main
bundle calls once at bootstrap, after `installIntegrationRegistry()`.
It SKIPS any id already registered — quietly, no collision warning —
so a consuming app can pre-register an id to override a built-in
(collision policy: first wins). It's idempotent and returns the ids
it newly registered.
Implements tasks 7.1–7.6 (the 5 JS registrations + reference-property
declarations) of the umbrella.
Files:
- src/integrations/builtin/{files,notes,tags,tasks,audit-trail}.js
(new) — one descriptor per built-in; notes/tasks include the
prop-rename adapter
- src/integrations/builtin/index.js (new) — `builtinIntegrations`
array + `registerBuiltinIntegrations()`
- src/integrations/index.js + src/index.js — export
`builtinIntegrations` + `registerBuiltinIntegrations`
- docs/utilities/builtin-integrations.md,
register-builtin-integrations.md (new)
- tests/integrations/builtin.spec.js (new) — 9 tests: descriptor
shape (ids, order, icon, group, referenceType, tab+widget
presence), registerBuiltinIntegrations (registers all five,
skip-on-collision keeping the pre-registered one, idempotent,
defaults to singleton)
Tests: 25 integration tests (registry + composable + builtin), all
green. Full suite: 71 suites, 1011 passing, 4 pre-existing failures
(CnMapWidget + CnAiChatPanel, unrelated, also red on origin/beta).
node scripts/check-docs.js — only pre-existing CnAi* / useAi* / default
gaps remain. npm run lint — no new errors.
…AND a widget (umbrella PR 10a)
Adds the AD-11/AD-13 parity gate as a CI + pre-commit check. The
registry already throws at `register()` time when `tab` or `widget`
is missing; this gate catches the same condition statically before
merge for the descriptors shipped in this repo's `src/integrations/`.
- scripts/check-integration-parity.js (new) — imports
`builtinIntegrations` and asserts each descriptor has a string
`id` + `label` and truthy `tab` + `widget`; exits non-zero with a
clear message listing every offender. Falls back to a source scan
of `src/integrations/builtin/*.js` if the module can't be required.
- package.json — `npm run check:integration-parity`
- .github/workflows/code-quality.yml — new "Integration parity gate"
step (after the JSDoc ratchet)
- scripts/precommit-regenerate-partials.sh — runs the gate on
commit when any `src/integrations/` file is staged (placed before
the Cn* SFC early-exit so integrations-only commits aren't skipped)
Implements tasks 12.1, 12.2, 12.4 of the umbrella (12.3 — the hydra
quality-gate hook — and 13.x — the leaf scaffold script — and the
ADR-019 authoring are tracked as separate PRs per the task list).
Verification: `npm run check:integration-parity` → exit 0,
"every registered integration has both a tab and a widget".
Pre-commit smoke-tested. Lint clean (7 pre-existing errors in
useObjectLock.js unchanged). 25 integration tests green.
…f, frontend) The @conduction/nextcloud-vue half of the `integration-xwiki` leaf (openregister issue #1326) — the JS twin of the PHP XwikiProvider. - src/components/CnXwikiTab/ — sidebar "Articles" tab: lists the linked XWiki pages with their full breadcrumb (AD-3), a link form that accepts a full XWiki URL or a `Space.Page` path (AD-2), an unlink button (removes the OR sub-resource pairing only — never deletes the page in XWiki), and a reconnect/unavailable banner when the endpoint answers 503 (AD-23). Emits `linked` / `unlinked`. - src/components/CnXwikiCard/ — surface-aware widget (AD-19): on `detail-page` the linked list plus a TEXT preview of the first page (HTML stripped to plain text, ~500 chars, macros NOT executed — AD-1) plus an "Open in XWiki" link; on `user-dashboard` / `app-dashboard` a compact list; on `single-entity` a title + breadcrumb chip resolved from the `value` prop (for `referenceType: 'xwiki'` properties), with a minimal-chip fallback when the lookup fails. A 503 renders a quiet unavailable state. - src/integrations/builtin/xwiki.js — `xwikiIntegration` descriptor (id `xwiki`, group `external`, requiredApp `openconnector`, referenceType `xwiki`, tab + widget) + `registerXwikiIntegration()`. This is a LEAF, not a built-in: it is not in `builtinIntegrations` and `registerBuiltinIntegrations()` does not register it — OpenRegister's bundle calls `registerXwikiIntegration()` explicitly (skip-on-collision: a consuming app may pre-register `xwiki` to override it). - barrels (`src/components/index.js`, `src/index.js`, `src/integrations/index.js`) export `CnXwikiTab`, `CnXwikiCard`, `xwikiIntegration`, `registerXwikiIntegration` - docs: `docs/components/cn-xwiki-tab.md`, `cn-xwiki-card.md`, `docs/utilities/xwiki-integration.md`, `register-xwiki-integration.md` - tests: `tests/components/CnXwikiTab.spec.js` (7 — list+breadcrumb, empty, link-by-URL POST, unlink DELETE on the OR sub-resource, 503→reconnect banner, 503→generic banner), `CnXwikiCard.spec.js` (8 — compact list, maxDisplay cap, detail-page text preview with HTML stripped + no macro execution, ~500-char truncation, single-entity chip from `value`, fallback chip, 503 unavailable, empty), `tests/integrations/xwiki.spec.js` (6 — descriptor shape, registerXwikiIntegration registers + skip-on-collision + not part of the built-in set + defaults to singleton) Implements the frontend tasks of the integration-xwiki leaf. Tests: 21 new (CnXwikiTab + CnXwikiCard + xwiki registration), all green. Full suite: 74 suites, 1032 passing, 4 pre-existing failures (CnMapWidget + CnAiChatPanel, unrelated, also red on origin/beta). npm run lint — no new errors/warnings on the xWiki files. check-docs — only pre-existing CnAi*/useAi*/default gaps. Note: the auto-generated `docs/components/_generated/CnXwikiTab.md` and `CnXwikiCard.md` partials still need regenerating (`cd docusaurus && npm run prebuild:docs`) — docusaurus deps weren't installed in this environment; same situation as CnFilesCard/CnTagsCard/ CnAuditTrailCard from #204.
…als for CnXwikiTab/CnXwikiCard
`check:jsdoc` requires new components to score 100%. Both CnXwikiTab
and CnXwikiCard had props grouped under a single `/** Pre-translated
labels. */` comment (only the first prop in each group counted) and
CnXwikiTab's `linked` / `unlinked` events were undocumented.
- CnXwikiTab: each label prop now has its own JSDoc; `linked` and
`unlinked` get a `/** @event … */` immediately before their
`this.$emit(…)` call (the placement vue-docgen-api attaches event
descriptions to)
- CnXwikiCard: `noPagesLabel` / `openInXwikiLabel` /
`unavailableLabel` each get their own JSDoc
- generated `docs/components/_generated/{CnXwikiTab,CnXwikiCard}.md`
via `cd docusaurus && npm run prebuild:docs`
- `scripts/.jsdoc-baselines.json` records both at 100%
(`npm run jsdoc-baselines:update`)
(This commit also carries forward — via the merge from
feature/integration-builtins — the JSDoc/parser fixes already landed
on the parent branches: the parser-safe `@type {object|null}` on the
PR-8 surface-component props, the `show-all` event docs on
CnFilesCard/CnAuditTrailCard, and their regenerated partials.)
Verification: `npm run check:jsdoc` → "All 79 components meet their
JSDoc baseline" (exit 0); `npx jest tests/components/CnXwiki*.spec.js
tests/integrations/xwiki.spec.js` → 21 tests green; `npm run lint` —
clean on the xWiki files.
…th (/integrations/xwiki), not /xwiki
`CnXwikiTab` and `CnXwikiCard` were hitting
`/api/objects/{r}/{s}/{id}/xwiki`, but the umbrella's registry-driven
sub-resource (handled by `ObjectIntegrationsController`) lives at
`/api/objects/{r}/{s}/{id}/integrations/{integrationId}` — i.e.
`.../integrations/xwiki`. (The `{integrationId}` is namespaced under
`/integrations/` so it can't collide with the legacy
`/objects/{r}/{s}/{id}/{type}` sub-resources for files/notes/etc.)
The old path would have 404'd against a live deployment.
- `CnXwikiTab.baseUrl()` / `CnXwikiCard.baseUrl()` →
`${apiBase}/objects/${register}/${schema}/${objectId}/integrations/xwiki`
- updated the component header comments to reference
`ObjectIntegrationsController` + the correct path
- updated the two URL assertions in `CnXwikiTab.spec.js` /
`CnXwikiCard.spec.js` accordingly
Verification: `npx jest tests/components/CnXwiki*.spec.js
tests/integrations/xwiki.spec.js` → 21 green; `npm run lint` clean
on the xWiki files; `npm run check:jsdoc` → "All 79 components meet
their JSDoc baseline".
…try / excludeIntegrations (manifest-driven registry sidebar)
Closes the gap found while live-testing the xWiki leaf: a consuming
app whose detail sidebar is manifest-driven (CnDetailPage via
CnAppRoot's #sidebar slot) couldn't flip that sidebar to registry
mode — `useRegistry` was a `CnObjectSidebar` prop only, with no path
from `pages[].config.sidebar`.
- CnDetailPage: the `sidebar` Object form (and `sidebarProps`) now
accept `useRegistry: boolean` and `excludeIntegrations: string[]`;
`mergeSidebarSources()` merges them and `syncSidebarState()`
pushes them into the injected `objectSidebarState`
(`objectSidebarState.useRegistry` / `.excludeIntegrations`), reset
when the page is suppressed. The `sidebar` prop JSDoc documents
the new keys + the mutual exclusion with `tabs` + the host-app
binding requirement.
- app-manifest.schema.json + validateManifest.js: `config.sidebar`
on detail pages may now carry `useRegistry` (boolean) and
`excludeIntegrations` (string[]); validation rejects a non-boolean
`useRegistry`, non-array / non-string `excludeIntegrations`, and
`useRegistry: true` together with `tabs` (mutually exclusive). The
schema descriptions are updated to mention both.
- docs: cn-object-sidebar.md gets a "Registry-driven tabs
(useRegistry)" section including the `#sidebar`-slot binding
snippet and the manifest example; cn-detail-page.md's sidebar
config object section mentions `useRegistry` / `excludeIntegrations`;
CLAUDE.md's "Surface components consume the registry" list grows a
CnDetailPage bullet; CnDetailPage's `_generated` partial regenerated.
- tests: CnDetailPageSidebarConfig.spec.js — 4 new tests (Object
form pushes useRegistry/excludeIntegrations into objectSidebarState;
defaults; cleared on suppression; sidebarProps fallback);
app-manifest.schema.spec.js — 4 new tests (accepts both keys;
rejects non-boolean useRegistry; rejects useRegistry+tabs; rejects
bad excludeIntegrations).
The consuming app still has to bind `:use-registry` /
`:exclude-integrations` from `objectSidebarState` onto its
`#sidebar`-slot `<CnObjectSidebar>` (documented) — that's the one
non-automatic step, the same shape as the existing `:tabs` /
`:hidden-tabs` binding it already does.
Verification: `npx jest` → 72 suites, 1040 passing (4 pre-existing
failures: CnMapWidget + CnAiChatPanel, unrelated); `npm run lint` —
no new errors; `npm run check:jsdoc` → "All 79 components meet their
JSDoc baseline".
…tionsController shape) The AD-23 failure-path contract on OpenRegister's ObjectIntegrationsController surfaces the ProviderUnavailableException cause under `details.cause`, but CnXwikiTab.degradedMessage() only checked `details.reason` / a bare `reason` — so a real "openconnector-source-missing" 503 fell through to the generic "unavailable" banner instead of the more actionable "reconnect connector" one. Accept `details.cause` (preferred) plus the legacy shapes, and treat `openconnector-down` as a reconnect cause too. Tests + the SFC header updated.
…IntegrationsController shape)
OpenRegister's `ObjectIntegrationsController::index` returns `{ items: [...] }`,
but the xwiki tab/card only un-wrapped `{ results: [...] }` (or a bare array) —
so against the real OR endpoint `this.pages` ended up being the envelope object,
`v-for` iterated its values, and the linked-page row rendered blank. Accept
`results` ?? `items` ?? bare-array for both components (list fetch and the
single-entity `loadEntity` lookup). +1 test for the `{ items }` envelope.
There was a problem hiding this comment.
[BLOCKER] Hardcoded apiBase default '/apps/openregister/api' in CnNotesCardAdapter
The apiBase prop default is hardcoded to '/apps/openregister/api'. This breaks any deployment where the app is installed under a sub-path or uses a different route prefix. The value should either be injected by the caller (no default, or '') and documented as required, or derived at runtime via generateUrl() from @nextcloud/router. The same issue exists in tasks.js.
There was a problem hiding this comment.
[BLOCKER] Hardcoded apiBase default '/apps/openregister/api' in CnTasksCardAdapter
Same issue as notes.js: apiBase defaults to '/apps/openregister/api'. Fix: remove the default or use generateUrl('openregister', 'api') from @nextcloud/router so the path is resolved against the actual Nextcloud installation base URL.
There was a problem hiding this comment.
[CONCERN] register() result checked against null instead of truthiness — undefined slips through
if (result !== null) will push the descriptor id if register() returns undefined (e.g. on a validation failure that returns void rather than null). Use if (result) or if (result != null) to cover both null and undefined, or add a comment citing the spec that guarantees null-on-failure.
There was a problem hiding this comment.
[CONCERN] objectId prop typed [String, Number] but adapter always casts to String — misleading
The objectId prop is typed [String, Number] but the render function unconditionally calls String(this.objectId). The Number type is misleading — it implies Number is a valid passthrough, but it gets coerced. Either accept only String (and document that callers must stringify) or pass the raw value and let CnNotesCard handle coercion. The same pattern exists in CnTasksCardAdapter.
There was a problem hiding this comment.
[CONCERN] objectId prop typed [String, Number] but adapter always casts to String (tasks)
Same as the notes adapter: objectId: { type: [String, Number], default: '' } but the render always does String(this.objectId). Align the declared type with the actual behaviour.
There was a problem hiding this comment.
[CONCERN] Individual descriptor exports widen the public API surface unintentionally
filesIntegration, notesIntegration, tagsIntegration, tasksIntegration, and auditTrailIntegration are re-exported from the barrel. The docs only describe builtinIntegrations (the array) and registerBuiltinIntegrations (the function) as public API. Exporting the five individual descriptors locks the library into maintaining each object's identity and shape as semver-breaking changes forever. If intentionally public, document them. If not, remove them from the barrel.
There was a problem hiding this comment.
[CONCERN] surface and objectType props silently dropped by adapter — no pass-through or warning
Both CnNotesCardAdapter and CnTasksCardAdapter declare surface and objectType as accepted props but never forward them to the wrapped card. If a future version gains surface support, the adapter will silently swallow the value. Consider forwarding all $attrs via v-bind or adding a dev-mode warning. CnTasksCardAdapter also lacks the inline comment present in CnNotesCardAdapter noting these are ignored.
WilcoLouwerse
left a comment
There was a problem hiding this comment.
Review
🔴 Blockers (2)
- Hardcoded apiBase default '/apps/openregister/api' in CnNotesCardAdapter —
src/integrations/builtin/notes.js:283 - Hardcoded apiBase default '/apps/openregister/api' in CnTasksCardAdapter —
src/integrations/builtin/tasks.js:393
🟡 Concerns (5)
- register() result checked against null instead of truthiness — undefined slips through —
src/integrations/builtin/index.js:236 - objectId prop typed [String, Number] but adapter always casts to String — misleading —
src/integrations/builtin/notes.js:280 - objectId prop typed [String, Number] but adapter always casts to String (tasks) —
src/integrations/builtin/tasks.js:390 - Individual descriptor exports widen the public API surface unintentionally —
src/integrations/index.js:437 - surface and objectType props silently dropped by adapter — no pass-through or warning —
src/integrations/builtin/notes.js:281
🟢 Minor (3)
- Doc table omits defaultSize column despite mentioning it in prose (
docs/utilities/builtin-integrations.md:25)
The descriptor shape description listsdefaultSizeas part of the contract, but the summary table only hasid | order | group | tab | widget. Adding adefaultSizecolumn would help consumers who read it for layout purposes —tagsuses{ w: 4, h: 2 }while all others use{ w: 4, h: 3 }. - Test file mixes require() (CJS) with ESM source — may mask transform issues (
tests/integrations/builtin.spec.js:464)
The test usesrequire()while the source files use ES moduleexportsyntax. If the Jest/Babel transform is misconfigured, tests silently pass against the wrong module version. Preferimportstatements for consistency with the source. - Docs don't explain why collision warning is suppressed for builtin registrations (
docs/utilities/register-builtin-integrations.md:52)
The doc states builtins skip any already-registered id quietly without the dev-mode collision warning, but doesn't explain why. A one-sentence rationale (to avoid noise for the common bootstrap pattern) would prevent future maintainers from "fixing" the intentionalhas()guard.
Reviewed by WilcoLouwerse via automated batch review.
…orm accepts useRegistry / excludeIntegrations (manifest-driven registry sidebar)' (#218) from feature/integration-sidebar-manifest into feature/integration-xwiki automerge: nextcloud-vue#218
… + xwiki registration (leaf, frontend)' (#213) from feature/integration-xwiki into feature/integration-builtins automerge: nextcloud-vue#213
… must declare a tab AND a widget' (#211) from feature/integration-ci into feature/integration-builtins automerge: nextcloud-vue#211
Part of the openregister pluggable-integration-registry umbrella. Cross-repo PR 9/N. Stacked on #209 → #204 → #202 →
beta; GitHub auto-retargets as upstream merges.Summary
Mirrors OpenRegister's five built-in PHP
IntegrationProviders on the JS side, soCnObjectSidebar(registry mode),CnDashboardPage, andCnDetailPageactually have something to render. Implements tasks 7.1–7.6 of the umbrella; the frontend-test tasks (10.1–10.4) are covered cumulatively across PRs #202 / #204 / #209 / this one (registry unit tests, widget tests, surface-component tests, built-in registration tests).filescoreCnFilesTabCnFilesCardnotescoreCnNotesTabCnNotesCard(adapter)tagscoreCnTagsTabCnTagsCardtaskscoreCnTasksTabCnTasksCard(adapter)audit-trailcoreCnAuditTrailTabCnAuditTrailCardid/order/icon/groupmatch the PHP providers exactly so PHP + JS stay in lockstep. Each declaresreferenceType === idfor the AD-18 crossover.Adapter note:
CnNotesCard/CnTasksCardpredate the registry and takeregisterId/schemaIdprops, whereas the surface components forward the OpenRegister-conventionregister/schema. Thenotes/tasksbuiltins wrap their card in a tiny render-fn adapter that renames the props — those existing components are untouched.registerBuiltinIntegrations(registry?)What OpenRegister's main bundle calls once at bootstrap (after
installIntegrationRegistry()). Skips any id already registered — quietly, no collision warning — so a consuming app can pre-register an id to override a built-in (collision policy: first wins). Idempotent; returns the ids it newly registered.Files
src/integrations/builtin/{files,notes,tags,tasks,audit-trail}.js(new)src/integrations/builtin/index.js(new) —builtinIntegrationsarray +registerBuiltinIntegrations()src/integrations/index.js+src/index.js— public exportsdocs/utilities/builtin-integrations.md,register-builtin-integrations.md(new)tests/integrations/builtin.spec.js(new) — 9 testsTest plan
npx jest tests/integrations→ 25 tests (registry + composable + builtin), greenCnMapWidget+CnAiChatPanel, unrelated, also red onorigin/beta)node scripts/check-docs.js— only pre-existing CnAi* / useAi* /defaultgaps remainnpm run lint— no new errorsinstallIntegrationRegistry()+registerBuiltinIntegrations()and the 5 built-ins should appear inCnObjectSidebar(useRegistry) and be available astype: 'integration'widgets