Skip to content

Retry CC feature - #17

Merged
detain merged 7 commits into
masterfrom
gtest
Apr 24, 2026
Merged

detain merged 7 commits into
masterfrom
gtest

var cc_id undefined error and other updates - considering few other s…

5c5fad5
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 23, 2026 in 11m 26s

Code review found 4 important issues

Found 5 candidates, confirmed 4. See review comments for details.

Details

Severity Count
🔴 Important 4
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/cc.inc.php:765-766 retry_charge_card leaks ot_cc/retry_cc into shared request state
🔴 Important src/cc.inc.php:573 RETRY_CC constant is never defined (fatal on PHP 8+)
🔴 Important src/cc.inc.php:13 make_cc_decline/email_cc_decline deletion breaks Plugin.php registrations and tests
🔴 Important src/cc.inc.php:567 Decline email never sent (multiMail call removed)

Annotations

Check failure on line 766 in src/cc.inc.php

See this annotation in the file changed.

@claude claude / Claude Code Review

retry_charge_card leaks ot_cc/retry_cc into shared request state

🟠 `retry_charge_card()` writes `App::variables()->request['ot_cc']` and `['retry_cc']` at lines 765-766 but never clears them before returning — no `unset()`, no `try/finally`. In a long-running PHP process that iterates over customers (billing cron, queue worker), customer A's retry poisons subsequent iterations: customer B's `charge_card()` reads the stale `ot_cc` and, because `parse_ccs()` uses sequential 0/1/2 int keys, is highly likely to collide with B's `$ccs` — silently charging B's **wr

Check failure on line 573 in src/cc.inc.php

See this annotation in the file changed.

@claude claude / Claude Code Review

RETRY_CC constant is never defined (fatal on PHP 8+)

RETRY_CC at src/cc.inc.php:573 is referenced bare (`RETRY_CC == 1`) without a `defined()` guard, but the constant is declared nowhere in the plugin (repo-wide grep for RETRY_CC returns only this line and the two `retry_cc` request-param uses). On a host that hasn't externally defined RETRY_CC, PHP 8.0+ throws a fatal Error and the entire decline branch 500s before the retry logic, the paypal fallback, or the decline email run; on PHP 7.x it emits E_WARNING and `'RETRY_CC' == 1` evaluates to fals

Check failure on line 13 in src/cc.inc.php

See this annotation in the file changed.

@claude claude / Claude Code Review

make_cc_decline/email_cc_decline deletion breaks Plugin.php registrations and tests

This PR deletes `make_cc_decline()` and `email_cc_decline()` from `src/cc.inc.php` but leaves three stale references: `src/Plugin.php:67-68` still registers them as lazy-loadable requirements, `tests/CcFunctionsTest.php:255-256` asserts their `function make_cc_decline(`/`function email_cc_decline(` strings are present (so `testAllExpectedFunctionsDeclared()` now fails deterministically), and `CLAUDE.md:33` still documents them as core CC functions. Fix by either dropping the Plugin.php registrat

Check failure on line 567 in src/cc.inc.php

See this annotation in the file changed.

@claude claude / Claude Code Review

Decline email never sent (multiMail call removed)

The decline branch builds the payment-failed email body into $email via $smarty->fetch('email/client/payment_failed.tpl') at src/cc.inc.php:567, but the `(new \MyAdmin\Mail())->multiMail(...)` call that actually sent it was deleted in this PR — the variable is computed and silently discarded. Customers now receive no decline notification in any path (retry succeeds, retry fails, or the else-branch paypal downgrade), which is a regression from the prior unconditional send. Fix by restoring the mu