laravel / cashier-mollie

MIT License
376 stars 63 forks source link

Coupon first payment issues #53

Closed RobertBoes closed 4 years ago

RobertBoes commented 5 years ago

I've tried to create my own coupon, but the documentation is very minimal, so I've basically copied the FixedDiscountHandler and changed a few things.

Now my own coupon didn't work, so I applied the default coupon (with the default plan etc.), and I think a lot of things are not working correctly:

The code I used is the same as in the "documentation" / readme, except I changed the part where I make a new subscription, like this:

$result = $user
            ->newSubscription($name, $plan)
            ->withCoupon('welcome')
            ->create();

This is the complete error log:

[2019-09-09 13:40:38] local.ERROR: Trying to get property 'currency' of non-object {"exception":"[object] (ErrorException(code: 0): Trying to get property 'currency' of non-object at D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\cashier-mollie\\src\\Order\\Order.php:75)
[stacktrace]
#0 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\cashier-mollie\\src\\Order\\Order.php(75): Illuminate\\Foundation\\Bootstrap\\HandleExceptions->handleError(8, 'Trying to get p...', 'D:\\\\GitHub\\\\molli...', 75, Array)
#1 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Concerns\\ManagesTransactions.php(29): Laravel\\Cashier\\Order\\Order::Laravel\\Cashier\\Order\\{closure}(Object(Illuminate\\Database\\SQLiteConnection))
#2 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\DatabaseManager.php(349): Illuminate\\Database\\Connection->transaction(Object(Closure))
#3 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Support\\Facades\\Facade.php(239): Illuminate\\Database\\DatabaseManager->__call('transaction', Array)
#4 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\cashier-mollie\\src\\Order\\Order.php(102): Illuminate\\Support\\Facades\\Facade::__callStatic('transaction', Array)
#5 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\cashier-mollie\\src\\Order\\Order.php(119): Laravel\\Cashier\\Order\\Order::createFromItems(Object(Laravel\\Cashier\\Order\\OrderItemCollection), Array, false)
#6 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\cashier-mollie\\src\\FirstPayment\\FirstPaymentHandler.php(52): Laravel\\Cashier\\Order\\Order::createProcessedFromItems(Object(Laravel\\Cashier\\Order\\OrderItemCollection), Array)
#7 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Concerns\\ManagesTransactions.php(29): Laravel\\Cashier\\FirstPayment\\FirstPaymentHandler->Laravel\\Cashier\\FirstPayment\\{closure}(Object(Illuminate\\Database\\SQLiteConnection))
#8 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\DatabaseManager.php(349): Illuminate\\Database\\Connection->transaction(Object(Closure))
#9 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Support\\Facades\\Facade.php(239): Illuminate\\Database\\DatabaseManager->__call('transaction', Array)
#10 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\cashier-mollie\\src\\FirstPayment\\FirstPaymentHandler.php(54): Illuminate\\Support\\Facades\\Facade::__callStatic('transaction', Array)
#11 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\cashier-mollie\\src\\Http\\Controllers\\FirstPaymentWebhookController.php(27): Laravel\\Cashier\\FirstPayment\\FirstPaymentHandler->execute()
#12 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\ControllerDispatcher.php(48): Laravel\\Cashier\\Http\\Controllers\\FirstPaymentWebhookController->handleWebhook(Object(Illuminate\\Http\\Request))
#13 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Route.php(219): Illuminate\\Routing\\ControllerDispatcher->dispatch(Object(Illuminate\\Routing\\Route), Object(Laravel\\Cashier\\Http\\Controllers\\FirstPaymentWebhookController), 'handleWebhook')
#14 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Route.php(176): Illuminate\\Routing\\Route->runController()
#15 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Router.php(680): Illuminate\\Routing\\Route->run()
#16 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Pipeline.php(30): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#17 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Pipeline\\Pipeline.php(104): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#18 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Router.php(682): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#19 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Router.php(657): Illuminate\\Routing\\Router->runRouteWithinStack(Object(Illuminate\\Routing\\Route), Object(Illuminate\\Http\\Request))
#20 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Router.php(623): Illuminate\\Routing\\Router->runRoute(Object(Illuminate\\Http\\Request), Object(Illuminate\\Routing\\Route))
#21 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Router.php(612): Illuminate\\Routing\\Router->dispatchToRoute(Object(Illuminate\\Http\\Request))
#22 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Foundation\\Http\\Kernel.php(176): Illuminate\\Routing\\Router->dispatch(Object(Illuminate\\Http\\Request))
#23 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Pipeline.php(30): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}(Object(Illuminate\\Http\\Request))
#24 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\fideloper\\proxy\\src\\TrustProxies.php(57): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#25 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Pipeline\\Pipeline.php(163): Fideloper\\Proxy\\TrustProxies->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#26 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#27 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest.php(21): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#28 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Pipeline\\Pipeline.php(163): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#29 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#30 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest.php(21): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#31 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Pipeline\\Pipeline.php(163): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#32 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#33 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize.php(27): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#34 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Pipeline\\Pipeline.php(163): Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#35 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#36 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Foundation\\Http\\Middleware\\CheckForMaintenanceMode.php(62): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#37 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Pipeline\\Pipeline.php(163): Illuminate\\Foundation\\Http\\Middleware\\CheckForMaintenanceMode->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#38 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Routing\\Pipeline.php(53): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#39 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Pipeline\\Pipeline.php(104): Illuminate\\Routing\\Pipeline->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#40 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Foundation\\Http\\Kernel.php(151): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#41 D:\\GitHub\\mollie-subscriptions-5.8-poc\\vendor\\laravel\\framework\\src\\Illuminate\\Foundation\\Http\\Kernel.php(116): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter(Object(Illuminate\\Http\\Request))
#42 D:\\GitHub\\mollie-subscriptions-5.8-poc\\public\\index.php(55): Illuminate\\Foundation\\Http\\Kernel->handle(Object(Illuminate\\Http\\Request))
#43 D:\\GitHub\\mollie-subscriptions-5.8-poc\\server.php(21): require_once('D:\\\\GitHub\\\\molli...')
#44 {main}

I think the subscriptions are not saved to my own DB because of this error, this kind of worries me, since we have no control over this and it's hard to debug if a user actually bought a product.

The coupon that's not applied also seems very strange, it might be applied during the next payment, but that's a weird flow, right?

sandervanhooft commented 5 years ago

@RobertBoes that's unfortunate, will have to look into this later this week.

It may help to understand in debugging that using the coupon is actually a two-step process:

  1. redeeming the coupon
  2. applying the coupon to the Order
sandervanhooft commented 5 years ago

Can you tell me what subscription it is? I.e. trial subscription with payment upfront?

RobertBoes commented 5 years ago

It's a subscription with a generic trial, so the user doesn't have a subscription yet and no payments have been made yet, i.e. the first_payment (1 euro) is not used. I'm using the example code that's provided in the readme, but I've added ->withCoupon('welcome').

I would assume that when creating a new subscription with a coupon, the coupon would be applied when making the first payment. I don't know if this doesn't work because of the errors, or because the coupon is never applied but only on subsequent payments.

This is basically the full code I'm using:

namespace App\Http\Controllers;

use Illuminate\Http\RedirectResponse;
use Illuminate\Support\Facades\Auth;

class CreateSubscriptionController extends Controller
{
    /**
     * @param string $plan
     * @return \Illuminate\Http\RedirectResponse
     */
    public function __invoke(string $plan)
    {
        $user = Auth::user();

        $name = ucfirst($plan) . ' membership';

        if(!$user->subscribed($name, $plan)) {

            $result = $user->newSubscription($name, $plan)->withCoupon('welcome')->create();

            if(is_a($result, RedirectResponse::class)) {
                return $result; // Redirect to Mollie checkout
            }

            return back()->with('status', 'Welcome to the ' . $plan . ' plan');
        }

        return back()->with('status', 'You are already on the ' . $plan . ' plan');
    }
}
sandervanhooft commented 5 years ago

(FYI, your code looks like you're starting a normal subscription, not a generic one, but that's beside the point here 😄)

Ok. So there are a few things going on here that need attention. Let me share my thoughts here.

Right now, the business logic is as such that the coupon should not be applicable to a trial.

But...

a) it should also be possible to redeem in trial mode.

