Provided code example for detecting successful transactions is misleading

[Synchro]

General information

The example here suggests that this is how you check for a successful transaction, doubly so since the else case demonstrates handling an unsuccessful one:

if ($result->success || !is_null($result->transaction)) {
    $transaction = $result->transaction;
    header("Location: " . $baseUrl . "transaction.php?id=" . $transaction->id);
} else {
    $errorString = "";

    foreach($result->errors->deepAll() as $error) {
        $errorString .= 'Error: ' . $error->code . ": " . $error->message . "\n";
    }

    $_SESSION["errors"] = $errorString;
    header("Location: " . $baseUrl . "index.php");
}

However, this is not true. This check will succeed even for failed transactions because it's not obvious that a failed request will still contain a transaction object, even though the docs say that the transaction object is

Returned directly or within a successful result object

the implication being that it will not be returned in an unsuccessful one.

Basing our code on this misleading example has caused us actual financial losses, so I really recommend that you present this differently. For example starting with:

if ($result->success && $result->transaction) {

would detect a successful transaction (as far as I can make out from your docs), and other result combinations could be handled appropriately.

[hollabaq86]

👋 @Synchro thanks for reaching out. We do have a disclaimer that these examples are NOT production ready and this example is only meant for educational purposes.

That said, we could make the error handling a little more robust, but I can't provide an ETA on when those changes will get made.

[kimboslice99]

Examples containing poor practice shouldn't be used whatsoever.

Are we looking to set a good example here? or just throwing up the absolute minimum half-baked code.

[hollabaq86]

@kimboslice99 you're always welcome to submit a PR