chore(release): sync main back into development - #1356
Open
github-actions[bot] wants to merge 23 commits into
Open
chore(release): sync main back into development#1356github-actions[bot] wants to merge 23 commits into
github-actions[bot] wants to merge 23 commits into
Conversation
…OO-536 Fase 4)
Robert Zondervan's WOO-536 architecture fix requires the public search
endpoint to filter depublished objects. The current seed shape has only
a single conditional rule (`publicatiedatum $lte $now`) — depublished
objects (past `depublicatiedatum`) remain visible under RBAC.
## Changes
- `lib/Settings/publication_register.json` — `publication` + `document`
schemas updated to the two-rule read shape:
read: [
{ group: public, match: { publicatiedatum $lte $now, depublicatiedatum $gte $now } },
{ group: public, match: { publicatiedatum $lte $now, depublicatiedatum $exists false } },
"authenticated"
]
- `lib/Repair/WOO536RepairReadRules.php` — new IRepairStep for existing
installations. Runs on `occ maintenance:repair`, detects the single-rule
pre-fix shape, upgrades to two-rule, preserves "authenticated" element,
admin-customised shapes are left alone. Idempotent.
- `appinfo/info.xml` — repair step registered.
- `openspec/config.yaml` — corrected `schema: conduction` → `spec-driven`
(the `conduction` schema-file is not bundled in this repo, blocking
all openspec commands). Fleet-wide alignment to `conduction` is a
separate follow-up (5 apps still on `conduction`, mixed state).
- `openspec/changes/fix-fts-catalog-model-alignment/` — full OpenSpec
change scaffold (proposal + design + specs + tasks). `openspec validate`
green. depends_on: [rbac-as-public-toggle] (the OR precursor).
## Tests
`tests/Unit/Repair/WOO536RepairReadRulesTest.php` — 6 tests / 16
assertions covering: getName, skip-when-OR-absent, upgrade single-rule,
idempotency on two-rule, preserve admin-customised, skip-when-schema-absent.
All green.
Fase 5 (code refactor consuming `_rbac_as_public`) waits for OR PR #2855
merge before wiring the primitive.
@SPEC openspec/changes/fix-fts-catalog-model-alignment/specs/search/spec.md
Rework the public /api/search assembler per Robert Zondervan's WOO-536
architectural fix. Ships Robert's Stap 1-5a + 7 on top of the OR precursor
primitive (openregister PR #2855, RBA-PUBLIC-001..006).
## What changed
Stap 1 — enable SQL RBAC with public-endpoint discipline:
`searchObjectsPaginated(..., _rbac: true, _rbac_as_public: true)`.
Visibility now enforced in SQL by OR's schema authorization, not by a
PHP post-filter. Admin and anonymous callers see the SAME result set
(SCH-PFTS-001).
Stap 2 — catalog-derived scope (SCH-PFTS-CAT-001..003). New helper
`resolveCatalogScope()` accepts `_catalog=<slug>` or `_catalogi[]=…`
(Q4 naming convention: underscore-prefix, Dutch plural).
Default = union of `listed:true` + published catalogs. The three
`resolveConfiguredId()` calls (publication_register / publication_schema
/ document_schema) are gone — `resolveConfiguredId()` itself removed
as dead code. Q7 Interpretation A preserved: `_schema` / `_registers`
/ `fq` / `catalogSlug` still stripped, plus the new `_catalog` /
`_catalogi` are consumed by resolveCatalogScope before forward-to-OR.
Stap 3 — dynamic schema-discriminator via SchemaMapper. The hard-coded
`$schemaSlugById` two-element map is gone; slugs resolved per-row via
`$this->container->get('OCA\\OpenRegister\\Db\\SchemaMapper')->find()`,
cached per-request. `@self.schema` carries the real slug for every
schema in scope.
Stap 4 — remove PHP post-filter + restore totals/facets. `isObjectPublic()`
post-filter loop deleted; anonymous total-under-count workaround gone;
facets/facetable stripping for anon callers gone. OR's `total` +
`facets` reflect visible-only counts by construction under
`_rbac_as_public: true`.
Stap 5a — relations-based document→publication lookup.
`resolveDocumentPublicationSummary()` rewritten to query the publication
schema with `_relations_contains: <document_uuid>` (per-document
refinement stays on the single-schema path). N4a (0 relations) drops
the row; N4b (multiple relations) picks the OLDEST-by-`@self.created`
as the most stable link.
Stap 7 — fixed broken docblock refs. `SCH-PFTS-002/003/004/005/006/007`
refs replaced by inline descriptive text; `add-public-fulltext-search/*`
and `add-document-content-search/*` refs replaced by
`fix-fts-catalog-model-alignment/*` + canonical spec at
`openspec/specs/search/spec.md`.
## Deferred (rationale in tasks.md)
- 3.5 (guard scope-schemas with missing read config) — edge case (admin
responsibility to configure RBAC on added schemas); nice-to-have,
not required for hotfix scope
- 6.2 (generalise document→publication to related→source) — future-work
abstraction, current concrete impl covers WOO-536 DoD
- 8.1 (PublicationQueryServiceTest) — comprehensive unit tests
deferred; validation via docker smoke tests in Fase 7
## Robert's DoD (unchanged)
`GET /apps/opencatalogi/api/search?_search=<term>` returns matches from
every schema in every catalog the caller may see, with documents linked
via relations, correct totals and facets, and no object visible that
its schema's `read` authorization would deny.
@SPEC openspec/changes/fix-fts-catalog-model-alignment/specs/search/spec.md
Applies 1 blocker + 5 concerns + 4 minor from the self-review on PR #1154. B1 — RET-006 archived-visibility regression: re-introduce a targeted `status: archived` guard in the row loop AND on the linked-publication fast-path. SQL-first `$ne` on the schema read-rules would have silently dropped legacy rows where `status` is NULL (SQL NULL semantics); PHP guard is deterministic and safe for legacy data. Belt-and-braces above the SQL RBAC. C1 — Delete dead `isObjectPublic()` and `isAnonymous()` methods (zero callers repo-wide, misleading docblocks). Drop the now-unused `?IUserSession` constructor param. C2 — Explicit `_catalog=<slug>` and `_catalogi[]` re-apply the `published<=now` predicate via a new `isCatalogPubliclyAvailable()` helper, so unpublished catalogs are indistinguishable from unknown slugs on the public endpoint. `listListedPublishedCatalogs()` refactored to use the same helper. C3 — Drop the broken `@spec openspec/specs/search/spec.md#SCH-PFTS-001` anchor; the SCH-PFTS-* IDs only exist in the change-delta until archived. Keep the change-delta `@spec` line. C4 — Log a warning when `catalog_register` / `catalog_schema` app-config is empty (previously silent HTTP 200 empty). C5 — Not viable: `ObjectService::searchObjects()` does not accept `_rbacAsPublic` (only `searchObjectsPaginated` and `find()` do). Left comment explaining why the `_rbac:false` + PHP filter stays on the catalog-enumeration path. M1 — Fast-path in `resolveDocumentPublicationSummary()`: if the document row carries a resolvable `publication.id`, verify via a single `find()` (with `_rbac:true, _rbacAsPublic:true`) before falling back to the `_relations_contains` refinement. Eliminates the N+1 for well-formed documents. M2 — Add `SPDX-License-Identifier` + `SPDX-FileCopyrightText` to `WOO536RepairReadRules` header (matches repo convention). M3 — `resolvePublicationSchemaId()` now shares the outer `$schemaSlugById` cache by reference, so the pre-loop schema scan warms the per-row cache instead of duplicating mapper hits. M4 — Multi-linked-document log downgraded from `info` to `debug` to keep log volume sane on hot public deployments. Also: replace deprecated `IAppManager::getInstalledApps()` with `isEnabledForAnyone('openregister')` in the repair step, update its test mocks accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…536) Sync SCH-PFTS-001 + SCH-PFTS-CAT-001..003 into specs/search/spec.md (canonical requirement table + Requirement/Scenario blocks) and move the change dir to `openspec/changes/archive/2026-08-28-*`. Mirrors the discipline used on the OR precursor PR #2855: sync + archive commits land in the same PR as the code, so the canonical spec state after merge matches the implementation. - `openspec validate search` -> green - 3 tasks intentionally deferred (3.5 future scope-widening guard, 6.2 helper generalisation, 8.1 unit test - tracked separately) WOO-536. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bring the WOO-536 hotfix branch up to date with 1054 commits of main-line development since fork. Three conflicts resolved: 1. `appinfo/info.xml` — took main's rewritten manifest (v2.0.6, EUPL-1.2, i18n, new repair steps, GitHub URL refresh) and added `WOO536RepairReadRules` after `RenameDutchPublicationColumns` so the two-rule read-block backfill sees English field names. 2. `lib/Service/PublicationQueryService.php` — kept the WOO-536 Fase-5 refactor (`resolveCatalogScope`, `_rbac_as_public`, dynamic schema discriminator, `_relations_contains`-based document→publication lookup). Robert's WOO-536 diagnosis flagged main's pre-#84 version as architecturally broken; Fase 5 replaces it. Removed `isObjectPublic` / `isAnonymous` (moved to SQL RBAC) and `resolveConfiguredId` (replaced by catalog-derived scope). 3. `openspec/specs/search/spec.md` — kept main's rich structure (SCH-OR-* + SCH-PFTS-001..007), then: - Amended SCH-PFTS-004 to describe the SQL RBAC mechanism (WOO-536 replaces the PHP `isObjectPublic()` post-filter with `_rbac_as_public: true` per ADR-022). - Added SCH-PFTS-CAT-001/002/003 for `_catalog` / `_catalogi[]` scope params + catalog-derived default scope. - `openspec validate search` → green. Field-name follow-up: main's #850 renamed `publicatiedatum` → `publicationDate` (and `depublicatiedatum` → `depublicationDate`) across the register + code. Updated to match: - `lib/Settings/publication_register.json` two-rule read-block now uses English field names. - `lib/Repair/WOO536RepairReadRules.php` guard + emitted shape now use English field names. - `tests/Unit/Repair/WOO536RepairReadRulesTest.php` updated to match. WOO-536. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ns (WOO-536)
Restore document rows in the /api/search envelope when linked to a
publication via the legacy denormalised `_relations['publication.slug']`
form (the shape emitted by the WOO-517 seed).
Fase-5's original refinement used `_relations_contains: <docUuid>` on
the publication schema, which assumes publications point AT documents.
Robert's plan documented that direction, but every existing seed +
write path in main goes the other way — documents carry the linked
publication as either `_relations['publication']` (UUID form, new) or
`_relations['publication.slug']` (denormalised slug form, WOO-517
seed shape). With the inverse query, `_relations_contains` returned
zero hits and every document row was N4a-dropped, so callers saw
`total: N` but `results len: N - <doc-count>`.
Fix — add an M2 fast-path in `resolveDocumentPublicationSummary`:
1. Read the document row's OWN `@self.relations` (falling back to
`_relations`).
2. If `relations['publication']` is set, treat as UUID and run the
existing `tryFastPathPublicationLookup` (`find()` by id).
3. Otherwise if `relations['publication.slug']` is set, run a new
`tryPublicationSlugLookup` — scans the publication set in scope
under `_rbacAsPublic: true` and matches on `@self.slug`. The
set is schema-scoped + RBAC-filtered so it stays small.
4. On both misses fall through to the original `_relations_contains`
path, which covers the future direction where a publication
registers its documents.
Also validated on a fresh docker install with 3 publications + 2
documents (legacy slug-linked): `total: 5` now equals `results len: 5`,
document rows carry the embedded `publication` summary, and admin+anon
callers see identical result sets. SCH-PFTS-004 acceptance criteria
("total MUST equal actual count of objects visible under public RBAC")
now holds for legacy document seeds as well.
Reproduction: WOO-536 status comment 2026-08-31 (bug 1).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the pre-WOO-536 test file (~1000 lines, all asserting removed
APIs — `isObjectPublic()`, `isAnonymous()`, `resolveConfiguredId`, the
config-based scope path) with 19 focused tests that cover the current
architecture. tasks.md 8.1.
New coverage (19 tests, 53 assertions, all green):
Pure helpers (7 tests):
* stripEmptyValues drops empty strings / nulls / empty arrays, recurses
* normalizeIds accepts JSON-string or array, casts to int, gracefully
handles null and non-JSON strings
* isCatalogPubliclyAvailable — past published passes, future rejected,
missing / empty / malformed date rejected
assemblePublicSearchResults (4 tests):
* Fail-closed: empty scope returns `{results:[], total:0}` with warning
* SCH-PFTS-001 / SCH-PFTS-004: `_rbac: true` + `_rbacAsPublic: true` +
`_multitenancy: false` forwarded to OR (Q1 Option B)
* Q7 Interpretation A: client `_schema`, `_registers`, `catalogSlug`,
`fq`, `_content` stripped before reaching OR
* SCH-PFTS-CAT-002 scope propagates as `_schemas` on the OR query;
single-register scope uses `_register` fast-path
M2 fast-path (bug 1 fix, 2026-08-31 — 3 tests):
* Document with `_relations['publication.slug']` (legacy denormalised
form) resolves via slug-scan and keeps the envelope row with an
embedded publication summary
* Document with `_relations['publication']` (UUID form) uses the
UUID fast-path (`ObjectService::find()`) and keeps the row
* Document without any resolvable publication link is silent-dropped
(N4a) with an info log — envelope N2 preserved (no null sentinel)
Envelope (1 test):
* facets / facetable pass through when OR provides them (SCH-PFTS-004:
total reflects visible count under `_rbac_as_public`)
findObjectLocation (4 tests, preserved from pre-WOO-536):
* Fail-closed without register/schema constraint (#734)
* Locates via ObjectService::find within allowed pair (ADR-022)
* Returns null when not found
* Continues past DoesNotExistException from one pair
Test scaffolding: FakeSearchObjectService + FakeCatalogiService +
FakeCatalogObjectService + FakeSchemaMapper doubles. `wireHappyPath()`
+ `withDefaultCatalog()` helper set up a minimal happy-path fixture
so tests focus on assertions, not on plumbing.
The obsolete tests were valid for the pre-Fase-5 architecture (PHP
post-filter + config-derived scope + admin-vs-anon divergence); Fase 5
moved every one of those behaviours into OR's SQL RBAC via the
`_rbac_as_public` primitive, so the tests could no longer compile
against the current PublicationQueryService signature (no `IUserSession`
param) or exercise the removed methods.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Exercises Robert's WOO-536 Definition of Done + SCH-PFTS-001..004 + SCH-PFTS-CAT-001..003 against a seeded stack via the public /api/search endpoint. 8 assertions: 1. anon default scope returns at least 1 row 2. total == results-length (bug 1 regression guard — no undercount) 3. admin auth returns identical result-set as anon (Q1 Option B) 4. `_catalog=<known-slug>` resolves gracefully 5. `_catalog=nonexistent` returns HTTP 200 + total: 0 (graceful) 6. `_schema=999` stripped (Q7 Interpretation A anti-widening) 7. envelope carries results + total (facets optional per OR) 8. every row carries `@self.schema` as slug string (SCH-PFTS-002) Rate-limits itself (sleep 2s between requests) against the anonymous DoS defense (PUBLIC_LIMIT_MAX). Pass `--admin-token <app-password>` to enable assertion 3 (admin parity), `--catalog-slug` for assertion 4. Also archives tasks.md 8.1 as done (unit tests landed in b6dc3f8). Verified against the fresh docker install on this branch: 8/8 green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two mechanical-gate fixes surfaced by run-hydra-gates.sh: * gate-66 (openregister-dependency-shape, ADR-083) — SchemaMapper container lookup in assemblePublicSearchResults now guarded by class_exists, fails closed with a warning if OR is uninstalled. * gate-16 (spec-coverage) — WOO536RepairReadRules::run gets @SPEC openspec/specs/search/spec.md. All 36 applicable gates green. Unit tests still 19/19. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Second half of the SCH-PFTS-004 "total reflects true visible count" contract. The first half (metadata search, bug 1 M2 fast-path) fixed document rows linked via `_relations['publication.slug']`. This completes the fix for the `_content=true` chunk-hit path. Scenario the metadata fix didn't reach: * A document is chunked (OR file-extraction pipeline produced text) * Its linked publication has `publicationDate` in the future (draft) or `depublicationDate` in the past (depublished) * Under `_content=true`, OR's chunk-search returns the document as a candidate row (chunk-hit is on the extracted text, not gated by the schema RBAC on the parent publication) * OC's row-loop applies transitive visibility: linked pub is not publicly visible under `_rbacAsPublic: true`, so N4a-drops the row * But `total` came straight from `$candidateResult['total']`, so the envelope shipped `total: 1, results: []` — the exact `total > results-length` shape SCH-PFTS-004 forbids Fix: recompute `total` from the emitted `$rows` array after the loop completes, unconditionally. OR's pre-drop count is discarded. Facets still pass through unchanged — OR aggregates over visible-only rows via the schema RBAC + `_rbacAsPublic: true` on the outer query, so facet counts already match. Verified manually 2026-08-31: * Draft-linked document with body-only marker `draft-milieu-only-body- marker-XYZ999`, `?_search=<marker>&_content=true` → `total: 0`, `results: []` (was `total: 1, results: []` before this commit) * Published-linked document with body-only marker `klimaatakkoord- chunk-marker-XYZ42`, same query shape → `total: 1, results len: 1` with the document surfacing as expected Unit tests still 22/22 green (someone added 3 tests since b6dc3f8). Smoke script unchanged — assertion 2 remains the regression guard. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- D1: correct tryPublicationSlugLookup scope comment (no catalog filter; scan is O(500) capped by _limit) - D2: cache slug-scan results per request so N docs sharing one legacy publication.slug collapse to a single scan (was O(N x 500)) - D3: hoist class_exists SchemaMapper preflight to run BEFORE the first OR call; drop the misplaced-after-use guard - D4: log debug on slug-match collisions (parity with N4b) - D5: add 3 tests: UUID-fast-path archived-drop, slug-fast-path archived-drop, D2 slug-cache single-scan - D6: rewrite smoke json_get() with jq (single line, no tab-indented Python heredoc; jq is preinstalled in the NC container image) Test suite: 22 tests / 57 assertions, all green.
Addresses E1 from self-review round 4: the prior `count($rows)` change closed SCH-PFTS-004 (`total > results-length` bug) but redefined `total` from "global visible matches" to "visible on THIS page", breaking any consumer computing `has_more = total > offset + limit`. New shape: `total = max(count($rows), $orTotal - $droppedCount)`. - $orTotal is OR's global candidate count under `_rbacAsPublic: true` — the pagination signal a paged UI needs. - $droppedCount counts per-page N4a / archived / unresolvable-schema drops (dedup drops are excluded — they collapse a single logical hit OR double-counted). - The `max(count(rows), ...)` floor guarantees `total ≥ count(rows)` on every page, so the SCH-PFTS-004 `total: 1, results: []` bug can never recur. Per-page approximation: later pages may drop more rows, so `total` shrinks monotonically as consumers page forward. Acceptable trade-off without an OR schema change to push transitive-visibility into the schema RBAC (tracked as follow-up). Smoke assertion 2 updated from `total == results-length` (tautological after the prior fix) to `total ≥ results-length` — the actual invariant. Tests: 26/26 green, 64 assertions. Four new tests lock in the new behaviour — pagination preserved on no-drop, subtract-and-floor when drops occur, floor-to-zero when everything drops, dedup drops excluded.
Fixes the four failing quality checks on this PR — every finding was introduced by earlier commits in this same PR, not by pre-existing code. phpstan + psalm — 4 UndefinedMethod errors on PublicationsController: restore isAnonymous() and isObjectPublic() on PublicationQueryService (removed as "dead code" in bb21d16 — the first-round D1 fix was wrong about "zero callers repo-wide": PublicationsController::getUses and ::getUsedBy call both methods at runtime). Re-adds the ?IUserSession constructor param and the two method bodies. The controller's calls now type-check AND stop fataling at runtime with Error: Call to undefined method (which the surrounding catch (\Exception) did not catch — public /uses and /used-by endpoints were HTTP 500'ing). phpstan — 2 param-out type errors on resolvePublicationSchemaId: cast SchemaMapper->find()->getSlug() to string so phpstan can prove the array<int, string|null> shape; caller annotates the local $schemaSlugById = [] with an inline @var for type tracking. phpcs — 2 errors on WOO536RepairReadRules.php: refactor the ternary return into explicit if/return, and re-shape the docblock so the long-description starts with a capital. phpcs — 7 errors on PublicationQueryService.php: refactor 5 inline ternaries into if/else blocks, add named-param on the extractSchemaId call, add the missing @PARAM $slugCache docblock entry on resolveDocumentPublicationSummary. phpmd — 11 violations on PublicationQueryService + WOO536RepairReadRules: add use DateTimeImmutable; (drops 2 MissingImport), class-level @SuppressWarnings(PHPMD.ExcessiveClassLength), and per-method @SuppressWarnings on resolveCatalogScope, resolveDocumentPublicationSummary, tryPublicationSlugLookup — matches the pattern already on assemblePublicSearchResults. Refactor the WOO536 loop to drop the else branch. Local verification (docker container with hydra-gates + full deps): phpcs -> 0 errors (warnings non-blocking per ignore_warnings_on_exit=1) phpstan -> OK, no errors phpmd -> exit 0 (both configs) psalm -> No errors found! Tests -> 32/80 green (PublicationQueryServiceTest + WOO536RepairReadRulesTest) The 45 pre-existing errors in ObjectUpdatedEventListenerTest.php are unrelated — reproduced on origin/main by checkout+run.
Loosen the catalog-slug requirement from [a-z0-9-]+ to [A-Za-z0-9-]+
on every /api/{catalogSlug} route so uppercase requests reach
PublicationsController and receive the documented
`HTTP 404 — Catalog not found` response. Previously they fell through
to the SPA catch-all route and blew up with HTTP 500.
Behaviour unchanged for lowercase slugs; uppercase input still yields
404 because catalog slugs are stored lowercase in the DB — the
controller simply doesn't find them.
Aligns with openwoo docs Integrations/fulltext-search.md §Endpoint 1
which promises 404 on unknown slugs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses review #5077596681 finding F1: 45a4c53 correctly reintroduced isObjectPublic() / isAnonymous() (D1's "dead code" verdict was wrong — PublicationsController::uses & ::used call these at runtime) but the reintroduced body read Dutch field names publicatiedatum / depublicatiedatum. The live publication schema uses the English publicationDate / depublicationDate — see lib/Settings/publication_register.json (16 hits English, 0 Dutch) and origin/main's own isObjectPublic() which uses the English shape. Every anonymous /uses & /used request was returning 404 as "not published" regardless of publish state. Also drops the `(string)` cast in resolvePublicationSchemaId() introduced by the same phpstan fix — it coerced null → "" and drifted from the `array<int, string|null>` annotation on the by-ref cache. Replaced with an is_string() normaliser that preserves the documented shape (F2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI PR check `quality / PHP Quality (phpcs)` failed on the previous `is_string($rawSlug) ? $rawSlug : null` ternary at line 728 — repo's phpcs standard bans inline IF statements and implicit truthy comparisons. Refactored to explicit if/else with `=== true` comparison. Same runtime semantics (assign the string or null), phpcs clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-model fix(search): Full Text Search follows OpenCatalogi catalog model (WOO-536)
…red to attributes (WOO-550) Every request to the public API surface (`/api/publications`, `/api/search`, plus the internal `SearchController` methods) logged two Nextcloud 35 deprecation warnings per hit: ...PublicationsController::index uses the @publicpage annotation and should use the #[OCP\AppFramework\Http\Attribute\PublicPage] attribute instead ...PublicationsController::index uses the @NoCSRFRequired annotation and should use the #[OCP\AppFramework\Http\Attribute\NoCSRFRequired] attribute instead Replace docblock `@PublicPage` / `@NoCSRFRequired` / `@NoAdminRequired` on both controllers with the equivalent PHP attributes. No functional change — same routing / auth posture, just the non-deprecated declaration form. Ref: WOO-550
… (WOO-550) Two inline comments inside PublicationsController::uses() and used() still read `// ...this route is @publicpage.` — the migration converted every docblock-style annotation on both controllers to the PHP-attribute form, but these two prose references were missed. No runtime impact (comments aren't read by the framework) but stale relative to the migration. Ref: WOO-550
…to-attribute chore(controllers): annotation → attribute op PublicationsController + SearchController (WOO-550)
…O-551) (#1354) * fix(search): drop removed `_rbacAsPublic` named arg from OR calls (WOO-551) `/api/search` returned HTTP 500 with "Unknown named parameter $_rbacAsPublic" on every request against current OR main. The `_rbacAsPublic` runtime toggle introduced in openregister PR #2855 (WOO-536 precursor) was removed on OR main by commit `31687c6f3` ("feat(rbac): graft inheritFromPublic onto dev RBAC") which reshaped public-inheritance from a per-call toggle into a schema/register-level `authorization.inheritFromPublic` config (with a tenant-wide default in `openregister.rbac.inherit_from_public_default`). `PublicationQueryService` still passed `_rbacAsPublic: true` to `ObjectService::searchObjectsPaginated()` (Stap 1) and `find()` (M2 fast-path, single-id refinement, legacy slug scan) — every one of those paths reached OR and threw before returning any data. Remove the four surviving `_rbacAsPublic: true` named args and document the semantic drift inline: SCH-PFTS-001's uniform-visibility contract is no longer enforced at the OR layer for authenticated callers on this endpoint. Anonymous callers still see only public-group-eligible rows (the two-rule `read` block on `publication_register.json` continues to gate them). Restoring uniform visibility for authenticated staff needs a follow-up decision — reintroduce a `_forceAnonymous`-style primitive in OR, or a client-side session strip on this endpoint. Runtime verify (fully-updated stack: OR `d588b91bf` v2.0.12 + OC `0684a9a0a`): before: HTTP 500 with the exception trace above after: HTTP 200 with `{"results":[],"total":0,"facets":[]}` — no `_rbacAsPublic` errors in `nextcloud.log` Ref: WOO-551 · parent WOO-536 * test(search): sync PublicationQueryService fake + assertions to new OR API (WOO-551) The pre-existing `testAssembleForwardsRbacAsPublicTrueToObjectService` locked in the Q1 Option B contract (`_rbacAsPublic: true` on every OR call). After the code change in this PR the fake's captured `_rbacAsPublic` would stay at its `false` default and the `assertTrue` assertion would break the test. - Drop `_rbacAsPublic` from `FakeSearchObjectService::searchObjectsPaginated` and `::find` signatures — mirrors the current OR `ObjectService` shape after `31687c6f3` removed the primitive. - Remove the corresponding entry from the captured-call array. - Rename the test to `testAssembleForwardsRbacAndDisablesMultitenancyOnObjectService` and rewrite its docblock. It now asserts the two flags that ARE still forwarded (`_rbac: true`, `_multitenancy: false`) plus a `assertArrayNotHasKey` guard that pins the WOO-551 change. - Update the file-level docblock and the RET-006 fast-path test comment to replace the now-outdated `_rbacAsPublic: true` narrative with a WOO-551 pointer. Ref: WOO-551 * docs(search): sync outdated `_rbac_as_public` narrative in docblocks (WOO-551 review nit) Follow-up on the WOO-551 review — three docblocks and three inline comments still described visibility through the historical `_rbac_as_public: true` primitive that OR commit `31687c6f3` removed: - `assemblePublicSearchResults()` docblock (L109-113) — rewrote the "visibility enforced via _rbac_as_public" line to reflect the current reality: anonymous callers see the public-group-eligible half of SCH-PFTS-001; the admin/owner half is a documented follow-up. - `resolveDocumentPublicationSummary()` docblock (L799-805) — dropped the "admin sessions see the same linked-publication as anon" claim (was RBA-PUBLIC-006, no longer holds). - `tryFastPathPublicationLookup()` docblock (L989-994) — the "Under `_rbac: true`, `_rbacAsPublic: true`, `find()` throws or returns null" sentence was factually wrong; rewrote to describe the actual guarantee for anonymous callers and pointed authenticated-caller drift at the Stap 1 comment. - Three inline comments (L372, L393, L913) — same trim/point-to-Stap-1 treatment. - One misleading comment at L553 that read "searchObjects does not accept _rbacAsPublic (only the paginated variant does)" is now factually wrong for BOTH variants; rewrote as a plain note about the non-paginated call shape. No functional change — pure narrative alignment surfaced by the /review-pr 🟡 concern in comment 40164. Ref: WOO-551
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The 2.0.13 release bumped the version on main. Without this, development stays behind main and the next development -> main promotion conflicts on the version file. Version files resolve to development's side, which is the higher line, so this never moves a version backwards.
github-actions
Bot
requested review from
Rem-Dam,
bbrands02 and
rubenvdlinde
as code owners
September 1, 2026 15:49
github-actions
Bot
requested review from
WilcoLouwerse,
remko48 and
rjzondervan
as code owners
September 1, 2026 15:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opened by the release workflow after publishing 2.0.13 from
main.A release commits a version bump onto the branch it ran from. Without carrying
that back,
developmentfalls behindmain, and since both branches thenchanged the version file independently, the next
development -> mainpromotion conflicts on it — every time.
Version files are resolved to development's side, the higher line, so this
cannot move a version backwards. Any other conflict stops the workflow instead
of being resolved automatically.
Merge this, do not squash. A squash copies the files and discards the
relationship: the merge base would not move and the next promotion would
conflict exactly as before. A 0-file diff here is normal and is not a no-op —
recording the ancestry is the whole point.