Skip to content

Add SKU-based lookup for Inventory Synchronization#471

Merged
iamdharmesh merged 7 commits intotrunkfrom
SQUARE-264
May 7, 2026
Merged

Add SKU-based lookup for Inventory Synchronization#471
iamdharmesh merged 7 commits intotrunkfrom
SQUARE-264

Conversation

@faisal-alvi
Copy link
Copy Markdown
Collaborator

@faisal-alvi faisal-alvi commented Mar 11, 2026

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:

When push_inventory runs, products/variations without a stored _square_item_variation_id mapping were previously skipped. If an earlier upsert_new_products step timed out (or failed) after Square created the item but before the plugin saved the mapping, those products stayed at 0 quantity in Square and never received a stock push.

This PR adds SKU-based fallback so push_inventory can recover in that case:

  • A new method added Product::get_square_variation_id_by_sku()
    Looks up a Square catalog variation by SKU via SearchCatalogObjects (exact query on sku). Optionally persists the mapping so later sync steps use the stored ID. Exceptions are caught and logged; the method returns null so sync continues.
  • push_inventory (Manual_Synchronization)
    • For simple products: if there is no stored Square variation ID but the product has a SKU and manages stock, attempts the new SKU lookup, persists the mapping when found, then builds and pushes the inventory change.
    • For variable products: same logic per child variation (no mapping → try SKU lookup → persist when found → push inventory).
    • MAX_SKU_LOOKUPS_PER_PUSH_STEP = 20 limits how many SKU lookups run per push_inventory step to avoid rate limits; additional unmapped products are handled in later step runs.

Note

The timeout that prevents mapping from being saved is intermittent (e.g. slow API, server limits), so this PR does not address that root cause. Instead, it targets the incomplete-sync case: when products already exist in Square but WooCommerce never received or stored the variation IDs (e.g. after a timeout). For those unmapped products, the plugin now attempts a SKU-based lookup during push_inventory—matching the approach suggested in the original issue—and persists the mapping when found so inventory can be pushed. This makes inventory push resilient to missed meta and looks promising for the reported scenario.

Closes https://linear.app/a8c/issue/SQUARE-264/push-inventory-should-attempt-sku-based-matching-for-unmapped-products.

Steps to test the changes in this Pull Request

  1. Set Woo as system of record
  2. Inventory sync enabled
  3. Create a new simple product with SKU and stock
  4. Run manual sync so it upserts to Square.
  5. In DB, delete the product’s _square_item_variation_id meta (simulate lost mapping).
  6. Run sync again
  7. Confirm the Square inventory for that product matches Woo after sync
  8. Repeat with a variable product; remove only one variation’s _square_item_variation_id.
  9. Run sync; confirm that variation’s inventory is pushed to Square after SKU lookup.
  10. Confirm already-mapped products still push inventory as before (no behavior change).
  11. Confirm products without SKU or that don’t manage stock are still skipped for push (no new behavior for them).

Changelog entry

Fix - Push inventory now attempts SKU-based lookup for synced products missing a Square variation ID

@faisal-alvi faisal-alvi self-assigned this Mar 11, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 11, 2026

🔍 WordPress Plugin Check Report

❌ Status: Failed

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
8 8 0

❌ Errors (8)

📁 includes/WC_Payments_Compatibility.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 includes/Framework/PaymentGateway/Admin/views/html-user-payment-token-editor-token.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 includes/Framework/PaymentGateway/Admin/views/html-user-payment-token-editor.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 includes/Framework/PaymentGateway/Admin/views/html-order-partial-capture.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 includes/Framework/PaymentGateway/Admin/views/html-user-profile-field-customer-id.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 includes/Framework/PaymentGateway/Admin/views/html-user-profile-section.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 includes/Framework/PaymentGateway/Admin/views/html-admin-gateway-status.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 includes/Admin/Product_Editor_Compatibility.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

@faisal-alvi faisal-alvi requested a review from iamdharmesh March 12, 2026 08:42
Copy link
Copy Markdown
Collaborator

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @faisal-alvi. Have some questions could you please help answer it?

If an earlier upsert_new_products step timed out (or failed) after Square created the item but before the plugin saved the mapping, those products stayed at 0 quantity in Square and never received a stock push.

If upsert_new_products step timed out, it will be retried and product upsertion will be retried. So, I don't anticipate the background process will reach to the push_invetory step without adding a reference to the meta. @faisal-alvi have you tried reproducing the issue by adding an intentional wait in the upsert_new_products function and making it timed out? Is it reaching to the push_invetory step? Thanks

Comment thread includes/Sync/Manual_Synchronization.php
@faisal-alvi
Copy link
Copy Markdown
Collaborator Author

@iamdharmesh You’re right that we don’t call complete_step( 'upsert_new_products' ) until the upsert queue is drained, and that inventory_push_product_ids is only merged from upsert_catalog_objects() after that method returns. So under normal operation we shouldn’t advance to push_inventory while a batch is still “in flight” inside upsert_catalog_objects.

I agree that reproducing the original report purely by “slowing upsert_new_products until timeout” may not mirror the failure mode unless we kill the process after Square returns and before job/product state is fully persisted, or we hit an operational edge case (meta removed from DB, restore, cache, etc.).

So the SKU lookup in push_inventory is partly defense in depth: if Woo is missing_square_item_variation_id but Square already has a matching ITEM_VARIATION for the SKU, we recover instead of skipping the physical count. I’m happy to tighten the PR wording so it doesn’t over-claim the timeout scenario versus “missing mapping for any reason.”

@faisal-alvi
Copy link
Copy Markdown
Collaborator Author

faisal-alvi commented Apr 13, 2026

@iamdharmesh 👋🏻

I dug into this in depth in the codebase. You raised a fair point: if the sync dies mid-upsert_new_products, the stepped job does not advance past that step, so push_inventory does not run in that same execution - so SKU recovery at push time is not the fix for that narrow failure mode.

I still think we should merge this PR as defense in depth. Theinventory_push_product_ids is filled from $staged_product_ids, not “mapping definitely succeeded for every line”. So push_inventory can still run after upsert_new_products has finished (complete_step), with some lines still missing _square_item_variation_id. So SKU reconciliation at push time still targets real gaps that are not the same as “timeout before push.”

This PR is still justified as belt-and-suspenders at push_inventory so we don’t silently skip stock when mapping is absent at push time. Read more on this here.

It's worth noting that PR #483 is addressing the timeout-related issues in search_matched_products.

Copy link
Copy Markdown
Collaborator

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Thanks for changes @faisal-alvi. I haven't tested this but based on the code this looks good overall now. I have added minor note to check.

I still think we should merge this PR as defense in depth. The inventory_push_product_ids is filled from $staged_product_ids, not “mapping definitely succeeded for every line”.

Agree, However, could you please check if $staged_product_ids are not in the $successful_product_ids then is it even uploaded to Square? Because we are adding alert here in that case. Thanks.

Comment thread includes/Sync/Manual_Synchronization.php Outdated
Also, update SKU lookup logic to simplify incrementing lookup count
@faisal-alvi faisal-alvi requested a review from iamdharmesh April 17, 2026 15:36
@iamdharmesh
Copy link
Copy Markdown
Collaborator

Agree, However, could you please check if $staged_product_ids are not in the $successful_product_ids then is it even uploaded to Square? Because we are adding alert here in that case. Thanks.

@faisal-alvi did you got a chance to look at this?

@faisal-alvi
Copy link
Copy Markdown
Collaborator Author

Hi @iamdharmesh yes, I looked into this.

The $staged_product_ids are the Woo product IDs we included in the batch_upsert_catalog_objects request for that run. When we reach the response-handling loop, we already have a successful BatchUpsertCatalogObjectsResponse from Square for that batch.

The $successful_product_ids** is built only when we can resolve a returned catalog item to a Woo product with Product::get_product_by_square_id( $remote_item_id ) and complete the “store Square data to WooCommerce” path for that object.

So $successful_product_ids means “Woo-side processing of the response succeeded for this product,” not “Square accepted the HTTP request” (that’s already true for the batch we’re processing).

So for array_diff( $staged_product_ids, $successful_product_ids ): in the typical case, Square has already received the catalog upsert for the batch; the alert is fired when we couldn’t finish linking/updating that staged product from the response in Woo (e.g. lookup didn’t match), which is why I think the copy is a bit misleading - it should be: “we couldn’t update Woo from Square’s response for this product” but it is: “Square never got it.”. Please let me know if this is not the case. Thanks again for the review.

Copy link
Copy Markdown
Collaborator

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification @faisal-alvi. Code looks good to me. However, I haven't tested this. @qasumitbagthariya / @ankitguptaindia Could you please test this thoroughly. Thanks

Copy link
Copy Markdown
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 SQUARE-264 branch, which has been fixed and is functioning as intended.

I tested the following on this branch:

  • Simple product sync works correctly after removing _square_item_variation_id
  • Variable product sync works; missing variation mapping restored via SKU lookup
  • Inventory updates pushed successfully to Square

Testing Environment

Details
  • WordPress: 6.9.4
  • Theme: Storefront 4.6.2
  • Theme: Twenty Twenty-Five 1.4
  • WooCommerce - 10.7.0
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS 15.2
  • Branch: SQUARE-264

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

@iamdharmesh iamdharmesh added this to the 5.3.3 milestone May 7, 2026
@ankitguptaindia
Copy link
Copy Markdown
Collaborator

Regression Test Report- QA Verified ✅

Testing Environment -

Details
  • WordPress: 6.9.4
  • PHP: 8.4.18
  • Server: nginx/1.26.1
  • Database: mysqli (Server: 8.0.35 / Client: mysqlnd 8.4.18)
  • Browser: Chrome 147.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.4
  • Plugins:
    • WooCommerce 10.7.0
    • WooCommerce Square - Dev version

Result:

Verified all key flows, user flows related to this PR, no blocking or functional issues detected.

Final Status:

Marked as Ready for Merge 🚀

@iamdharmesh iamdharmesh marked this pull request as ready for review May 7, 2026 17:22
@iamdharmesh iamdharmesh merged commit 58f6c16 into trunk May 7, 2026
13 of 14 checks passed
@iamdharmesh iamdharmesh deleted the SQUARE-264 branch May 7, 2026 17:24
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.

4 participants