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

Disable WooPay for suspended and rejected accounts - Take 2 #8942

Merged
merged 12 commits into from
Jun 17, 2024

Conversation

asumaran
Copy link
Contributor

@asumaran asumaran commented Jun 12, 2024

Closes https://github.com/Automattic/woopay/issues/2641

Original PR (#8857) introduced some fatal PHP errors that others also saw it (p1717672645619179-slack-CGGCLBN58).

I decided to revert the PR in #8912 to find and fix the error.

Changes in this PR are the same as #8857 except for the last commit which contains the fix to the fatal errors.

Given that the changes were already reviewed in #8857 I'll focus on explaining the fix.

  • The error happens when the account cache expires and it's refreshed.
  • If the cache is refreshed the woocommerce_payments_account_refreshed is fired (ref)
  • The Compatibility_Service class already registered a callback for that hook. (ref)
  • During the update_compatibility_data() method call, get_permalink() will be called. (ref)
  • get_permalink() calls get_page_link() which calls _get_page_link() which uses the $wp_rewrite global variable (ref).
  • Given that our code started running with the plugins_loaded action there's no $wp_rewrite global defined. Since plugins_loaded is fired before $wp_rewrite is defined. That explains the error:
    Fatal error:  Uncaught Error: Call to a member function get_page_permastruct() on null in /Users/asumaran/valet/wcpay/wp-includes/link-template.php:434
    
  • So, in this PR, I'm proposing delaying registering actions that could refresh the account cache to the next immediate action, setup_theme.

Testing instructions

  • Visit the Woopayments page in the admin area.
  • Modify this line to the following to decrease the "time to live" of the account cache to 10 seconds.
    diff --git a/includes/class-database-cache.php b/includes/class-database-cache.php
    index 68b4c7d4d..0dafaae24 100644
    --- a/includes/class-database-cache.php
    +++ b/includes/class-database-cache.php
    @@ -335,14 +335,7 @@ class Database_Cache {
      	  switch ( $key ) {
      		  case self::ACCOUNT_KEY:
      			  if ( is_admin() ) {
    -					// Fetches triggered from the admin panel should be more frequent.
    -					if ( $cache_contents['errored'] ) {
    -						// Attempt to refresh the data quickly if the last fetch was an error.
    -						$ttl = 2 * MINUTE_IN_SECONDS;
    -					} else {
    -						// If the data was fetched successfully, fetch it every 2h.
    -						$ttl = 2 * HOUR_IN_SECONDS;
    -					}
    +					$ttl = 10;
      			  } else {
      				  // Non-admin requests should always refresh only after 24h since the last fetch.
      				  $ttl = DAY_IN_SECONDS;
  • Refresh the page after 10 seconds and there shouldn't be any fatal error.
  • Confirm the fix by moving outside of the setup_theme hook each individual line and refreshing the page after 10 seconds. You should see a different fatal error for each line.
    • self::maybe_register_woopay_hooks();
    • self::maybe_display_express_checkout_buttons();
    • self::maybe_init_woopay_direct_checkout();
    • self::maybe_enqueue_woopay_common_config_script( WC_Payments_Features::is_woopay_direct_checkout_enabled() ); <-- here WC_Payments_Features::is_woopay_direct_checkout_enabled() is what triggers the error.

  • Run npm run changelog to add a changelog file, choose patch to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Post merge

@asumaran asumaran self-assigned this Jun 12, 2024
@botwoo
Copy link
Collaborator

botwoo commented Jun 12, 2024

Test the build

Option 1. Jetpack Beta

  • Install and activate Jetpack Beta.
  • Use this build by searching for PR number 8942 or branch name as-disable-woopay-rejected-suspended-accounts in your-test.site/wp-admin/admin.php?page=jetpack-beta&plugin=woocommerce-payments

Option 2. Jurassic Ninja - available for logged-in A12s

🚀 Launch a JN site with this branch 🚀

ℹ️ Install this Tampermonkey script to get more options.


Build info:

  • Latest commit: 7ffd8f0
  • Build time: 2024-06-17 22:22:00 UTC

Note: the build is updated when a new commit is pushed to this PR.

Copy link
Contributor

github-actions bot commented Jun 12, 2024

Size Change: 0 B

Total Size: 1.25 MB

ℹ️ View Unchanged
Filename Size
release/woocommerce-payments/assets/css/admin.css 1.08 kB
release/woocommerce-payments/assets/css/admin.rtl.css 1.08 kB
release/woocommerce-payments/assets/css/success.css 172 B
release/woocommerce-payments/assets/css/success.rtl.css 172 B
release/woocommerce-payments/dist/blocks-checkout-rtl.css 2.07 kB
release/woocommerce-payments/dist/blocks-checkout.css 2.07 kB
release/woocommerce-payments/dist/blocks-checkout.js 51.1 kB
release/woocommerce-payments/dist/bnpl-announcement-rtl.css 530 B
release/woocommerce-payments/dist/bnpl-announcement.css 531 B
release/woocommerce-payments/dist/bnpl-announcement.js 20 kB
release/woocommerce-payments/dist/cart-block.js 15.3 kB
release/woocommerce-payments/dist/cart.js 4.57 kB
release/woocommerce-payments/dist/checkout-rtl.css 599 B
release/woocommerce-payments/dist/checkout.css 599 B
release/woocommerce-payments/dist/checkout.js 31.5 kB
release/woocommerce-payments/dist/express-checkout-rtl.css 155 B
release/woocommerce-payments/dist/express-checkout.css 155 B
release/woocommerce-payments/dist/express-checkout.js 3.59 kB
release/woocommerce-payments/dist/index-rtl.css 41.1 kB
release/woocommerce-payments/dist/index.css 41.1 kB
release/woocommerce-payments/dist/index.js 294 kB
release/woocommerce-payments/dist/multi-currency-analytics.js 1.05 kB
release/woocommerce-payments/dist/multi-currency-rtl.css 3.28 kB
release/woocommerce-payments/dist/multi-currency-switcher-block.js 59.5 kB
release/woocommerce-payments/dist/multi-currency.css 3.29 kB
release/woocommerce-payments/dist/multi-currency.js 54.7 kB
release/woocommerce-payments/dist/order-rtl.css 733 B
release/woocommerce-payments/dist/order.css 735 B
release/woocommerce-payments/dist/order.js 41.8 kB
release/woocommerce-payments/dist/payment-gateways-rtl.css 1.21 kB
release/woocommerce-payments/dist/payment-gateways.css 1.21 kB
release/woocommerce-payments/dist/payment-gateways.js 38.6 kB
release/woocommerce-payments/dist/payment-request-rtl.css 155 B
release/woocommerce-payments/dist/payment-request.css 155 B
release/woocommerce-payments/dist/payment-request.js 5.92 kB
release/woocommerce-payments/dist/plugins-page-rtl.css 388 B
release/woocommerce-payments/dist/plugins-page.css 388 B
release/woocommerce-payments/dist/plugins-page.js 19.3 kB
release/woocommerce-payments/dist/product-details-rtl.css 398 B
release/woocommerce-payments/dist/product-details.css 402 B
release/woocommerce-payments/dist/product-details.js 11.2 kB
release/woocommerce-payments/dist/settings-rtl.css 11 kB
release/woocommerce-payments/dist/settings.css 10.9 kB
release/woocommerce-payments/dist/settings.js 201 kB
release/woocommerce-payments/dist/subscription-edit-page.js 669 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal-rtl.css 527 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.css 527 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.js 19.4 kB
release/woocommerce-payments/dist/subscription-product-onboarding-toast.js 693 B
release/woocommerce-payments/dist/subscriptions-empty-state-rtl.css 120 B
release/woocommerce-payments/dist/subscriptions-empty-state.css 120 B
release/woocommerce-payments/dist/subscriptions-empty-state.js 18.5 kB
release/woocommerce-payments/dist/tokenized-payment-request-rtl.css 155 B
release/woocommerce-payments/dist/tokenized-payment-request.css 155 B
release/woocommerce-payments/dist/tokenized-payment-request.js 6.57 kB
release/woocommerce-payments/dist/tos-rtl.css 235 B
release/woocommerce-payments/dist/tos.css 236 B
release/woocommerce-payments/dist/tos.js 21 kB
release/woocommerce-payments/dist/woopay-direct-checkout.js 4.94 kB
release/woocommerce-payments/dist/woopay-express-button-rtl.css 155 B
release/woocommerce-payments/dist/woopay-express-button.css 155 B
release/woocommerce-payments/dist/woopay-express-button.js 15.2 kB
release/woocommerce-payments/dist/woopay-rtl.css 4.25 kB
release/woocommerce-payments/dist/woopay.css 4.22 kB
release/woocommerce-payments/dist/woopay.js 69.4 kB
release/woocommerce-payments/includes/subscriptions/assets/css/plugin-page.css 622 B
release/woocommerce-payments/includes/subscriptions/assets/js/plugin-page.js 815 B
release/woocommerce-payments/vendor/automattic/jetpack-assets/build/i18n-loader.js 2.44 kB
release/woocommerce-payments/vendor/automattic/jetpack-assets/src/js/i18n-loader.js 1.01 kB
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.css 196 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.js 20 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.rtl.css 196 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.css 627 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.js 20 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.rtl.css 628 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-users.js 390 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-ajax.js 522 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-callables.js 581 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-admin-create-user.css 214 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-admin-create-user.js 523 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-login.css 722 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-login.js 408 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-users.js 517 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/babel.config.js 160 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.css 2.36 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.js 13.5 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.rtl.css 2.36 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/about.css 1.03 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-empty-state.css 291 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-order-statuses.css 403 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin.css 3.6 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/checkout.css 299 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/modal.css 742 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/view-subscription.css 572 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/wcs-upgrade.css 411 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin-pointers.js 544 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin.js 9.4 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.js 6.8 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.min.js 3.83 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-coupon.js 544 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-subscription.js 2.52 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.js 22.1 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.min.js 11.6 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/payment-method-restrictions.js 1.29 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/wcs-meta-boxes-order.js 502 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/payment-methods.js 355 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/single-product.js 429 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/view-subscription.js 1.38 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/wcs-cart.js 781 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/modal.js 1.1 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/wcs-upgrade.js 1.27 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.css 392 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.js 3.05 kB

compressed-size-action

Also removes active webhooks if the account is suspended or rejected
Fixes a fatal error when clearing the account data cache.

In this PR we call `WC_Payments::get_account_service()->is_account_rejected()` to know if the account is rejected. This method will get the data from Stripe and the `WC_Payments_Account->get_cached_account_data()` will be called. Then the action `woocommerce_payments_account_refreshed` will be fired.

The `Compatibility_Service` class is registering a hook for that action. https://github.com/Automattic/woocommerce-payments/blob/trunk/includes/class-compatibility-service.php#L41
Then `Compatibility_Service->get_compatibility_data()` is called. Which calls the `get_permalink()` function.

This function internally calls to `_get_page_link` which uses `$wp_rewrite` global but at the time it’s not defined resulting in a PHP fatal error. This commit fixes it.

Note that the request to the server is not done on every page refresh, only after clearing the account cache contents.
…er_review`

We are already checking if the account is rejected or under review in the `WC_Payments_Features::is_woopay_eligible()` method.
Fixes a fatal PHP error that happens when the account cache is expired.

Check #8942 for more details.
@asumaran asumaran force-pushed the as-disable-woopay-rejected-suspended-accounts branch from f8c9c4b to 270d47c Compare June 13, 2024 00:33
@asumaran asumaran marked this pull request as ready for review June 13, 2024 00:33
@asumaran asumaran requested a review from ricardo June 13, 2024 17:10
Copy link
Member

@ricardo ricardo left a comment

Choose a reason for hiding this comment

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

The fix tests well, but I was able to reproduce the error only when moving self::maybe_register_woopay_hooks() and self::maybe_display_express_checkout_buttons() out of setup_theme, but not the last 2 methods.

Also, please check my comment about WooPay_Order_Status_Sync::remove_webhook(); before merging.

includes/class-wc-payments.php Outdated Show resolved Hide resolved
@asumaran asumaran added this pull request to the merge queue Jun 17, 2024
Merged via the queue into develop with commit 602e466 Jun 17, 2024
23 checks passed
@asumaran asumaran deleted the as-disable-woopay-rejected-suspended-accounts branch June 17, 2024 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants