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

Refactor remote specs structure and naming #45547

Merged
merged 19 commits into from Mar 19, 2024

Conversation

ilyasfoo
Copy link
Contributor

@ilyasfoo ilyasfoo commented Mar 13, 2024

Changes proposed in this Pull Request:

Closes #44451.

This PR:

  • Deprecates RuleProcessors and Transformers classes using DeprecatedClassFacade to proxy to new classes
  • Deprecates DataSourcePoller.php and proxy public functions to new abstract class
  • Updates all imports to deprecated classes to new path
  • Adds check for DeprecatedClassFacade to ensure no duplicate message in a single request
  • Some lint fixes to old files

How to test the changes in this Pull Request:

Regression test

  1. In a fresh site, go to Setup Wizard
  2. Click continue
  3. Choose Hong Kong or other countries that do not support smart shipping defaults
  4. Click continue
  5. Observe the free features extensions are shown (screenshot)
  6. Go to WooCommerce > Home
  7. Observe inbox notes are displayed, and look for a remote note Save big on foreign exchange (FX) fees (screenshot)
  8. Click on Get paid task
  9. Observe payment list are shown (screenshot)
  10. Go back to WooCommerce > Home
  11. Click on Get your products shipped task
  12. Fill in the forms with any value and proceed to step 3
  13. Observe shipping list are shown (screenshot)
  14. Click on Grow your business task
  15. Observe marketing list are shown (screenshot)
  16. Go to WooCommerce > Settings > Payments
  17. Observe payment extensions are shown (screenshot)
  18. Go to Payments
  19. Observe there is promo note Save 10% on payment processing costs in your first three months (screenshot)

(Optional) Developer test

Test deprecation message
  1. After running all above step, look at the debug file, debug.log. You should not find any deprecated warning such as the following:
    Automattic\WooCommerce\Admin\RemoteInboxNotifications\GetRuleProcessor::get_processor is deprecated since version 8.8.0! Use Automattic\WooCommerce\Admin\RemoteSpecs\RuleProcessors\GetRuleProcessor::get_processor instead.
Test daily job
  1. Go to Tools > WCA Test Helper > Tools
  2. Click on Run wc_admin_daily job
  3. Observe it's successful
Test specs saved
  1. With SQL query tool, run the query SELECT option_name, option_value FROM wp_options WHERE option_name LIKE '%specs%';
  2. Observe multiple specs are saved and have expiry timestamps that's around 7 days (screenshot)

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 13, 2024
Copy link
Contributor

github-actions bot commented Mar 13, 2024

Test Results Summary

Commit SHA: 4c07862

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 37s
E2E Tests760027603527m 42s

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.

@github-actions github-actions bot added the plugin: woocommerce beta tester Issues related to the WooCommerce Beta Tester plugin. label Mar 14, 2024
Copy link
Contributor

Hi @chihsuan, @rjchow, @woocommerce/ghidorah

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

Copy link
Member

@chihsuan chihsuan left a comment

Choose a reason for hiding this comment

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

Nice work! Tested well and looks good.. 👍

Observe payment list are shown (screenshot)

For other reviewers, if you use JN live branch for testing, you will need to rename the plugin folder wc_beta_tester_live_branch_* to woocommerce first. Otherwise, you will not be able to see WCPay in the payment list.

With SQL query tool, run the query SELECT option_name, option_value FROM wp_options WHERE option_name LIKE '%specs%';

We need to use local to test this. Because JN site is using persistent cache, so transients are not stored in the database.

Copy link
Contributor

@rjchow rjchow left a comment

Choose a reason for hiding this comment

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

Nice work! Tests well according to instructions, can't say I got into the code changes too much but seems like minimal actual changes

@ilyasfoo
Copy link
Contributor Author

For other reviewers, if you use JN live branch for testing, you will need to rename the plugin folder wc_beta_tester_live_branch_* to woocommerce first. Otherwise, you will not be able to see WCPay in the payment list.

Thanks for the note! This is so easy to forget 😬

We need to use local to test this. Because JN site is using persistent cache, so transients are not stored in the database.

Ah, good catch, thanks!

@ilyasfoo ilyasfoo merged commit 5485665 into trunk Mar 19, 2024
45 checks passed
@ilyasfoo ilyasfoo deleted the dev/44451-refactor-remote-specs-structure-and-naming branch March 19, 2024 14:15
@github-actions github-actions bot added this to the 8.8.0 milestone Mar 19, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 19, 2024
@alvarothomas alvarothomas added needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. plugin: woocommerce beta tester Issues related to the WooCommerce Beta Tester plugin. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reorganize RemoteSpec directory and naming
4 participants