b) if applicable, an InvalidCouponException should be thrown before the customer is redirected to the checkout. This probably needs to happen in the SubscriptionBuilders.

c) an exception while handling the coupon after payment should not kill the rest of the flow. This relates to payment actions and how to (gracefully) handle exceptions.

sandervanhooft commented 5 years ago

@robertboes can you share the metadata from the payment (you can find this in the Mollie dashboard)?

RobertBoes commented 5 years ago

Sure, here it is:

{
  "owner": {
    "type": "App\\User",
    "id": 2
  },
  "actions": [
    {
      "handler": "Laravel\\Cashier\\FirstPayment\\Actions\\StartSubscription",
      "description": "Monthly payment",
      "subtotal": {
        "currency": "EUR",
        "value": "10.00"
      },
      "plan": "example-1",
      "name": "main",
      "quantity": 1,
      "coupon": "welcome"
    }
  ]
}
RobertBoes commented 5 years ago

I've done some more testing and it seems like the preprocessors are not applied on the first payment, is that correct?

When I create a subscription with a trial, like this:

$result = $user
    ->newSubscription($name, $plan)
    ->trialUntil(Carbon::now()->addMinutes(30))
    ->withCoupon('welcome')
    ->create();

The first_payment is used, so the first payment is the amount that specified there (which I've set to 0.01). After I run php artisan cashier:run the subsequent payment is processed, which is then charged as 10.00 minus the coupon discount of 5.00, so the total that's paid is 5.00.

However, when I run this without a trial and with a coupon, the amount that needs to be paid for the first payment is 10.00 instead of 5.00 (10.00 plan - 5.00 coupon). So the first payment ignores the preprocessors.

This might be related to the error I get within the webhook when using a coupon upon creating the subscription, but I'm not sure.

sandervanhooft commented 5 years ago

Thanks for following up. I have been recreating this issue this afternoon, and can confirm your suspicions about the first payment not having applied the discount yet.

sandervanhooft commented 5 years ago
// FirstPaymentSubscriptionBuilderTest.php

/** @test */
    public function buildsPayloadWithCouponNoTrial()
    {
        $this->withMockedCouponRepository();

        $this->assertSame(0, RedeemedCoupon::count());
        $this->assertSame(0, AppliedCoupon::count());

        $builder = $this->getBuilder()
            ->withCoupon('test-coupon'); // 5 EUR off

        $builder->create();

        // Coupons will not be handled before payment is completed via checkout
        $this->assertSame(0, RedeemedCoupon::count());
        $this->assertSame(0, AppliedCoupon::count());

        $payload = $builder->getMandatePaymentBuilder()->getMolliePayload();
        $this->assertSame('EUR', $payload['amount']['currency']);
        $this->assertSame('7.00', $payload['amount']['value']); // <-- fails here (12.00 instead of 7.00)
    }
ciungulete commented 5 years ago

and with trial is same problem

$this->assertSame('7.00', $payload['amount']['value']); // <-- fails here (0.05 instead of 7.00)

sandervanhooft commented 5 years ago

I think that's correct behaviour actually @ciungulete

sandervanhooft commented 5 years ago

I'm digging in today/tomorrow in the coupon logic. Complex stuff, simplifying some code while going over it.

sandervanhooft commented 5 years ago

Generally speaking (there may be some exceptions here as I noted above, but let's start here):

Mandate available, no trial: validate, redeem on creating the subscription. Apply coupon during processing the scheduled payment ✅ Mandate available, with trial: validate, redeem coupon on creating the subscription. Apply coupon during processing first scheduled normal payment

Through checkout, no trial

Before checkout:

  1. validate ✅
  2. apply coupon to payment amount ✅

After checkout:

  1. skip validation ✅
  2. redeem ✅
  3. mark as already applied ✅

Through checkout, with trial

Before checkout:

  1. validate coupon ✅
  2. do not apply coupon to payment amount ❓

After checkout:

  1. skip validation ✅
  2. redeem ✅
  3. do not apply (wait for first normal payment)✅
sandervanhooft commented 5 years ago

Still at it, hope to release an improved version later this week.

RobertBoes commented 4 years ago

Any update on the progress? We really want to start implementing this, but we're depending on preprocessors to work for the very first payment.

sandervanhooft commented 4 years ago

Hi @RobertBoes ,

Good news: I'm preparing the PR right now.

The issue was more complex than I expected, so I had to sit on it for a bit before moving forward. Sorry for that.

sandervanhooft commented 4 years ago

I had to make one change, probably not breaking anything

sandervanhooft commented 4 years ago

I'll open a separate issue for documenting the coupon logic.

sebastiaands commented 4 years ago

Hi @sandervanhooft ,

Thanks for your work! I tried your fix and there seems to be an issue. When I apply a coupon to a new subscription, the following metadata is sent to Mollie:

{
  "owner": {
    "type": "App\\Company",
    "id": 1
  },
  "actions": [
    {
      "handler": "Laravel\\Cashier\\FirstPayment\\Actions\\StartSubscription",
      "description": "Pro abonnement",
      "subtotal": {
        "currency": "EUR",
        "value": "19.00"
      },
      "taxPercentage": "21.0000",
      "plan": "pro",
      "name": "default",
      "quantity": 1,
      "coupon": "ptquku"
    },
    null
  ]
}

The last item in the array is null, which causes the following error when Mollie does its callback: Trying to get property 'handler' of non-object {"exception":"[object] (ErrorException(code: 0): Trying to get property 'handler' of non-object at \\vendor\\laravel\\cashier-mollie\\src\\FirstPayment\\FirstPaymentHandler.php:84)

sandervanhooft commented 4 years ago

Thanks for the super fast response!

I've just released v1.2.1 to fix this issue, can you give it a spin?

sandervanhooft commented 4 years ago

https://github.com/laravel/cashier-mollie/releases/tag/v1.2.1

sebastiaands commented 4 years ago

Yes, that works! Is it correct that coupons are not mentioned on invoices?