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

[stripe] Use AttributionID metadata when querying for customers #12762

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Sep 8, 2022

Description

Customers in staging & prod have been updated with this metadata.

Counterpart PR for usage component is here #12755

Related Issue(s)

Fixes #

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@easyCZ easyCZ requested a review from a team September 8, 2022 09:00
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Sep 8, 2022
@roboquat roboquat added the size/S label Sep 8, 2022
@roboquat roboquat merged commit 05141be into main Sep 8, 2022
@roboquat roboquat deleted the mp/stripe-use-attribution-id-query-server branch September 8, 2022 09:18
// userId is deprecated, use attributionId where possible
userId: user.id,
attributionId: AttributionId.render({ kind: "user", userId: user.id }),
ATTRIBUTION_ID_METADATA_KEY: AttributionId.render({ kind: "user", userId: user.id }),
Copy link
Contributor

@jankeromnes jankeromnes Sep 8, 2022

Choose a reason for hiding this comment

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

I don't think this is going to work (you're creating an object key that's literally "ATTRIBUTION_ID_METADATA_KEY").

If you really want to make ATTRIBUTION_ID_METADATA_KEY configurable with a global const in this file, you'd want to first create a const customerSpec = { email, name, metadata: {} } and then set customerSpec.metadata[ATTRIBUTION_ID_METADATA_KEY] = ....

Copy link
Contributor

Choose a reason for hiding this comment

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

However, I don't think it's useful to factor out ATTRIBUTION_ID_METADATA_KEY. So, maybe it's simpler to just use attributionId everywhere in-line. (This is unlikely to change in the future, but still easy to change if needed.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Revert is here. Thanks for spotting this. I applied Go style thinking it would use it as a key, not as a literal..

@jankeromnes
Copy link
Contributor

jankeromnes commented Sep 8, 2022

@easyCZ @andrew-farries FYI, this breaks the creation of new customers (please see comment above). This could have been caught immediately with a deployed preview + minimal "How to test" instructions.

@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants