Prevent unwanted helper api calls when loading subscription notes

[thilinah]

All Submissions:

• Have you followed the WooCommerce Contributing guideline?
• Does your code follow the WordPress' coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Connected WooCommerce stores send requests to helper API for fetching details related to subscriptions and to get information about plugin updates. The responses from these API endpoints are cached to prevent stores from sending multiple requests frequently.

For example, the GET subscriptions request is cached for 1 hour and POST update-check request is cached for 12 hours.

But for some stores, the cache can be completely broken due to DB having a full disk. This is because when set_transient is called if the object cache is not configured for the site wp options table will be used to store the transient value. (further discussions in this P2 post and the issue).

Problem

• Subscription notes are loaded via admin_init
• In order to prevent subscription notes from on every call to admin_init the option LAST_REFRESH_OPTION_KEY is used.
• But when the db disk is full, saving the latest updated time in wp_option should fail.
• This leads to subscription notes being loaded on every admin page load and cron run.
• Since the transients are not saved when the disk is full, this leads to store making get subscription API call multiple times.

Fix

• There is no need to check if subscription notes need to be refreshed on every admin page load. Instead, it can be checked only when the user visits a WC page showing the Activity tab.

Closes # .

How to test the changes in this Pull Request:

• Have you followed the Writing high-quality testing instructions guide?

Cron should not load subscription notes

• Make sure the site is connected to WCCOM via wp-admin/admin.php?page=wc-addons&section=helper
• Every time subscription notes are updated the option woocommerce_admin-wc-helper-last-refresh will be set to the current time. We will use this fact for testing.
• Checkout trunk.
• Run wp option delete woocommerce_admin-wc-helper-last-refresh
• Run cron wp cron event run --all
• Run wp option get woocommerce_admin-wc-helper-last-refresh and notice it exists.
• Checkout this branch.
• Run wp option delete woocommerce_admin-wc-helper-last-refresh
• Run cron wp cron event run --all
• Run wp option get woocommerce_admin-wc-helper-last-refresh and notice it doesn't exist.

Only visiting a WC Admin or a page with WC Admin Header Shall load subscription notes

• Checkout this branch.
• Run wp option delete woocommerce_admin-wc-helper-last-refresh
• Visit a non WC page like wp-admin/edit-comments.php
• Run wp option get woocommerce_admin-wc-helper-last-refresh and notice it doesn't exist.
• Visit a WC related page that shows Activity tab, such as wp-admin/edit.php?post_type=shop_order
  
• Run wp option get woocommerce_admin-wc-helper-last-refresh and notice that it exists. (this means subscription notes are loading on this page)
• Repeat this test on any page to make sure all pages showing Activity tab are loading subscription notes.

Make sure the subscription notes are loading after this update

• Checkout this branch.
• Make sure your store is connected.
• You need to have an active product that is expiring soon (less than in 60 days)
• If this is not the case, you may update the expiry date of the subscription via the dashboard.
• Under My Subscriptions you should be able to see the expiry date.
  
• Disable the auto-renew for this product via My Subscriptions
• Run the following in wp shell to remove subscription notes.

\Automattic\WooCommerce\Admin\Notes\Notes::delete_notes_with_name('wc-admin-wc-helper-subscription');

• Update subscriptions under My Subscriptions
  
• Run wp option delete woocommerce_admin-wc-helper-last-refresh reset last subscription notes refresh time.
• Load any WC page.
• Notice there is a new subscription note as a reminder to enable auto-renew for the above mentioned product.
  

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?
• Have you included testing instructions?

FOR PR REVIEWER ONLY:

• I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

[github-actions]

Test Results Summary

Commit SHA: f754dc5

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 49s
E2E Tests	190	0	0	18	0	208	13m 57s

---

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

[codecov]

Codecov Report

Merging #37378 (8f15452) into trunk (f814f09) will decrease coverage by 5.9%.
The diff coverage is n/a.

❗ Current head 8f15452 differs from pull request most recent head f754dc5. Consider uploading reports for the commit f754dc5 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##             trunk   #37378      +/-   ##
===========================================
- Coverage     51.7%    45.8%    -5.9%     
+ Complexity   17257    17196      -61     
===========================================
  Files          429      429              
  Lines        79689    64911   -14778     
===========================================
- Hits         41200    29702   -11498     
+ Misses       38489    35209    -3280     

see 345 files with indirect coverage changes

[github-actions]

Hi @rcstr, @psealock, @denho, @vedanshujain,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[vedanshujain]

Requesting a minor change if its possible

[vedanshujain]

@thilinah This looks good to me, but I am assuming you are waiting for wc-admin folks to also take a look?

[rcstr]

Changes looks good and work as expected

[thilinah]

@thilinah This looks good to me, but I am assuming you are waiting for wc-admin folks to also take a look?

@vedanshujain if the changes look good can you please approve the PR, I see it's blocked due to the change request.

feedback addressed

[vedanshujain]

@thilinah dismissed my review, should be mergeable now

[denho]

@thilinah The WC notes are shown not only on wc-admin and wc-addons pages. They are also loaded on other WC pages in admin, such as orders list or settings.

On those pages, the subscription notes are not refreshed due to changes in your PR, and the user will not get the latest notes until they visit wc-admin and wc-addons pages.

[thilinah]

On those pages, the subscription notes are not refreshed due to changes in your PR, and the user will not get the latest notes until they visit wc-admin and wc-addons pages.

@denho in this case I think skipping subscription notes loading based on the page won't work. I've noticed that other WC plugins like subscriptions and bookings also show the Activity button. So post_types such as shop_order, shop_subscription, shop_coupon, product.

Due to this, I think we should consider moving forward with #36846 and move the discussion to that PR for refining the approach.

[thilinah]

Also, another way could be to make sure subscription notes are not loaded via the cron.

[thilinah]

@denho @nirolph @rcstr re-requesting reviews for this PR since the code and test instructions are updated.

[rcstr]

Changes looks good and works as described

[jonathansadowski]

I'm going to clear the milestone from this for now, so that this doesn't inadvertently get missed once it's merged. A milestone will be added automatically by our automation upon merge.

[denho]

Added some suggestions in the code. Otherwise, LGTM!

[thilinah]

@jonathansadowski, the PR is approved. Is it possible to get this merged?

[jonathansadowski]

Hi @thilinah, please feel free to merge once checks have passed (as they have in this case), and you've gotten your PR approved. If you don't have access to merge yourself, let me know and I can take a look further.

[thilinah]

@jonathansadowski I'm not authorized to merge.

[jonathansadowski]

Thanks @thilinah, I've merged this for now — I'll look into whether or not that should be the case.