mollie / laravel-cashier-mollie

Official Mollie integration for Laravel Cashier
https://www.cashiermollie.com/
MIT License
137 stars 44 forks source link

swapAndInvoice giving unexpected result #263

Open Tailchakra opened 2 months ago

Tailchakra commented 2 months ago

I am using $subscription->swapAndInvoice of the Mollie Cashier repository. Usually it should refund the unused days and invoice for the full new price. Which normally works. However with the latest swapAndInvoice my customer suddenly doesn't get their old price refunded but instead the cycle gets lengthened to 30-09-2024. So now his cycle runs from 14-08-2024 to 30-09-2024. This brings two issues:

Issue 1. He didn't pay the new price for 14-08-2024 to 30-08-2024 but did instantly get the new features; Issue 2. Maybe he didn't want the extend from 30-08-2024 to 30-09-2024.

This is in the Cashier\Subscription model:

<?php

namespace App\Models\Cashier;

use Carbon\Carbon;
use Closure;
use Illuminate\Support\Facades\DB;
use Laravel\Cashier\Cashier;
use Laravel\Cashier\Coupon\AppliedCoupon;
use Laravel\Cashier\Order\OrderItem;
use Laravel\Cashier\Order\OrderItemCollection;
use Laravel\Cashier\Refunds\RefundItemCollection;
use Laravel\Cashier\Subscription as CashierSubscription;
use LogicException;
use Money\Currency;
use Money\Money;

class Subscription extends CashierSubscription
{
    /**
     * The maximum amount that can be reimbursed, ranging from zero to a positive Money amount.
     *
     * @return Money
     */
    protected function reimbursableAmount(): Money
    {
        $zeroAmount = new Money('0.00', new Currency($this->currency));

        // Determine base amount eligible to reimburse
        $latestProcessedOrderItem = $this->latestProcessedOrderItem();
        if (!$latestProcessedOrderItem) {
            return $zeroAmount;
        }

        $reimbursableAmount = $latestProcessedOrderItem->getTotal()
            ->subtract($latestProcessedOrderItem->getTax()); // tax calculated elsewhere

        // Subtract any refunds
        /** @var RefundItemCollection $refundItems */
        $refundItems = Cashier::$refundItemModel::where('original_order_item_id', $latestProcessedOrderItem->id)->get();

        if ($refundItems->isNotEmpty()) {
            $reimbursableAmount = $reimbursableAmount->subtract($refundItems->getTotal());
        }

        // Subtract any applied coupons
        $order = $latestProcessedOrderItem->order;
        $orderId = $order->id;
        $appliedCoupons = $this->appliedCoupons()->with('orderItems')->get();

        $appliedCouponOrderItems = $appliedCoupons->reduce(function (OrderItemCollection $carry, AppliedCoupon $coupon) use ($orderId) {
            $items = $coupon->orderItems->filter(function (OrderItem $item) use ($orderId) {
                return $item->order_id === $orderId;
            });

            return $carry->concat($items->toArray());
        }, new OrderItemCollection);

        if ($appliedCouponOrderItems->isNotEmpty()) {
            $discountTotal = $this->getTotalOfOrderItems($appliedCouponOrderItems);
            $reimbursableAmount = $reimbursableAmount->subtract($discountTotal->absolute());
        }

        // Guard against a negative value
        if ($reimbursableAmount->isNegative()) {
            return $zeroAmount;
        }

        return $reimbursableAmount;
    }

    /**
     * @return \Money\Money
     *
     * @throws \LogicException
     */
    public function getTotalOfOrderItems($appliedCouponOrderItems): Money
    {
        if (count($appliedCouponOrderItems->currencies()) > 1) {
            throw new LogicException('Calculating the total requires items to be of the same currency.');
        }

        return new Money($appliedCouponOrderItems->sum('unit_price'), new Currency($appliedCouponOrderItems->currency()));
    }
}

Plan Model:

<?php

namespace App\Models;

use Laravel\Cashier\Plan\Plan as CashierPlan;
use Illuminate\Database\Eloquent\Model;
use Laravel\Cashier\Order\OrderItemPreprocessorCollection as Preprocessors;

