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.

Sign up for GitHub

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

Jump to bottom

feat: I can choose a storage pricing tier (issue 1869) #1878

Merged
merged 55 commits into from
Sep 20, 2022
Merged

feat: I can choose a storage pricing tier (issue 1869) #1878

merged 55 commits into from
Sep 20, 2022

Conversation

gobengo
Copy link
Contributor

@gobengo gobengo commented Sep 10, 2022 •
edited
Loading

Motivation:

Parts

  • implement api contract stubbed in userPaymentPut
    added test here 
    const desiredPaymentSettings = {
  method: { id: `pm_${randomString()}` },
  subscription: { storage: { price: `price_test_${randomString()}` } }
}
  • @cmunns ensure /account/payment page can write the pricing tier to the PUT /account/payment api
  • @gobengo add quick hack where GET /account/payment?mockSubscription=true will include a mock subscription with storage price - this should unblock frontend fetching/rendering it for dev/testing
    added here
    Opt-in to getting a realistic paymentSettings.subscription:
    • Request GET /user/payment?mockSubscription=true
    • Response 
      {
  subscription: { storage: { price: 'price_mock_userPaymentGet_mockSubscription' } }
}
  • create use case object saveStorageSubscription and use from userPaymentPut
  • can saveStorageSubscription with services backed by stripe.com APIs
  • @cmunns ensure /account/payment page can read the pricing tier from the GET /account/payment?mockSubscription=true api
  • can getStorageSubscription(customer)
  • getStorageSubscription(customer) called from api userPaymentGet
  • test for a new user choosing a paid tier for first time
  • test for upgrading from one paid tier to another
  • test for downgrading from one paid tier to another
  • test for downgrading from one paid tier to a free tier

@gobengo @cmunns
chore: feat/accounts-payments-ui-2 -> issue 1869 (choose pricing tier) (
a99ae8b 
#1876)

* Add typechecking via tsc to packages/api and start WIP billing/stripe save logic

* fix type check

* rm new stripe stuff for now. just focus on tsc

* fix bug in pagination from bad merge

* remove unneeded .paths in packages/api/tsconfig.json

* build: packages/api build script runs typecheck

* Revert "rm new stripe stuff for now. just focus on tsc"

This reverts commit 7957f38.

* wip

* packages/api typecheck checks test dir

* feat: split out plans page, start planning onboard mods

* fix typecheck on new billing tests

* fix test

* fix tests

* PUT /user/payment API calls saveUserPayments

* add packages/api/test/stripe.spec.js

* wip stripe

* add user_customer table and use in StripeCustomersService

* finish StripeCustomersService#getOrCreateUser

* cf workers tests use a createTestBillingContext

* narrow type on createStripeBillingContext

* StripeCustomersService has a stripe instance with paymentMethods.attach

* StripeBillingService calls stripe.paymentMethods.attach

* tests pass

* working stripe userPaymentGet

* fix getUserCustomer when no customer is set

* Fix StripeBillingService tests

* add test 'can userPaymentPut and then userPaymentGet'

* userPaymentGet includes stripe.com card info if paymentMethod is a stripe card

* wip

* feat: split out plans page, start planning onboard mods

* website playwright has globalTeardown that runs 'npm stop'

* ci: website workflow sets STRIPE_SECRET_KEY

* wip

* get rid of playwright globalTeardown as it seems to prevent writing of html reporter

* add db migration 023-add-user-customer-table.sql

* add STRIPE_SECRET_KEY (commented) to .env.tpl

* rm env.mockStripePaymentMethodId

* relocate some things

* rename db stripe.spec.js -> billing.spec.js

* increase timeout of test w3name backward compatibility proxy

* db mocha hooks log more to debug failed ci

* feat: get and update cards with loading

* feat: rework payment page to consolidated version

* feat: plan switching by role

* feat: re-arrange payment page, add loading states and move form into modal

* fix: while loading cards dont show buttons

* fix: move buttons into component

* website/pages/account/payment uses env var NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY instead of old one ('..._TEST_PK')

* website test-e2e has env var NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY

* account payment page throws if not STRIPE_PUBLISHABLE_KEY

* fix how payment page constructs Stripe

* fix accountPayment.e2e test with new design

* change how website gets stripe config and logging for debug

* wip

* website test-e2e doesnt set STRIPE_SECRET_KEY at all

* only log STRIPE_SECRET_KEY length

* clean up diff

Co-authored-by: Adam Munns <adam@whetinteractive.com>
@gobengo gobengo linked an issue Sep 10, 2022 that may be closed by this pull request
I can choose a storage subscription price #1869
Closed
@github-actions
Copy link
Contributor

github-actions bot commented Sep 10, 2022 •
edited
Loading

Website preview 🔗✨

build log

@gobengo gobengo changed the title feat: I can choose a storage pricing tier issues/1869 feat: I can choose a storage pricing tier (issue 1869) Sep 10, 2022
gobengo and others added 16 commits September 9, 2022 19:04
@gobengo
fix api test
9e59448
@gobengo
fix userPaymentPut + userPaymentGet test to actually assert that paym…
e44ed86 
…entMethod is saved
@cmunns
feat: add paymentplan mock ui and put
bb4a04d
@gobengo
wire up mock SubscriptionsService and use in tests
4f2b55b
@gobengo
fix small bug on pages/account/payment when .storage.subscription is …
22720b1 
…null
@gobengo
fix mockSubscription
533503b
@gobengo
fix broken test
150c193
@gobengo
fix test
8f71483
@gobengo
add temporary feature to userPaymentPut allowing omitting the .subscr…
05adb2a 
…iption
@gobengo
addPaymentMethodForm uses ?dangerouslyDefaultSubscriptions=true (temp…
27fdfdd 
…orarily)
@cmunns
fix: update modal open and always pass userPlan to payment put
29984b1
@gobengo
start StripeSubscriptionsService and test, though it needs to work di…
04fd5f2 
…fferently on save
@gobengo
small lint
31aa1c9
@gobengo
Improve StripeSubscriptionsService to better handle changing subsription
283a57e
@gobengo
website no longer uses mockSubscription=true
3d9508d
@gobengo
StripeSubscriptionService can unset storage subscription
d5ba424
gobengo
gobengo commented Sep 19, 2022
View reviewed changes
packages/website/lib/api.js Outdated Show resolved Hide resolved
yusefnapora
yusefnapora approved these changes Sep 19, 2022
View reviewed changes
Copy link
Contributor

@yusefnapora yusefnapora left a comment

Choose a reason for hiding this comment

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

This looks great 💯 with the caveat that I didn't pull it down and try to run it, just read through the diff.

@gobengo you did a great job testing the stripe API boundary IMO - I especially like the assertions on the number of API calls & verification that they were called with the right args.

Looks really nicely put together overall, great job guys.

packages/api/src/user.js
const method = { id: paymentMethodId }
const subscriptionInput = requestBody?.subscription
if (typeof subscriptionInput !== 'object') {
throw Object.assign(new Error(`subscription must be an object, but got ${typeof subscriptionInput}`), { status: 400 })
Copy link
Contributor

Choose a reason for hiding this comment

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

Hadn't seen this pattern for adding "ad-hoc" fields to an Error before. Pretty clever 👍

packages/api/src/utils/logs.js
@@ -117,6 +117,10 @@ export class Logging {
}

async postBatch () {
if (process.env.NODE_ENV === 'development') {
Copy link
Contributor

Choose a reason for hiding this comment

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

good call 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was slowing down my tests! :P

packages/api/test/stripe.spec.js
it('can saveSubscription between prices using real stripe.com API', async function () {
// ensure stripeSecretKey and use it to construct stripe
const stripeSecretKey = process.env.STRIPE_SECRET_KEY
if (!stripeSecretKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

that's cool - for some reason it never occurred to me that you could skip a test dynamically like that; I've always just added it to the it call if I knew it wasn't going to pass for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(FWIW me too, I think I copypastad the it.skip() otherwise I might not have thought about it)

packages/website/components/account/paymentTable.js/paymentTable.js Outdated
<span>
Your current plan is: <strong>{currentPlan.title}</strong>
</span>
<small>Billing Cycle: Aug 18 - Sept 18</small>
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably shouldn't be hard-coded 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oy. Good catch. Thank you!

packages/website/pages/account/payment.js Outdated Show resolved Hide resolved
@gobengo gobengo merged commit 58de180 into main Sep 20, 2022
@gobengo gobengo deleted the 1869-i-can-choose-a-storage-subscription-price branch September 20, 2022 15:00
This was referenced Sep 20, 2022
Merged
Merged
olizilla pushed a commit that referenced this pull request Sep 21, 2022
@github-actions
chore(main): release api 7.6.0 (#1884)
db387f8 
🤖 I have created a release *beep* *boop*
---


##
[7.6.0](api-v7.5.1...api-v7.6.0)
(2022-09-20)


### Features

* I can choose a storage pricing tier (issue 1869)
([#1878](#1878))
([58de180](58de180))
* put write to cluster behind a flag
([#1785](#1785))
([eae75d2](eae75d2))
* send timing info to logtail. Time r2 & s3.
([#1908](#1908))
([8dff635](8dff635))
* use sha256 checksum for r2.put
([#1910](#1910))
([d277ba0](d277ba0))


### Other Changes

* add a script that generates many uploads
([#1790](#1790))
([e38b7a0](e38b7a0))
* encode sha256 hash as hex for r2 put
([#1911](#1911))
([3d66fa9](3d66fa9))
* silence logging debug statements in test/dev
([#1870](#1870))
([a576131](a576131))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@gobengo gobengo mentioned this pull request Sep 21, 2022
Merged
gobengo added a commit that referenced this pull request Sep 21, 2022
@gobengo @cmunns
fix: bugs saving credit card at /account/payment for the first time (#…
724e3ab 
…1913)

Motivation:
* fix bug discovered while testing on staging
#1878
* bug prevented adding a payment method for the first time in both 'add
payment form' in lower left of payment page as well as the form in the
accountPlanModal
* I think this was introduced in [this
commit](8adff7b)
after last PR review
[approval](#1878 (review))

Co-authored-by: Adam Munns <adam@whetinteractive.com>
gobengo pushed a commit that referenced this pull request Sep 21, 2022
@github-actions
chore(main): release website 2.20.0 (#1888)
e60c880 
🤖 I have created a release *beep* *boop*
---


##
[2.20.0](website-v2.19.1...website-v2.20.0)
(2022-09-21)


### Features

* I can choose a storage pricing tier (issue 1869)
([#1878](#1878))
([58de180](58de180))


### Bug Fixes

* bugs saving credit card at /account/payment for the first time
([#1913](#1913))
([724e3ab](724e3ab))
* make browser update versions absolute
([#1887](#1887))
([bca96fa](bca96fa))


### Other Changes

* **docs:** add curl CAR upload example to store howto
([#1735](#1735))
([54e31d9](54e31d9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@gobengo
Copy link
Contributor Author

gobengo commented Sep 23, 2022

works on prod

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yusefnapora yusefnapora yusefnapora approved these changes

@cmunns cmunns cmunns approved these changes

Assignees

@gobengo gobengo

@cmunns cmunns

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

I can choose a storage subscription price
3 participants
@gobengo @yusefnapora @cmunns