BookingHooks::forgetPolicyOutcomes() drops two things when an order or one of its lines changes: the resolved constraint policy outcomes it is named for, and the basket lines a request keeps, added in #3615974: The basket's held lines are read again for every caller in a request, so the visitor's own endpoint costs a query it already paid.
The name says one of them. Someone reading it to learn what a change invalidates gets half the answer, and the next thing that needs dropping has no obvious home: adding it here makes the name wronger, and adding a second hook on the same events splits one rule across two places.
Naming it after writing would also be wrong in a third of its cases. It is attributed to yoyaku_booking_delete as well, and a line that has gone invalidates as much as one that has arrived.
Proposed resolution
Name it for the rule: forgetStaleAnswers(). Both members are answers worked out once and kept for the length of a request, which is exactly the property that makes a change invalidate them, and a name saying that holds whatever is added next. The hook attributes carry the real hook names, so the method name is free, and nothing outside the class calls it.
The docblock should lead with the rule and keep the per-member reasoning underneath: why the policy evaluation is remembered at all, which shipped test fails without it, and why the hosts an attachment hangs on are deliberately left out.
Remaining tasks
- Rename, and rewrite the docblock around the rule rather than around the policy outcomes.
- Say in it that this runs on deletes too, so the next reader does not narrow it back to writes.
- Check nothing refers to the old name.
Issue fork yoyaku-3615978
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 #5
mably commented