-
Notifications
You must be signed in to change notification settings - Fork 212
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(payments): rename downloadURL to successActionButtonURL #12904
Conversation
c6fa9c2
to
bdbc5bf
Compare
Because: * The button on the success screen allows for a configurable label, which means it can be used for more than just downloading the product, which makes the configuration field "downloadURL" misleading. This commit: * Adds support for a new configuration field "successActionButtonURL" to replace "downloadURL". * Updates the code to reflect the name change. Closes #9338
bdbc5bf
to
e784754
Compare
@@ -2354,9 +2354,13 @@ export class StripeHelper extends StripeHelperBase { | |||
const productMetadata = this.mergeMetadata(plan, abbrevProduct); | |||
const { | |||
emailIconURL: planEmailIconURL = '', | |||
downloadURL: planDownloadURL = '', | |||
downloadURL, |
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.
Why is this still here since we are replacing the "download" url in this patch?
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.
Ok, I see a TODO below that's likely the reason for this. Is there a jira file to remove this?
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.
Ah yes that's what I forgot. There is one now, thank you.
@@ -67,7 +67,8 @@ class SetPassword extends FormView { | |||
const plan = plans.find((p) => p.product_id === productId); | |||
const url = new URL( | |||
plan && plan.product_metadata | |||
? plan.product_metadata.downloadURL | |||
? plan.product_metadata.successActionButtonURL || | |||
plan.product_metadata.downloadURL | |||
: 'https://mozilla.org' |
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'm surprised that prettier didn't wrap some parens around this.
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.
👍
Because
which means it can be used for more than just downloading the product,
which makes the configuration field "downloadURL" misleading.
This pull request
replace "downloadURL".
Issue that this pull request solves
Closes: #9338
Checklist
Put an
x
in the boxes that applyAdditional Info
successActionButtonURL
to all products in Stripe Dev and Staging