feat(archives): RiC-CM full roadmap (Phases 1-6) — RiC-O Linked Data, agents/activities/places, admin UI, OAI-PMH ric-o (v0.7.12, #122)#136
Closed
fabiodalez-dev wants to merge 50 commits into
Closed
Conversation
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAggiunta del builder RiC-O JSON-LD, tre endpoint pubblici /archives/*/ric.json, integrazione OAI‑PMH ric-o (RDF/XML), migrazioni RiC‑CM, admin UI Phase‑5 (activities/places/relations), aggiornamenti UNIMARC→MARCXchange, e ampio insieme di test e aggiornamenti CI/documentazione. ChangesRiC-O JSON-LD Export Phase 1–6 + MARCXchange
🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@storage/plugins/archives/ArchivesPlugin.php`:
- Around line 1322-1324: The alternate RiC-O link is only added in the admin
headLinks but not in the public page; update the public renderer by modifying
renderPublic() to append the same alternate link (rel="alternate",
type="application/ld+json", title="RiC-O (Records in Contexts)",
href=absoluteUrl('/archives/' . $id . '/ric.json')) to whatever head/links array
or HTML head builder the public view uses so the public HTML includes the
/archives/{id}/ric.json discoverability link; reference the existing headLinks
usage so the public output matches the admin output.
- Around line 5317-5337: The current code builds and returns an empty collection
when the DB query fails; change the flow to detect a failed query (when $result
is false or not an instance of \mysqli_result) before using $rootUnits and, on
failure, return an error response via $this->ricJsonError($response, /* include
$this->db->error or a short message */, 500) instead of calling
$this->makeRicBuilder()->buildCollection(...) and ricJsonResponse(); only call
makeRicBuilder() / buildCollection and ricJsonResponse() when $result is a valid
\mysqli_result and rows were fetched.
- Around line 5288-5296: The handler ricUnitAction currently treats any null
from findById as a 404; however findById (and findAuthorityById used in the
similar block at 5344-5352) can return null due to DB prepare/fetch failures, so
update the logic to distinguish "not found" from "persistence error": modify
findById/findAuthorityById to either throw a specific exception on DB errors or
expose the DB error state (e.g., a getLastDbError() / errorCode), then in
ricUnitAction (and the analogous authority handler) check for a DB error first
and return ricJsonError($response, 500, 'persistence_error', 'Database error:
...') when a persistence failure occurred, otherwise return the existing 404
'not_found' when the record truly doesn't exist. Ensure the change references
the same response helper ricJsonError so callers get consistent JSON error
payloads.
- Around line 5369-5374: Il payload JSON-LD viene costruito con il locale
corrente in makeRicBuilder (usa RicJsonLdBuilder) ma la risposta è pubblicamente
cacheable senza Vary, quindi devi fissare una lingua canonica per l'export
oppure garantire la variazione della cache; modifica makeRicBuilder per usare un
locale canonico esplicito (es. 'en' o un valore da config come
config('app.export_locale')) invece di App\Support\I18n::getLocale(), applica la
stessa correzione agli altri builder nella stessa classe che usano il locale per
il JSON-LD, oppure se non puoi fissare la lingua assicurati che il codice che
invia la risposta imposti l'intestazione HTTP "Vary: Accept-Language" per
evitare che risposte in lingue diverse vengano servite dalla stessa cache.
- Around line 5467-5495: The GROUP_CONCAT separator in the SQL uses the literal
string '\x1f', which MySQL treats as the characters "x1f" and therefore doesn't
produce the byte 0x1F expected by explode("\x1f", $alt); update the query in the
ArchivesPlugin PHP code (the GROUP_CONCAT(... SEPARATOR '\x1f') expression) to
use a real byte separator such as SEPARATOR 0x1F or SEPARATOR CHAR(31) so the
returned alt_uris value matches the explode("\x1f", $alt) logic and restores
correct URI splitting.
In `@storage/plugins/archives/RicJsonLdBuilder.php`:
- Around line 245-250: Le ric:Relation vengono create come blank node
(nell'array costruito in RicJsonLdBuilder.php con le chiavi
'@type'=>'ric:Relation', 'ric:relationType'=>$predicate,
'ric:relationHasSource'=>['@id'=>$entityId], 'ric:relationHasTarget'=>$target) e
devono ricevere un '@id' deterministico: genera un $relationId deterministico
(es. concatenazione baseUrl + unitId + agentId + role oppure UUID v5 derivato da
$entityId, $target e $predicate) e inseriscilo nell'array come '@id' =>
$relationId in modo che la stessa relazione costruita da unit e agent converga
sullo stesso IRI.
- Around line 430-445: La funzione buildDateRange usa xsd:gYear ma tronca/filtra
anni < 1000 e BCE; modifica i controlli $hasStart/$hasEnd a "$start !== null" /
"$end !== null" per consentire anni negativi, e formatta i valori come literal
xsd:gYear a 4 cifre zfill con segno per i BCE (es. $formatted = ($year < 0 ? '-'
: '') . sprintf('%04d', abs($year))). Poi imposta ric:hasBeginningDate/@value e
ric:hasEndDate/@value su questi $formatted mantenendo '@type' => 'xsd:gYear'
(puoi estrarre la logica in una funzione helper come formatGYear(int $year):
string).
- Around line 316-321: The loop in RicJsonLdBuilder.php that iterates over
['parallel_forms','other_forms'] currently pushes raw strings into
$doc['ric:hasOrHadName'] via $this->str($auth,$variantKey), which collapses
distinct semantics and emits xsd:strings instead of ric:Name objects; change the
loop to create a structured object for each non-empty $val — e.g. $nameObj =
['@type'=>'ric:Name','rdfs:label'=>$val,'ric:type'=> $variantKey ===
'parallel_forms' ? 'parallel' : 'other'] — and push $nameObj into
$doc['ric:hasOrHadName'] so ric:hasOrHadName preserves type and label semantics
(use the existing $this->str call to get $val and keep other behavior
unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: db87c7cc-16de-4674-b706-6c7165ddfb52
📒 Files selected for processing (5)
installer/database/migrations/migrate_0.7.7.sqlstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/archives/RicJsonLdBuilder.phpstorage/plugins/archives/plugin.jsonversion.json
fabiodalez-dev
added a commit
that referenced
this pull request
May 14, 2026
Round 1 review on PR #136 surfaced 5 major + 3 minor findings. All are fixed here with minimal, targeted changes — no behavioural change for endpoints that were already working, only correctness improvements where the previous code was either subtly broken or insufficiently defensive. ArchivesPlugin.php ================== #1 (Major) Public archive page was missing the RiC-O alternate link. renderPublic() builds its own headLinks array separate from the admin showAction one. Added a 5th entry matching the DC/EAD3/METS/IIIF shape so HTTP harvesters crawling the public URL discover the JSON-LD serialisation via standard <link rel="alternate"> tags. #2 (Major) findById()/findAuthorityById() collapse DB errors and missing rows into the same null return value. The RiC endpoints therefore returned 404 for transient persistence failures, which a harvester would interpret as "record deleted". Introduced two new private helpers — findUnitForRic() and findAuthorityForRic() — that return a discriminated union {status:'found'|'missing'|'error', ...} so the action methods can emit 404 only for genuine misses and 500 for DB errors. Existing callers of findById/findAuthorityById are unchanged. #3 (Major) ricCollectionAction() returned a synthetic 200 RecordSet with zero items when the DB query failed. Now bails out with a 500 + persistence_error payload the moment $this->db->query() returns false, so harvesters can tell repository emptiness from a transient outage. #4 (Major) The endpoints emit Cache-Control: public,max-age=300 but the builder was reading I18n::getLocale() which can vary per request (session-bound user locale). Two clients hitting the same URL with different sessions could share a cached body in a different language. Switched to I18n::getInstallationLocale() so the body is fully determined by the URL, and added Vary: Accept-Language as defense in depth in case a future change re-introduces per-request locale logic. #5 (Major, real bug) The SQL GROUP_CONCAT(... SEPARATOR '\x1f') did NOT produce the byte 0x1F as a separator. MySQL string literals do not recognise \xhh escape sequences — the backslash is silently dropped and the separator becomes the three-character string "x1f". Verified empirically: mysql> SELECT HEX(GROUP_CONCAT(x SEPARATOR 0x1F)), HEX(GROUP_CONCAT(x SEPARATOR '\x1f')) FROM (SELECT 'AA' UNION ALL SELECT 'BB') t; 0x1F → 41411F4242 (correct: AA <0x1F> BB) '\x1f' → 4141783166...4242 (wrong: AA "x1f" BB) The PHP-side explode("\x1f", $alt) then never split the URIs, so any authority with 2+ alternate URIs in author_authority_alternates would surface them as a single concatenated string in owl:sameAs. Replaced the literal with the hex form 0x1F so MySQL produces the real byte and the PHP split works correctly. RicJsonLdBuilder.php ==================== #6 (Nitpick) ric:Relation nodes were emitted as anonymous blank nodes. This is syntactically valid but means the relation between unit A and agent B emerges twice in the merged graph: once as a blank node when buildUnit() materialises it, and once as a different blank node when buildAuthority() materialises the inverse. Consumers cannot collapse them. Added a deterministic @id of the form {baseUrl}/archives/relations/{unitId}-{agentId}-{role-slug} computed in a new public relationIri() helper. The role is included in the slug so an agent who is BOTH creator and subject of the same unit yields two distinct relations rather than one. #7 (Minor) parallel_forms (ISAAR 5.1.3) and other_forms (5.1.5 — pseudonyms, historical variants) were pushed into ric:hasOrHadName as bare strings. The RiC-O ontology declares the range of ric:hasOrHadName as ric:Name, not xsd:string, so the previous output would fail SHACL validation. Each variant is now wrapped as {'@type':'ric:Name', 'rdfs:label':{...,'@language':lang}, 'ric:type':'parallel'|'other'} so the distinction between the two ISAAR categories is preserved. #8 (Minor) xsd:gYear literals were emitted via (string) $year, producing values like "850" for early medieval items. xsd:gYear requires YYYY format (4 digits, optional leading "-" for BCE). The validation would fail at any downstream RDF parser. Extracted formatGYear() to zero-pad to 4 digits and emit "-YYYY" for negative years (BCE), and relaxed the date_start/date_end gate from "$start > 0" to "!== null" so BCE dates the SMALLINT column can legitimately hold are no longer silently dropped. Verification ============ - PHPStan level 5: clean (no errors). - Live verification on local Apache: all 3 RiC endpoints still 200 with correctly-formatted JSON-LD; 404 path now returns the JSON-LD error envelope; Vary: Accept-Language header confirmed; ric:Relation @id visible on the unit endpoint as http://localhost:8081/archives/relations/168-4-custodian; xsd:gYear values unchanged for 4-digit years. - MySQL SEPARATOR fix empirically verified at the SQL level (HEX dump of GROUP_CONCAT output before/after).
Owner
Author
|
Addressed all 8 CodeRabbit findings in commit
PHPStan level 5: clean. Live verification on all 3 endpoints passes. |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Owner
Author
🔍 /adamsreview:review — Round 25 internal lenses (L1 diff-local · L2 structural · L3 CLAUDE.md · L4 comments · L6 security) dispatched in parallel. 17 raw candidates → deduped to 7 unique findings. Severity-sorted below. 🟠 Major (1)F001 — Relation direction inverted in Surfaced by L1, L2, and L4 (3 lenses agree). The Verified live: The two serialisations (unit-side and agent-side) thus emit conflicting triples on the same Fix: in 🟡 Minor (4)F002 — The query selects F003 — The response body is intentionally pinned to F004 — The new F005 — URIs are emitted into the public JSON-LD verbatim without scheme validation. A ⚪ Trivial (2)F006 —
F007 — Both PHPDoc Phases summary
Note: this is a structurally compressed adamsreview — the schema-strict artifact write at Phase 1.5 was skipped because the value lives in the findings, not the JSON shape. Full lens output + trace in The CodeRabbit round 1 fixes (8 findings on commit F001 is the only blocker — the relation direction inversion produces semantically wrong RDF. The other 6 are quality-of-output improvements. |
Owner
Author
|
✅ All 7 adamsreview findings addressed in commit
Verification:
PR is now clean against both CodeRabbit round 1 (8 findings, addressed in |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@storage/plugins/archives/ArchivesPlugin.php`:
- Around line 5301-5327: Le query secondarie (fetchAuthoritiesForArchivalUnit,
fetchDirectChildren, fetchArchivalUnitsForAuthority, collectSameAsForAuthority)
possono degradare silenziosamente a [] su errore e causare un 200 con RiC
incompleto; modifica ricUnitAction (e i blocchi analoghi indicati) per rilevare
e propagare gli errori di queste chiamate come fa findUnitForRic: cambia le
funzioni secondarie per restituire una struttura discriminata (es.
['status'=>'ok'|'error'|'missing','row'|…] o lanciare un'eccezione), nel
chiamante controlla lo status (o cattura l'eccezione), effettua
SecureLogger::error con il messaggio e ritorna ricJsonError($response, 500,
'persistence_error', ...) invece di continuare e serializzare un grafo parziale;
applica la stessa logica anche nei metodi/azioni alle altre porzioni indicate
(le righe/aree duplicate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 99bab26c-e0b5-47d3-bf82-c2b7876447b4
📒 Files selected for processing (3)
README.mdstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/archives/RicJsonLdBuilder.php
fabiodalez-dev
added a commit
that referenced
this pull request
May 14, 2026
… 16 bundled plugins Documentation catch-up for the v0.6.x → v0.7.7 work. The site previously stopped at v0.5.9.6 in the changelog and at v0.5.9.4 in the bundled-plugin table, hiding 12 months of feature growth from new users browsing the docs. releases/changelog.md ===================== - Replaces the stale "In arrivo (branch attivi)" block (which still described PRs #118/#119/#120/#127 as in-flight) with proper version entries v0.7.7 / v0.7.6 / v0.7.5 / v0.7.4 / v0.7.0-0.7.3, plus a forward- looking entry for the current PR #136 RiC-CM Phase 1 work. guida/archivi.md + en/guide/archives.md ======================================= Adds six new sections covering features that already shipped but were undocumented: - IIIF Presentation 3.0 manifests (v0.7.6+, #123): endpoint + seeAlso block - RiC-O JSON-LD — Records in Contexts (v0.7.7+, #122): three endpoints, role → predicate table, deterministic Relation @id, owl:sameAs allow-list - AtoM ISAD(G) area labels (v0.7.6+, #121): canonical area names - Multi-document upload (v0.7.6+): cover + downloadable files per unit - ARK identifiers + rightsstatements.org URIs admin/plugin.md + en/admin/plugins.md ===================================== - Bundled plugin table expanded from 10 to 16 entries (catching up the missing OAI-PMH Server, NCIP 2.0, BIBFRAME, OpenURL Resolver, ResourceSync, and VIAF Authority Control). - Plugins grouped into three categories: "Metadata scraping & enrichment", "Interoperability protocols", "Cataloging & specialised collections". - Header label bumped from "v0.5.9.4" to "v0.7.7". tecnico/sru.md + en/technical/sru.md ==================================== - Added BNF (Bibliothèque nationale de France) entry to preconfigured servers list with UNIMARC parser + field 676 Dewey extraction note. - Added "Security hardening (v0.7.6+)" subsection covering CQL injection escape fix.
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
fabiodalez-dev
added a commit
that referenced
this pull request
May 14, 2026
…abbit R2) Round 2 of the CodeRabbit review on PR #136 surfaced 1 Major finding: the four secondary helpers used by the RiC endpoints (fetchAuthoritiesForArchivalUnit, fetchDirectChildren, fetchArchivalUnitsForAuthority, collectSameAsForAuthority) still degraded silently to [] on DB prepare/execute failure, even after the F006 fix added SecureLogger calls. The action methods then continued to serialise a 200 RiC graph with the missing pieces invisible — and to a harvester an empty children list or empty owl:sameAs looks like a real structural change, not a transient outage. The fix ======= All four helpers now THROW `\RuntimeException` on prepare/execute failure rather than logging-and-returning. The two RiC actions wrap the secondary fetches in `try { } catch (\RuntimeException) { return ricJsonError(500, 'persistence_error', ...); }` so the harvester gets an honest 500 instead of an incomplete 200. Behavioural notes ================= - `collectSameAsForAuthority` keeps the silent-degrade ONLY when the prepare fails with `errno=1146` (ER_NO_SUCH_TABLE) — the legitimate case where the viaf-authority plugin has never been activated. Any other DB error propagates. - `fetchAuthoritiesForArchivalUnit` and `fetchArchivalUnitsForAuthority` are also called from admin HTML views (showAction, authorityShowAction). Those don't catch — the exception bubbles up to Slim's default error handler and the admin sees a 500 page. This is correct behaviour for a DB error: a half-rendered HTML view is worse than an explicit error. Verification ============ - PHPStan level 5: clean. - Live: all 3 RiC endpoints still return 200 on the happy path. - The 404 / 500 distinction from R1's F002 fix is preserved — only the secondary-fetch failure class is new.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
storage/plugins/archives/ArchivesPlugin.php (1)
5321-5327:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftNon pubblicare un export RiC parziale con
200.Qui il lookup primario distingue bene
404e500, ma i fetch secondari (fetchAuthoritiesForArchivalUnit(),fetchDirectChildren(),fetchArchivalUnitsForAuthority(),collectSameAsForAuthority()) continuano a degradare a[]su errore DB. Il risultato è un grafo incompleto con200, che per un harvester equivale a relazioni davvero rimosse.Fai propagare anche questi errori fino a
ricJsonError(..., 500, 'persistence_error', ...)invece di serializzare il payload parziale.Also applies to: 5392-5398
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@storage/plugins/archives/ArchivesPlugin.php` around lines 5321 - 5327, The secondary fetch helpers (fetchAuthoritiesForArchivalUnit, fetchDirectChildren, fetchArchivalUnitsForAuthority, collectSameAsForAuthority) currently swallow DB errors and return empty arrays, producing partial RiC payloads with 200; change them to propagate failures (throw exceptions or return a sentinel error) and update the calling flow around makeRicBuilder()/buildUnit() so that when any secondary fetch fails you call ricJsonError($response, 500, 'persistence_error', ...) and return early instead of ricJsonResponse; alternatively, have those helper methods return false on failure and add explicit checks after each call to invoke ricJsonError on failure before proceeding to buildUnit()/ricJsonResponse.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@storage/plugins/archives/RicJsonLdBuilder.php`:
- Around line 202-219: The embedded-child block is emitting 'rdfs:label' even
when preferTitle($child) returns an empty string; change the logic in
RicJsonLdBuilder (the loop that builds $parts) to call preferTitle($child), trim
and check for a non-empty value, and only add the 'rdfs:label' key when the
title is non-empty; apply the same pattern in buildAuthority (the
relation/target construction around ~395) so targets do not get empty literal
labels either, ensuring both unitIri(...) and `@type` remain but omitting empty
rdfs:label entries.
- Around line 439-446: The rdfs:label and ric:title emitted by buildCollection()
are hardcoded in English but tagged with $this->lang (causing literals like
"Archival collections"@it); update buildCollection() so the English literal is
correctly declared — either set the rdfs:label '@language' to 'en' for that
literal, or replace the plain English string with a translated string based on
$this->lang, and make ric:title consistent (either provide a language-tagged
object or omit the incorrect tag); ensure you update the entries referencing
rdfs:label and ric:title accordingly.
---
Duplicate comments:
In `@storage/plugins/archives/ArchivesPlugin.php`:
- Around line 5321-5327: The secondary fetch helpers
(fetchAuthoritiesForArchivalUnit, fetchDirectChildren,
fetchArchivalUnitsForAuthority, collectSameAsForAuthority) currently swallow DB
errors and return empty arrays, producing partial RiC payloads with 200; change
them to propagate failures (throw exceptions or return a sentinel error) and
update the calling flow around makeRicBuilder()/buildUnit() so that when any
secondary fetch fails you call ricJsonError($response, 500, 'persistence_error',
...) and return early instead of ricJsonResponse; alternatively, have those
helper methods return false on failure and add explicit checks after each call
to invoke ricJsonError on failure before proceeding to
buildUnit()/ricJsonResponse.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d5add8d2-249a-49ff-88a9-161a064f345e
📒 Files selected for processing (6)
README.mdinstaller/database/migrations/migrate_0.7.7.sqlstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/archives/RicJsonLdBuilder.phpstorage/plugins/archives/plugin.jsonversion.json
Owner
Author
|
@coderabbitai full review R2 finding (secondary fetcher silent degradation) addressed in commit
|
|
✅ Actions performedFull review triggered. |
Owner
Author
|
Round 3 findings addressed in commit
Live verify: |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@storage/plugins/archives/ArchivesPlugin.php`:
- Around line 2463-2500: The helper fetchAuthoritiesForArchivalUnit currently
throws raw RuntimeException on DB errors, which breaks non-RiC exporters
(dc.xml, ead.xml, mets.xml, export.xml, oai) by producing generic 500 HTML;
change the helper to not throw raw exceptions on prepare/execute/get_result
failures — instead log the DB error and return a distinct, non-exception value
(e.g. false or null) or an empty array to indicate failure, and remove the throw
statements from fetchAuthoritiesForArchivalUnit; then update the RiC-specific
caller(s) to detect that return value and explicitly throw a contextual
exception (or rethrow a wrapped one) so only RiC endpoints still produce a 500
while all exporter callers translate the failure into their appropriate XML/OAI
error handling.
- Around line 5508-5522: The ricJsonError method sets Content-Type to
application/ld+json while returning a simple error object; update ricJsonError
to either (A) change the response Content-Type header to
"application/problem+json" and keep the existing payload keys, or (B) make the
payload valid JSON-LD by adding an "@context" and appropriate JSON-LD fields
before writing the body; locate the function ricJsonError in ArchivesPlugin.php
and modify the withHeader('Content-Type', ...) call and/or the $payload
construction so the header matches the payload (prefer option A unless you
intentionally produce JSON-LD).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7d4107d-3fde-48f7-a806-b302401f4eed
📒 Files selected for processing (11)
installer/database/migrations/migrate_0.7.7.sqlstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/archives/RicJsonLdBuilder.phpstorage/plugins/oai-pmh-server/OaiPmhServerPlugin.phptests/archives-plugin.unit.phptests/archives-ric-jsonld.unit.phptests/archives-unit-files.unit.phptests/oai-pmh-server.spec.jstests/sru-unimarcxml.spec.jstests/unimarc-export.spec.jstests/unimarc-standards.unit.php
Owner
Author
|
R4 findings addressed in commit
Live verify:
PHPStan level 5: clean. |
Owner
Author
|
Test coverage estesa in commit
Nuovo unit coverage (PHP, +25 check)
Nuovo E2E (Playwright, 16 test)
Tutti i 72 check passano in 2.4s sul seed locale. |
Owner
Author
|
@coderabbitai full review Latest commits since R4:
Test results: 56/56 unit + 16/16 E2E = 72 dedicated RiC-O checks, all green. PHPStan level 5 clean. |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@storage/plugins/archives/RicJsonLdBuilder.php`:
- Around line 287-290: Trim and sanitize the $ark value returned by
$this->str($unit, 'ark_identifier') (strip CR/LF and other control/whitespace)
and validate it matches an ARK canonical pattern (e.g. starts with "ark:/"
followed by NAAN and name) before concatenating into 'https://n2t.net/'.$ark; if
validation fails, do not add the rdfs:seeAlso entry. Use the same defensive
approach as for rights_statement_url/owl:sameAs (reuse or mirror
isValidLodUri-style checks) and update the block that builds $seeAlso to perform
these checks against $ark first.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1a912246-8153-4e93-841e-482518b7471a
📒 Files selected for processing (15)
README.mdinstaller/database/migrations/migrate_0.7.7.sqlstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/archives/RicJsonLdBuilder.phpstorage/plugins/archives/plugin.jsonstorage/plugins/oai-pmh-server/OaiPmhServerPlugin.phptests/archives-plugin.unit.phptests/archives-ric-jsonld.spec.jstests/archives-ric-jsonld.unit.phptests/archives-unit-files.unit.phptests/oai-pmh-server.spec.jstests/sru-unimarcxml.spec.jstests/unimarc-export.spec.jstests/unimarc-standards.unit.phpversion.json
Owner
Author
|
R5 (1 Minor) addressed in commit Finding: Fix: New
Test coverage: 16 new Reflection-based assertions cover positive cases (canonical + SNAC real-world + bare-NAAN + trim) and 12 negative paths (short/non-digit NAAN, absolute URL, leading slash, path traversal, CR/LF/NUL/whitespace). Unit total: 56 → 72, all green. PHPStan level 5 clean. |
Owner
Author
|
@coderabbitai full review R5 finding (ark_identifier validation) was already addressed in commit The applied fix (
Live verify: Please re-review against |
|
✅ Actions performedFull review triggered. |
Discovered by reinstall-test.sh Test B (upgrade v0.7.6 → v0.7.12): the upgrade failed with 'Table archive_activities doesn't exist' because Updater::runMigrations() uses glob() + sort() and string sort places '0.7.10' BEFORE '0.7.9' (because '1' < '7' in lex order). Order became: migrate_0.7.10.sql, 0.7.12, 0.7.7, 0.7.8, 0.7.9 migrate_0.7.10.sql declared an FK referencing archive_activities, which migrate_0.7.9.sql had not yet created. migrate_0.7.12.sql then DROP COLUMN'd a column from a table that didn't exist yet. The Updater.php sort fix (e9bc4bf) only helps installs that ALREADY have the new Updater code loaded — but the migration runner that executes during an upgrade FROM v0.7.6 is the OLD code in PHP-FPM's opcache. We can't retroactively fix the bytecode that's already running the upgrade. Workaround: rename every migrate_0.7.X.sql with a single-digit X to migrate_0.7.0X.sql so lex sort matches semver: migrate_0.7.00.sql (was 0.7.0) migrate_0.7.04.sql (was 0.7.4) migrate_0.7.05.sql (was 0.7.5) migrate_0.7.06.sql (was 0.7.6) migrate_0.7.07.sql (was 0.7.7) migrate_0.7.08.sql (was 0.7.8) migrate_0.7.09.sql (was 0.7.9) migrate_0.7.10.sql (unchanged) migrate_0.7.12.sql (unchanged) version_compare normalises decimal segments so '0.7.07' == '0.7.7' (verified: php -r 'echo version_compare("0.7.07", "0.7.7");' prints 0). The migrations table tracks by version string, but a fresh upgrade from v0.7.6 has zero entries for 0.7.7+ so the renamed migrations all run. Future minor families (0.8.x onwards) should follow the same 2-digit padding convention. The e9bc4bf usort fix in Updater.php serves as a safety net for 0.8+ installs but is not retroactive.
…et installed Reinstall-test Test B (upgrade v0.7.6 → v0.7.12) revealed a second class of failure beyond the lex-sort issue (e61a51e): migrations 0.7.08 and later ALTER TABLE authority_records / CREATE TABLE archive_activities (with FK→authority_records) / DROP COLUMN place_id (from archive_activities) — but none of those tables exist on a fresh upgrade from v0.7.6 because the archives plugin landed in v0.7.7 and its tables are created by ensureSchema() at first onActivate(), not by the migrations. The flow is: 1. Updater::installUpdate() copies files (incl. new plugin) 2. updateBundledPlugins() copies plugin code to storage/plugins/ 3. autoRegisterBundledPlugins() INSERTs the plugin row with is_active=0 (or syncs the row if already present and active) 4. Updater::runMigrations(0.7.6 → 0.7.12) tries to ALTER tables that don't exist yet → fail 5. (Plugin would only call ensureSchema/onActivate AFTER the admin visits /admin/plugins and clicks Attiva — but the upgrade already failed at step 4.) Fix: gate every ALTER / UPDATE / CREATE TABLE (with FK) in the four migration files behind an INFORMATION_SCHEMA.TABLES check that the required base table(s) actually exist. When absent, the statement short-circuits to `SELECT 1` and the upgrade succeeds. The archives plugin's ensureSchema() then creates the full Phase 2-6 schema (including authority_records / archive_activities / archive_places / the polymorphic relations table) on first activation — which is the intended single source of truth for the plugin schema anyway. Files touched: - migrate_0.7.08.sql: ALTERs on authority_records + CREATE TABLE archive_agent_identifiers + archive_agent_relations (both FK→ authority_records) now skip when authority_records is absent. - migrate_0.7.09.sql: CREATE TABLE archive_activities + archive_unit_activities (both FK→archival_units / authority_records) skip when either base table is absent. - migrate_0.7.10.sql: CREATE TABLE archive_places + archive_relations same guard. - migrate_0.7.12.sql: ALTER TABLE archive_activities DROP COLUMN place_id now wrapped in @col_exists guard (also covers the case where ensureSchema already created the table without place_id since the DDL fix). The migrations record table (`migrations`) still tracks 0.7.07..0.7.12 as executed, so re-running the upgrade is idempotent. Reinstall-test Test B now passes.
The Full E2E suite now exercises 13 spec files (was 8): * [9/13] archives-ric-jsonld.spec.js — RiC-O JSON-LD endpoints (Phase 1-4) * [10/13] archives-crud.spec.js — archival_units CRUD (#103) * [11/13] archives-authorities.spec.js — authority_records CRUD * [12/13] archives-upload-assets.spec.js — cover/document upload pipeline (now passing 10/10 after the WAV-fixture fix in 7458d07) * [13/13] archives-pr-extended.spec.js — extended CRUD + CSRF regression These specs were previously local-only (only runnable via the developer's /tmp/run-e2e.sh wrapper). Wiring them into CI gives PR regressions on archives-touching changes the same visibility that the rest of the suite already enjoys. Also fixed stale 'v0.8.0' string-substitutions in the existing two RiC-CM step names — the shipped version is v0.7.12 (the F016 revert restored the documented 0.7.x interop pinning rule).
CodeRabbit full-review on the post-CI-fix state of PR #136 surfaced 7 actionable inline findings. All addressed in this commit; the one outside-diff "consolidate migrations into a single 0.7.12 file" recommendation is deliberately skipped — the per-phase migration split lets pre-Phase-1 installs (≤ v0.7.6) silently skip the DDL chain via the INFORMATION_SCHEMA guards, which a consolidated file would break by re-introducing a single ALTER on a non-existent table. * CR1 (🔴 Critical) — locale key drift across 4 JSONs de_DE.json had keys (e.g. "+ Nuova attività" → "+ Neue Aktività", "+ Nuovo luogo" → "+ Neuer Ort") that were absent from it_IT.json and en_US.json. Synchronised all 4 locale files to identical 5072-key set (was: it=581, en=4746, fr=5071, de=4746). it_IT uses identity for source strings; other locales reuse existing sibling translations or fall back to Italian source. All files alphabetically sorted. * CR2 (🟠 Major) — year-only is not valid xsd:date buildActivity() and buildPlace() always tagged date_start/date_end as xsd:date, but "1890" is NOT a valid xsd:date lexical form. Now detect year-only via /^\d{4}$/ and emit xsd:gYear; full YYYY-MM-DD values still get xsd:date. * CR3 (🟠 Major) — buildRelationNode emitted /archives/relations/0 buildRelationNode() now rejects rows with id<=0 (returning null), preventing IRI collisions when callers pass a projection missing the PK. * CR4 (🟡 Minor) — sanitizeArkIdentifier allowed ../ after NAAN Inputs like 'ark:/12345/../../foo' previously survived normalisation because the dot-segment guard only inspected the input prefix. Now scans the full post-NAAN path for any './' / '../' / '/.$' / '/..$' segments and returns null. * CR5 (🟠 Major) — ric-o missing isArchivesSetExposed() gate at runtime. oaiListMetadataFormats correctly gated ric-o behind isArchivesSetExposed(), but oaiListRecords + oaiGetRecord did not — meaning the format remained accessible if the plugin was deactivated while archive_relations / archival_units still existed in the DB. Both runtime endpoints now emit cannotDisseminateFormat early when the gate is closed. * CR6 (🟡 Minor) — invalid-enum attach test missed 5xx assertions archives-phase5-admin-ui.spec.js test 14 now asserts res.status() < 500 BEFORE checking DB persistence, so a 5xx regression on the relations/attach handler cannot pass undetected. * CR7 (🟠 Major) — Phase 6 default-set test was too permissive archives-phase6-oai-ric-o.spec.js test 8 previously accepted badArgument/noRecordsMatch/cannotDisseminateFormat as valid responses to "metadataPrefix=ric-o without set=". The spec contract is auto-routing to set=archives — assertions are now strict (no <error>, at least one <record>, contains <rdf:RDF>). Verification: - PHPStan level 5 clean on archives + oai-pmh + Updater - Unit tests 183/183 (was 172/172, +11 new for CR2/CR3/CR4 edges) - Local E2E 25/25 (Phase 5 + Phase 6) - Locale parity: 5072 keys per file, all 4 sets identical
…erministic test locators
- RicJsonLdBuilder: centralise xsd date typing in dateLiteral(); historical
years (850, "-44") now correctly emit xsd:gYear instead of invalid xsd:date
literals. Applies to buildActivity(), buildAuthority() birth/death,
agent-rel date ranges, buildPlace(), and the polymorphic relation builder.
- OaiPmhServerPlugin::ListMetadataFormats: tighten the ric-o gate so the
archival_unit identifier branch also respects $ricExposed. Previously
discovery could advertise ric-o for an archival_unit identifier when the
archives set was not exposed, causing GetRecord/ListRecords to respond
cannotDisseminateFormat for an advertised format.
- archives-phase5/phase6 specs: scope the activateBtn selector to the
archives plugin card (`activatePlugin('archives')`) instead of `.first()`,
so the plugin-bootstrap step is deterministic regardless of plugin order
in /admin/plugins.
- archives-ric-jsonld.unit.php: drop the unused third element from the
$gyear closure return (callers only destructure [$b, $e]).
Validation: phpstan clean on touched files, 183/183 RiC-O JSON-LD unit,
60/60 OAI resumption-token unit, 25/25 archival_unit_files unit, 16/16
archives-ric-jsonld E2E, 15/15 archives-phase5-admin-ui E2E, 10/10
archives-phase6-oai-ric-o E2E (test 8 now strictly enforces auto-default
set=archives, no error-branch fallback).
Reconciled fix (one merge pass after Phase 9.pre overlap):
Findings: F001, F002, F003, F006, F010, F011, F028, F032, F033, F034, F042
Files: 9 (4 locale JSONs + 5 PHP/views)
Overlaps: locale/de_DE.json, locale/en_US.json, locale/fr_FR.json, locale/it_IT.json,
storage/plugins/archives/ArchivesPlugin.php,
storage/plugins/archives/views/places/show.php
Fixes applied:
[FG-1] F001 — OaiPmhServerPlugin.fetchRecordsPage SELECT expanded from 8 to ~17 columns
so ListRecords ric-o emits ric:RecordSet for fonds and includes
date/extent/ARK/parent (parity with GetRecord SELECT *)
[FG-2] F002 — activityUpdateAction: check execute() return + affected_rows; HTTP 422
with __('Unable to save activity. Please retry.') on failure
F003 — placeUpdateAction: early-return guard on prepare() failure
(SecureLogger::error + renderNotFound) mirrors activityUpdateAction
F006 — SRU CQL + searchArchivalUnits: addcslashes($value, '%_\\') before LIKE
F010 — detachAuthorityAction: Location wrapped with url()
F011 — handleAssetUpload: 7 Location headers wrapped with url()
F042 — ricActivitiesListAction + ricPlacesListAction: LIMIT 500 on public lists
[FG-3] F028 — locale/{en_US,de_DE}.json: 315 French-residual values retranslated to
proper English/German
[FG-4] F032 — activities/form.php placeholders wrapped with __(); 'AAAA o AAAA-MM-GG'
key added to all 4 locale JSONs
[FG-5] F033 — placeShowAction fetches parent name; places/show.php renders $parent_label
with fallback to '#id'
[FG-6] F034 — $relationTargetLabels map in activities/show.php and places/show.php;
'Autorità' key added to all 4 locale JSONs
Post-fix review: 11/11 findings verified complete via reconcile re-trace;
0 partial, 0 regression. PHPStan clean on touched files, 183/183 RiC-O JSON-LD
unit tests green, all 4 locale JSONs valid (5074 keys each).
Review: rev_01KRYDV6VDPQKCZXGZHS4E20WQ
Run: fixrun_01KRZ6ZMBCDH378SY0XEP9PX1D
Threshold: 50
Two promoted findings applied after :promote +human_confirmation override.
[FG-1] F007 — removeAssetAction (storage/plugins/archives/ArchivesPlugin.php)
Add realpath()-based containment check after str_starts_with prefix
gate. Resolves both allowed root and candidate path; rejects unlink
when resolved path is not strictly under the resolved allowed root.
SecureLogger::error logs containment failures for audit. Defends
against /uploads/archives/covers/../../../etc.../passwd style traversal
segments that pass the prefix check but escape the directory.
[FG-2] F022 — buildActivity (storage/plugins/archives/RicJsonLdBuilder.php)
Replace 'ric:hasOrHadPartOf' (not a defined RiC-O property) with
'ric:isOrWasPartOf' (the correct child→parent inverse). Existing
'ric:hasOrHadPart' usages (parent→child direction, valid RiC-O) are
unaffected. Unit test pinned the prior incorrect string — updated to
match the corrected semantics.
Post-fix review: 2/2 findings verified. PHPStan clean on touched PHP files;
183/183 RiC-O JSON-LD unit tests green.
Review: rev_01KRYDV6VDPQKCZXGZHS4E20WQ
Run: fixrun_01KRZ9PH4E5MJ7QW7JN8SYPXWK
Threshold: 50
Six findings promoted via /adamsreview:walkthrough --threshold 40,
applied via :fix 35 (manual fallback after API overload on parallel
Opus agents).
[F009] OaiPmhServerPlugin::isArchivesSetExposed — per-request memoization
via $ricExposedCache instance prop. ListRecords + GetRecord +
ListMetadataFormats no longer fire ≥3 PluginManager constructions
+ ≥3 INFORMATION_SCHEMA queries per OAI request.
[F013] ArchivesPlugin cycle-walk hard cap 100 → MAX_HIERARCHY_DEPTH=1000
in 4 helpers (activityWouldCreateCycle, activityAncestorChainHasCycle,
placeWouldCreateCycle, parentWouldCreateCycle). Real archival
hierarchies stay <20 deep; 1000 cap defends against pathological
input without false-negative-as-no-cycle.
[F016] ArchivesPlugin relationAttachAction — INSERT IGNORE INTO
archive_unit_activities / archive_agent_relations replaced with
INSERT ... ON DUPLICATE KEY UPDATE ric_predicate=VALUES(ric_predicate).
Mirrors stay in sync when same (unit,activity) or (agent,agent) pair
is re-attached with a different predicate.
[F036] activities/show.php + places/show.php typeahead — ARIA combobox
on the input (aria-autocomplete, aria-expanded, aria-controls,
aria-label), role=listbox on the results div with stable id,
role=option on each result; aria-expanded toggled in
renderResults/hideResults. aria-label on ric_predicate input.
[F038] placeShowAction now fetches relation count + passes
'relation_count' to view; places/show.php delete-button SweetAlert
uses dynamic message ("È collegato a N relazione/i; le relazioni
saranno orfane.") when count > 0. Mirrors activities/show.php
destructive-action blast-radius declaration.
[F045] ArchivesPlugin relationAttachAction — ISO 8601-style format
validation on date_start/date_end (/^-?[0-9]{4}(-[0-9]{2}-[0-9]{2})?$/)
and 500-char length cap on source_ref. Rejects upstream instead
of letting malformed strings reach the public RiC-O JSON-LD output.
Locale: 3 new keys added to all 4 locale files (5077 keys each, parity
preserved). PHPStan clean on touched PHP, 183/183 RiC-O JSON-LD unit
tests green.
Review: rev_01KRYDV6VDPQKCZXGZHS4E20WQ
Run: fixrun_01KS0AKXCGJ5MD4M7WK1PERMC8
Threshold: 35
[F027] migrate_0.7.12.sql header — rephrased to not quote a non-existent
comment from migrate_0.7.09.sql. Describes column history factually:
"added in migrate_0.7.09.sql; reserved for the Phase 4 FK that
Phase 4 never delivered" instead of citing a fabricated quote.
[F037] aria-label on ric_predicate input — already added in commit e332a05
as part of F036 (combobox ARIA fix). No code change; outcome
recorded as verified for audit completeness.
[F046] User predicate log injection — already mitigated upstream at
ArchivesPlugin.php:7843-7844 where the predicate is validated by
/^[a-z]{2,10}:[A-Za-z][A-Za-z0-9]{1,80}$/ before reaching
SecureLogger; the regex's character class excludes newlines so
log-line injection is structurally impossible. No code change;
outcome recorded as verified for audit completeness.
Review: rev_01KRYDV6VDPQKCZXGZHS4E20WQ
Threshold: 34
[F014] safeReturnTo restricted to /admin/archives/* allow-list
(was: any /-starting non-// path). Closes the confused-deputy
redirect surface between admin routes.
[F015] handleAssetUpload basename entropy 32-bit → 64-bit
(bin2hex(random_bytes(8))) so per-unit collisions stay
negligible even at scale.
[F017] Phase 6 (v0.8.0) → Phase 6 (v0.7.12) in OaiPmh comments (2 sites).
[F019] ensureSchema docblock: add migrateAuthorityRecordsRicColumns.
[F020] RicJsonLdBuilder docstring: 'rico:' → 'ric:' (real prefix).
[F021] findActivityForRic comment: drop 'place_id stay' (col removed).
[F023] Phase 5 (target v0.8.0) → (v0.7.12) Admin UI section header.
[F024] OaiPmh writeArchivalUnitMetadata comment: acknowledge in-plugin
ric-o RDF/XML emission instead of suggesting only the archives
plugin's /archives/oai handles richer formats.
[F025] RicJsonLdBuilder class docblock: replace 'Phase 1 ... no new
tables' with Phase 1–6 scope (archive_activities, archive_places,
archive_relations).
[F026] Route registration comment: 'three public, read-only endpoints'
→ describe Phase 1/3/4 endpoints (collection/unit/agents +
activities + places).
[F039] activities/show.php + places/show.php: typeahead now renders a
'…' loading row (role=status, aria-live=polite) between input
and fetch resolve, so slow networks no longer look frozen.
Review: rev_01KRYDV6VDPQKCZXGZHS4E20WQ
Skipped: F004 (false-positive per validator; is_ongoing branch
intentionally appends '[ongoing]' to description).
PHPStan clean; 183/183 RiC-O unit tests green; both views php -l OK.
…ycle After @coderabbitai full review on PR #136 head b3b077b (33/37 prior findings resolved), CR's full-file pass surfaced 5 new actionable items that the diff-only incremental reviews had missed. [CR-1] README.md — OAI-PMH endpoint documented as `/archives/oai` when the actual route registered by the OAI-PMH server plugin is `/oai`. Rewritten to clarify that archival units harvest via `set=archives` on the shared /oai endpoint, and to mention ric-o (added in v0.7.12) alongside oai_dc/marc21/ead3. [CR-2] RicJsonLdBuilder::buildPlace — coordinate guard only rejected null; empty strings or non-numeric inputs cast to (float) 0.0 and shipped a bogus 0,0 ric:CoordinateLocation in public JSON-LD. Now trims string inputs and requires both endpoints non-empty + numeric before emitting the node. [CR-3] OaiPmhServerPlugin::oaiGetRecord — when CannotDisseminateFormat fires after startElement('metadata'), the catch emitted <error> while <metadata>/<record>/<GetRecord> were still open, producing malformed XML. Catch now closes the three open elements via best-effort endElement() calls before oaiError(). [CR-3 + CR-4] activities/show.php + places/show.php typeahead — F036 added ARIA combobox/listbox/option roles but the listbox was still mouse-only: keyboard-only admins could search but not select. Now ArrowDown/ArrowUp move the active option, Enter selects (populates hidden target_id + closes listbox), Escape closes; aria-activedescendant tracks the active option and aria-selected is toggled on the active item. Same implementation pattern in both views. Validation: PHPStan clean, 183/183 RiC-O JSON-LD unit tests green, php -l clean on both views.
After the 2nd @coderabbitai full review on commit b1f41cc, CR surfaced 6 additional finer-grained findings the 1st full review had missed. [CR-A] README.md:708 (Minor) — Plugin formats list omitted `ead3` while another README section declared it supported. Added. [CR-B] RicJsonLdBuilder.php:858-863 (Minor, defence-in-depth) — geonames_id / wikidata_id / tgn_id were interpolated into owl:sameAs IRIs without read-path validation. Write-path rejects malformed values, but a DB seed or future bypass could let a crafted value (path traversal, query string, control chars) leak into public IRIs. Added inline preg_match guards (^\d+$ for GeoNames/TGN, ^Q\d+$ for Wikidata) before each $sameAs append, matching the sanitizeArkIdentifier() defence-in-depth pattern used elsewhere in the file. [CR-C] activities/show.php typeahead JS — aria-expanded was kept at the default false in the "no results" branch even though the listbox was visible (screen-reader inconsistency). Now sets aria-expanded=true in the empty branch too, mirroring the populated branch. [CR-D] places/show.php typeahead JS — same aria-expanded fix as activities, same pattern. [CR-E] places/form.php — the red border on date_start/date_end on validation failure was the only signal; the actual error message (which tells the user the expected format) was not rendered. Added a `<p class="text-red-600">` block under each field, matching the existing pattern for other fields in the same form. [CR-F] OaiPmhServerPlugin.php writeBookUnimarc:1587-1590 (Major) — In the standalone /admin/books/{id}/unimarc.xml download (where <record> is the document root, not nested under the OAI envelope) the `xmlns:xsi` namespace was not declared before the `xsi:schemaLocation` attribute was written. Result: the xsi prefix was unbound and the standalone XML was technically malformed. Added the explicit xmlns:xsi declaration immediately before the schemaLocation attribute. PHPStan clean, 183/183 RiC-O JSON-LD unit green, php -l clean on all three views. The OAI dissemination path is unaffected — only the direct-download endpoint was missing the xsi namespace binding.
After /adamsreview:review on commit bd0ec44, 3 lens dispatch (L1+L2+L6) surfaced 24 candidates of which 6 were real sibling-unfixed / fix-incomplete patterns. Applied here. [F013-followup] placeWouldCreateCycle was 1 of 4 cycle-walkers and the F013 sed-pass missed it (uses while+iterations++ instead of a for-loop). Replaced literal 100 with self::MAX_HIERARCHY_DEPTH so all 4 walkers are consistent. [F002-followup] activityUpdateAction error branch gated on `!$execOk || $affected < 1`. mysqli affected_rows returns 0 by default (no CLIENT_FOUND_ROWS) on a successful UPDATE that changes nothing, so a legitimate no-op resubmit was falsely routed to the 422 + "Unable to save" path. Removed the affected_rows < 1 clause; only $execOk is the failure signal. [F002 i18n followup] Activity-save error message was emitted with an English-source key __('Unable to save activity. Please retry.') that was absent from all 4 locale JSONs, so it_IT/fr_FR/de_DE installs would have displayed raw English. Switched to an Italian-source key __('Impossibile salvare l\'attività. Riprovare.') and added it + the parallel "Impossibile salvare il luogo" key to all 4 locale files (5077 → 5080 keys each). [F045-sibling validateActivity] F045 added ISO 8601 / length validation to relationAttachAction. archive_activities.date_start /date_end / source_ref had the same VARCHAR shape but validateActivity ignored them. Added same regex + mb_strlen check so malformed dates can't reach the public RiC-O JSON-LD output. [F045-sibling validatePlace] Same gap on validatePlace. archive_places.date_start/date_end are also unvalidated. Added same regex check. The places/form.php view already had the error message slot from the CR confirm-review fix; now it actually gets populated when the user submits a malformed date. [F002-sibling placeUpdateAction] placeUpdateAction only checked prepare() failure (F003). A failed execute() (deadlock, constraint, lost connection) would have returned 302 silently implying success. Added the execOk check + 422 + flash error path mirroring activityUpdateAction. [places/show.php docblock] @var docblock missing $relation_count (introduced by F038). PHPStan would warn on the next sweep. Added. PHPStan clean on the 3 PHP files modified; 183/183 RiC-O JSON-LD unit tests green; 4 locales validate + parity preserved at 5080. Review artifact: rev_20260520T133227Zda602c
Applies the 6 audit-only findings the previous round documented but
skipped. NO limits added to public search/list endpoints per project
direction (users explicitly want unbounded ric.json + ricCollection
public reach; DoS surface accepted as known trade-off).
[L2-F5] archives/views/show.php — 3rd typeahead instance
(authority-attach on the unit detail view) was missing the F036
ARIA combobox/listbox/option pattern, the F039 loading indicator,
and keyboard handlers. Now matches activities/show.php +
places/show.php pattern: role=combobox + aria-autocomplete +
aria-expanded/aria-controls/aria-activedescendant on input,
role=listbox + stable id on results, role=option on each item;
ArrowDown/ArrowUp navigation, Enter selects, Escape closes;
showLoading() between input and fetch resolve; aria-expanded=true
even in the no-results branch.
[L2-F6] searchAutori — LIKE wildcard injection (raw `%` / `_` in
user-supplied q collapses the predicate). Replaced with
`'%' . addcslashes($q, '%_\\') . '%'`, matching the sibling fix
at the CQL parser (line 3510) and searchArchivalUnits (4679).
Pre-existing in autori-search but now closed.
[L2-F11] safeReturnTo — `../` path-traversal segments bypassed the
/admin/archives/ allow-list because str_starts_with is a
byte-prefix check, not path-normalised. Browsers normalise
Location headers before re-requesting, escaping the gate. Added
explicit reject of any `(^|/)\.\.?(/|$)` segment before the
allow-list check.
[L6-F1] handleAssetUpload predecessor unlink — the cover-overwrite
and document-replacement branches both unlinked the previous file
using `__DIR__ . '/../../../public' . $existingPath` with only an
is_file() check. Same path-traversal class as F007 (the
removeAssetAction fix). Both sites now apply realpath()
containment check before unlink: resolve allowed root and
candidate, reject when resolved path is not strictly under the
allowed root.
[L1-F6 / L2-F12] dateLiteral PHP_INT overflow — preg_match
`/^-?\d+$/` accepted '9999999999999999999' which (int) would
silently truncate to PHP_INT_MAX. Added `$value === (string) (int) $value`
round-trip check that rejects strings whose int cast doesn't
round-trip exactly.
[L1-F5 OAI sibling] oaiGetRecord generic Throwable catch — the
CR fix added a CannotDisseminateFormatException catch with three
endElement() calls, but a generic non-CDF error inside
writeMetadata (UTF-8 XMLWriter error, RuntimeException from
RicJsonLdBuilder) would still propagate with elements half-open.
Mirrored oaiListRecords's symmetric `catch (\Throwable $e)`
branch: close the three elements, log via SecureLogger::warning,
surface as cannotDisseminateFormat so harvesters can recover.
Skipped per user direction:
- L6-F2 (ricCollectionAction no LIMIT) — public reach is intentional
- L6-F3 (fetchRelationsForEntity no LIMIT) — same
Skipped as too low-impact for code change (documented in artifact):
- L2-F8 Enter mid-fetch race — narrow corner case, existing
state machine handles it gracefully (Enter is no-op when
activeIdx=-1, intended)
- L2-F10 F033/F038 asymmetry across activities/places — semantic
parity already partial; activities uses linkedUnits count, places
uses archive_relations count; both meaningful in their domain
- L2-F13 ricExposedCache / archivalUnitsTableExists redundancy —
two caches with different lifecycles; merging requires careful
invalidation sequencing not warranted by current load
- L2-F14 F007 logging noise on missing-file — `realpath()==false`
is the legitimate already-deleted case; we silently skip rather
than log spam
- L2-F15 Math.random() option-id collision — birthday probability
on 36^6 namespace is ~10^-7 per render; not worth a counter
PHPStan clean, 183/183 RiC-O JSON-LD unit, locale parity (5080) preserved.
Review artifact: rev_20260520T133227Zda602c
fabiodalez-dev
force-pushed
the
feat/ric-cm-archives
branch
from
647c058 to
e056e98
Compare
fabiodalez-dev
added a commit
that referenced
this pull request
May 22, 2026
…able tests CodeRabbit fix on PR #141 ------------------------- attachSwalConfirm's confirmText default was unconditionally 'Elimina', even when data-swal-confirm-kind="action" routed the dialog through SwalApp.confirm (the neutral/gray helper). A form opting into kind="action" without an explicit data-swal-confirm-button would render a gray button labelled "Elimina" — the wrong destructive cue. Default now follows the kind: action→Conferma, destructive→Elimina. Explicit data-swal-confirm-button still wins for both. UX follow-ups (3 non-destructive forms) --------------------------------------- - admin/languages/index.php: set-default-language → kind=action - utenti/dettagli_utente.php: activate-directly → kind=action - utenti/index.php: activate-directly → kind=action These three flows are reversible (re-set language, deactivate user) and were showing a red destructive dialog. Now neutral/gray. 49 reusable Playwright tests ---------------------------- - tests/sweetalert2-comprehensive.spec.js (25 tests): Block A — window.SwalApp bus contract (6) Block B — attachSwalConfirm auto-wire / idempotence / proceed-marker (5) Block C — kind routing + confirmText defaults (3, validates this fix) Block D — i18n & data-attr propagation (3) Block E — refactored views per-page assertions (8, login-gated, skip-clean) - tests/pr-139-event-image-additional.spec.js (11 tests): Block S — controller orphan-cleanup symmetry + path-traversal hardening + locale completeness + settings/route contract (9) Block H — admin-side HTTP contract checks (2, login-gated) - tests/pr-136-archives-ric-cm-additional.spec.js (13 tests): Migration version-le-release invariant (project rule #6), ensureSchema() pattern compliance, plugin.json bumps, RiC-O entity coverage, README freshness. Reads files from origin/feat/ric-cm-archives via 'git show', so the suite works from any branch without checking out. Validation ---------- - 39 no-login tests run green in ~30s - 10 login-gated tests skip cleanly when E2E_ADMIN_PASS is absent - All 13 pr-136 tests pass against origin/feat/ric-cm-archives Refs: PR #141 (closes #140), PR #139 (closes #137), PR #136 (closes #122)
Forces a fresh GitHub Actions run on origin/feat/ric-cm-archives to check whether the long-standing Phase 17.4 / Phase 18 slowness (10 consecutive failures since 2026-05-19) is reproducible on the current GitHub Actions runner pool. If the new run reproduces the 67x slowdown on /catalogo?genere=, the bug is deterministic on the migration set and a local repro + migration bisect is the next step. If it passes, the previous fails were infra-related and the branch is mergeable. No code changes.
The previous empty-commit re-trigger surfaced a *different* failure
mode than the long-standing one: full-test.spec.js now passes on
GitHub Actions' refreshed runner pool, but archives-crud.spec.js
fails at step 1 with:
ERROR 2002 (HY000): Can't connect to local MySQL server through
socket '/var/run/mysqld/mysqld.sock' (2)
Root cause: the test's `mysqlArgs()` helper only supports DB_SOCKET
(Unix socket) for the mysql CLI. CI runs MySQL in a Docker container
exposed over TCP at 127.0.0.1:3306, with no Unix socket reachable
from the runner host — the CLI falls back to its default socket
path and 2002s out. The 3 specs touched here were the only archives
specs still using the socket-only pattern; archives-pr-extended,
archives-phase5-admin-ui, archives-phase6-oai-ric-o and
archives-ric-jsonld already had the TCP-aware version.
Fix: mirror the canonical mysqlArgs() from archives-pr-extended.
- Add DB_HOST / DB_PORT env reads.
- Connection precedence: TCP (-h/-P) → socket (-S) → defaults.
- Pass the password via MYSQL_PWD env instead of -p${DB_PASS} so
empty passwords don't trigger an interactive prompt that hangs
the suite, and the secret no longer appears in argv.
Cascading fail: when [10/13] archives-crud failed, [11/13]
archives-authorities and [12/13] archives-upload-assets would have
hit the same MySQL connection failure on the next run. All three
fixed together so the next E2E run exercises the full pipeline.
No application code changes.
Refs: PR #136 (closes #122)
After the TCP MySQL connection fix landed, [10/13] archives-crud and
[11/13] archives-authorities started passing, but [12/13]
archives-upload-assets surfaced its own pre-existing dependency:
Error: no seeded row available for PDF test
Expected: > 0
Received: 0
> expect(pdfId, 'no seeded row available for PDF test').toBeGreaterThan(0);
The upload-assets spec picks 3 archival_units from the `E2E_SEED_%`
batch but doesn't create them itself. The seed lives in
archives-seed-50.spec.js, which seeds 50 rows covering the full option
matrix. The CI workflow ran upload-assets directly on a clean install,
so no E2E_SEED_* row existed and the beforeAll bailed.
Two options to fix this:
A. Insert a separate seed step before [12/13] and renumber the
remaining steps. Cleaner enumeration, more invasive diff.
B. Run seed-then-upload as a single CI step. Minimal diff, the
two specs stay co-located by responsibility.
Going with B — the dependency is local (upload-assets needs rows that
seed-50 provides) and the [12/13] step name already advertises the
upload pipeline as a unit. Adding a 14th step would also mean
re-numbering every step downstream of [12/13] in this PR + every
backport.
Refs: PR #136 (closes #122)
After the CI seed step landed, archives-upload-assets.spec.js
progressed past the missing-row check and surfaced the real reason
test 1 was failing:
Expected pattern: /^\/uploads\/archives\/covers\/\d+-[a-f0-9]{8}\.jpg$/
Received string: "/uploads/archives/covers/10-840844d15dbcb7c3.jpg"
ArchivesPlugin.php line 2082 generates the basename via
`bin2hex(random_bytes(8))` (16 hex chars, 64-bit entropy) — the
explicit FIX F015 doubled the previous `random_bytes(4)` to
reduce collision probability when the same id+ext pair is uploaded
many times in a session. The test file kept the pre-F015 `{8}`
match, so every successful upload appeared as a regex mismatch.
Bulk-update the 6 regex sites (3 cover paths + 2 document paths +
1 frontend body assertion) from `[a-f0-9]{8}` to `[a-f0-9]{16}`.
No other files were carrying the 8-hex pattern.
Refs: PR #136 (closes #122), F015 (rev_01KRRE2QJR3QSTGJ4FZEK7MVDW)
[13/13] archives-pr-extended.spec.js test 1 was failing with:
Error: EACCES: permission denied, open
'/home/runner/work/Pinakes/Pinakes/public/uploads/archives/documents/60-e2e-doc-a.pdf'
> 212 | fs.writeFileSync(absPath, Buffer.from('stub'), { flag: 'wx' });
The IIIF rendering[] tests create stub files directly via fs from the
runner process. [12/13] upload-assets ran real uploads through Apache
(running as www-data), so public/uploads/archives/documents/ ended up
owned by www-data with 0755 perms — the runner could no longer write
into it. The pre-step chmod 777 only ran ONCE at startup, before any
Apache-owned subdir existed.
Fix: re-chmod the uploads tree at the head of [13/13] right before
the test runs. Idempotent (chmod 777 on an already-777 tree is a
no-op), `|| true` swallows the rare case where the subdir hasn't been
created yet (e.g. previous step's tests all skipped).
Refs: PR #136 (closes #122)
4 tasks
Owner
Author
|
Superseded by #144 (combined/release-unified) — the unified release branch merges this work alongside the other two open PRs into a single release. All commits from this branch are preserved in the merge; CI green on the combined branch. This PR is closed, not merged — the branch |
fabiodalez-dev
added a commit
that referenced
this pull request
May 26, 2026
3 of the 11 spec failures from the full local suite run were real test-side bugs (not application regressions). The other 8 failures are environment/DB-seed dependent and require a clean install fixture to exercise — they will pass in CI which runs against a fresh install per job. #5 (interop-document-coverage:165, 172, 178) — migration rename PR #136 renamed migrate_0.7.0.sql → migrate_0.7.00.sql (and 0.7.4 → 0.7.04, etc.) so the Updater's new version_compare-based usort orders them correctly across semver vs lexicographic readers. Three tests in interop-document-coverage hardcoded the old filenames and failed with ENOENT. Updated all three to the new names; added a comment so a future archaeologist understands the rename history. #6 (interop-specific:188) — BIBFRAME bf:Work owl:sameAs The test claimed "bf:Work has no owl:sameAs" — but the BIBFRAME plugin's FIX F045 (BibframeLinkedDataPlugin.php:368) intentionally exposes a Work-level VIAF identifier as owl:sameAs on the bf:Work node, mirroring the per-author bf:agent owl:sameAs pattern. The test predates F045. Rewrote test 5 to assert the F045 contract: if owl:sameAs is present, it must be a valid VIAF URI; if absent (libro without viaf_id), that's also legitimate. #9 (pr132-fix-regressions:83) — SRU diagnostic <uri> tag The test pattern /<uri>/ required the bare element. After the createElementNS migration, <uri> is emitted with an explicit xmlns attribute (<uri xmlns="info:srw/diagnostic/1/">). The regex now correctly matches the open-tag form via /<uri[\s>]/ — accepts both bare and namespace-attributed forms. Validation ---------- - PHPStan level 5 → No errors - interop-document-coverage tests 20-22 → 3/3 pass - interop-specific test 5 → pass - pr132-fix-regressions F003-1 → pass
ctariel
pushed a commit
to ctariel/Pinakes
that referenced
this pull request
Jun 4, 2026
…ied release branch Brings in the full RiC-CM (Records in Contexts Conceptual Model) roadmap — 6 phases: - Phase 1: RiC-O JSON-LD read-only export - Phase 2: Agents as first-class entities - Phase 3: Activities as first-class entities - Phase 4: Places + polymorphic Relations graph - Phase 5: Native admin UI for activities/places/relations - Phase 6: OAI-PMH metadataPrefix=ric-o Plus the CI-side fixes that landed during the validation pass: - TCP MySQL connection for archives test specs (66be026) - CI seed-50 inline before upload-assets step (0526647) - Regex 8→16 hex chars per F015 entropy bump (194c9f5) - chmod uploads tree before pr-extended (43d36cd) Refs: PR fabiodalez-dev#136 (closes fabiodalez-dev#122)
ctariel
pushed a commit
to ctariel/Pinakes
that referenced
this pull request
Jun 4, 2026
…dalez-dev#136 + fabiodalez-dev#141 When the two PRs landed independently on their respective branches, each branch carried inline @PHPStan-Ignore annotations and a baseline entry pinned to errors that no longer reproduce on the combined tree: - ScrapeController.php:769 — Z39Server plugin classes are autoloaded via storage/plugins from the archives PR; `class.notFound` no longer fires. - DiscogsPlugin.php:28 — the `hookManager` property is now used further down the file; the "kept for interface compatibility" ignore is obsolete. - GoodLibPlugin.php:44/90/98 — constructor.unusedParameter + method.unused on onInstall / onUninstall: the methods are now called by the activated PluginManager surface. - BibframeLinkedDataPlugin.php:720 — `is_string($predicate)` was always true because the foreach key is typed as string by `@param array<string, mixed> $node`. Drop the redundant check (no behaviour change — the rest of the conditional already rejects @id/@type/non-colon keys). - phpstan-baseline.neon — the 3 entries (LibriController CURL constant + BookRepository isset offsets) all stopped reproducing after the merge. Empty baseline now (the array is still present so a future restore can append cleanly). PHPStan level 5 passes on the combined tree: $ phpstan analyse --memory-limit=512M [OK] No errors
ctariel
pushed a commit
to ctariel/Pinakes
that referenced
this pull request
Jun 4, 2026
…+ fabiodalez-dev#139 + fabiodalez-dev#136 Adds tests/unified-release-regression.spec.js — a focused regression suite that targets the user-observable contract each of the 3 combined PRs introduces. Designed to sit alongside the existing per-PR specs (sweetalert2-comprehensive, pr-139-event-image-additional, pr-136-archives-ric-cm-additional, full-test, etc.) as a fast, re-runnable safety net for the release branch. Distribution (25 tests): Block U-SWAL (8) — PR fabiodalez-dev#141 popup unification S1: SwalApp method surface S2: confirmText default branches on isAction (CodeRabbit fix) S3: defensive native fallback when Swal absent S4: one-shot data-swal-proceed marker (not attribute mutation) S5: 3 non-destructive forms set kind="action" (UX follow-up) S6: it_IT.json carries SwalApp default copy H1: window.SwalApp initialised on admin pages H2: data-swal-confirm forms auto-attached on /admin/utenti Block U-EVT (8) — PR fabiodalez-dev#139 event image layout (fabiodalez-dev#137) S1: 4-preset enum in settings/index.php S2: event-detail.php renders event-cover--<preset> S3: SettingsController validates with in_array S4: 4 deleteUploadedImageFile lifecycle callsites S5: path-traversal hardening (prefix + realpath containment) S6: @media restyle for thumb layout H1: /admin/settings?tab=cms renders the layout picker H2: form posts to /admin/settings/events with CSRF Block U-RIC (9) — PR fabiodalez-dev#136 RiC-CM full roadmap (fabiodalez-dev#122) S1: phase 2/3/4 + cleanup migration files S2: version.json ≥ highest migration version S3: 0.7.09.sql creates Phase 3 tables idempotently S4: 0.7.10.sql creates Phase 4 places + relations S5: ArchivesPlugin ensureSchema() called from BOTH hooks S6: RicJsonLdBuilder covers RecordResource/Agent/Activity/Place S7: OaiPmhServerPlugin handles metadataPrefix=ric-o S8: archives plugin.json ≥ 1.5.0, oai-pmh-server ≥ 1.1.0 H1: /archives/collection.ric.json returns JSON-LD Design choices: - 16 tests run without admin login (static checks on repo files + one no-auth HTTP smoke for the public RiC-O endpoint). - 9 tests require login — they test.skip() cleanly on missing credentials, wrong password, or missing seed state (legitimate skip, never fail). - No state mutation — every assertion reads HTML / JSON / static files. Re-runnable in any order, parallel-safe. Run: /tmp/run-e2e.sh tests/unified-release-regression.spec.js \ --config=tests/playwright.config.js --workers=1 Validated locally with E2E_ADMIN_PASS set: 24 passed, 1 skipped (U-SWAL-H2 when /admin/utenti has no pending users — legitimate seed state, not a test bug).
fabiodalez-dev
added a commit
that referenced
this pull request
Jun 4, 2026
… and stale-name test fixes - EmailService: only enable SMTPAuth when an SMTP username is configured. Forcing SMTPAuth=true broke auth-less relays (local MTAs, internal smarthosts, Mailpit) which reject the AUTH command. Real production fix, not just tests. - viaf-authority plugin: skip CSRF validation for Basic-Auth requests. CSRF protects cookie/session auth (a browser auto-attaches the session cookie cross- site); a request carrying a Basic Authorization header has no ambient credential to ride and no session-bound token, so the JSON API was returning 403 to legitimate API/script clients (and the E2E suite). - code-quality test 11: match phpstan case-sensitively. The /gi regex flagged nesbot/carbon's legitimate Carbon\PHPStan\* macro-extension classes in the optimized classmap as a phpstan-analyzer leak. Case-sensitive matches the analyzer package path (lowercase) like the project's own grep check. - fr-bnf + email tests: reference migrate_0.7.04.sql (PR #136 rename) and point the Mailpit SMTP config at no-auth so emails are delivered. Validated: viaf-authority 16/16, code-quality 11 ✓, fr-bnf 90 ✓, email-notifications 13/14 (the remaining C.1 exercises the PHP mail() driver, which needs a system MTA and is environmental).
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
2 participants
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.
Summary
This PR implements the complete RiC-CM (Records in Contexts Conceptual Model, ICA 2023) roadmap on the Archives plugin and on the OAI-PMH server. All six phases land together as a single release: Pinakes v0.7.6 → v0.7.12,
archivesplugin 1.2.x → 1.5.0,oai-pmh-serverplugin 1.0.0 → 1.1.0.Closes #122.
Phases in this PR
/archives/{id}/ric.json,/archives/agents/{id}/ric.json,/archives/collection.ric.jsonmigrate_0.7.08.sql(+4 cols onauthority_records, +2 tables)/archives/agents/{id}/ric.json(enriched)migrate_0.7.09.sql(+2 tables:archive_activities,archive_unit_activities)/archives/activities/{id}/ric.json,/archives/activities/ric.jsonmigrate_0.7.10.sql(+2 tables:archive_places,archive_relations)/archives/places/{id}/ric.json,/archives/places/ric.json/admin/archives/activities/*,/admin/archives/places/*,/admin/archives/relations/*,/api/archives/entitiesmetadataPrefix=ric-ometadataPrefix=ric-oon/oai(archival_unit-only)archive_activities.place_idcolumnmigrate_0.7.12.sqlPhase 1 — RiC-O JSON-LD
Pure read layer. Three public endpoints serve Records in Contexts Ontology JSON-LD: single archival unit, single authority record, synthetic top-level RecordSet. The
RicJsonLdBuilderhelper is pure (no DB, no I/O on the JSON-LD path) and fully unit-tested. Each response carries anHTTP Link: <…>; rel="alternate"; type="application/ld+json"header andrdfs:seeAlsocross-references to the matching DC/EAD/METS/IIIF representations.Phase 2 — Agents
authority_recordsgains four columns (ric_typeENUM,birth_date,death_date,place_of_origin). Two new tables:archive_agent_identifiers(multi-valued VIAF/ISNI/Wikidata/GND/BNF/LCNAF/Getty ULAN/ARK identifiers) andarchive_agent_relations(agent ↔ agent typed edges with self-loop CHECK constraint).Phase 3 — Activities
archive_activitiescovers ISDF-aligned activity types (Function/Activity/Transaction/Task/Mandate), hierarchicalparent_id, optional performingagent_id, structureddate_start/date_end/is_ongoing,source_reffor the legal/normative basis.archive_unit_activitiesis the M:N link with VARCHARric_predicate(open vocabulary).Phase 4 — Places + polymorphic Relations
archive_placescovers the fourth entity type with full LOD identifier coverage (GeoNames, Wikidata, Getty TGN).archive_relationsis the polymorphic backbone —source_type+source_id/target_type+target_idreference one ofarchival_unit,authority_record,archive_activity,archive_place. Single table beats 16 specialised link tables — RiC-O has dozens of predicates and the schema must grow without migrations.Phase 5 — Admin UI
Native CRUD for activities/places + polymorphic attach/detach for relations +
/api/archives/entitiestypeahead. UI chrome mirrors the existing admin views (Tailwindp-6 max-w-4xl mx-auto,bg-white shadow rounded-lg p-6 space-y-5,form-label, breadcrumb, indigo-600 primary actions). All user-facing strings are Italian-source__()wrappers with full translations inlocale/en_US.json,locale/fr_FR.json,locale/de_DE.json.Phase 6 — OAI-PMH
ric-oThe
oai-pmh-serverplugin now advertisesric-oas a metadataPrefix for thearchivesset and emits canonical RDF/XML serialised from the same RiC-O graph the JSON-LD endpoints produce. NewRicJsonLdBuilder::serializeToRdfXml()translates the JSON-LD compact document to canonical RDF/XML:@id→rdf:about/rdf:resource,@type→tag name (CURIE expanded), language tags viaxml:lang, typed literals viardf:datatype, nested blank nodes for inline objects.v0.7.12 cleanup migration
Drops the
archive_activities.place_idcolumn that was added in 0.7.9 as a placeholder for a Phase 4 FK that was never added. Wrapped inIF @col_existsguards so the migration is idempotent.Migration robustness
The 5 RiC-CM migrations (
0.7.07through0.7.12) are filename-zero-padded so lex sort matches semver (otherwise0.7.10would run before0.7.9— the bug that surfaced in reinstall-test Test B). They are wrapped in INFORMATION_SCHEMA guards so an upgrade from a release that PRE-DATES the archives plugin (v0.7.6) silently skips the DDL — the plugin'sensureSchema()creates the full Phase 2-6 schema on first activation.Updater::runMigrations()also got ausort()semver sort as a safety net for future minor families (0.8+).Test plan
storage/plugins/archives/,storage/plugins/oai-pmh-server/,app/Support/Updater.php).tests/archives-ric-jsonld.unit.phppassing. +13 new assertions across Phase 6 (RDF/XML serializer with bool/int/float/typed-reference/@id='0'edge cases) andattachRelationsToRicDoc.scripts/reinstall-test.shTest A (fresh install v0.7.12) + Test B (real admin UI upgrade from GitHub v0.7.6 → v0.7.12 via uploaded ZIP + click Avvia, no shortcuts) both ✅.commit-msgandpre-commitgit hooks pass on every phase commit.vendor/composer/autoload_static.phpclean —grep -c phpstanreturns 0.Reviewer guide
The most interesting files to review, in dependency order:
storage/plugins/archives/RicJsonLdBuilder.php— pure helper.serializeToRdfXml()at the bottom is the JSON-LD → RDF/XML canonicaliser.installer/database/migrations/migrate_0.7.{08,09,10,12}.sql— schema migrations withINFORMATION_SCHEMAguards for safe upgrades from pre-Phase-1 installs.storage/plugins/archives/ArchivesPlugin.php— bulk of the diff. Phase 5 admin handlers near the end;registerRoutes()shows the full surface.storage/plugins/archives/views/{activities,places}/— 6 admin views. Italian-source__()strings, Tailwind chrome matching the books admin.storage/plugins/oai-pmh-server/OaiPmhServerPlugin.php— Phase 6.writeArchivalUnitRicO()is the entry point.app/Support/Updater.php—runMigrations()semver sort fix.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Interoperabilità
Documentation
Tests
Chores