Skip to content

WIP: Fixed stale pending acceptances on checkin and transfer, and reconcile accessory quantities - #19555

Draft
marcusmoore wants to merge 18 commits into
grokability:developfrom
marcusmoore:fixes/handle-stale-pending-acceptances
Draft

WIP: Fixed stale pending acceptances on checkin and transfer, and reconcile accessory quantities#19555
marcusmoore wants to merge 18 commits into
grokability:developfrom
marcusmoore:fixes/handle-stale-pending-acceptances

Conversation

@marcusmoore

@marcusmoore marcusmoore commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Currently, an unanswered acceptance request could outlive the checkout it belonged to and the user will continue to see it on /account/accept.

Scenarios where this occurs

Mostly shared setup

  1. Checkout the develop branch
  2. Silence all three notification channels. Clear CC Email and uncheck Always send copy upon checkin/checkout; /admin/slack: leave the webhook endpoint empty.
  3. Create a categories for accessories, licenses, and assets with "Require users to confirm acceptance of assets in this category" on and "Send email to user upon checkin/checkout" off.
  4. Create two users with email addresses that have the ability to login.
  5. Create an accessory and check it out to the user but do not accept or decline it. This is to show it survives the clean up migation included in this PR.

Scenario 1. Checkin never clears the acceptance when notifications are off

  1. Check out the accessory to the user.
  2. As that user, /account/accept lists the request. Leave it unanswered.
  3. As an admin, check the accessory back in.
  4. Reload /account/accept: the request is still there, and still signable.

The same steps will show the same issue for a license whose category has the settings from above.

Scenario 2. Transferring a license seat never cleared the source user's acceptance

  1. Create a license and check out a license seat to a user and leave the request unanswered.
  2. Use the "Transfer Items" feature to transfer the license from that user to another user.
  3. See that the seat moved to the other user but the original user can still see the request.

Scenario 3. Checking in one unit of a multi-unit accessory clears the whole acceptance

  1. Update the accessory category and enable Send email to user on check-in/check-out.
  2. Create an accessory for the category with the quantity set to 3 or more.
  3. Check out 3 units to a user in one action.
  4. As A, /account/accept shows one request with a quantity of 3. Leave it unanswered.
  5. Check one unit back in (or Users → A → Transfer Items one unit to user B).
  6. See that the original user still holds two units but the request has been removed.

In this PR: the request remains with a quantity of 2.

Scenario 4. Checking in one item clears a same-id item's acceptance

  1. Keep Send email to user on checkin/checkout enabled for the accessory category.
  2. Create an accessory for the category.
  3. Take the new accessory's id and find an asset that shares that id.
  4. Check out the asset and accessory that share an id to a user.
  5. See the pending acceptances for the user (/account/accept) but do not respond.
  6. Check in the accessory.
  7. See that the asset's acceptance has been removed along with the accessory's acceptance.

The same collison happens for license seat ids.


The Fix

Snapshot the pending rows from the checkout_acceptances table:

SELECT id, checkoutable_type, checkoutable_id, qty
FROM checkout_acceptances
WHERE assigned_to_id = <A> AND deleted_at IS NULL
  AND accepted_at IS NULL AND declined_at IS NULL
ORDER BY id;
  1. Checkout this branch
  2. Run migrations which includes 2026_08_24_234955_clean_stale_pending_acceptances
  3. Re-run the query
Row left by develop What A actually has After migrate
Scenario 1, accessory none of it gone
Scenario 1, license seat seat unassigned gone
Scenario 2, license seat seat is B's now gone
Scenario 3 quiet, accessory qty 3 2 units survives at qty 2
Control accessory all of it untouched

Note: Scenario 4 is not repaired. On develop that bug soft-deleted the asset's valid request, and a destroyed request is not something this can distinguish from one a normal checkin retired. The asset stays checked out to A with nothing pending; re-requesting acceptance is the only remedy. The fix stops it happening again but it does not undo it.

What changed

The core issue is that the CheckoutableListener could exit before doing any checkout acceptance clean up depending on notification settings. This PR ensures that clean up logic runs.

For accessories: a checkin retires one unit from the oldest pending row (via decrement or deletion if the qty is 1) rather than a row that may be worth three.


AI Disclosure: I used Claude to help investigate the underlying issues as well as write a lot of the code. I went through and cleaned stuff up.

@marcusmoore
marcusmoore requested a review from snipe as a code owner August 26, 2026 00:54
@marcusmoore
marcusmoore marked this pull request as draft August 26, 2026 00:54
@codacy-production

codacy-production Bot commented Aug 26, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 medium

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category Results
Complexity 2 medium

View in Codacy

🟢 Metrics 62 complexity

Metric Results
Complexity 62

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant