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

Pre sync errors are not always cleared after successful sync #2737

Closed
mikkamp opened this issue Dec 18, 2024 · 0 comments · Fixed by #2738
Closed

Pre sync errors are not always cleared after successful sync #2737

mikkamp opened this issue Dec 18, 2024 · 0 comments · Fixed by #2738
Assignees
Labels
priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue is a confirmed bug.

Comments

@mikkamp
Copy link
Contributor

mikkamp commented Dec 18, 2024

Describe the bug:

If we encounter pre sync errors we store these in the product meta with the key _wc_gla_errors. These are mostly for validation issues before the product is sent to the Merchant Center. One such error is an empty description field.

If we previously had these errors for specific products, we'd expect them to be cleared up after resolving the error and syncing the product. These issues are cleared up in the product edit view. But they continue to popup in the product issues table on the Product Feed page:

Image

The reason this is not cleared up is because of a mismatch in synced countries with target countries. We can see this in the following code:

// check if product is synced completely and remove any previous errors if it is
$synced_countries = array_keys( $google_ids );
$target_countries = $this->target_audience->get_target_countries();
if ( count( $synced_countries ) === count( $target_countries ) && empty( array_diff( $synced_countries, $target_countries ) ) ) {

That's asking to fulfill two conditions:

  1. That the count of synced countries matches the count of target countries
  2. That all synced countries are included in target countries

Before v2 when this code was added, that was correct, because we were syncing each product individually for each target country. Later on that was refactored to only sync once for the base country, and include a list of target countries. However it seems this check wasn't refactored. So in case where there is only one target country it would still work, but if we have multiple target countries, it won't clear up old errors after a successful sync.

This went unnoticed in unit testing because the tests also remained the same to explicitly test the old scenarios:

  1. test_mark_as_synced
  2. test_mark_as_synced_doesnt_delete_errors_unless_all_target_countries_synced

In particular that second test needs to be changed to match the correct behaviour.

Workaround

One workaround to get around this issue is to follow these steps:

  • Edit a product and set the channel visibility to "Do not Sync and Show"
  • Save the product which triggers the product to be saved as unsynced (which clears previous errors)
  • Update the channel visibility to "Sync and Show"
  • Save the product and wait for the scheduled sync to complete

Steps to reproduce:

  1. On a site where we completed onboarding (and use a live URL so we are allowed to sync)
  2. During onboarding make sure to select multiple target countries
  3. Create a product with no description (complete all other requirements such as product image and price)
  4. Save the product with Sync and Show status to trigger the sync
  5. Wait for Action Scheduler to complete sync
  6. Edit the product and confirm the error has shown up after sync attempt
    Image
  7. Update the product to add a description and complete all requirements so it can sync
  8. Wait for the product sync to complete
  9. Edit the product and confirm the errors don't show up in the product
  10. View the product issues in the Product Feed page and confirm that the issues remain
  11. Clear the product status cache to reload it (tool in connection test page)
  12. View the product issues table again and confirm that the errors are still not cleared up

Expected behavior:

Local product errors to be cleared up after a successful sync

Actual behavior:

Clearing the product errors is skipped because the synced countries doesn't match the full list of target countries.

Additional details:

p1734476920010379-slack-C0410KV3YLW

@mikkamp mikkamp added type: bug The issue is a confirmed bug. priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. labels Dec 18, 2024
@mikkamp mikkamp self-assigned this Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant