Problem/Motivation
An audit before the next release, over performance, security, Drupal standards, documentation, translations and duplication.
Six dimensions came back clean, recorded here so the next audit starts from them: no interpolated SQL, no superglobals, no dynamic t(), and every route in all 14 routing files carries an access requirement; no deprecated core APIs, and every static \Drupal:: call is in an entity or a render element, which core builds rather than the container; 1974 strings extracted and every one translated, with 7 documented exceptions; every module shipping config ships schema; seats on the plan are real buttons with a label apiece; all 42 info files agree on ^11.3 || ^12.
Worth naming among those: no N+1 on the booking path. Every per-iteration read the audit found sits on a cache fill or an admin screen, and the seat map is already cached above the loop.
Three things did not come back clean.
Five loops read once per turn. Each asked the database once per iteration where one read does, and none of them is on the request a booker waits for. That is worth saying plainly, because the seat map looks like it should be: getAreaSections() loaded a section per area, so sixteen areas cost sixteen reads, but getSettledState() above it is cached per slot, content language and booking channel, so the loop runs on a cache fill rather than per booking. The venue-scoped widget did the same per option on an admin form, an admin table read a grade per row, a resource type a message per attached id on config save, and the demo controller a booking per id.
They are real reductions on the paths they sit on: a cache fill still pays sixteen reads for one on every invalidation, and an admin table pays one per row. They are not a fix to booking latency.
Nine routes spelled out the same five lines to open an add form for a child of the thing being looked at: create the child with its parent's id and tenant, then hand back its add form.
Two documents named a class that no longer exists. The no-code page said the example's CheckoutController "is gone", which is history rather than a description. The resource type design described the world before its own change in the present tense, down to a literal gateway string that appears nowhere in the code now.
Proposed resolution
Each loop reads what it needs in one go, and the detector that found them reports none.
The storage and form-builder plumbing moves into AddChildFormTrait, while the type created and the fields naming the parent stay at the call site where a reader can see them. VenueInventoryController's own venue helper goes through it as well.
Both documents say what is true today.
The instanceof in calculateDependencies() went with the loop it guarded: an id naming no message is absent from what loadMultiple() returns, so there was nothing left to check.
Naming: four names that said the wrong thing
SeatingShareOut told a reader nothing. Its own first line says what it is: it decides which of a run's places goes to which tariff, so it is PlaceTariffMatcher, with matchToTariffs() and tryToMatchPlace() inside it rather than shareOut() and an augmenting path.
The matcher also called its demand side slots, and a slot is the event here: read that way the code looked like bookings being shuffled between events, when nothing in the class touches a booking or a slot at all. It now says which seat was asked for and which place fills it, naming both sides so that neither is mistaken for the other.
Three resolvers had borrowed unit, which belongs to UnitAssignment and BookingUnit, for a bare variable holding 'hours' or 'days'. Those say $time_unit. The field names carrying a unit of time explain themselves and are left alone.
Four predicates asked "is what?", because the subject of the question sat in the first argument: isPricing($grade_ids, $place) does not say that grades never price, a tariff does. Each one's own summary said what it meant, and core names a predicate is or has, so they are isPlacePriced(), isPlaceInArea(), isPlaceMovedByClaim() and isRowStillAcceptable().
Remaining tasks
None.
User interface changes
None. The same pages, with fewer queries behind them.
API changes
ChildEntityAddFormTrait::getChildEntityAddForm() is new, for a controller extending ControllerBase.
The service yoyaku_placement.seating_share_out is now yoyaku_placement.place_tariff_matcher, and its class SeatingShareOut is PlaceTariffMatcher. Nothing outside the module referenced either, in yoyaku, orchestra or kessai.
Data model changes
None.
AI-Generated: Yes (Claude Code was used to help draft this issue summary and to write the code on the merge request. I reviewed and ran the work myself before posting it.)
Issue fork yoyaku-3620326
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mably commentedComment #4
mably commentedComment #5
mably commentedComment #7
mably commented