Issue #3616340: Let an operator name a subsection, and store a main-body place as zero rather than null

Two halves, both about the number a place carries

A place in the main body of its section stores the sentinel, not NULL. Numbers start at 1, so 0 is free to mean that no subsection is named, and a section setting has always stored it: a unique key cannot tell one NULL from another, so a NULL scope would let one section hold two contradicting section-wide settings. A place insisted on NULL instead, which left one idea with two encodings and made every query narrowing places by subsection spell out the NULL case beside the comparison, since a NOT IN is neither true nor false for a NULL row and drops every main-body place out of the answer.

SubsectionNumber is now the one home for the sentinel and says why it exists; ConfigurationSection::WHOLE_SECTION is gone, so 0 has one name. PlaceStorageSchema declares the column NOT NULL with the sentinel as its default, which makes the invariant real rather than conventional - that is what lets the isNull() disjunct come out of restrictToBookablePlaces() instead of being left as a fallback. The accessors go on answering NULL, so nothing above storage learns a new value.

Three reads changed meaning once no row is NULL, and each is fixed where it sits: the area form asked which rows exist to find the subsections a venue uses, which would have loaded the whole hall; the exporter wrote any non-empty value, which would have put subsection: 0 into every package for every place; and the layout round trip built its identity key from the raw column, which would have read key/0/A/12 and matched no marker already written.

A section names its subsections. A translatable multi-value subsection_names field: the first item names subsection 1, the second names subsection 2. So a screen offering the front of the balcony can call it what the house calls it instead of "Balcon, subsection 2".

The names go on the section rather than in records of their own, and the reason is the cost. Every screen that names an area has already loaded the section, so the names arrive with it and naming an area runs no query at all. A record of its own cost a read per section - or a priming pass to get that back to one - plus two tables, a unique key, a delete cascade, an admin screen, and a schema install in every test that touches a map. SubsectionTest measures the claim with a query log rather than asserting it from the design.

The names are positional and the field says so plainly: give them in order and leave none blank, because Drupal keeps no empty item in a multi-value field, so a gap closes up and every name after it would move onto the wrong subsection. A test pins that too, since it is the one way this can mislead.

AreaName is the one reader, because the surfaces have to agree: it chooses between the section's own name, the name the section gives that subsection, and the section with the number where nothing names it. The picker said "Balcon, subsection 2" while the numbering report said "Balcon, part 2" and the layout labels said "Balcon 2". The picker now asks the reader; the other two follow when they hold a section rather than an array of columns.

A name never becomes an identity. A place, a section setting and a booking all go on carrying the bare number, so a name is only ever a name.

Verification

Kernel classes covering the sentinel, the pool scope keys, the area names, the translations, the layout round trip, the io package and the map query budgets pass locally on SQLite. phpcs (Drupal + DrupalPractice) and phpstan are clean over the module, and msgfmt --check passes on the .po.

The two sentinel tests were run against the unpatched entity and seen to fail - the column held NULL, and the main-body place fell out of a NOT IN - then passed with the change. VenueIoTest needed one change worth naming: it asserted the storage detail rather than the promise, so it now asks the accessor whether an imported place has a subsection; its neighbouring assertion, that a place naming no subsection exports without the key, passed unchanged and is what pins the exporter fix.

Note on the history

The branch was force-pushed once. An earlier commit modelled the names as a yoyaku_subsection entity; that was dropped in favour of the field for the query cost above, and rewritten out rather than left in the history as a design nobody should copy.

Edited by Frank Mably

Merge request reports

Loading
Loading