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

Only sync selected categories as product type #2233

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

mikkamp
Copy link
Contributor

@mikkamp mikkamp commented Feb 1, 2024

Changes proposed in this Pull Request:

This PR changes the behaviour to only sync the selected categories as product type. Previously it would sync parent categories, both included as the parent of a child category as well as the parent individually even if the parent wasn't selected directly.

This change switches from using wc_get_product_cat_ids to using wc_get_product_term_ids as the first function will include a list of ancestors (regardless of whether they are selected or not). The latter function will return a list of all the selected categories. Which means we can influence which parent categories get synced individually by selecting them or not.

I altered the unit test to match the expected behaviour as well as testing some more edge cases to ensure they are synced correctly:

  • test for unselected parent not included
  • test 2 levels of ancestors
  • test parent and child are included individually when both are selected
  • test unrelated category is not included

Closes #2230

Detailed test instructions:

  1. Setup a site with the extension and onboarded to Merchant Center
  2. Create some categories with hierarchy
  3. Create a test product where only the child category is selected
    image
  4. Save the product and wait for it to sync, and then confirm the final attributes in MC, the parent category should not be included as an individual line
    image
  5. Create a test product where both the parent and child categories are selected
    image
  6. Save the product and wait for it to sync, and then confirm the final attributes in MC, the parent category should be included as an individual line and also as part of the child line
    image

Changelog entry

  • Fix - Only sync selected categories as product type.

@mikkamp mikkamp self-assigned this Feb 1, 2024
@github-actions github-actions bot added changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug. labels Feb 1, 2024
@mikkamp mikkamp requested a review from a team February 1, 2024 13:30
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (876e3fe) 60.1% compared to head (8f886e3) 60.1%.
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             develop   #2233   +/-   ##
=========================================
  Coverage       60.1%   60.1%           
  Complexity      4141    4141           
=========================================
  Files            454     454           
  Lines          17586   17586           
=========================================
  Hits           10572   10572           
  Misses          7014    7014           
Flag Coverage Δ
php-unit-tests 60.1% <100.0%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/Product/WCProductAdapter.php 97.8% <100.0%> (ø)

Copy link
Contributor

@martynmjones martynmjones left a comment

Choose a reason for hiding this comment

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

Hey @mikkamp, thanks for the fix.

Tested syncing products with different category selections to confirm it will sync with:

  • Only the parent category
  • Parent and child categories
  • Child categories only

LGTM ✅

@mikkamp mikkamp merged commit 252164a into develop Feb 5, 2024
13 checks passed
@mikkamp mikkamp deleted the fix/2230-sync-selected-categories branch February 5, 2024 17:21
@tomalec tomalec mentioned this pull request Feb 6, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google product type: Parent category gets synced when only sub-category is selected
2 participants