Skip to content

feat(opsx batch 2): apply + archive shipped specs - #1391

Merged
rubenvdlinde merged 1 commit into
platform-integration-2026-04from
opsx-batch-2
May 1, 2026
Merged

feat(opsx batch 2): apply + archive shipped specs#1391
rubenvdlinde merged 1 commit into
platform-integration-2026-04from
opsx-batch-2

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Five-spec audit + small actionable shipments on the in-flight platform-integration-2026-04 branch. Per-spec table:

Spec Tasks after Status Files touched
data-import-export 12/15 partial — banner fixed (was wrongly 9/12) openspec/changes/data-import-export/tasks.md
file-actions 76/110 (+4) partial — phantom ticks surfaced + 4 missing unit tests added openspec/changes/file-actions/tasks.md, tests/Unit/Service/File/FileLockHandlerTest.php, tests/Unit/Service/File/FileVersioningHandlerTest.php
nextcloud-entity-relations 44/53 nothing-actionable for PHP-backend pass — 9 open are Vue + integration tests openspec/changes/nextcloud-entity-relations/tasks.md
opt-in-files-extend 24/32 nothing-actionable — 8 open are cross-repo opencatalogi docs + manual smoke openspec/changes/opt-in-files-extend/tasks.md
deprecate-published-metadata 37/56 nothing-actionable — 19 open are explicitly OUT OF SCOPE (other repos) openspec/changes/deprecate-published-metadata/tasks.md

Phantom ticks found in file-actions

  1. Phase 5 lock unit tests (lines 73 / 74 / 75) — claimed but not present at handler level. Added the three missing tests: testUnlockByNonOwnerThrows, testAdminForceUnlockSucceeds, testTtlExpiryAutoClears. Controller-level testUnlockNonOwner did exist in FilesControllerFileActionsTest, but the handler-level cases were absent.
  2. Architectural gap (NOT a tick lie, but structurally insufficient): FileLockHandler stores locks in a private in-memory $locks array with no FileMapper write-through. Phase-1 migration columns (locked_by, locked_at, lock_expires) are present but unused. Locks evaporate between requests. Surfaced in tasks.md banner; fix needs a File entity that doesn't exist yet (Phase 1 line 6 still [ ]).

Plus a small genuine ship for Phase 4 line 56: added testRestoreVersionRejectsMalformedId for FileVersioningHandler::restoreVersion parse-side defensive path.

Validation

All 5 changes pass openspec validate <spec> --type change.

Test plan

  • composer check:strict on touched files (tests/Unit/Service/File/FileLockHandlerTest.php, tests/Unit/Service/File/FileVersioningHandlerTest.php) — local environment without composer install; PHP -l clean.
  • PHPUnit unit run tests/Unit/Service/File/ — local env without composer install in this worktree; relies on CI for verification.
  • openspec validate <spec> --type change for all 5 — passes locally.

Archive recommendation

None of the 5 specs auto-archived. Recommend opening cross-repo follow-up issues for opt-in-files-extend Section 7 and deprecate-published-metadata Phases 4-7, then archiving those two.

Five-spec audit + small actionable shipments on the in-flight
platform-integration-2026-04 branch.

## Shipped (genuine new code)

- file-actions Phase 5 lock unit tests: added the three previously
  phantom-ticked cases at handler level. Controller-level cases existed
  in FilesControllerFileActionsTest, but FileLockHandlerTest was missing
  testUnlockByNonOwnerThrows / testAdminForceUnlockSucceeds /
  testTtlExpiryAutoClears.
- file-actions Phase 4 line 56: added testRestoreVersionRejectsMalformedId
  for FileVersioningHandler::restoreVersion parse-side defensive path.

## Status banner refreshes (no code change)

- data-import-export: corrected count "9 of 12" to "12 of 15"
  (9 implemented + 3 test coverage; 3 genuinely open).
- opt-in-files-extend: noted all in-repo work shipped; remaining 8
  items split between cross-repo opencatalogi docs and manual smoke
  checklists.
- deprecate-published-metadata: clarified all 19 open items live in
  Phases 4-7 explicitly marked OUT OF SCOPE; recommend cross-repo
  follow-up + archive.
- nextcloud-entity-relations: noted backend complete (44/53); 9 open
  are frontend Vue components + Greenmail/CalDAV/CardDAV integration
  tests requiring live services.

## Phantom ticks surfaced (file-actions)

Spot-check of "[x]" ticks turned up two architectural / test gaps that
the agent did NOT fix silently:

1. FileLockHandler stores locks in a private in-memory $locks array
   with no FileMapper write-through. The Phase-1 migration columns
   (locked_by, locked_at, lock_expires) are present but unused. Locks
   evaporate between requests. Phase-1 line 7 + Phase-5 acquisition
   ticks are structurally insufficient for production use. Surfaced
   in tasks.md status banner; fix needs a File entity that does not
   yet exist (Phase-1 line 6 still [ ]).
2. Phase-5 unit-test ticks (lines 73-75) were lying — three required
   tests were absent at the handler level. Corrected by adding the
   three tests this batch, with explicit reference in the banner.

## Validation

All 5 changes still pass `openspec validate <spec> --type change`.

## Not done in this batch

- Genuine backend work for nextcloud-entity-relations: skipped
  (frontend Vue + integration tests, not 2-smallest-most-self-contained).
- data-import-export error CSV download: not shipped this pass
  (touches ImportService chunked-batch architecture; deeper than the
  smallest-task constraint).
- file-actions 38 other open items: most are genuine implementation
  gaps (FileMapper File entity, persistent locking, audit trail
  integration on rename/restore, formatFile description+category,
  category filtering, ViewObject.vue batch wiring, OpenAPI spec, etc).
  Out of scope for the smallest-tasks pass but tracked.

## Archive recommendation

None of the 5 specs are ready to archive automatically — each still
has [ ] items the user/parent flow has chosen to defer rather than
delete. Recommend opsx-archive on opt-in-files-extend and
deprecate-published-metadata once cross-repo follow-up issues are
opened (Section 7 / Phases 4-7 respectively).
@rubenvdlinde
rubenvdlinde merged commit 1ce33de into platform-integration-2026-04 May 1, 2026
1 check was pending
@rubenvdlinde
rubenvdlinde deleted the opsx-batch-2 branch May 1, 2026 12:45
rubenvdlinde added a commit that referenced this pull request Sep 3, 2026
… no object (#3346)

Apps grew their own document object to hold a publication window over an
attached file. Measuring opencatalogi's, every other property it carries
already has a home: filename and mime type are the file, description and
labels are the OR-side metadata row, the owning publication is the folder
the file lives in, and the file's text is already extracted into
openregister_chunks and searchable.

The window was the only real gap. publishFile() is a boolean, so an
attachment could not be depublished on a date independently of the record
it belongs to, which is what a WOO bijlage needs, and that alone forced a
whole object type into existence.

The second reason is stronger. ContentSearchHandler already resolves a
file chunk to its owning object through findOwningObjectUuid(), so a
keyword hit inside a file attached to a publication resolves straight to
the publication. The schema widening in opencatalogi #1391 exists only
because the attachment is a separate object outside the catalog's schema
scope. Files on publications would have made that class of bug impossible.

The window is a rule rather than a comparison because each null in it
means something different: no publication date is never published and
must not fall back to the creation time, a future one is not yet, and no
depublication date is no end date rather than an end date in the past.
Start inclusive, end exclusive, so a zero-length window publishes nothing.

A depublication date is written onto the public share's expiration, which
Nextcloud already honours. An OR-side flag alone would leave a public URL
that still serves the file, and a URL that still works is not a
depublication.

Two repairs on the way. formatFile() reported the creation time under
'published', so every file that had ever existed looked published and
'not published' was unrepresentable; the creation time is kept under
'created', where it is true. And FileMapper declared a @phpstan-type
alias named File describing a filecache ROW, which shadowed the entity of
the same name in every docblock in that file: a method annotated
@return File read as an array shape. Renaming it to FilecacheRow and
naming the entity in the generic removed 12 phpstan baseline entries.

Verified on the dev instance: the migration applies and is a no-op on
re-run, and all four window states behave correctly round-tripped through
the database, not only in memory.

Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant