Two defects in the place picker, found while reading the panel a booker checks their price against. The first is what a booker sees; the second is what the endpoint behind it allows.

The panel counted a house-given place twice

A booker taking four places in an area whose places the house gives out saw the panel's total at twice the basket's: the panel priced each seat, and priced the block below it as well. The block of such an area is rebuilt from its seats on every answer, so the same four units were in front of the panel twice, once named in the list and once counted in the block.

The basket, the order and the amount charged were right throughout. Only the panel's total was wrong, and it is the one number on that surface a booker checks against what they will be charged.

Such a place is now read in the panel and set nowhere: it says which offer the house gave it at, and its area's block is the one surface that prices it. The mix of offers in such an area is what the block's steppers are for, and a select on the seat was a second surface setting the same thing.

Repricing a held place asked the engine nothing

The endpoint's "tariff" operation wrote the tariff onto the held line and saved it. Every rule about which offer a place may be taken at is enforced where units are taken, and this operation went nowhere near it: not that the tariff is one of the resource's, not that it prices the seat's grade, not that its allotment has a unit left, not how many one booking may hold at it.

So the offers a booker was shown bounded nothing. A request naming any tariff on the site was obeyed, including one belonging to another resource, and the price a booker ended up holding was not one the hall had offered them for that seat.

One of the four is reachable from the panel as it stands, with no crafted request at all: the seat's control offers every offer that prices its grade, what an allotment has left is not something the client can see, and the operation asked nobody. A hall that put one place on sale at a reduced offer could be made to hold two of them.

The move is now asked of the engine as one call under one lock, with the held line given up as part of taking the new one. The unit leaving the offer it sat at pays for the unit joining the other, and a refusal rolls back both, which leaves the place theirs at the offer it was already held at. This is the engine's own exchange path, the one a party closing the gap in its row already uses.

Measured on the same fixture, first operation in a fresh process: a reprice goes from 13 queries to 31, against 17 for taking one place and 37 for giving one back and taking one in a single run. So it costs the same order as a click, less than the giving-back-and-clicking-again it exists to save, and it takes one lock where that route takes two.

Why neither was caught

The panel's total is asserted for chosen places and for areas offered by quantity, and never for an area whose places the house gives out: that class had seven scenarios and none of them read the total. The "tariff" operation had no test at all, of any kind, so nothing described what it was allowed to refuse.

Both gaps are closed. The browser class now takes two places in such an area and reads the total, and four kernel tests drive the operation: the reprice that works, and one for each rule that was going unasked. Three of the four were seen to fail against the code as it stands.

One finding this does not fix

One finding this does not fix, and a correction to the first version of this paragraph, which said the rule was read-side only. It is not: whether the front being served offers a resource or a tariff is enforced at the hold, by RequestBooker. What is true is narrower and odder. This endpoint enforces it on some of its own operations and not others: an assigned-area take goes through RequestBooker and is refused, while a seat click, a relocate and this reprice compose their own lines and call the engine directly, so they are not. Filed as #3618660: Move the offer-on-this-front rule inside the engine, where every other rule already is, which moves the rule inside the engine, where every other eligibility rule already is.

AI-Generated: Yes (Claude Code was used to find both defects, to draft this summary, and to write the fix, its four kernel tests and its browser scenario. The measurements quoted were taken with the query logger; three of the four kernel tests were confirmed to fail against the code as it stands and to pass with the change.)

Issue fork yoyaku-3618650

Command icon 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

mably created an issue. See original summary.

mably’s picture

Status: Active » Needs review

  • mably committed bde8eed7 on 1.x
    fix: #3618650 Count a house-given place once, and put a reprice through...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

mably’s picture

Issue summary: View changes

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.