feat(files): a publication window on the file, so an attachment needs no object - #3346
Merged
Merged
Conversation
… no object 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.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-03 05:26 UTC
Download the full PDF report from the workflow artifacts.
…ication-window # Conflicts: # lib/Migration/Version1Date20260903090000.php
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-03 08:21 UTC
Download the full PDF report from the workflow artifacts.
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
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.
Why
Apps grew their own
documentobject to hold a publication window over an attached file. Measuring opencatalogi's, every other property it carries already has a home:filename,mimeTypedescriptionFile.descriptiontitlepublication,organizationsummarydescriptionpublicationDate/depublicationDateThe window was the only real gap.
publishFile()is a boolean: it creates a public share or it does not. So an attachment could not be depublished on a date independently of the record it belongs to, which is exactly what a WOO bijlage needs, and that alone forced a whole object type into existence.The second reason is the stronger one.
ContentSearchHandler::resolveOwningObject()already resolves a file chunk to its owning object throughFileMapper::findOwningObjectUuid(), andopenregister_chunksalready carriessource_type='file'rows. 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 living outside the catalog's schema scope. Files attached to publications would have made that class of bug impossible.The rule
The window is a method rather than an inline comparison because each null in it means something different, and treating them alike is the mistake it exists to prevent:
Start inclusive, end exclusive, so a zero-length window publishes nothing. An inverted window publishes nothing either, rather than reading as published because the start has passed.
A depublication date is written onto the public share's
expiration, which Nextcloud already honours. An OpenRegister-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 found on the way
formatFile()reported the creation time underpublished. Every file that had ever existed therefore looked published, and "not published" was unrepresentable. The creation time is kept, undercreated, where it is true.FileMapperdeclared@phpstan-type Filedescribing a filecache ROW, named after the entity the mapper maps. That alias shadowed the entity in every docblock in the file, so a method annotated@return Fileread as an array shape. It is why the phpstan baseline carried an entry for each accessor, and why adding one method made an unrelated ignore count wrong. Renaming it toFilecacheRowand naming the entity in the generic removed 12 baseline entries. Full-tree phpstan is clean.Verification
Not only in tests. On the dev instance:
addTypehydration rather than assuming ittests/Unit/Db, and 1373 across every File-related test, greenphpcs, phpmd, psalm and phpstan clean on every changed file.
What this does not do
It does not retire any app's
documentschema. That is per-app work with its own migration and its own repointing, and it should follow this rather than ride along with it. Task 6 of the change records it, including reverting #1391's widening once nothing needs it.