[16.x] Fix incorrect subscription validation for canceled Stripe subscriptions#1797
Closed
Diddyy wants to merge 4 commits intolaravel:16.xfrom
Closed
[16.x] Fix incorrect subscription validation for canceled Stripe subscriptions#1797Diddyy wants to merge 4 commits intolaravel:16.xfrom
Diddyy wants to merge 4 commits intolaravel:16.xfrom
Conversation
…ding tests - Modified the Subscription model to ensure that the `active`, `canceled`, `onTrial`, and `onGracePeriod` methods correctly account for the `stripe_status` being canceled. - Added new unit tests to verify the behavior of the Subscription model when the `stripe_status` is set to canceled, including scenarios with and without `ends_at` and `trial_ends_at`. - Updated .gitignore to exclude .idea directory.
Member
|
Thanks for your pull request to Laravel! I appreciate you taking the time to submit this; however, it appears this contribution may have been primarily AI-generated without careful human review and consideration. We've found that AI-generated code often doesn't align well with Laravel's conventions, architectural decisions, and the specific context of what we're trying to accomplish with the framework. Quality contributions require thoughtful human insight into the codebase. If you're interested in contributing to Laravel, I'd encourage you to familiarize yourself with the existing codebase, engage with the community, and submit PRs that reflect your own understanding and careful consideration of the problem you're solving. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Target Branch:
16.x(bug fix for currently supported version)Description
This PR fixes a critical bug where subscription status validation methods (
valid(),active(),onTrial(),onGracePeriod(),canceled()) did not properly check thestripe_statusfield when subscriptions are canceled via Stripe's API (e.g., when a test clock is canceled) or when subscriptions are immediately canceled during trial periods.Problems Fixed
Issue 1: Test Clock Cancellation
When a Stripe test clock is canceled, Stripe automatically cancels all associated subscriptions and sets their status to
canceled. The webhook correctly updatesstripe_statustoSTATUS_CANCELED, but the validation methods did not respect this status:canceled()only checkedends_atand ignoredstripe_statusactive(),onTrial(), andonGracePeriod()did not check forSTATUS_CANCELEDvalid()would incorrectly returntruefor canceled subscriptionsReproduction Steps:
valid()incorrectly returnstrueIssue 2: Immediate Cancellation During Trial
When a subscription with an active trial period is canceled immediately using
cancelNow(), the subscription would still be considered valid becauseonTrial()only checked iftrial_ends_atwas in the future, without verifying if the subscription had actually ended.Reproduction Steps:
$user->newSubscription('default', $price)->trialUntil('tomorrow')->add()$user->subscription('default')->cancelNow()$user->subscribed('default')incorrectly returnstrueSolution
Updated all subscription status validation methods to properly check
stripe_status === STATUS_CANCELED:canceled(): Now returnstrueif eitherends_atis set ORstripe_status === STATUS_CANCELEDactive(): Added explicit check forstripe_status !== STATUS_CANCELEDonTrial(): Now returnsfalseifstripe_status === STATUS_CANCELEDOR ifends_atis set and not in the futureonGracePeriod(): Now returnsfalseifstripe_status === STATUS_CANCELED(grace period only applies whenends_atis in the future AND status is not canceled)Changes Made
Code Changes (
src/Subscription.php)canceled()method (line 304-307): Added check forstripe_status === STATUS_CANCELEDactive()method (line 229-237): Added explicit check forstripe_status !== STATUS_CANCELEDonTrial()method (line 357-363): Added checks forstripe_status !== STATUS_CANCELEDandends_atnot being in the pastonGracePeriod()method (line 410-413): Added check forstripe_status !== STATUS_CANCELEDTests Added (
tests/Unit/SubscriptionTest.php)Added 17 comprehensive test cases covering:
canceled()behavior withSTATUS_CANCELEDstatus (with and withoutends_at)active()behavior withSTATUS_CANCELEDstatusonTrial()behavior withSTATUS_CANCELEDstatus and pastends_atonGracePeriod()behavior withSTATUS_CANCELEDstatusvalid()behavior withSTATUS_CANCELEDstatusended()behavior withSTATUS_CANCELEDstatusends_atand active statusBenefits to End Users
Backward Compatibility
✅ No breaking changes: This fix maintains full backward compatibility:
ends_at) still work correctlyends_atand active statusTesting
All new tests pass, and existing tests continue to pass, ensuring no regressions were introduced.
Related Issues
#1550 #1791