#5002: the rescue stamp is spent once the appointee uses it
VERIFY #5008 attacked the state #5002's own tests never built: not the rescue that goes wrong and is undone within the minute, but the one that WORKS. A group that was orphaned, was rescued, and is now being run by the person the rescue appointed — with `recovered_by` still sitting on that person's row, because the only thing that cleared it was the appointee re-saving their own row, a write nobody makes. So arm 3 of requireRemoveAccess() stayed open on the sole working manager of a group that is plainly somebody else's business, which is the line the method's own docblock says the stamp is narrow enough not to cross: strip the manager, then "rescue" the group that press just orphaned, and a site admin who never ran the desk manages it with the manager who staffed it gone. Two presses. settleRescueStamp() clears the stamp on the actor's own row the moment they ACT as the manager the rescue made them — staff the group, rename it, archive it, give it a project or an email connection, remove a row: anything this file lets a manager do to their own group. It is called from the three gates, not from each action, because the gates are where "this write was authorised by the actor's OWN manager row" is already decided, and a read of the page never comes through them. The undo therefore lasts exactly as long as the mistake it is for: until somebody takes the appointment up. removableAgentIds follows without a change — it reads `recovered_by`, so the button disappears with the gate. green: test_a_successful_rescue_does_not_leave_the_rescuer_able_to_take_the_group_over test_a_stamp_earned_on_one_group_is_not_a_claim_on_another all 17 of OrphanRecoveryIsReversibleTest, 215 Feature/Ticketing, 3917 tests/Unit Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -94,6 +94,40 @@ class TicketingSettingsController extends Controller
|
||||
if (!$this->isManagerOf($groupId)) {
|
||||
abort(403);
|
||||
}
|
||||
|
||||
$this->settleRescueStamp($groupId);
|
||||
}
|
||||
|
||||
/**
|
||||
* THE RESCUE STAMP ENDS WHEN THE APPOINTEE USES WHAT IT GAVE THEM — #5008.
|
||||
*
|
||||
* `recovered_by` (#5002) exists so a rescuer can take back an appointment
|
||||
* they got wrong, including a `manager` press that shut the orphan arm
|
||||
* behind it. It was written as an undo and it has to expire like one. As
|
||||
* first built, the only thing that cleared it was the appointee re-saving
|
||||
* their own row — a write nobody makes — so the stamp sat on the row for
|
||||
* good, and a successful rescue left the rescuer a standing lever over a
|
||||
* desk somebody else was now running: strip the manager, then "rescue" the
|
||||
* group that press just orphaned, and a site admin who never ran it
|
||||
* manages it (#5008). That is #4710's line, which the undo must not cross.
|
||||
*
|
||||
* So the stamp settles the moment the appointee ACTS as the manager they
|
||||
* were made: staffs the group, renames it, archives it, gives it a project
|
||||
* or an email connection — anything this file lets a manager do to their
|
||||
* own group. The rescue has been taken up, the group is theirs, and the
|
||||
* rescuer's claim on it is spent. Until then nothing has happened but the
|
||||
* press itself, which is exactly the case the undo is for.
|
||||
*
|
||||
* Called from the gates and not from each action because the gates are
|
||||
* where "this write was authorised by the actor's OWN manager row" is
|
||||
* already decided; a read of the page never comes through them.
|
||||
*/
|
||||
private function settleRescueStamp(int $groupId): void
|
||||
{
|
||||
TicketingAgentAccess::where('user_id', Auth::id())
|
||||
->where('group_id', $groupId)
|
||||
->whereNotNull('recovered_by')
|
||||
->update(['recovered_by' => null]);
|
||||
}
|
||||
|
||||
/** Does the signed-in user hold a `manager` row on this group? */
|
||||
@@ -195,6 +229,8 @@ class TicketingSettingsController extends Controller
|
||||
private function requireManagerAccessOrOrphanRecovery(int $groupId): void
|
||||
{
|
||||
if ($this->isManagerOf($groupId)) {
|
||||
$this->settleRescueStamp($groupId);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -687,12 +723,20 @@ class TicketingSettingsController extends Controller
|
||||
* rescuer taking back their own appointment claims nothing they did
|
||||
* not already hold at the moment they made it.
|
||||
*
|
||||
* And it lasts only until the appointee USES the access — see
|
||||
* settleRescueStamp(). Otherwise the undo outlives the mistake it is
|
||||
* for: a rescue that WORKED leaves the stamp on the row of the person
|
||||
* now running the desk, and arm 3 becomes a permanent lever for
|
||||
* stripping them and taking the group over (#5008).
|
||||
*
|
||||
* Removing the last manager re-orphans the group, which is the correct
|
||||
* end state: it is recoverable again, by whoever gets there next.
|
||||
*/
|
||||
private function requireRemoveAccess(TicketingAgentAccess $access): void
|
||||
{
|
||||
if ($this->isManagerOf($access->group_id)) {
|
||||
$this->settleRescueStamp($access->group_id);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user