/**
 * @property mixed $name
 * @property mixed $amount
 * @property mixed $interval
 * @property mixed $description
 * @property mixed $first_payment_method
 * @property mixed $first_payment_amount
 * @property mixed $first_payment_description
 * @property mixed $first_payment_redirect_url
 * @property mixed $first_payment_webhook_url
 * @property mixed $order_item_preprocessors
 * @method static where(string $string, string $name)
 * @method static whereIn(string $string, mixed[] $getSelected)
 */
class Plan extends Model
{

    public $fillable = [
        'name',
        'amount',
        'interval',
        'description',
        'first_payment_method',
        'first_payment_amount',
        'first_payment_description',
        'first_payment_redirect_url',
        'first_payment_webhook_url',
        'order_item_preprocessors',
    ];

    /**
     * Builds a Cashier plan from the current model.
     *
     * @returns CashierPlan
     */
    public function buildCashierPlan(): CashierPlan
    {
        $plan = new CashierPlan($this->name);

        return $plan->setAmount(mollie_array_to_money($this->amount))
            ->setInterval($this->interval)
            ->setDescription($this->description)
            ->setFirstPaymentMethod($this->first_payment_method)
            ->setFirstPaymentAmount(mollie_array_to_money($this->first_payment_amount))
            ->setFirstPaymentDescription($this->first_payment_description)
            ->setFirstPaymentRedirectUrl($this->first_payment_redirect_url)
            ->setFirstPaymentWebhookUrl($this->first_payment_webhook_url)
            ->setOrderItemPreprocessors(Preprocessors::fromArray(config('cashier_plans.defaults.order_item_preprocessors')));
    }
}

PlanRepository:

<?php
namespace App\Repositories;

use App\Models\Plan;
use Laravel\Cashier\Exceptions\PlanNotFoundException;
use Laravel\Cashier\Plan\Contracts\PlanRepository as PlanRepositoryContract;

class PlanRepository implements PlanRepositoryContract
{
    public static function find(string $name)
    {
        $plan = Plan::where('name', $name)->first();

        if (is_null($plan)) {
            return null;
        }

        if ($plan['first_payment_method'] == '[]') $plan['first_payment_method'] = [];

        $plan['amount'] = json_decode($plan['amount'], true);
        $plan['first_payment_amount'] = json_decode($plan['first_payment_amount'], true);

        return $plan->buildCashierPlan();
    }

    /**
     * @throws PlanNotFoundException
     */
    public static function findOrFail(string $name)
    {
        if (($result = self::find($name)) === null) {
            throw new PlanNotFoundException;
        }

        return $result;
    }
}

This is the expected result: image

But it simply gives a new invoice with only the new price and adds this to the database cycle: image

sandervanhooft commented 2 months ago

Hi @Tailchakra ,

Thanks for reporting this. We're attempting to reproduce the error. Has anything happened to the user balance in cashier?

Naoray commented 2 months ago

Hi @Tailchakra,

I can’t reproduce the issue without more data. Could you provide the details for the OrderItems used in this calculation?

I noticed you overrode the logic that calculates the reimbursement amount. Have you recently introduced this change, so that it might be responsible for the missing reimburseable amount? Why do you want to keep the discount to be calculated from the unit_price instead of the total including taxes?

Tailchakra commented 2 months ago

sandervanhooft

Not as far as I can see currently.

Hi @Tailchakra,

I can’t reproduce the issue without more data. Could you provide the details for the OrderItems used in this calculation?

I noticed you overrode the logic that calculates the reimbursement amount. Have you recently introduced this change, so that it might be responsible for the missing reimburseable amount? Why do you want to keep the discount to be calculated from the unit_price instead of the total including taxes?

image

@Naoray Hi, I altered this because the total field does not exist. When logging the OrderItemCollection it gives no total in return so I had to rewrite it for it to work.

image

I also had to alter the percentage coupon because it gave a 10% over the total price and not the original price.

image

But no, it does not seem to keep it from reimbursing, little note - I cannot seem to reproduce this on my own account as well. When I swapandinvoice it works for me, but this random occurrence made me worried it may happen more often.

Naoray commented 2 months ago

I altered this because the total field does not exist. When logging the OrderItemCollection it gives no total in return so I had to rewrite it for it to work.

