jetpack icon indicating copy to clipboard operation
jetpack copied to clipboard

Move Simple Payments to PayPal Payments package

Open allie500 opened this issue 9 months ago • 16 comments

Fixes PAYPAL-13

Proposed changes:

  • This PR moves the Simple Payments block code to the PayPal Payments package.

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

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

Testing instructions:

Reviewers: Please check Slack. I have sent you a message with the PayPal Sandbox test account credentials for testing this PR.

  • Test this on the following sites:
    • WoA site without the business plan
    • WoA site with the business plan
    • JN site without Jetpack Complete plan
    • JN site with Jetpack Complete plan
    • Simple site without the business plan
    • Simple with the business plan
  • Use Jetapack Beta plugin to checkout this branch.

For each site check the following:

  • Is the 'Pay with PayPal' block available in the editor?
    • It should be for sites with plans.
    • It should not be for sites without plans.
  • For sites with plans:
    • In the editor, can you make changes to the product and save? Do the changes persist?
    • Add the PayPal Sandbox business email address in the block's address field for later payment testing.
    • Does the block render correctly on the front end?
    • If you click the 'Pay with PayPal' button does the PayPal modal successfully load?

It is possible to test payments using the PayPal Sandbox with the Simple Payments block. However, it is currently only possible to test payments on WoA and JN sites. Simple sites encounter a CORS issue when sandboxed, preventing payment testing regardless of the version of Jetpack running on the sandbox.

To test payments, do the following:

  • Visit your WoA or JN site.
  • Open the browser inspector's application tab
  • Under local storage, select your site's URL.
  • Then add the following key - value pair:
    • Key: simple-payments-env
    • Value: sandbox
  • Refresh the page, you should now see https://www.sandbox.paypal in the list of sites in local storage in the browser inspector application tab.
  • Navigate to the frontend page on your site with a Simple Payment block on it.
  • Click the Pay with PayPal button.
  • Add the PayPal Sandbox customer test account email address to the email field in the PayPal modal.
  • Click Next
  • Then click the Use Password Instead button.
  • Enter the password that you were provided in Slack.
  • Click login.
  • Then click Pay.
  • Wait a few moments (5-10 seconds). The modal should disappear.
  • You should see a success message show within the Simple Payments block
  • You should receive an email at the site's admin email address notifying you of a successful sale.

allie500 avatar May 08 '25 19:05 allie500

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 WoA dev 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, select your plugin (Jetpack or WordPress.com Site Helper), and enable the update/move-simple-payments-to-paypal-payments-pkg branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack update/move-simple-payments-to-paypal-payments-pkg
bin/jetpack-downloader test jetpack-mu-wpcom-plugin update/move-simple-payments-to-paypal-payments-pkg

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[bot] avatar May 08 '25 19:05 github-actions[bot]

Thank you for your PR!

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

  • :white_check_mark: Include a description of your PR changes.
  • :white_check_mark: Add a "[Status]" label (In Progress, Needs Review, ...).
  • :white_check_mark: Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  • :white_check_mark: Add testing instructions.
  • :white_check_mark: Specify whether this PR includes any changes to data or privacy.
  • :white_check_mark: 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 :robot:


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!


Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

  • WordPress.com Simple releases happen as soon as you deploy your changes after merging this PR (PCYsg-Jjm-p2).
  • WoA releases happen weekly.
  • Releases to self-hosted sites happen monthly:
    • Scheduled release: August 5, 2025
    • Code freeze: August 4, 2025

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

github-actions[bot] avatar May 08 '25 19:05 github-actions[bot]

@jeherve, so far I've only moved and lightly modified the Simple Payments block code. I see there is also a simple-payments directory within the jetpack/projects/plugins/jetpack/modules directory: https://github.com/Automattic/jetpack/tree/update/move-simple-payments-to-paypal-payments-pkg/projects/plugins/jetpack/modules/simple-payments

Should I make a modules directory in projects/packages/paypal-payments and move the three files there? Or would it be better to simply move the three files directly into the projects/packages/paypal-payments directory?

Thanks in advance for your feedback!

allie500 avatar May 08 '25 19:05 allie500

Code Coverage Summary

Coverage changed in 5 files.

File Coverage Δ% Δ Uncovered
projects/plugins/jetpack/extensions/shared/help-message.js 0/2 (0.00%) -100.00% 2 ❤️‍🩹
projects/plugins/jetpack/modules/module-headings.php 206/1000 (20.60%) 0.08% 0 💚
projects/plugins/jetpack/extensions/blocks/simple-payments/simple-payments.php 0/5 (0.00%) 0.00% -31 💚
projects/plugins/jetpack/modules/widgets/simple-payments.php 0/3 (0.00%) 0.00% -281 💚
projects/plugins/jetpack/modules/simple-payments/simple-payments.php 0/25 (0.00%) 0.00% -331 💚

