Skip to content

Conversation

@ilyasfoo
Copy link
Contributor

@ilyasfoo ilyasfoo commented May 25, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

Closes #38456.

Adds consumption of shipping partner suggestions from WCCOM API.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Testing row layout

  1. Go to WooCommerce > Settings > Advanced > WooCommerce.com (/wp-admin/admin.php?page=wc-settings&tab=advanced&section=woocommerce_com)
  2. Uncheck Display suggestions within WooCommerce option and save
  3. Go to Settings > General and choose Italy country and save
  4. Delete all shipping zone in Settings > Shipping and delete the option woocommerce_admin_created_default_shipping_zones.
  5. Go to WooCommerce > Home and click on Add shipping costs
  6. Click on Save shipping options
  7. Observe SendCloud and Packlink are shown
image

Testing column layout

  1. Go to Settings > General and choose Canada country and save
  2. Delete all shipping zone in Settings > Shipping and delete the option woocommerce_admin_created_default_shipping_zones.
  3. Go to WooCommerce > Home and click on Add shipping costs
  4. Click on Save shipping options
  5. Observe that ShipStation is shown
image

Optional further testing
7. Optionally, repeat testing for any countries from this list 'CA', 'AU', 'GB', 'ES', 'IT', 'DE', 'FR', 'MX', 'CO', 'CL', 'AR', 'PE', 'BR', 'UY', 'GT', 'NL', 'AT', 'BE' and ensure it follows the rules in DefaultShippingPartners.php

@github-actions github-actions bot added focus: react admin [team:Ghidorah] package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 25, 2023
'$schema' => 'http://json-schema.org/draft-04/schema#',
'title' => 'payment-gateway-suggestions',
'type' => 'array',
'type' => 'object',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Random fix: array in schema was wrong, it's supposed to be an object

@ilyasfoo ilyasfoo changed the title Update/use shipping partner suggestions Update shipping partner suggestions to use data from API May 25, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 25, 2023

Test Results Summary

Commit SHA: 40a31fc

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 50s
E2E Tests1940010020415m 31s

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.

@ilyasfoo ilyasfoo requested review from a team, moon0326 and rjchow May 25, 2023 15:35
@github-actions
Copy link
Contributor

github-actions bot commented May 25, 2023

Hi @moon0326,

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
Contributor

@moon0326 moon0326 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this 👍

I have one minor comment regarding the onboarding store changes.

Do you think it's worth implementing shipping method codes in a separate package, similar to payment gateways? It seems that the logic related to shipping methods does not fit into the onboarding package since it is not used during the onboarding process. However, I also noticed payment suggestion logic included in the onboarding package.

Other than that, everything LGTM 👍

rjchow
rjchow previously approved these changes May 29, 2023
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.

Tests well for Mexico! Wondering why we should set remote suggestions to false to test this though? (tested both true and false and they are both ok)

@ilyasfoo
Copy link
Contributor Author

Thanks for the suggestion, @moon0326! I've added the new data module in this commit 4ea2c65 and removed the old changes here 13386af.

Please take another look! 🙏

Tests well for Mexico! Wondering why we should set remote suggestions to false to test this though? (tested both true and false and they are both ok)

Thanks for testing it out @rjchow! Reasons why:

  1. Previously, the old WCCOM data was outdated and would error on some places (now it's already merged)
  2. Because I'd like to get the offline defaults tested 😄

@ilyasfoo ilyasfoo requested a review from moon0326 May 29, 2023 14:54
@moon0326
Copy link
Contributor

Thanks for the suggestion, @moon0326! I've added the new data module in this commit 4ea2c65 and removed the old changes here 13386af.

LGTM! 👍 🚀

@ilyasfoo ilyasfoo merged commit edf95bf into trunk May 30, 2023
@ilyasfoo ilyasfoo deleted the update/use-shipping-partner-suggestions branch May 30, 2023 07:56
@github-actions github-actions bot added this to the 7.9.0 milestone May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ShippingPartnerSuggestions does not use WooCommerce.com API

4 participants