Skip to content

Decide the status from the last approved operation - #65

Open
loevgaard wants to merge 1 commit into
2.xfrom
fix/status-last-approved-operation
Open

Decide the status from the last approved operation#65
loevgaard wants to merge 1 commit into
2.xfrom
fix/status-last-approved-operation

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Fixes robustness finding R2 of #57. Supersedes #59, which was auto-closed when its stacked base branch (#58's) was deleted on merge — #58 is now in 2.x, so this targets 2.x directly and the diff is the R2 change alone.

Problem

StatusAction decided from Operations::latest() — the last operation recorded, not the last one that succeeded:

  • processed + [capture 1000, refund attempt the acquirer rejected] → markUnknown(), although the 1000 is demonstrably still held (balance > 0 is already in hand at that point).
  • processed + [capture, async refund still pending] → markUnknown() until the callback lands.
  • new + [approved authorize, rejected capture attempt] → markFailed(), although the authorization is intact and a retry is possible.

Downstream consumers (Sylius state machines) treat unknown/failed as dead ends, so a payment holding real money fell out of the flow.

Fix

New Operations::latestApproved() returns the most recent operation with an approved status code, skipping trailing rejected/pending attempts. Both the new and processed branches of StatusAction now decide from it. The refund branch's balance logic (partial refund stays captured, zero balance is refunded) is unchanged, as is markUnknown when nothing approved exists at all.

Operations::isLatestApproved() is no longer used by the package but stays — it is public API.

Tests

Rejected-refund-stays-captured, pending-refund-stays-captured, rejected-capture-stays-authorized, new with no operations pins failed, unmodeled state string pins unknown, plus direct Operations::latestApproved() coverage.

StatusAction decided from Operations::latest() alone, so a trailing
rejected or still-pending attempt masked what actually happened to the
money: capture 1000 then a bounced refund attempt reported unknown while
1000 was demonstrably still held, and an authorized payment whose capture
attempt failed reported as failed while its authorization was intact.

Both branches now read Operations::latestApproved() — the last operation
that DID succeed. Payum consumers (Sylius state machines in particular)
treat unknown/failed as dead ends, so the mark must not degrade just
because the most recent attempt did not go through.
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