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

Reset duplicate notifications from new plugins #8919

Merged
merged 15 commits into from
Jun 10, 2024

Conversation

timur27
Copy link
Contributor

@timur27 timur27 commented Jun 6, 2024

Closes #8809

Changes proposed in this Pull Request

This PR ensures that a previously dismissed duplicate notice on the Settings page will reappear if a new plugin triggers duplicates again. This is accomplished by enhancing the dismissed notification object stored in wp_options. Previously, only the payment method was stored. With this PR, we also store the list of gateway IDs of plugins causing duplicates. This enhancement allows us to handle scenarios where new plugins cause duplicate notices.

Testing instructions

💡 In order to test multiple plugins enabling the same payment method, you can either follow the instructions in #8513, which mention installing/configuring plugins like Mollie, Stripe and Klarna, or add the following line of code in this place: $this->gateways_qualified_by_duplicates_detector['card'][] = 'plugin_gateway_id'; (more details in the testing steps)
In the testing instructions for this PR, I will follow the second approach to save a bit of time, but feel free to proceed with installing & configuring plugins which is described in the PR mentioned above.

Quick reminder on how the inline notice looks like:

image

  • Before this line, add the following code snippet to simulate Stripe plugin enabling card and Apple Pay/Google Pay
$this->gateways_qualified_by_duplicates_detector['card'][] = 'stripe';
$this->gateways_qualified_by_duplicates_detector['apple_pay_google_pay'][] = 'stripe_apple_pay_google_pay';
  • Navigate to Payments settings and you will see the inline notification about duplicates for both payment methods
  • Dismiss for both payment methods and confirm they both don't appear upon page refresh
  • Simulate one more enablement of credit card w/o removing the previously added lines
$this->gateways_qualified_by_duplicates_detector['card'][] = 'mollie_cc';
  • Refresh Settings page and confirm you see the duplicates notification again - this is the main goal of this PR
  • Dismiss the notification for credit card, refresh the page and confirm it's not reappearing
  • Remove two last lines of code you previously added in class-duplicates-detection-service.php by keeping only card from stripe now, refresh the Settings page
    • upon page refresh, there should be no notification displayed again
    • this is important because we want to ensure that if a user dismisses notifications from multiple plugins in sequence, and then disables one of those plugins, the dismissed notifications from the other plugins remain hidden. For example, if notifications from the second and third plugins are dismissed, turning off the third plugin should not cause the second plugin's notification to reappear, as it was dismissed before

Optional: after going through all the testing instructions, the wp_options table in your local database should contain the wcpay_duplicate_payment_method_notices_dismissed option with the following values. It's not a must-have to check this (feel free to do so if you wish), but rather, it provides additional information on what happened in the background when we dismissed notifications for multiple plugins.

Array
(
    [card] => Array
        (
            [0] => stripe
            [1] => mollie_cc
        )

    [apple_pay_google_pay] => Array
        (
            [0] => stripe_apple_pay_google_pay
        )

)

  • 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

@botwoo
Copy link
Collaborator

botwoo commented Jun 6, 2024

Test the build

Option 1. Jetpack Beta

  • Install and activate Jetpack Beta.
  • Use this build by searching for PR number 8919 or branch name add/reset-duplicate-notifications-from-new-plugins 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: 79084a2
  • Build time: 2024-06-10 07:03:47 UTC

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

Copy link
Contributor

github-actions bot commented Jun 6, 2024

Size Change: +86 B (0%)

Total Size: 1.25 MB

Filename Size Change
release/woocommerce-payments/dist/settings.js 201 kB +86 B (0%)
ℹ️ 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.5 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 kB
release/woocommerce-payments/dist/index.js 293 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/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.33 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.86 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.1 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

@timur27 timur27 force-pushed the add/reset-duplicate-notifications-from-new-plugins branch from 138f740 to dd5b853 Compare June 7, 2024 07:55
@timur27 timur27 marked this pull request as ready for review June 7, 2024 13:17
@timur27 timur27 force-pushed the add/reset-duplicate-notifications-from-new-plugins branch from 631d987 to af447bc Compare June 7, 2024 13:28
@timur27 timur27 requested review from a team, brettshumaker and mdmoore and removed request for a team, brettshumaker and mdmoore June 7, 2024 13:31
@timur27 timur27 requested a review from FangedParakeet June 7, 2024 13:32
@timur27
Copy link
Contributor Author

timur27 commented Jun 7, 2024

I re-assigned the review because of the AFKs.

Copy link
Contributor

@FangedParakeet FangedParakeet left a comment

Choose a reason for hiding this comment

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

Code here LGTM and is working as described. I'm trying to think of any edge-cases, whereby an extension, for which we have already dismissed a notice, might reintroduce a duplicated payment method on update or some other mysterious circumstance...but in any case, I think it's a pretty trivial hypothetical and it's hurting my hollow head even thinking too hard about it.

I think this is a really valuable enhancement and I wasn't able to find a way to break it. Nice one!

@timur27 timur27 added this pull request to the merge queue Jun 10, 2024
Merged via the queue into develop with commit cbeaa01 Jun 10, 2024
23 checks passed
@timur27 timur27 deleted the add/reset-duplicate-notifications-from-new-plugins branch June 10, 2024 07:21
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.

Reset duplicate notifications from new plugins
3 participants