Issue #3614361: The seating strategies left PlaceAssignment and availablePlaces without a caller

Follow-up to !150 (merged) (merged as ceb5317), which introduced the SeatingStrategy plugin and left three leftovers behind in the released 1.x branch.

1. An unused injection made PlaceAssignment dead

VenuePlacementProvider::place() now asks the session's seating strategy for a proposal, but the constructor still took PlaceAssignment, and yoyaku_placement.services.yml still passed it. Nothing in the class referenced $this->placeAssignment: git log -S"placeAssignment->" shows ceb5317 removed the last call.

With that argument gone, PlaceAssignment::proposeForTariff() (its only public method) had no caller but its own test, and the yoyaku_placement.place_assignment service no consumer at all. Its logic is a strict subset of what Together does: the first contiguous run in the first section that fits, otherwise a scattered slice. So the class, its service and PlaceAssignmentTest go.

Three docblocks cited PlaceAssignment as a co-holder of the "a rate naming no grade prices every place" rule (VenueAvailabilityBound::freeFor(), VenuePlacementProvider::pools(), Together). They now name freeCandidates(), which is what applies it.

2. availablePlaces() outlived the path it was written for

PlaceAvailability::availablePlaces() had no production caller either, only three test call sites. It ran an entity query over a whole venue, loadMultiple()ed the result and filtered taken and closed places in PHP, which is the pattern #3614038 and #3614075 removed from the map and the proposal paths. Keeping it meant the tests held a hall-loading read alive purely to assert against.

Removed, and the three call sites moved onto freeCandidates():

  • ConfigurationTest gains a freePlaceIds() helper, so the closed-section assertions read assertContains() on ids instead of assertArrayHasKey().
  • VenueMapBuilderTest takes $free[0]->id instead of array_key_first().
  • ApiPlacementTest's two assertCount() calls are unchanged.

The test conditions are preserved rather than weakened: both methods call the same activeConfiguration() and restrictToPlaceMarket(), and filter the same CONSUMING_STATES bookings, so "the closed subsection is not available" and "the seats the order took are no longer on offer" still mean what they meant. The rewritten assertions can still fail, too: each closed-place test asserts the open place is present in the same array it asserts the closed ones are absent from, so an empty read fails instead of passing vacuously.

placeOpen() stays, because PlaceBookable still calls it.

3. A stray duplicate docblock

PlaceAvailability carried a verbatim copy of freeCandidates()'s docblock floating above candidatesByIds()'s own, documenting a $grade_ids parameter the next method does not have. phpcs never flagged it: a docblock followed by a blank line is attached to nothing, so the Drupal.Commenting.* sniffs never inspect it.

Scope

No behavior change, no user-facing string, no schema and no config, so there is nothing to document or translate. Net: 26 insertions, 633 deletions.

Verified locally before pushing: phpcs (Drupal + DrupalPractice, --warning-severity=1) clean on the whole module; phpstan on the CI ruleset clean apart from the pre-existing new.static findings d.o ignores; cspell clean on the touched files; and the whole yoyaku kernel suite green, 113 of 113 classes passing, including every class whose assertions changed (ConfigurationTest, ApiPlacementTest, VenueMapBuilderTest) as well as VenuePlacementProviderTest and SeatTogetherTest. The Functional and FunctionalJavascript lanes are left to CI here.

Edited by Frank Mably

Merge request reports

Loading
Loading