-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(api-service): Nv 5304 update pricing and billing pages #7718
base: next
Are you sure you want to change the base?
Conversation
❌ Deploy Preview for dashboard-v2-novu-staging failed. Why did it fail? →
|
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
e764002
to
ea985f6
Compare
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.
Please fix the spellcheck errors as well.
Also, we need to change this dashboard part as well:
https://github.com/novuhq/novu/blob/bfb8b9517dd4d8342c0f9850974fb226882ec10c/apps/web/src/ee/billing/components/PlanActionButton.tsx
The action button to "Upgrade plan" has a hardcoded http call like this:
...
api.post(checkoutUrl, {
billingInterval: selectedBillingInterval,
apiServiceLevel: ApiServiceLevelEnum.BUSINESS,
}),
...
The apiServiceLevel on the button need to change dynamically based on selected plan.
apps/api/src/app/billing/e2e/checkout-session-completed.e2e-ee.ts
Outdated
Show resolved
Hide resolved
apps/api/src/app/environments-v1/usecases/create-environment/create-environment.usecase.ts
Outdated
Show resolved
Hide resolved
apps/api/src/app/environments-v1/usecases/create-environment/validate-tiers-use.case.ts
Outdated
Show resolved
Hide resolved
libs/dal/src/repositories/organization/community.organization.repository.ts
Outdated
Show resolved
Hide resolved
9d4d26f
to
814ffb7
Compare
c1eb199
to
4a1cd50
Compare
17fe776
to
53f9a99
Compare
53f9a99
to
330ddad
Compare
@@ -27,9 +27,6 @@ export class CreateEnvironment { | |||
_organizationId: command.organizationId, | |||
}); | |||
|
|||
if (environmentCount >= 10) { |
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.
I am not quite sure how do we enforce the environment creation limit after this change.
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 done with a decorator on the controller, we said the number is irrelevant it's a Yes / No Question, the create + delete enpooints are blocked if the tier decides so
apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts
Outdated
Show resolved
Hide resolved
.../api/src/app/workflows-v2/usecases/service-level-tier-validator/tier-validation-type.enum.ts
Outdated
Show resolved
Hide resolved
apps/api/src/app/workflows-v2/usecases/service-level-tier-validator/validate-tiers-use.case.ts
Outdated
Show resolved
Hide resolved
}; | ||
|
||
function augmentBasedOfFeatureFlags(highlightsArray: Partial<PlanHighlights>, featureFlags: FeatureFlags) { | ||
if (!featureFlags[FeatureFlagsKeysEnum.IS_2025_Q1_TIERING_ENABLED]) { |
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.
Consider using the useFeatureFlag
to read the feature flad to avoid introducing a new pattern on feature flag consumption.
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.
I cannot use this function locally, it has to propegate between function, in such case I prefer to create code that doesn't have to be deleted completly, that's why I introduced a new pattern, the feature flags can be there propagated and if needed to make augmentations they can be done there
apps/api/src/app/workflows-v2/usecases/service-level-tier-validator/index.ts
Outdated
Show resolved
Hide resolved
apps/api/src/app/workflows-v2/usecases/service-level-tier-validator/tier-exceeded.exception.ts
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,678 @@ | |||
import { ApiServiceLevelEnum, FeatureFlags, FeatureFlagsKeysEnum } from '../types'; | |||
|
|||
export enum FeatureNameEnum { |
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.
Enlisting all features of the product in a enum in shared along with presentational concerns feels convoluted a this point. I suggest modelling only the features that are different per plan and affect the business logic and the execution. The rest are just static values for the pricing page.
The best way to think about it is do the exercise of putting the features that differ per plan in Stripe metadata. We will do the clean up right after this PR. In that case we wouldn't add all of these from shared to Stripe. We would only add the things that change per plan.
I recognize the effort of putting everything in one place but the result is more convoluted than the complexity of our pricing model at this point.
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.
Even features in stripe should be populated from a single source of truth, that's the attempt to create it, you don't want this pricelist to exist partially in 6 locations, it makes the maintenance extremely uncomfortable, the point here it to make the next time much easier
d226632
to
1d18922
Compare
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer