Skip to content

Conversation

faisal-alvi
Copy link
Collaborator

All Submissions:

  • Does your code follow the WooCommerce Sniffs variant of WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Will this change require new documentation or changes to existing documentation?

Changes proposed in this Pull Request:

This PR fixes an issue where Square product imports were creating unnecessary attribute terms in WooCommerce. Previously, when importing products from Square, the plugin would import ALL possible option values from Square's Option Name, even if the specific product didn't use all of those values.

  • Modified extract_attributes_from_square_options() method to accept variations data
  • Added logic to collect only the option values actually used by the product's variations
  • Filter attribute options to only include values that are genuinely used by the product

Closes https://linear.app/a8c/issue/SQUARE-159/general-product-options-being-synced-to-woo.

Steps to test the changes in this Pull Request:

  1. Create Square Product with Partial Option Usage:

    • In Square Dashboard, go to Items → Options and create an option called "Colour" with 6-7 terms (e.g., Navy, Grey, Green, Black, Red, Yellow, Pink)
    • Create a product in Square that uses this "Colour" option but only uses some of the terms (e.g., only Navy, Grey, Green)
    • Create variations for the product using only the selected terms
  2. Import Product to WooCommerce:

    • In WooCommerce admin, go to WooCommerce → Settings → Square
    • Use "Import all Products from Square" functionality
    • Verify the imported product appears correctly
  3. Verify Attribute Terms:

    • Edit the imported product in WooCommerce
    • Check the Attributes tab
    • Confirm that only the attribute terms actually used by the product variations are present (Navy, Grey, Green)
    • Verify that unused terms (Black, Red, Yellow, Pink) are NOT present in the attribute

Changelog entry

Fix - Sync Only the Utilized Option Values

if ( ! isset( $used_option_values[ $attribute_name ] ) ) {
$used_option_values[ $attribute_name ] = array();
}
$used_option_values[ $attribute_name ][] = $attribute['option'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as above. If for some reason this isn't already an array, this will cause an error. May want an is_array check in the above conditional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the code $used_option_values[ $attribute_name ] will always be an array, since we initialize it as one on line 827.

Co-authored-by: Darin Kotter <[email protected]>
Copy link
Collaborator

@qasumitbagthariya qasumitbagthariya left a comment

Choose a reason for hiding this comment

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

QA Update ✅


I have verified this PR in the fix/SQUARE-159 branch, which has been fixed and is functioning as intended.

I tested the following on this branch:

  • Imported the product into WooCommerce using “Import all Products from Square.”
  • Verified the imported product appears correctly in WooCommerce.
  • Unused terms (Black, Red, Yellow, Pink) are not imported.

Before

Square Dashboard

image

Woo
image

After

image image

Testing Environment

  • WordPress: 6.8.2
  • Theme: Storefront 4.6.1
  • Theme: Twenty Twenty-Five 1.3
  • WooCommerce - 10.2.0
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS 15.2
  • Branch: fix/SQUARE-159

Steps to Test- As mentioned in the PR description.
Test Results - It is working as expected.
Functional Demo / Screencast -
Special Notes - Ready for UAT
Testing Document status:
Cases related to this Issue/PR are added to the Critical Flow Wiki pages:

  • Yes
  • Not Required/Applicable for this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants