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

Allow mapping of a Google ID without a prefix #2773

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

mikkamp
Copy link
Contributor

@mikkamp mikkamp commented Jan 17, 2025

Changes proposed in this Pull Request:

This PR makes the prefix when fetching status from the Merchant Center as optional. We do want to make sure that the ID is a strictly numeric ID otherwise it's not accepted. In the previous regex we searched till end of string with $, but didn't force it to search from the start of the string with ^. This meant that it was possible to a map an ID like foo_gla_12345. We fixed this in the PR to ensure it scans the whole string for a matching pattern:

  • gla_ + numeric ID
  • numeric ID

Any other patterns will result in an unmapped ID. I've added additional unit tests to cover all these variations.

Closes #2768

Detailed test instructions:

  1. Add the snippet from the documentation: https://woocommerce.com/document/google-listings-and-ads/faqs/#how-do-i-remove-the-gla-prefix-from-synced-products
  2. Setup the extension to sync some products to Merchant Center
  3. Use the Connection Test helper to clear the product statuses
  4. Refresh the product feed page
  5. Wait for the statuses to reload and confirm that we have products that are a different status then unsynced

Changelog entry

  • Tweak - Allow mapping of a Google ID without a prefix.

@mikkamp mikkamp self-assigned this Jan 17, 2025
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Jan 17, 2025
@mikkamp mikkamp requested a review from a team January 17, 2025 14:31
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.2%. Comparing base (9949c75) to head (a9dbb55).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             develop   #2773   +/-   ##
=========================================
  Coverage       67.2%   67.2%           
  Complexity      4677    4677           
=========================================
  Files            480     480           
  Lines          19571   19571           
=========================================
  Hits           13159   13159           
  Misses          6412    6412           
Flag Coverage Δ
php-unit-tests 67.2% <100.0%> (ø)

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

Files with missing lines Coverage Δ
src/Product/ProductHelper.php 92.3% <100.0%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When the product ID prefix is removed statuses always remain unsynced
1 participant