woocommerce / google-listings-and-ads

Sync your store with Google to list products for free, run paid ads, and track performance straight from your store dashboard.
https://woo.com/products/google-listings-and-ads/
GNU General Public License v3.0
47 stars 21 forks source link

[GTIN] Add GTIN migration JOB #2645

Closed puntope closed 4 weeks ago

puntope commented 1 month ago

Changes proposed in this Pull Request:

Closes part of #2616 as part of pcTzPl-2p2-p2.

This PR creates the logic for the Job responsible for the migration GTIN in batches for all the products.

Screenshots:

Screenshot 2024-10-17 at 18 29 40

Detailed test instructions:

  1. Set a simple product and add a GTIN in Google for WooCommerce tab
  2. Set a simple product and add a different GTIN in Google for WooCommerce tab and in the Inventory tab
  3. Set a Variable product and set GTIN on one of the variations in Google for WooCommerce only and in both (inventory and Google for WooCommerce) in other.
  4. Go to Connection Test page and click on "Start GTIN Migration"
  5. See the job gla/jobs/migrate_gtin/create_batch scheduled in WooCommerce - Status - Scheduled actions
  6. Run the job and see the gla/jobs/migrate_gtin/process_items created with the product ids.
  7. Run it
  8. Go back to the products created in steps 1,2,3 and verify that:
  9. For the product in step 1- the GTIN was loaded in the inventory GTIN
  10. For the product in step 2 - The GTIN is still the GTIN you set initially and is not override by the GTIN in google for WooCommerce GTIN
  11. Check the same in the variations (GTIN set in core should not be overrided in the migration).

Additional details:

In follow-up PRs I will add:

Changelog entry

Add - Add GTIN Migration Job

puntope commented 1 month ago

I noticed a different behavior for simple products when the GTIN is already set to the same value or when it's set to a different one. Is that expected? if there are differences, it keeps the core GTIN but adds the G4W as SKU, while for the case where they're the same, it does not set SKU.

Hey @tomalec that's odd I was not able to reproduce this. Can you verify again or help me to reproduce this issue?

The expected behaviour is:

puntope commented 1 month ago

Plus, for variable ones, it didn't migrate the GTINs. The process job was executed.

The variables are required to process the Variations. I just added a tweak here because I notice I was processing only available variations (those with price set) Now I do get all the variations.

Notice the GTIN in the variable inventory is useless because we don't support GTIN in Variables. So you need to check each of the variations (they have their own GTIN fields in core part and G4W tab) .

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 38 lines in your changes missing coverage. Please review.

Project coverage is 65.4%. Comparing base (a2c3654) to head (02e6224). Report is 10 commits behind head on add/support-core-gtin-field.

Files with missing lines Patch % Lines
src/Jobs/MigrateGTIN.php 0.0% 27 Missing :warning:
src/ConnectionTest.php 0.0% 6 Missing :warning:
...ternal/DependencyManagement/JobServiceProvider.php 0.0% 5 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645/graphs/tree.svg?width=650&height=150&src=pr&token=UROWUPF1LX&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce)](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) ```diff @@ Coverage Diff @@ ## add/support-core-gtin-field #2645 +/- ## =============================================================== - Coverage 65.5% 65.4% -0.1% - Complexity 4608 4622 +14 =============================================================== Files 474 475 +1 Lines 19299 19337 +38 =============================================================== Hits 12638 12638 - Misses 6661 6699 +38 ``` | [Flag](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) | Coverage Δ | | |---|---|---| | [php-unit-tests](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) | `65.4% <0.0%> (-0.1%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce) | Coverage Δ | | |---|---|---| | [...ternal/DependencyManagement/JobServiceProvider.php](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645?src=pr&el=tree&filepath=src%2FInternal%2FDependencyManagement%2FJobServiceProvider.php&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-c3JjL0ludGVybmFsL0RlcGVuZGVuY3lNYW5hZ2VtZW50L0pvYlNlcnZpY2VQcm92aWRlci5waHA=) | `0.0% <0.0%> (ø)` | | | [src/ConnectionTest.php](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645?src=pr&el=tree&filepath=src%2FConnectionTest.php&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-c3JjL0Nvbm5lY3Rpb25UZXN0LnBocA==) | `0.0% <0.0%> (ø)` | | | [src/Jobs/MigrateGTIN.php](https://app.codecov.io/gh/woocommerce/google-listings-and-ads/pull/2645?src=pr&el=tree&filepath=src%2FJobs%2FMigrateGTIN.php&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=woocommerce#diff-c3JjL0pvYnMvTWlncmF0ZUdUSU4ucGhw) | `0.0% <0.0%> (ø)` | |
puntope commented 1 month ago

Hey @tomalec I addressed all your comments and questions. Can you follow up with another review?