Team trial mode #1611
Team trial mode #1611
Conversation
forge/ee/lib/billing/index.js
Outdated
| if (!subscription) { | ||
| // No billing subscription - not allowed to create anything | ||
| return false | ||
| } else if (subscription.isActive()) { |
There was a problem hiding this comment.
General feedback for this file: The nested if statements aren't needed here as each path is returning. Following the return early pattern might make this slightly easier to follow.
forge/containers/wrapper.js
Outdated
| this._app.log.error(`Problem adding project to subscription: ${err}`) | ||
| throw new Error('Problem adding project to subscription') | ||
| const billingState = await project.getSetting(this.KEY_BILLING_STATE) | ||
| if (billingState !== this.BILLING_STATES.TRIAL) { |
There was a problem hiding this comment.
Additionally, #1642 added two helper methods, SubscriptionHandler.addProject and SubscriptionHander.removeProject that this logic should be moved into to keep subscription related logic in once place.
| @@ -42,14 +65,22 @@ module.exports = { | |||
| // Should this subscription be treated as active/usable | |||
| // Stripe states such as past_due and trialing are still active | |||
| isActive () { | |||
| return !this.isCanceled() | |||
| return this.status === STATUS.ACTIVE | |||
There was a problem hiding this comment.
The comment above is now out of date. I also think we should have a clearer naming convention to clarify that we're talking about the Stripe subscription, rather than the local subscription. Arguably a person in a trial has an active local subscription, they just don't have a stripe subscription yet. I'd suggest isActiveInStripe() and isActive(), which would make the intension much clearer.
Do I want to guard against them not being set up in stripe? => isActiveInStripe()
Do I want to guard against the team having any form of active subscription? => isActive()
Edit: perhaps isBillingActive()?
There was a problem hiding this comment.
This can be addressed as part of any wider billing changes in a follow up.
| @@ -240,7 +252,7 @@ module.exports.init = async function (app) { | |||
| const billingIds = getBillingIdsForTeam(team) | |||
|
|
|||
| const subscription = await app.db.models.Subscription.byTeamId(team.id) | |||
| if (subscription) { | |||
| if (subscription && subscription.isActive()) { | |||
There was a problem hiding this comment.
This is an example where I feel subscription?.isActiveInStripe() communicates the intension much clearer.
Not all subscriptions that are 'active' need to go through this step.
| await app.auditLog.User.account.verify.autoCreateTeam(request.session?.User || verifiedUser, null, team) | ||
|
|
||
| if (app.license.active() && app.billing && app.settings.get('user:team:trial-mode')) { |
There was a problem hiding this comment.
[nb] General feedback for our codebase, I think app.settings should offer helpers like app.settings.trialModeEnabled() non blocking
|
@knolleary Tests pass with the change I made above 442d1fd, I'm 👍, I'll leave it to you to merge and trigger a staging deploy. |
|
We had an unrelated test failure -waiting the re-run to pass then will merge etc. |
Description
Work-in-progress : will update this description with progress
This introduces the new Team Trial mode as described in #1578.
Database changes
Subscription.statusenum extended to includetrialSubscription.trialStatuscolumn added - an enum that tracks the state of the trial (such as what emails have been sent)Subscription.trialEndsAt- date when the trial endsAPI Changes
The
/api/v1/team/:teamidendpoint has been updated to include billing information:billingobject is added to the response if billing is enabled on the platformbilling.active- boolean, whether there is an active Stripe subscription for this teambilling.canceled- boolean, whether the Stripe subscription is canceledbilling.trial- boolean, whether this is a trial Subscriptionbilling.trialEnded- boolean, whether the trial has endedbilling.trialEndsAt- date, when the trial endsbilling.trialProjectAllowed- boolean, whether the team is allowed to create a trial project (ie, they don't already have a trial project created)Project Create api returns 403/Payment Required if called but no billing setup and not a permitted trial project
Housekeeper
A house keep task is added that runs every 30 minutes. It looks for teams that have ended their trial and does the work to either suspend projects (because billing not configured) or to add project to billing. It then sends the team owners an email to notify them.
It also looks for trials that end in 8 days or 2 days and sends a reminder email about the pending end of the trial
UI
The team gets a new banner when in trial mode that clearly communicates how much time is left in the trial and whether they need to setup billing or not.
Create Project
When in trial mode, without billing configured, the create project page disables all of the non-trial type projects and labels the Trial Type project as 'Free', but with the regular cost also listed. The Charges summary table reflects this as well.
Once they setup billing, the create project page allows them to create anything as normal.
I'm trying to structure the commits to make review easier, however the work has evolved to a point the earlier commits are reworked by previous ones.
trialEndsAtto theSubscriptionobject for better tracking (and to not pollute non-ee model with ee concepts)Tasks
billing_stateadded by Store project billing state when adding/removing subscription #1619)Related Issue(s)
#1578
Checklist
flowforge.yml?flowforge/helmto update ConfigMap Templateflowforge/CloudProjectto update values for Staging/ProductionLabels
backportlabelarea:migrationlabel