Skip to content

Decide the status from the last approved operation - #59

Closed
loevgaard wants to merge 1 commit into
fix/confirm-payment-approved-authorizefrom
fix/status-last-approved-operation
Closed

Decide the status from the last approved operation#59
loevgaard wants to merge 1 commit into
fix/confirm-payment-approved-authorizefrom
fix/status-last-approved-operation

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Fixes robustness finding R2 of #57. Stacked on #58 (shares the pending-operation test fixture) — merge that first; this PR's own diff is the last commit.

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.
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.75%. Comparing base (5cfe38b) to head (9123481).

Additional details and impacted files
@@                             Coverage Diff                              @@
##             fix/confirm-payment-approved-authorize      #59      +/-   ##
============================================================================
+ Coverage                                     97.43%   97.75%   +0.31%     
- Complexity                                      126      129       +3     
============================================================================
  Files                                            14       14              
  Lines                                           351      356       +5     
============================================================================
+ Hits                                            342      348       +6     
+ Misses                                            9        8       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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