-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[PM-20090] Restarting subscription for unpaid organizations #5635
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5635 +/- ##
==========================================
+ Coverage 45.73% 45.81% +0.08%
==========================================
Files 1603 1609 +6
Lines 72659 73181 +522
Branches 6507 6558 +51
==========================================
+ Hits 33229 33529 +300
- Misses 38018 38221 +203
- Partials 1412 1431 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
| CancelledDate = sub.CanceledAt; | ||
| CancelAtEndDate = sub.CancelAtPeriodEnd; | ||
| Cancelled = sub.Status == "canceled" || sub.Status == "unpaid" || sub.Status == "incomplete_expired"; | ||
| Cancelled = sub.Status is StripeConstants.SubscriptionStatus.Canceled or StripeConstants.SubscriptionStatus.IncompleteExpired; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ This property is used in 13 places throughout the web application so changing it here is almost certainly going to have downstream effects.
For example:
- Users with unpaid subscriptions will be able to download their license
- They'll be able to adjust their storage
- Unpaid organizations would be able to adjust their seats
- They'd also be able to subscribe to Secrets Manager
Organization disablement may handle some of this, but if all we're trying to do is prevent the POST restart-subscription endpoint from being invoked when the subscription is "unpaid", I think it would be more straightforward to find that point of invocation in the web app and prevent it there using the subscription.status property.
Let me know your thoughts.
| CancelledDate = sub.CanceledAt; | ||
| CancelAtEndDate = sub.CancelAtPeriodEnd; | ||
| Cancelled = sub.Status == "canceled" || sub.Status == "unpaid" || sub.Status == "incomplete_expired"; | ||
| Cancelled = sub.Status is StripeConstants.SubscriptionStatus.Canceled or StripeConstants.SubscriptionStatus.Unpaid or StripeConstants.SubscriptionStatus.IncompleteExpired; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a stylistic change. But might help navigate the code base faster related to specific subscription statuses.
|
cyprain-okeke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do if have a situation where customer that has other status are able to call this endpoint today?





🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-20090
📔 Objective
An additional check has been placed on the
POST /organizations/{orgid}/billing/restart-subscriptionendpoint to prevent an additional subscription from being created if the customer already has an active subscription in Stripe (definition not in acanceledstatus).📸 Screenshots
https://bitwarden.atlassian.net/browse/PM-20090?focusedCommentId=93746
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes