feat: logins of already-existing users result in the user's customer contact info being updated (e.g. in stripe.com dashboard) #2103

gobengo · 2022-11-10T17:31:29Z

Motivation:

business wants the stripe dashboard to be useful as a CRM of sorts, i.e. have pretty up to date contact info on the customer records there
- before this PR, we only write that contact info for new user signups. This PR makes logins of previously-existing-users also result in an update of the customer contact info in stripe.
https://www.notion.so/on-user-login-update-stripe-com-customer-with-latest-name-email-fed8d970688f4bfd8281dc6cc31581bc

Todo

write main business logic, tests that work against mock (not against stripe apis)
unit test for StripeCustomerService updateContact
integration test for StripeCustomerService updateContact
manual test login locally after stripe, ensure no errors

Challenges

testing everything locally against an old user is hard because localdev always has an emptyish database of users.

How to Test on staging/production

log in with email should work without error
log in with github should work without error
in prod, @gobengo can test with his oldish user and ensure that re-logging in will update the corresponding name/email of customer in stripe

…t being updated

github-actions · 2022-11-11T00:16:16Z

`package-lock.json` changes

Click to toggle table visibility

Name	Previous	Current
`@types/sinon`	-	10.0.13
`@types/sinonjs__fake-timers`	-	8.1.2
`nise`	5.1.1	5.1.2

github-actions · 2022-11-11T00:20:52Z

Website preview 🔗✨

build log

… error when extra props are passed as part of contact

yusefnapora

Looks solid to me and nicely tested.

Out of curiosity, is it actually possible for an email-registered user to change their email but keep the same issuer? I noticed you have a test for it, but wasn't aware that was a thing 😄 Either way, the test is a good idea and makes sense to include.

gobengo · 2022-11-11T17:05:59Z

@yusefnapora tysm!

Out of curiosity, is it actually possible for an email-registered user to change their email but keep the same issuer? I noticed you have a test for it

I haven't seen in happen. But it's kind of an implementation detail of how magic.link works, which I don't really understand! :P That scenario might happen if you log in with github, but your github-profile email changed since last login. magic.link might give same issuer (which is our pk in the user table) but the updated email? idk but I wouldn't be surprised if it's possible.

It seemed like reasonable business logic to encode as a regression test. and if we learn otherwise it's hopefully easy to adjust it

travis

this all looks great to me! see my comment re: Stripe's "test mode" API and investigating a version of the CustomerService that uses it for end-to-end testing - if you think that's worth digging into I'd be happy to spend some time today/next week spiking this out

travis · 2022-11-11T18:41:01Z

packages/api/test/user.spec.js

@@ -454,4 +454,174 @@ describe('userLoginPost', function () {
    assert.ok(gotSubscription, 'gotSubscription is truthy')
    assert.equal(gotSubscription.storage?.price, storagePriceNames.free)
  })
