Jump to content
thirty bees forum
  • 0

Paypal shipping bug report (not Paypal specific)


Question

Posted (edited)

[Bug] Order created with €0.00 shipping after PayPal payment when customer adds/changes a delivery address mid-checkout — root cause traced to Cart::updateAddressId()

Summary

When a customer adds or changes their delivery address during checkout and pays via PayPal (Website Payments Plus / "plussubmit" flow), PayPal correctly charges the full amount including shipping — but the Order that gets created in thirty bees shows €0.00 shipping, and this incorrect value is also what appears in the back-office order view and the admin notification email.

I traced this through the actual module and core code (thirty bees 1.x, modules/paypal — the classic module, not tbpaypalcheckout) and found what looks like a genuine core bug in Cart::updateAddressId(), not something PayPal-module-specific — PayPal's redirect-based flow just happens to be a reliable way to trigger it.

Real-world data (anonymized shop, values otherwise unmodified)

  • id_cart: 8659
  • id_address_delivery on the cart: 27116
  • delivery_option on the cart (raw JSON): {"27115":"174,"}
  • id_carrier on the cart: 174 ("Worldwide", weight-tiered pricing, includes Asia zone)
  • Delivery country: Kazakhstan (Asia zone)
  • PayPal charged: correct total, shipping included
  • Resulting Order: total_shipping = 0, admin notification email also shows no shipping line

Note the address ID mismatch: 27115 (key in delivery_option) vs 27116 (actual id_address_delivery) — a difference of exactly 1, consistent with a new/duplicate address record having just been created during checkout.

Steps to reproduce (best guess based on the trace below — not yet lab-confirmed)

  1. Customer has an existing address, or starts checkout normally
  2. Customer adds a new delivery address during checkout (e.g. a second address for a different country)
  3. Customer pays via PayPal Website Payments Plus (redirects to PayPal, then returns)
  4. Order is created with total_shipping = 0, despite PayPal having charged the correct amount

What I ruled out first

My first suspicion was this hardcoded value in modules/paypal/controllers/front/plussubmit.php, inside displayAjax():

$transaction = [
    'id_transaction' => $payment->transactions[0]->related_resources[0]->sale->id,
    'payment_status' => $payment->state,
    'total_paid' => $payment->transactions[0]->amount->total,
    'id_invoice' => 0,
    'shipping' => 0,   // <-- hardcoded, always zero
    'currency' => $payment->transactions[0]->amount->currency,
    'payment_date' => date("Y-m-d H:i:s"),
];

This looked like a smoking gun, but tracing it through PayPal::validateOrder()parent::validateOrder() (core PaymentModule::validateOrder()) shows the $transaction/$extra_vars array is only used for transaction_id (via $order->addOrderPayment($amount_paid, null, $transaction_id)). It is not used anywhere to compute or set total_shipping. So this hardcoded 0 doesn't feed into the actual Order's shipping value — it's dead/misleading, but not the root cause of this symptom.

(It is passed to PayPalOrder::saveOrder() right after, into the module's own paypal_order tracking table used for refunds — so it may be worth fixing separately, since refund calculations reading from that table would see a wrong shipping value there. But that's a secondary finding, not the main bug.)

The actual root cause: Cart::updateAddressId() never touches delivery_option

classes/Cart.php:

public function updateAddressId($idAddress, $idAddressNew)
{
    $toUpdate = false;
    if (!isset($this->id_address_invoice) || $this->id_address_invoice == $idAddress) {
        $toUpdate = true;
        $this->id_address_invoice = $idAddressNew;
    }
    if (!isset($this->id_address_delivery) || $this->id_address_delivery == $idAddress) {
        $toUpdate = true;
        $this->id_address_delivery = $idAddressNew;
    }
    if ($toUpdate) {
        $this->update();
    }

    $conn = Db::getInstance();
    $conn->update('cart_product', ['id_address_delivery' => (int) $idAddressNew], '`id_cart` = '.(int) $this->id.' AND `id_address_delivery` = '.(int) $idAddress);
    $conn->update('customization', ['id_address_delivery' => (int) $idAddressNew], '`id_cart` = '.(int) $this->id.' AND `id_address_delivery` = '.(int) $idAddress);
}

This method correctly rewrites the address ID everywhere else on the cart — id_address_delivery, id_address_invoice, cart_product.id_address_delivery, customization.id_address_deliveryexcept in $this->delivery_option, the JSON field that maps {id_address: chosen_delivery_option_key}. That field is never read, updated, or cleared here, so it keeps pointing at the old address ID indefinitely.

How that stale key produces exactly €0.00 downstream

Two places in Cart.php are affected once delivery_option references an address ID that no longer matches the cart's current delivery address:

1. getDeliveryOption() (validates the stored option against a freshly-computed, currently-keyed list):

if (isset($this->delivery_option) && $this->delivery_option != '') {
    $deliveryOption = json_decode($this->delivery_option, true);
    $validated = true;
    if (is_array($deliveryOption)) {
        foreach ($deliveryOption as $idAddress => $key) {
            if (!isset($deliveryOptionList[$idAddress][$key])) {   // fails: '27115' isn't a key anymore
                $validated = false;
                break;
            }
        }
        if ($validated) {
            ...
        }
    }
}
// falls through to auto-select logic when $validated is false

