Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

swapAndInvoice giving unexpected result #263

Open
Tailchakra opened this issue Aug 15, 2024 · 6 comments
Open

swapAndInvoice giving unexpected result #263

Tailchakra opened this issue Aug 15, 2024 · 6 comments

Comments

@Tailchakra
Copy link

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
Copy link
Collaborator

Hi @Tailchakra ,

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

@Naoray
Copy link
Collaborator

Naoray commented Aug 19, 2024

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
Copy link
Author

Tailchakra commented Aug 19, 2024

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
Copy link
Collaborator

Naoray commented Aug 20, 2024

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
Copy link
Author

@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
Copy link
Collaborator

Naoray commented Aug 26, 2024

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?

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

No branches or pull requests

3 participants