@Tailchakra there is neither a unit_price nor a total stored on the collection itself. The unit_price you use is coming from the underlying OrderItem model. When calling $orderItemCollection->sum('total'), the total value of OrderItem is accessed through the getTotalAttribute accessor method.

    public function getTotalAttribute()
    {
        $beforeTax = $this->getSubtotal();

        return (int) $beforeTax->add($this->getTax())->getAmount();
    }

I also had to alter the percentage coupon because it gave a 10% over the total price and not the original price.

As you can see, the getTotalAttribute() (an earlier version for Laravel's accessors) - which can be accessed by calling the property total on the OrderItem model like $orderItem->total - adds the tax to the subtotal value. The subtotal value itself is again derived from another accessor getSubtotalAttribute(), which multiplies the unit_price with the quantity.

    public function getSubtotalAttribute()
    {
        return (int) $this->getUnitPrice()->multiply($this->quantity ?: 1)->getAmount();
    }

So the "issue" why you received a 10% discount over the total price is because it takes the total price into account for this discount and not only the unit_price. I guess the 10% is likely the taxes that were added to the initial invoice?!


A tip for your PlanRepository. You are accessing your DB columns on the Plan model like an array. But (1) you can access them like normal properties $plan->amount instead of $plan['amount'] and (2) you can use casts so you don't have to json_decode them manually.

// App\Models\Plan
class Plan extends Model
{
    public $casts = [
        'amount' => 'array',
        'first_payment_amount' => 'array',
        'first_payment_method' => 'array',
    ];

    //...
}

// App\Repositories\PlanRepository
class PlanRepository implements PlanRepositoryContract
{
    public static function find(string $name)
    {
        $plan = Plan::where('name', $name)->first();

        if (is_null($plan)) {
            return null;
        }

        return $plan->buildCashierPlan();
    }

    // ...
}

In general I'd advise to delete your custom code for Subscriptions and see if this issue occurs again. If so, make sure to provide all necessary data, including the config files from cashier.

If you have further questions, please feel free to ask them :).

Tailchakra commented 2 months ago

@Naoray For the record, I can't seem to reproduce this myself either. With the original code it completely miscalculated the coupon discount during reimbursement , with my edits it calculates it correctly. However with this one client it not only didn't reimburse, it also extended the time of the cycle to the end of next month?

My edits are documented here: https://discord.com/channels/1037712581407817839/1257434407766200351/1260923312876294225 Sander should've made a note for it IIRC.


@Tailchakra there is neither a unit_price nor a total stored on the collection itself. The unit_price you use is coming from the underlying OrderItem model. When calling $orderItemCollection->sum('total'), the total value of OrderItem is accessed through the getTotalAttribute accessor method.

The getTotal() was a part of the original laravel-cashier-mollie code though. Altering that to the unit_price made it work in the first place.


As you can see, the getTotalAttribute() (an earlier version for Laravel's accessors) - which can be accessed by calling the property total on the OrderItem model like $orderItem->total - adds the tax to the subtotal value. The subtotal value itself is again derived from another accessor getSubtotalAttribute(), which multiplies the unit_price with the quantity.

    public function getSubtotalAttribute()
    {
        return (int) $this->getUnitPrice()->multiply($this->quantity ?: 1)->getAmount();
    }

So the "issue" why you received a 10% discount over the total price is because it takes the total price into account for this discount and not only the unit_price. I guess the 10% is likely the taxes that were added to the initial invoice?!

The 10% was a discount through a coupon. What it originally did was this:

Price is 10EUR but with taxes it's 12,10EUR. Coupon took 10% from 12,10EUR and then added 21% on top of THAT causing it to give 14,64EUR discount with a 10% discount over 10EUR.

That was because it took the unitprice of the getTotal instead of getSubtotal.


Deleting my custom code for subscriptions will put me back into my original problem I documented in Discord. Thanks for the PlanRepository tip, will get to that once I know everything works as is haha

Naoray commented 2 months ago

With the original code it completely miscalculated the coupon discount during reimbursement

@Tailchakra could you provide me with the exact prices of the previous paid subscription (subtotal and taxes) please? If the calculation is wrong I'd like to address the issue asap.

However with this one client it not only didn't reimburse, it also extended the time of the cycle to the end of next month?

Are you sure that the ->swapAndInvoice() method was called and that no other interaction with cashier took place? How does your implementation look like for updgrading/downgrading plans?

Can you also paste your cashier*.php config files please?