Since $deliveryOptionList is freshly built and keyed by the current delivery address, the stale key (27115) is never found, validation fails, and the method falls into its "no valid option, pick the best automatically" branch — silently discarding whatever the customer actually chose.

2. getTotalShippingCost() (if a stale $deliveryOption array is passed in explicitly rather than recomputed):

foreach ($deliveryOption as $idAddress => $key) {
    if (!isset($deliveryOptionList[$idAddress]) || !isset($deliveryOptionList[$idAddress][$key])) {
        continue;   // silently contributes nothing for this address
    }
    $totalShipping += ...;
}

If the stale, old-address-keyed array reaches this method directly, the mismatch causes a silent continue rather than a fallback or an error — and since there's nothing else to add, $totalShipping stays exactly 0. This maps very cleanly onto the observed exact-zero result.

I also checked whether address changes flush the other, Cache::store()-backed cache that getDeliveryOptionList() uses (Cart::getDeliveryOptionList_{id_cart}_{country_id}) — I found no Cache::clean() call anywhere in Cart.php targeting that key, while other caches (getCartRules_*, getContextualValue_*) are explicitly cleaned elsewhere. I haven't fully confirmed whether this contributes as well, but it's consistent with the same general pattern: address-dependent state isn't being kept in sync when the address itself changes.

Why PayPal (and likely other redirect-based payment modules) surfaces this

This isn't PayPal-specific in root cause, but redirect-based payment flows are a reliable way to trigger it: the customer can add/change an address, get redirected off-site to PayPal, and return — by the time the order is validated, enough has happened for the stale delivery_option reference to matter. This lines up with an existing, official PrestaShop core issue that's architecturally very similar:

PrestaShop/PrestaShop#16864"Cart::getDeliveryOption() method with a payment module" — reports that for payment modules requiring an external redirect, the selected carrier gets silently redefined after payment, and traces it to Cart::getDeliveryOption()'s $useCache parameter defaulting to true while PaymentModule::validateOrder() never overrides it. That issue is filed against PrestaShop 1.7, but the exact same method signature and the exact same "no delivery option selected or not valid, get the better for all options" fallback logic exists identically in PrestaShop 1.5, 1.6, and in thirty bees' own Cart.php — so it's plausible both issues are two symptoms of the same general fragility around address/delivery-option consistency across a redirect.

Possibly related existing issues on thirtybees/paypal

None of these match exactly, but they share the same general shape — a mismatch between what's calculated/shown and what actually gets saved:

  • #26 — "PayPal instant checkout doesn't import address from PP if customer is logged in to TB and has an address in TB (but suggests order will be sent to paypal address anyway)"
  • #40 — "Paypal Checkout: Error reported due to apparently separated taxes (?)"
  • #37 — "Server error if refreshing payments plus confirmation page" (same Plus confirmation code path as this report)

Suggested fix (for whoever picks this up) @datakick

The cleanest fix is probably in Cart::updateAddressId() itself: when $idAddress is being replaced by $idAddressNew, also rewrite (or simply clear) the corresponding key in $this->delivery_option, e.g.:

if ($this->delivery_option != '') {
    $deliveryOption = json_decode($this->delivery_option, true);
    if (is_array($deliveryOption) && isset($deliveryOption[$idAddress])) {
        $deliveryOption[$idAddressNew] = $deliveryOption[$idAddress];
        unset($deliveryOption[$idAddress]);
        $this->delivery_option = json_encode($deliveryOption);
        $toUpdate = true;
    }
}

(Untested — just illustrating the idea; whoever fixes this should verify against the exact key format used elsewhere, e.g. getIdCarrierFromDeliveryOption().) Clearing it entirely (forcing a fresh auto-selection) would also work and is simpler, at the cost of not necessarily preserving the customer's original explicit choice.

Separately, modules/paypal/controllers/front/plussubmit.php's hardcoded 'shipping' => 0 in the $transaction array is misleading dead weight and worth removing/fixing independently, since it's saved into paypal_order and could affect refund-related calculations that read from that table.

Environment

  • thirty bees 1.x
  • modules/paypal (classic module — I understand tbpaypalcheckout is a newer, actively developed replacement; worth checking whether it has the same Cart::updateAddressId() exposure, since the root cause is in core, not in the PayPal module itself)
  • PHP 8.1+
  • Payment method: PayPal Website Payments Plus ("plussubmit" flow specifically — haven't tested Express Checkout or standard PayPal for the same symptom)

Impact

Orders silently ship for free when they shouldn't. Not a security issue, but a real, silent revenue-loss bug — no error is thrown, nothing appears in logs, the order just looks normal except for the missing shipping line.


Happy to provide more detail, the full Cart.php/plussubmit.php/paypal.php excerpts I traced this from, or test a patch if someone wants to attempt a fix.

It would also be good if someone volunteered to submit this as a GitHub request. I can't do it myself.  Please give feedback here whenever this is done.

paypal-shipping-bug-report.md

Edited by DRMasterChief

0 answers to this question

Recommended Posts

There have been no answers to this question yet

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...