+  it('login to email-originating user updates customer contact', async function () {


not totally clear to me what "email-originating user" means here - is this just any user who signed up with an email?

travis · 2022-11-11T18:41:28Z

packages/api/test/stripe.spec.js

+    }
+    await customers.updateContact(customer.id, contactUpdate2)
+    const contact2 = await customers.getContact(customer.id)
+    assert.deepEqual(contact2, contactUpdate2, 'getContact returns same contact as the udpate')


very minor tpyo here ('udpate' -> 'update')

travis · 2022-11-11T18:44:30Z

packages/api/src/utils/stripe.js

+      if (hasOwnProperty(error, 'code')) {
+        switch (error.code) {
+          case 'resource_missing':
+            return new CustomerNotFound('customer not found')


I have complicated feelings about errors as flow control so don't disagree with this pattern, but curious if there's a deeper philosophical reason we're returning this CustomerNotFound rather than throwing it?

@travis in short, it's for type safety of the 'error' result value types. afaik tsc won't typecheck the type of what gets thrown. It's definitely unusual compared to the whole world of js, but I like the pattern of having 'result types' that are union of both 'happy path' and specific error tyeps. iirc @yusefnapora seemed fine with it too in earlier review. When I used it in here originally I was a bit inspired by how @Gozala was using them in ucanto. https://github.com/web3-storage/ucanto/blob/b752b398950120d6121badcdbb639f4dc9ce8794/packages/interface/src/lib.ts#L267

travis · 2022-11-11T19:05:56Z

packages/api/src/utils/stripe.js

+/**
+ * Create a mock stripe sdk instance that can be used with the StripeCustomersService
+ * @returns {StripeComForCustomersService}
+ */
 export function createMockStripeForCustomersService () {


this looks good and makes sense, AND is starting to get to the level of complexity where it seems worth investigating whether we can use Stripe's API in "test mode" (https://stripe.com/docs/keys#test-live-modes) to get some end-to-end testing into place.

I dug around in the Stripe API docs enough to convince myself it would probably be worth investigating this but not enough to be sure they actually implement this particular API functionality in test mode - I would definitely be down to do a quick spike on this if you think it makes sense to keep digging into this!

adding - if I do find this is possible, I'd want to add it to a test suite that primarily gets run during CI so we can still run the existing tests offline/on a plane. it seems like test:e2e in the top level package.json might be a good place for this - seems like I'd need to add a similar test:e2e script to the api project, does that sound right?

🤖 I have created a release *beep* *boop* --- ## [7.12.0](api-v7.11.3...api-v7.12.0) (2022-11-11) ### Features * logins of already-existing users result in the user's customer contact info being updated (e.g. in stripe.com dashboard) ([#2103](#2103)) ([a365b4c](a365b4c)) --- 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>

logins of already-existing users result in the user's customer contac…

c7f0700

…t being updated

gobengo changed the title ~~logins of already-existing users result in the user's customer contac…~~ feat: logins of already-existing users result in the user's customer contac… Nov 10, 2022

gobengo self-assigned this Nov 10, 2022

gobengo changed the title ~~feat: logins of already-existing users result in the user's customer contac…~~ feat: logins of already-existing users result in the user's customer contact info being updated (e.g. in stripe.com dashboard) Nov 10, 2022

gobengo added 2 commits November 10, 2022 11:33

fix unit tests

e6d85c0

test StripeCustomersService updateContact / getContact

8b5c44a

fix how StripeCustomersService#updateContact calls stripe sdk, fixing…

27db32b

… error when extra props are passed as part of contact

gobengo marked this pull request as ready for review November 11, 2022 00:26

gobengo requested review from yusefnapora and travis November 11, 2022 00:26

gobengo added 2 commits November 10, 2022 16:29

lint

da8ab71

fix test

3ad6def

yusefnapora approved these changes Nov 11, 2022

View reviewed changes

gobengo merged commit a365b4c into main Nov 11, 2022

gobengo deleted the 1667928778-githubsigninnames branch November 11, 2022 19:05

travis approved these changes Nov 11, 2022

View reviewed changes

github-actions bot mentioned this pull request Nov 11, 2022

chore(main): release api 7.12.0 #2104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: logins of already-existing users result in the user's customer contact info being updated (e.g. in stripe.com dashboard) #2103

feat: logins of already-existing users result in the user's customer contact info being updated (e.g. in stripe.com dashboard) #2103

gobengo commented Nov 10, 2022 •

edited

Loading

github-actions bot commented Nov 11, 2022

github-actions bot commented Nov 11, 2022 •

edited

Loading

yusefnapora left a comment

gobengo commented Nov 11, 2022 •

edited

Loading

travis left a comment

travis Nov 11, 2022

travis Nov 11, 2022

travis Nov 11, 2022

gobengo Nov 11, 2022

travis Nov 11, 2022

travis Nov 11, 2022

feat: logins of already-existing users result in the user's customer contact info being updated (e.g. in stripe.com dashboard) #2103

feat: logins of already-existing users result in the user's customer contact info being updated (e.g. in stripe.com dashboard) #2103

Conversation

gobengo commented Nov 10, 2022 • edited Loading

github-actions bot commented Nov 11, 2022

package-lock.json changes

github-actions bot commented Nov 11, 2022 • edited Loading

Website preview 🔗✨

yusefnapora left a comment

Choose a reason for hiding this comment

gobengo commented Nov 11, 2022 • edited Loading

travis left a comment

Choose a reason for hiding this comment

travis Nov 11, 2022

Choose a reason for hiding this comment

travis Nov 11, 2022

Choose a reason for hiding this comment

travis Nov 11, 2022

Choose a reason for hiding this comment

gobengo Nov 11, 2022

Choose a reason for hiding this comment

travis Nov 11, 2022

Choose a reason for hiding this comment

travis Nov 11, 2022

Choose a reason for hiding this comment

gobengo commented Nov 10, 2022 •

edited

Loading

`package-lock.json` changes

github-actions bot commented Nov 11, 2022 •

edited

Loading

gobengo commented Nov 11, 2022 •

edited

Loading