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

Prioritise core WooCommerce GTIN field in product adapter #2621

Conversation

martynmjones
Copy link
Contributor

Changes proposed in this Pull Request:

Closes #2614

Adjusts WCProductAdapter.php to map the GTIN field from WooCommerce core if it's available. This is checked after the attribute mapping rules and Google for WooCommerce attributes as we want to prioritise the core field.

We return early if the product does not have a get_global_unique_id method to maintain backwards compatibility.

Screenshots:

Screenshot 2024-09-20 at 12 20 42

Detailed test instructions:

  1. Checkout update/2614-prioritise-core-gtin-field-in-product-adapter
  2. Set up an attribute mapping rule for GTIN to some field
  3. Create a product and enter different valid GTIN values (for example 00000001, 00000002, 00000003) in
    • The core GTIN field on the Inventory tab
    • The GTIN field on the Google for WooCommerce tab
    • The field set via the attribute mapping rule created in step 2
  4. Go to wp-admin/admin.php?page=connection-test-admin-page and sync the product to Google
  5. View the product in Merchant Center and confirm the GTIN number matches the value entered in the Inventory tab
  6. Delete the GTIN value from the Inventory tab
  7. Re-sync the product and then confirm the value in Merchant Center now matches the value from the Google for WooCommerce tab
  8. Delete the GTIN value from the Google for WooCommerce tab
  9. Re-sync the product and then confirm the value in Merchant Center now matches the value from the attribute mapping rule

Changelog entry

Update - Product adapter to map GTIN value from WooCommerce core field if it's available

@martynmjones martynmjones self-assigned this Sep 20, 2024
@github-actions github-actions bot added the changelog: update Big changes to something that wasn't broken. label Sep 20, 2024
@martynmjones martynmjones requested a review from a team September 20, 2024 17:30
Copy link
Contributor

@puntope puntope left a comment

Choose a reason for hiding this comment

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

Just a little comment regarding docs. But approved in advance

Screenshot 2024-09-23 at 16 59 12 Screenshot 2024-09-23 at 16 58 56

@puntope
Copy link
Contributor

puntope commented Sep 23, 2024

p.s I also tested with a non valid GTIN

Screenshot 2024-09-23 at 17 06 09

@martynmjones martynmjones changed the base branch from develop to add/support-core-gtin-field September 23, 2024 18:01
@martynmjones martynmjones merged commit f268f36 into add/support-core-gtin-field Sep 23, 2024
5 of 10 checks passed
@martynmjones martynmjones deleted the update/2614-prioritise-core-gtin-field-in-product-adapter branch September 23, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: update Big changes to something that wasn't broken.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GTIN] Prioritise core WooCommerce GTIN field when syncing
2 participants