8 files are newly checked for coverage. Only the first 5 are listed here.

File Coverage
projects/packages/paypal-payments/src/block/class-block.php 0/55 (0.00%) 💔
projects/packages/paypal-payments/src/legacy/class-paypal-payments-currencies.php 0/17 (0.00%) 💔
projects/packages/paypal-payments/src/legacy/class-simple-payments.php 0/344 (0.00%) 💔
projects/packages/paypal-payments/src/widget/class-simple-payments-widget.php 0/274 (0.00%) 💔
projects/packages/paypal-payments/src/widget/simple-payments/admin-warning.php 0/21 (0.00%) 💔

Full summary · PHP report · JS report

If appropriate, add one of these labels to override the failing coverage check: https://github.com/Automattic/jetpack/labels/Covered%20by%20non-unit%20tests https://github.com/Automattic/jetpack/labels/Coverage%20tests%20to%20be%20added%20later https://github.com/Automattic/jetpack/labels/I%20don%27t%20care%20about%20code%20coverage%20for%20this%20PR

jp-launch-control[bot] avatar May 08 '25 19:05 jp-launch-control[bot]

That's going to be a multi-step process I think. Here is how I would personally go about it, but that's just my personal opinion, feel free to take a different approach :)

The main entry point for the whole functionality is projects/plugins/jetpack/modules/simple-payments/simple-payments.php. I would recommend copying this to the package first, alongside the related files in that projects/plugins/jetpack/modules/simple-payments/ directory.

Once that's ported to the package, we can start requiring the package in the Jetpack plugin's composer.json file, stop requiring the file here: https://github.com/Automattic/jetpack/blob/5661c6259654a343b52c5cb1e0cfc8dfd3a401f3/projects/plugins/jetpack/modules/module-extras.php#L37 And instead create a new modules/simple-payments.php file that would instantiate the class from the package.

Once that's done, you could copy the contents of projects/plugins/jetpack/modules/widgets/simple-payments.php to the package, and then replace the contents of that widget file by a call to the same code in the package.

Then, the next step would be to copy the actual block from projects/plugins/jetpack/extensions/blocks/simple-payments/ into a new dir in the package, and create the webpack config file to build it, so you get build files that we'll be able to require in the Jetpack plugin and later on in the new standalone plugin. The Forms package has an example you could follow. Here is their webpack file for example: projects/packages/forms/tools/webpack.config.blocks.js. Once you have that set up, you would probably create a new class in the package, that would handle registering that block. You can see the Contact_Form_Block for an example of a block that lives in a package, is registered in a package, and then registration is called from the Jetpack plugin in projects/plugins/jetpack/extensions/blocks/contact-form/contact-form.php.

Once that's all done, you'll be able to deprecate the old classes from the Jetpack plugin, and they could then be removed entirely in a few releases.

At that point, you'll have a standalone package with code that can register a block, register a widget, and you'll be able to register that block or that widget from the Jetpack plugin and also from any other standalone plugin we create in the future.

Let me know if that makes sense.

jeherve avatar May 09 '25 09:05 jeherve

@jeherve I'm finally getting back over to this. Thanks so much for the detailed guidance. I'm going to follow that to get this moving forward.

allie500 avatar May 13 '25 16:05 allie500

That's going to be a multi-step process I think. Here is how I would personally go about it, but that's just my personal opinion, feel free to take a different approach :)

The main entry point for the whole functionality is projects/plugins/jetpack/modules/simple-payments/simple-payments.php. I would recommend copying this to the package first, alongside the related files in that projects/plugins/jetpack/modules/simple-payments/ directory.

Once that's ported to the package, we can start requiring the package in the Jetpack plugin's composer.json file, stop requiring the file here:

https://github.com/Automattic/jetpack/blob/5661c6259654a343b52c5cb1e0cfc8dfd3a401f3/projects/plugins/jetpack/modules/module-extras.php#L37

And instead create a new modules/simple-payments.php file that would instantiate the class from the package.

@jeherve I have completed all of the above. At this point, I have tried running the PHP unit tests for Jetpack plugin just to make sure everything is working. The tests fail to run with the error:

Verifying lock file contents can be installed on current platform.
Warning: The lock file is not up to date with the latest changes in composer.json. You may be getting outdated dependencies. It is recommended that you run `composer update` or `composer update <package name>`.
- Required package "automattic/jetpack-paypal-payments" is not present in the lock file.

I think at this point, I would need to publish the package to Packagist as the lock file doesn't get updated if I run composer update. I would publish using the PayPal Payments mirror repo, correct?

allie500 avatar May 13 '25 18:05 allie500

I just realized I totally skipped a command to properly add the package to the Jetpack plugin's composer.json file. I've run that and now I've got the lock file updated as well.

allie500 avatar May 13 '25 18:05 allie500

@jeherve I think I may have a mis-configuration in the package's phan config. It is throwing errors for things that I haven't changed in the file that weren't being flagged in the file when it was part of the Jetpack plugin.

projects/packages/paypal-payments/class-simple-payments.php:131
Error: TypeError PhanTypeMismatchArgument Argument 5 ($media) is false of type false but \wp_register_style() takes string defined at /home/runner/work/jetpack/jetpack/vendor/php-stubs/wordpress-stubs/wordpress-stubs.php:114599

And it is complaining about undeclared classes, but if I had the namespace to them, my IDE complains that those classes don't exist and Phan still flags them. 🤔

projects/packages/paypal-payments/class-simple-payments.php:115
Error: UndefError PhanUndeclaredClassMethod Call to method register_script from undeclared class \Automattic\Jetpack\Assets
projects/packages/paypal-payments/class-simple-payments.php:238
Error: UndefError PhanUndeclaredClassMethod Call to method is_connection_ready from undeclared class \Jetpack

Maybe I'm just missing a simple setting somewhere?

allie500 avatar May 14 '25 20:05 allie500

As we discussed in Slack:

  • Moving the new files into src will help.
  • Since you're introducing new files, you will indeed need to fix those issues reported by Phan, and then run jetpack phan --update-baseline packages/paypal-payments.

jeherve avatar May 15 '25 15:05 jeherve

Copying the Simple Payments block code to the package in this commit is the cause of unrelated extensions' JS tests to fail. I haven't been able to track down the root cause.

allie500 avatar May 16 '25 20:05 allie500

Copying the Simple Payments block code to the package in this commit is the cause of unrelated extensions' JS tests to fail. I haven't been able to track down the root cause.

I got some assistance in Slack (p1747425409956389/1746628203.330399-slack-CDLH4C1UZ) to figure out my error. Some of the dependencies in the package didn't match the dependencies in the the Jetpack plugin. This caused issues. I've addressed it in ff0eb51. Tests are passing for me locally now.

allie500 avatar May 16 '25 20:05 allie500

I left a few comments below, hopefully it helps a bit.

You'll need to update projects/packages/paypal-payments/.gitattributes so the block's raw files aren't shipped in the stable version of the package. The build directory, however, should be included. Check projects/packages/forms/.gitattributes for some examples.

You'll need a babel.config.js file as well.

Thanks for all the helpful tips! I've made an initial update to the .gitattributes file and added the Babel config file in 22a4d7d. I will need to circle back to the .gitattributes once I get the build stuff nailed down.

allie500 avatar May 20 '25 18:05 allie500

@jeherve I've removed the plan requirements and checks in https://github.com/Automattic/jetpack/pull/43413/commits/cbc1120caae63f7cff61b128e849bbeb1e34b2f1. Could you give it a look and let me know if it looks good and I'm not missing anything else?

I've tested the block after making these changes and everything looks good on my end.

allie500 avatar Jun 05 '25 16:06 allie500

We're getting there I think! I found some issues with the legacy widget though. Here is how I would recommend testing it.

  1. On a self-hosted site, connect the site to WordPress.com
  2. Purchase a paid plan
  3. Go to Appearance > Themes, install and activate a classic theme like Twenty Ten
  4. Go to Plugins > Add new, install and activate the Classic widgets plugin
  5. Go to Appearance > Customize > Widgets
  6. Add a Pay with PayPal widget to a widget area
  7. Try creating a product from there.

Thanks for the test. I tried this and couldn't get past step 6. The widget is not available. I'll look into it further.

allie500 avatar Jun 11 '25 15:06 allie500

Need some help fixing this broken lock file again after merging in trunk.

It is complaining about [email protected]([email protected]) but I can't get it to update to [email protected] like that found on trunk.

allie500 avatar Jun 20 '25 19:06 allie500

Functionally, this seems to test well for me. Minus the CORS issues on Simple sites.

I've fixed merge conflicts and have fixed a broken test. I think that it's ready for a review.

ebinnion avatar Jun 29 '25 03:06 ebinnion

I'm finishing up a merge to address conflicts.

@jeherve - Do you think that I could get some help from you or your team on review and testing? I don't believe that the team that Allie was on had much experience around Jetpack.

ebinnion avatar Jul 01 '25 19:07 ebinnion