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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.x] Fix deletion of subscription items in swap() #1509

Merged

Conversation

glspdotnet
Copy link
Contributor

This pull request fixes an issue in Subscription::swap() where subscription item models created using inline prices (via price_data) instead of an existing Stripe price are immediately deleted after creation:

// Delete items that aren't attached to the subscription anymore...
$this->items()->whereNotIn('stripe_price', $items->pluck('price')->filter())->delete();

Items using inline price data will not be keyed by price in parseSwapPrices(), meaning they will not be found by $items->pluck('price'). This change simply uses the item data returned by the updated Stripe subscription object as the source of truth instead of the array that is initially passed to swap(). FWIW, I considered adding post-creation/swapping item relation counts to the feature tests relating to inline prices but saw that you weren't testing for such things anywhere else, so I'll keep them to my local branch for now.

Presumably this hasn't been much of an issue for people who have followed the docs during initial setup, as the deleted items are re-created almost immediately via the customer.subscription.updated webhook (I initially started looking into this as I thought I was experiencing some sort of odd race condition in my site's codebase).

This is my first-ever PR, so if I've mucked anything up or can otherwise be of assistance please let me know! 馃槃

This commit fixes an issue in Subscription::swap() where subscription
items created via `price_data` instead of an existing Stripe price are
immediately deleted after creation.
@bastiaandewaele
Copy link

I noticed the same issue today. Purely by code review, I think @glspdotnet changes will work.

Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

@taylorotwell taylorotwell merged commit abd0d29 into laravel:14.x Mar 1, 2023
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.

None yet

4 participants