Skip to content

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Jul 20, 2023

Fixes #31982

Proposed changes:

We currently have the following logic for the support doc link in the Stripe connection nudge banner:

For the Payments buttons:

For the Donations block:

For the Premium Content block as well as any other block (this ends up being the default):

That last, default behavior seems to be the problem here, because we have at least one other block name, recurring-payments, that is not handled here. This PR fixes that by setting up a default.

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

  • N/A

Does this pull request change what data or activity we track or use?

  • No

Testing instructions:

This can be tested on WordPress.com Simple and on self-hosted, on a site where you have not tested those blocks before and have not connected your site to a Stripe account. In that scenario:

  1. Go to Posts > Add New
  2. Add a Payments block, and then pick each variation, one at a time, and check the support link in the Stripe connection banner.

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. [Pri] Low [Block] Payment Button aka Recurring Payments [Block] Payments aka Unified Intro [Block] Payments Intro [Block] Payment Buttons labels Jul 20, 2023
@jeherve jeherve self-assigned this Jul 20, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 20, 2023

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta > Jetpack and enable the update/payments-block-fees-link branch.

  • To test on Simple, run the following command on your sandbox:

    bin/jetpack-downloader test jetpack update/payments-block-fees-link
    

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Jul 20, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 20, 2023

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.


Jetpack plugin:

  • Next scheduled release: August 1, 2023.
  • Scheduled code freeze: July 24, 2023.

@jeherve jeherve added this to the jetpack/12.4 milestone Jul 20, 2023
@samiff samiff self-requested a review July 21, 2023 16:37
samiff
samiff previously requested changes Jul 21, 2023
Copy link
Contributor

@samiff samiff left a comment

Choose a reason for hiding this comment

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

Should the switch case statements be using break here? If I'm using a donations block for example, it currently falls through to the default for readMoreUrl.

@samiff samiff added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jul 21, 2023
jeherve and others added 2 commits July 24, 2023 17:02
@jeherve
Copy link
Member Author

jeherve commented Jul 24, 2023

Should the switch case statements be using break here? If I'm using a donations block for example, it currently falls through to the default for readMoreUrl.

Well yes that would help, wouldn't it? :) Done in 9ee7b91

@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jul 24, 2023
@zinigor zinigor dismissed samiff’s stale review July 25, 2023 07:55

I'm going to dismiss this review because the fix was implemented. Thanks for taking a look, Sami!

@zinigor zinigor merged commit deeeb2c into trunk Jul 25, 2023
@zinigor zinigor deleted the update/payments-block-fees-link branch July 25, 2023 07:55
@github-actions github-actions bot removed the [Status] Needs Review This PR is ready for review. label Jul 25, 2023
matticbot pushed a commit to Automattic/jetpack-storybook that referenced this pull request Jul 25, 2023
* Payments: add a default scenario for our upgrade nudges

Fixes #31982

* Add missing break statements

See Automattic/jetpack#31986 (review)

Co-authored-by: samiff <samiff@users.noreply.github.com>

---------

Co-authored-by: samiff <samiff@users.noreply.github.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/5654131963
matticbot pushed a commit to Automattic/jetpack-production that referenced this pull request Jul 25, 2023
* Payments: add a default scenario for our upgrade nudges

Fixes #31982

* Add missing break statements

See Automattic/jetpack#31986 (review)

Co-authored-by: samiff <samiff@users.noreply.github.com>

---------

Co-authored-by: samiff <samiff@users.noreply.github.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/5654131963
haqadn pushed a commit that referenced this pull request Aug 17, 2023
#31986)

* Payments: add a default scenario for our upgrade nudges

Fixes #31982

* Add missing break statements

See #31986 (review)

Co-authored-by: samiff <samiff@users.noreply.github.com>

---------

Co-authored-by: samiff <samiff@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Payment Button aka Recurring Payments [Block] Payment Buttons [Block] Payments Intro [Block] Payments aka Unified Intro [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Low [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connect to Stripe - Change Learn More URL
3 participants