Skip to content

Rename computed genre keys to resolved_* (+ escape breadcrumb label)#290

Merged
fabiodalez-dev merged 1 commit into
mainfrom
refactor/genre-resolved-keys
Jul 24, 2026
Merged

Rename computed genre keys to resolved_* (+ escape breadcrumb label)#290
fabiodalez-dev merged 1 commit into
mainfrom
refactor/genre-resolved-keys

Conversation

@fabiodalez-dev

Copy link
Copy Markdown
Owner

Follow-up to discussion #286 and PR #287.

The book genre hierarchy exposed two request-only computed keys —
genere_id_cascade / sottogenere_id_cascade — right next to the stored
libri columns genere_id / sottogenere_id, with nothing to signal that the
former are a derived view rather than persisted duplicates. That ambiguity is
what read as "tangled and misleading" in #286.

Change

  • Rename genere_id_cascaderesolved_genere_id, sottogenere_id_cascade
    resolved_sottogenere_id. The resolved_ prefix marks them as a
    computed view of the stored ids.
  • resolveGenreHierarchy() (producer) + both consumers — the edit-form cascade
    pre-population and the admin book-detail breadcrumb (from Fix admin book genre hierarchy display #287) — updated
    together. The keys are purely in-memory (never a column, never serialized to
    an API), so the change is contained to three files.
  • Documented the convention on resolveGenreHierarchy().
  • Switched the breadcrumb label to htmlspecialchars(..., ENT_QUOTES, 'UTF-8')
    (the view standard), resolving the remaining CodeRabbit nit on Fix admin book genre hierarchy display #287.

Not renamed (on purpose)

radice_id / radice_nome / genere_nome / sottogenere_nome are also base
getById() SELECT aliases consumed by LibriApiController, PublicApiController,
the UNIMARC/MARC exports and the LibraryThing import — renaming them would break
those contracts. Only the two purely-computed _cascade keys are in scope.

Verification

Live check on a book stored at root+direct-child (genere_id=root,
sottogenere_id=child): the breadcrumb renders Root → Child with the correct
distinct ids, not the old duplicate/empty chain. php -l + PHPStan level 5 clean;
the Phase 18 genre regression in full-test.spec.js covers it.

…cape in view

The book genre hierarchy exposed two request-only computed keys, genere_id_cascade
and sottogenere_id_cascade, sitting right next to the stored libri columns
genere_id/sottogenere_id with no signal that they are derived, not persisted —
which is exactly what read as redundant/misleading in discussion #286.

Rename them to resolved_genere_id / resolved_sottogenere_id so the `resolved_`
prefix marks them as a derived view of the stored ids. resolveGenreHierarchy()
(producer) and its two consumers — the edit form's cascade pre-population and
the admin book-detail breadcrumb (#287) — are updated together; the keys are
purely in-memory (never a column, never serialized to any API), so the blast
radius is these three files. radice_id/radice_nome/genere_nome/sottogenere_nome
are intentionally NOT renamed: they are also base-query SELECT aliases consumed
by the API controllers, exports and imports.

Also switched the breadcrumb label from HtmlHelper::e() to the view-standard
htmlspecialchars(..., ENT_QUOTES, 'UTF-8') (the earlier CodeRabbit nit on #287).

Verified live: a book stored at root+direct-child (genere_id=root,
sottogenere_id=child) renders the breadcrumb as "Root -> Child" with the correct
distinct ids, not the old duplicate/empty chain. php -l + PHPStan level 5 clean.
The genre regression in full-test.spec.js (Phase 18) covers it.
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@fabiodalez-dev, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: acd75ee7-294d-4d01-bae0-0cd1b09ee61c

📥 Commits

Reviewing files that changed from the base of the PR and between 5128fd2 and 1e77ec3.

📒 Files selected for processing (3)
  • app/Models/BookRepository.php
  • app/Views/libri/partials/book_form.php
  • app/Views/libri/scheda_libro.php
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/genre-resolved-keys

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@fabiodalez-dev

Copy link
Copy Markdown
Owner Author

Code review

Branch: refactor/genre-resolved-keysmain
PR: #290 (open)
Review ID: rev_20260724T112856Zefcc5b
Base freshness: local main was behind origin/main at run start; fast-forwarded before review
Fix threshold: not yet set (run /adamsreview:fix [threshold] to apply fixes)
Sub-agent tokens: 442,449 across 4 invocations

Found 1 finding across all lanes:



🤖 Generated with Adam's Claude Code Review Command

@fabiodalez-dev
fabiodalez-dev merged commit 37a1b7b into main Jul 24, 2026
6 checks passed
fabiodalez-dev added a commit that referenced this pull request Jul 24, 2026
The What's New section only described the Danish-language PR; this release
also bundles bulk loan extension (#281), the mobile catalogue language
filter (#282), the home available-count + responsive related-books fix
(#288), Book Club external-book reconciliation (#138), PHP 8.5 compat
(#289), and the genre-key rename (#286/#290). Documented each user-facing
change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant