Skip to content
This repository has been archived by the owner on Feb 2, 2022. It is now read-only.

Mandate method check in Billable Trait #203

Open
mmachatschek opened this issue May 5, 2020 · 9 comments
Open

Mandate method check in Billable Trait #203

mmachatschek opened this issue May 5, 2020 · 9 comments

Comments

@mmachatschek
Copy link
Contributor

I"m curios for what reason the check $method = MandateMethod::getForFirstPaymentMethod($planModel->firstPaymentMethod()); of the mandate method has been implemented on the newSubscription method in the Billable trait.

Because of this check, you are always redirected to the mollie checkout in case you have set null in the config cashier.first_payment.method to let the user choose the payment method at checkout. This is mostly relevant when you add multiple subscriptions to the same billable model

public function newSubscription($subscription, $plan, $firstPaymentOptions = [])
{
    if(! empty($this->mollie_mandate_id)) {

(...)

        $planModel = app(PlanRepository::class)::findOrFail($plan);
        $method = MandateMethod::getForFirstPaymentMethod($planModel->firstPaymentMethod());

        if(
            ! empty($mandate)
            && $mandate->isValid()
            && $mandate->method === $method
        ) {
            return $this->newSubscriptionForMandateId($this->mollie_mandate_id, $subscription, $plan);
        }
    }

    return $this->newSubscriptionViaMollieCheckout($subscription, $plan, $firstPaymentOptions);
}
@sandervanhooft
Copy link
Collaborator

Hi @mmachatschek,

I don't agree. The first line checks if there's a mandate id available, regardless of what method is configured on the subscription plan.

If there is a mandate id set, the subscription is created without having to go through the checkout.

@mmachatschek
Copy link
Contributor Author

@sandervanhooft as there is no early return you will always be redirected to the checkout even if you have a mandate set on the billable model because in case of the problem with the payment method I mentioned you won't get into the if statement to

return $this->newSubscriptionForMandateId($this->mollie_mandate_id, $subscription, $plan);

@sandervanhooft
Copy link
Collaborator

I'm trying to understand what you mean.

Is this issue with this line?

&& $mandate->method === $method

Would changing it to this solve your issue?

&& (is_null($method) || $mandate->method === $method)

@mmachatschek
Copy link
Contributor Author

mmachatschek commented Jun 18, 2020

@sandervanhooft

The MandateMethod::getForFirstPaymentMethod()-method will always return a string, so the is_null check is not really helping here.

The main problem here is that PayPal is missing in the mollie-php-api MandateMethod::getForFirstPaymentMethod(). The method of the mandate can return directdebit, creditcard or paypal according to the documentation.

If you set the first payment method in the cashier config config('cashier.first_payment.method to e.g. paypal, the check $mandate->method === $method is always false because $method can only return directdebit and creditcard atm.

Also if you pass a comma separated list of possible first payment methods, the value of $method will always be directdebit. So if a customer did the checkout with a credit card, he will be redirected to the checkout again at the next subscription setup even though he already setup everything correctly.
Same happens if the config of the first payment method is set to null.

@sandervanhooft
Copy link
Collaborator

Can you suggest a fix?

@mmachatschek
Copy link
Contributor Author

@sandervanhooft I added a PR at mollie/mollie-api-php for the paypal issue

@mmachatschek
Copy link
Contributor Author

mmachatschek commented Jul 15, 2020

Can you suggest a fix?

I would suggest to change the line to:

&& ($planModel->firstPaymentMethod() === null || $mandate->method === $method)

This way you can either let the user decide which payment method to use by setting the first payment method of the plan to null or (if explicitly set in the plan model) strictly check if the original mandate method is the same as the $method provided.

@juancruzmartino
Copy link

@mmachatschek I just used that and it works. I was having problems when I wanted to have multiple subscriptions for the same user. Nice one!

@sandervanhooft
Copy link
Collaborator

Thanks @mmachatschek for the suggestion. Let's do that. Can you send in a PR?

Thanks for confirming this @juancruzmartino .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants