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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(console): App storage billing integration #2837

Merged
merged 15 commits into from
Feb 20, 2024

Conversation

Cosmin-Parvulescu
Copy link
Contributor

Description

Related Issues

Testing

Checklist

  • I have read the CONTRIBUTING guidelines
  • I have tested my code (manually and/or automated if applicable)
  • I have updated the documentation (if necessary)

@Cosmin-Parvulescu Cosmin-Parvulescu added the enhancement Indicates new feature requests label Feb 6, 2024
@Cosmin-Parvulescu Cosmin-Parvulescu self-assigned this Feb 6, 2024
@Cosmin-Parvulescu Cosmin-Parvulescu changed the base branch from main to feat/console/App-storage-package-modal February 6, 2024 11:22
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/App-storage-package-modal branch 2 times, most recently from 191a751 to 06056e9 Compare February 12, 2024 11:19
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/App-storage-billing-integration branch from a052837 to f98bb5c Compare February 12, 2024 11:23
Base automatically changed from feat/console/App-storage-package-modal to main February 12, 2024 13:43
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/App-storage-billing-integration branch from 18d28f6 to 51219ee Compare February 12, 2024 13:49
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/App-storage-billing-integration branch from eeeccaa to 0191aed Compare February 13, 2024 10:50
@Cosmin-Parvulescu Cosmin-Parvulescu marked this pull request as ready for review February 13, 2024 11:09
@@ -24,6 +25,7 @@ const AppDataStorageModal: React.FC<AppDataStorageModalProps> = ({
subscriptionFetcher,
currentPackage,
topUp = false,
currentCost = 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to name this cost as opposed to price? Preferably all wording around this should be uniform, ie. price.

@@ -262,14 +263,19 @@ export const action: ActionFunction = getRollupReqFunctionErrorWrapper(

URN = metaDel.URN as IdentityRefURN

const delSub = event.data.object as Stripe.Subscription
delSub.items.data.forEach(async (item) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be converted to an array of promises which can be Promise.all()d, otherwise some of these async calls may run concurrently with the promises below or not finish at all.

@@ -22,6 +22,8 @@ interface Env {
SECRET_STRIPE_WEBHOOK_SECRET: string
SECRET_STRIPE_PRO_PLAN_ID: string
SECRET_STRIPE_GROUP_SEAT_PLAN_ID: string
SECRET_STRIPE_APP_DATA_STORAGE_STARTER_PRICE_ID: string
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we combine these two into a singular SECRET_STRIPE_APP_DATA_STORAGE_PRICE_IDS containing JSON content representing the same? The second level price IDs (ie. service-specific tiers) don't pollute the env var space too much.

@@ -66,6 +67,8 @@ jobs:
SECRET_STRIPE_WEBHOOK_SECRET: ${{ secrets.SECRET_STRIPE_WEBHOOK_SECRET_DEV }}
SECRET_STRIPE_PRO_PLAN_ID: ${{ secrets.SECRET_STRIPE_PRO_PLAN_ID_DEV }}
SECRET_STRIPE_GROUP_SEAT_PLAN_ID: ${{ secrets.SECRET_STRIPE_GROUP_SEAT_PLAN_ID_DEV }}
SECRET_STRIPE_APP_DATA_STORAGE_PRICE_IDS: ${{ secrets.SECRET_STRIPE_APP_DATA_STORAGE_PRICE_IDS_DEV }}
SECRET_STRIPE_APP_DATA_STORAGE_SCALE_PRICE_ID: ${{ secrets.SECRET_STRIPE_APP_DATA_STORAGE_SCALE_PRICE_ID_DEV }}
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't need this at the top level anymore, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, just the PRICE_IDS

@betimshahini betimshahini merged commit 9245b65 into main Feb 20, 2024
15 of 16 checks passed
@betimshahini betimshahini deleted the feat/console/App-storage-billing-integration branch February 20, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(console): App storage billing integration
2 participants