feat(archives): unified admin search now surfaces archival units#120
Merged
fabiodalez-dev merged 7 commits intomainfrom May 4, 2026
Merged
feat(archives): unified admin search now surfaces archival units#120fabiodalez-dev merged 7 commits intomainfrom
fabiodalez-dev merged 7 commits intomainfrom
Conversation
…ugin)
The admin global search bar now surfaces archival_units alongside books,
authors, and publishers when the Archives plugin is active.
Implementation:
- SearchController::unifiedSearch() applies a new filter hook
`search.unified.sources` after the existing publishers query, allowing
active plugins to append their own results to the JSON array.
- ArchivesPlugin registers `addArchivalSources` for that hook in onActivate()
and inserts it in the plugin_hooks DB table so the hook survives server
restarts (same pattern as app.routes.register / admin.menu.render).
- addArchivalSources() uses LIKE on constructed_title + formal_title +
scope_content (ISAD(G) 3.3.1) instead of FULLTEXT: short reference codes
("IT-MI-1") and year fragments ("1943") are found reliably regardless of
MySQL's minimum full-text word length.
- layout.php handles the new type:"archive" in both the desktop and mobile
search dropdowns with a green archive icon and reference_code as identifier.
Closes part of #103 (unified search milestone).
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughIl controller di ricerca unificata tronca i risultati core a 15 elementi, invoca l'hook ChangesUnified search + Archives plugin + UI
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Controller
participant Hooks
participant ArchivesPlugin as Plugin
participant DB
User->>Frontend: invia query q
Frontend->>Controller: GET /search?q
Controller->>DB: query core sources (books, authors, publishers)
DB-->>Controller: core results
Controller->>Controller: slice(core results, 0, 15)
Controller->>Hooks: apply('search.unified.sources', results, [q])
Hooks->>Plugin: call addArchivalSources(results, q)
Plugin->>DB: prepared LIKE query su archival_units
DB-->>Plugin: fino a 5 corrispondenze
Plugin->>Plugin: mappa corrispondenze (id,label,identifier,type='archive',url)
Plugin-->>Hooks: return risultati augmentati
Hooks-->>Controller: risultati finali
Controller->>Controller: slice(results, 0, 20)
Controller-->>Frontend: JSON response con risultati
Frontend->>User: render (elementi archive mostrano identifier & icona)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 38 minutes and 52 seconds.Comment |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
fabiodalez-dev
added a commit
that referenced
this pull request
May 2, 2026
…AD3, fix broken links - Fix broken links: create guida/arricchimento.md and en/guide/bulk-enrichment.md - Add guida/dischi.md + en/guide/discs.md: media types, dynamic labels, Discogs/MusicBrainz/Deezer plugins - Add guida/collane.md + en/guide/series.md: series hierarchy (cycles, seasons, spin-offs) from v0.5.9.6 - Update guida/archivi.md + en/guide/archives.md: unified search (PR #120) + interoperability section (OAI-PMH 2.0, Dublin Core XML, EAD3 bulk export — PR #127) - Update _sidebar.md + en/_sidebar.md: add Dischi/Discs and Collane/Series entries - Update changelog: add v0.5.9.6 entry (series hierarchy) and "In arrivo" section (PR #118/119/120/127) - Update tecnico/requisiti.md + en/technical/requirements.md: note on zip extension installer check (PR #118)
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Controllers/SearchController.php`:
- Around line 202-204: The plugin-added results from
\App\Support\Hooks::apply('search.unified.sources', ...) are being appended to
$results but then lost by the later array_slice(..., 0, 20); to fix, call
Hooks::apply into a separate variable (e.g. $pluginResults) instead of appending
directly to $results, decide on a reserved slot count (e.g. $reservedForPlugins
= 3), trim the core $results (from SearchController) to max 20 -
$reservedForPlugins, then merge $pluginResults into the trimmed core results and
only then apply array_slice(..., 0, 20) to produce the final list so extension
results are guaranteed reserved slots.
In `@storage/plugins/archives/ArchivesPlugin.php`:
- Around line 144-147: The hook registrations in ArchivesPlugin:: (calls to
registerHookInDb) are not atomic and may leave the plugin partially registered;
wrap the multiple registerHookInDb calls in a database transaction/atomic block
(beginTransaction/commit) and if any registerHookInDb fails, roll back and
rethrow the exception so the caller sees the error; specifically modify the
method that registers these hooks (the block containing
registerHookInDb('app.routes.register', 'registerRoutes', 10),
registerHookInDb('admin.menu.render', 'renderAdminMenuEntry', 10),
registerHookInDb('search.unified.sources', 'addArchivalSources', 10)) to start a
transaction before the first call, commit after all succeed, and catch
exceptions to rollback and rethrow.
🪄 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: 123bbd92-cfed-43d5-bd6e-1724435b4183
📒 Files selected for processing (3)
app/Controllers/SearchController.phpapp/Views/layout.phpstorage/plugins/archives/ArchivesPlugin.php
…ration - SearchController: pre-slice core results to 15 before Hooks::apply so plugin sources (archives) are not systematically truncated by the global cap of 20 (fixes CodeRabbit finding — array_slice after hook was a no-op) - ArchivesPlugin::onActivate: wrap 3 registerHookInDb calls in a single transaction so partial activation is impossible on DB error
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
storage/plugins/archives/ArchivesPlugin.php (1)
144-153:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLa transazione qui non è davvero atomica.
registerHookInDb()continua a loggare e tornare senza lanciare: se uno degli insert fallisce, ilcatchnon scatta e ilcommit()può confermare una registrazione parziale.Proposta di fix
- if ($stmt === false) { - SecureLogger::error(...); - return; - } + if ($stmt === false) { + SecureLogger::error(...); + throw new \RuntimeException(...); + } - if (!$stmt->execute()) { - SecureLogger::error(...); - } + if (!$stmt->execute()) { + SecureLogger::error(...); + throw new \RuntimeException(...); + }As per coding guidelines
storage/plugins/**: Hook registration: transazione + rethrow on failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@storage/plugins/archives/ArchivesPlugin.php` around lines 144 - 153, The transaction around the hook registration is not atomic because registerHookInDb() logs and returns on failure instead of throwing, so commit() can succeed after partial inserts; change the flow so failures trigger rollback by either making registerHookInDb() throw on error or by checking its return value in the transaction block and throwing an exception when it returns false; update the transaction code that calls begin_transaction(), registerHookInDb('app.routes.register', ...), registerHookInDb('admin.menu.render', ...), registerHookInDb('search.unified.sources', ...) to detect any failure and rethrow (so rollback() runs) before calling commit(), ensuring atomic hook registration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@storage/plugins/archives/ArchivesPlugin.php`:
- Around line 144-153: The transaction around the hook registration is not
atomic because registerHookInDb() logs and returns on failure instead of
throwing, so commit() can succeed after partial inserts; change the flow so
failures trigger rollback by either making registerHookInDb() throw on error or
by checking its return value in the transaction block and throwing an exception
when it returns false; update the transaction code that calls
begin_transaction(), registerHookInDb('app.routes.register', ...),
registerHookInDb('admin.menu.render', ...),
registerHookInDb('search.unified.sources', ...) to detect any failure and
rethrow (so rollback() runs) before calling commit(), ensuring atomic hook
registration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d2f0f818-afc5-4b10-99fe-18d968ce4ba2
📒 Files selected for processing (3)
app/Controllers/SearchController.phpapp/Views/layout.phpstorage/plugins/archives/ArchivesPlugin.php
The begin_transaction/commit/rollback wrapper in onActivate() was not truly atomic because registerHookInDb() swallowed errors via log+return. Now both failure paths (prepare() == false, execute() == false) log AND throw so the catch block in onActivate() actually triggers rollback.
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 the current code and only fix it if needed.
Inline comments:
In `@app/Support/PluginManager.php`:
- Around line 120-133: L'upgrade viene già scritto nella tabella plugins prima
di chiamare onActivate() (qui quando chiami instantiatePlugin(...) e poi
->onActivate()), quindi se onActivate fallisce bisogna impedire che l'upgrade
venga considerato riuscito: nella catch(\Throwable $e) al posto di limitarsi a
SecureLogger::warning(...) ripristina lo stato precedente (es. la colonna
version del plugin al valore precedente $dbVersion o imposta is_active a 0
usando l'id del plugin passato a instantiatePlugin) e poi fallisci
esplicitamente l'operazione (rilanciando l'eccezione o restituendo/propagando un
errore) in modo che version_compare() ritenti l'upgrade al prossimo boot; fai
riferimento a instantiatePlugin, onActivate e alla catch corrente per collocare
il rollback.
In `@storage/plugins/archives/ArchivesPlugin.php`:
- Around line 4019-4031: The unified search query omits reference_code from the
LIKE conditions so searches for codes like "IT-MI-1" never match; update the SQL
in ArchivesPlugin (the prepared statement selecting id, reference_code,
constructed_title from archival_units) to include "OR reference_code LIKE ?" in
the WHERE clause and adjust the bind_param call to pass the $s parameter for
that additional placeholder (i.e., add another 's' and another $s argument) so
reference_code is searched with the same wildcard pattern as
constructed_title/formal_title/scope_content.
🪄 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: df0280fc-9cc7-40cd-816f-f4f5b865883f
📒 Files selected for processing (3)
app/Support/PluginManager.phpstorage/plugins/archives/ArchivesPlugin.phpstorage/plugins/archives/plugin.json
When a bundled plugin is re-deployed with the same version already in the DB, autoRegisterBundledPlugins skipped onActivate because version_compare reported no change. Any hooks added to the codebase between the two deploys would be silently absent from plugin_hooks. Add an elseif branch for diskVersion === dbVersion + is_active: calls onActivate (which runs ensureSchema, a DELETE+INSERT idempotent cycle) so the DB always reflects the current hook set. Failure is non-fatal (logs a warning, does not rethrow) since running with stale hooks is better than breaking the admin plugin page entirely. Regression test added to tests/plugin-manager.unit.php.
Playwright spec used by reinstall-test.sh step A.7 to functionally exercise the autoRegisterBundledPlugins elseif branch: 1. reinstall-test.sh activates archives plugin via SQL and wipes its plugin_hooks rows (simulating a same-version re-deploy with new hooks) 2. This spec logs in as admin and navigates to /admin/plugins, which calls getAllPlugins() → autoRegisterBundledPlugins() 3. reinstall-test.sh then verifies via MySQL that all hooks were restored Confirms the fix works end-to-end without any deactivate/reactivate.
This was referenced May 5, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SearchController::unifiedSearch()now applies asearch.unified.sourcesfilter hook after the existing books/authors/publishers queries, so active plugins can append their own result sets without touching the controllerArchivesPluginregistersaddArchivalSourcesfor that hook inonActivate()(stored inplugin_hooksDB table, same pattern asapp.routes.register)addArchivalSources()searchesarchival_unitsvia LIKE onconstructed_title,formal_title, andscope_content(ISAD(G) 3.3.1) and returns results in the unified format withtype: 'archive'layout.phphandles the newtype: 'archive'in both desktop and mobile search dropdowns with a green archive icon (fas fa-archive) and thereference_codeas the identifier sub-lineDesign decisions
LIKE vs FULLTEXT:
addArchivalSourcesuses LIKE instead of FULLTEXT. Short reference codes (IT-MI-1), year fragments (1943), and partial call numbers work correctly with LIKE but are silently dropped by MySQL FULLTEXT (minimum word length + stopword list). FULLTEXT is still used in the dedicated/admin/archives/searchpage where users write longer queries.Hook pattern: The
search.unified.sourcesfilter hook was already documented inArchivesPlugin::plannedHooks()— this commit promotes it from "planned" to "active". New plugins can contribute to unified search with zero changes toSearchController.Test plan
Part of #103
Summary by CodeRabbit