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

[14.x] Fix array merge for checkout #1579

Merged
merged 1 commit into from
Oct 3, 2023
Merged

[14.x] Fix array merge for checkout #1579

merged 1 commit into from
Oct 3, 2023

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Oct 3, 2023

Makes sure the payload for checkout is recursively merged instead of overwritten. The create method of the builder doesn't suffers from this because the default payload doesn't have multidimensional options.

Fixes #1578

@driesvints driesvints changed the title Fix array merge for checkout [14.x] Fix array merge for checkout Oct 3, 2023
@taylorotwell taylorotwell merged commit 8191b96 into 14.x Oct 3, 2023
@taylorotwell taylorotwell deleted the fix-array-merge branch October 3, 2023 11:40
@edalzell
Copy link
Contributor

edalzell commented Oct 3, 2023

Thanks @driesvints!

I was heading back here to do a PR (did the issue at the end of my day) only to find you'd done it already! You rock.

@bjarn
Copy link

bjarn commented Oct 14, 2023

Just dropping this here as this PR might be a breaking change to others like it was for me:

When I first used Cashier, I had to (or at least read in Stripe or Cashier's docs) that I had to set mode to subscription in the checkout attributes.
Because of this change, the mode gets merged and changed into an array instead of string, causing this error:
CleanShot 2023-10-14 at 13 04 48

Fixed by removing the mode attribute from my checkout builder.

@driesvints
Copy link
Member Author

@bjarn can you share the code that causes this?

@bjarn
Copy link

bjarn commented Oct 14, 2023

@driesvints Sure!

I used the following builder:

        $builder = $team->newSubscription(
            'default',
            $plan->priceId($data->billing_cycle)
        );
        
        // ...

        return $builder->checkout([
            'mode' => 'subscription',
            'client_reference_id' => $team->id,
            'customer_update' => [
                'name' => 'auto',
                'address' => 'auto',
                'shipping' => 'never',
            ],
            'billing_address_collection' => 'required',
            'allow_promotion_codes' => true,
            'success_url' => xxx,
            'cancel_url' => xxx
        ])->asStripeCheckoutSession()->url;

I'm not sure why the mode ended up here, but I saw it was also set here:
https://github.com/laravel/cashier-stripe/pull/1579/files#diff-6ddbc0726400777bbee69d4448fa971720fb206f613abda0e165fffd8a340316L356

Now it is recursively merged, I think it's appended instead of overwritten of course. Probably have overlooked something when diving into the Stripe docs to see all available parameters not thinking about Cashier's abstractions.

Removed the mode from the builder and it was all fine again.

@driesvints
Copy link
Member Author

Yeah, I'm sorry but I don't feel we can consider this an issue since the builder already adds this for you.

@bjarn
Copy link

bjarn commented Oct 16, 2023

Yeah, I'm sorry but I don't feel we can consider this an issue since the builder already adds this for you.

Yea, no worries at all! It definitely was an error on my end, just sharing in case anyone else made the same mistake :)

Thanks!

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

Successfully merging this pull request may close these issues.

Checkout overwrites default subscription_data with $sessionOptions
4 participants