Skip to content

Commit 63b2853

Browse files
sol-loupsol-loup
authored
Refactor Product Identifer logic (#553)
* Refactor Product Identifer logic * Removing changes which would forcibly flip SKU to ID in builder * Refunds degenerated case * Diagnostics fix * Simplifying fallback code * Removing log statements * Reverting changes to catalog logic, so this diff contains only Sales and Order changes * Addressing comments
1 parent fd23c40 commit 63b2853

File tree

6 files changed

+92
-168
lines changed

6 files changed

+92
-168
lines changed

‎app/code/Meta/Catalog/Block/Adminhtml/Diagnostics.php‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -190,16 +190,14 @@ private function getProducts(array $retailerIds, int $storeId)
190190
$collection->addAttributeToSelect('*')
191191
->addStoreFilter($storeId)
192192
->setStoreId($storeId);
193-
194-
$productIdentifierAttr = $this->systemConfig->getProductIdentifierAttr($storeId);
195-
if ($productIdentifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_SKU) {
196-
$collection->addAttributeToFilter('sku', ['in' => $retailerIds]);
197-
} elseif ($productIdentifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_ID) {
198-
$collection->addIdFilter($retailerIds);
199-
} else {
200-
return [];
201-
}
202-
193+
// Because it is not guaranteed whether the seller uses SKU or entity_id as their
194+
// default, we must always query by both types.
195+
$collection->addAttributeToFilter(
196+
[
197+
['attribute' => 'sku', 'in' => $retailerIds],
198+
['attribute' => 'entity_id', 'in' => $retailerIds]
199+
]
200+
);
203201
return $collection->getItems();
204202
}
205203

‎app/code/Meta/Catalog/Block/Adminhtml/Product/Form/Diagnostics.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,13 @@ private function getFacebookProductDiagnostics(): array
123123
}
124124

125125
try {
126-
$retailerId = $this->productIdentifier->getMagentoProductRetailerId($this->product);
127-
$product = $this->graphApiAdapter->getProductByRetailerId($catalogId, $retailerId);
126+
$product = $this->graphApiAdapter->getProductByRetailerId($catalogId, $this->product->getSku());
127+
} catch (\Exception $e) {
128+
$product = $this->graphApiAdapter->getProductByRetailerId($catalogId, $this->product->getId());
129+
}
130+
131+
try {
132+
128133
$fbProductId = $product['data'][0]['id'] ?? false;
129134
if ($fbProductId) {
130135
$productErrors = $this->graphApiAdapter->getProductErrors($fbProductId)['errors'] ?? [];

‎app/code/Meta/Catalog/Helper/Product/Identifier.php‎

Lines changed: 8 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,6 @@ public function __construct(SystemConfig $systemConfig, ProductRepository $produ
5555
$this->productRepository = $productRepository;
5656
}
5757

58-
/**
59-
* Get product's identifier col name(SKU or entity ID, depending on configuration)
60-
*
61-
* @return string
62-
*/
63-
public function getProductIdentifierColName(): string
64-
{
65-
if ($this->identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_SKU) {
66-
return 'sku';
67-
} elseif ($this->identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_ID) {
68-
return 'entity_id';
69-
}
70-
return '';
71-
}
72-
7358
/**
7459
* Get product's identifier (SKU or ID, depending on configuration)
7560
*
@@ -86,24 +71,6 @@ private function getProductIdentifier(ProductInterface $product)
8671
return false;
8772
}
8873

89-
/**
90-
* Get product's other identifier for passing to Meta for product identifications
91-
*
92-
* @param ProductInterface $product
93-
* @return bool|int|string
94-
*/
95-
private function getOtherProductIdentifier(ProductInterface $product)
96-
{
97-
// If identifier is set to sku then we pass magento id, otherwise in case of magento id, we pass sku
98-
// This is done to pass both different type of IDs to meta for product deduplication
99-
if ($this->identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_SKU) {
100-
return $product->getId();
101-
} elseif ($this->identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_ID) {
102-
return $product->getSku();
103-
}
104-
return false;
105-
}
106-
10774
/**
10875
* Get product Retailer ID for Commerce
10976
*
@@ -123,67 +90,27 @@ public function getMagentoProductRetailerId(ProductInterface $product)
12390
*/
12491
public function getProductIDOtherThanRetailerId(ProductInterface $product)
12592
{
126-
return $this->getOtherProductIdentifier($product);
127-
}
128-
129-
/**
130-
* Get product by Facebook retailer id
131-
*
132-
* @param string|int $retailerId
133-
* @return ProductInterface|bool
134-
* @throws LocalizedException
135-
*/
136-
public function getProductByFacebookRetailerId($retailerId)
137-
{
138-
// Sometimes (without realizing), seller catalogs will be set up so that the Retailer ID they pass to Meta is
139-
// the entity ID of their magento product, not the SKU. There are legitimate reasons for sellers to do this,
140-
// but if they don't update their extension settings, calls to fetch magento products will fail.
141-
// This logic catches the product load failure, and makes an attempt to load by the inverted identifier instead
142-
// (Sku -> Entity Id, Entity Id -> Sku). If the config was misset, it will gracefully flip to the correct value.
143-
$product = $this->fetchProduct($retailerId, $this->identifierAttr);
144-
145-
if (!$product) {
146-
// Switch identifier attribute
147-
$newIdentifierAttr = $this->identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_SKU
148-
? IdentifierConfig::PRODUCT_IDENTIFIER_ID
149-
: IdentifierConfig::PRODUCT_IDENTIFIER_SKU;
150-
151-
$product = $this->fetchProduct($retailerId, $newIdentifierAttr);
152-
153-
if ($product) {
154-
$this->systemConfig->setProductIdentifierAttr($newIdentifierAttr);
155-
$this->identifierAttr = $newIdentifierAttr;
156-
} else {
157-
throw new LocalizedException(__(sprintf(
158-
'Product with %s %s does not exist in Magento catalog',
159-
strtoupper($this->identifierAttr),
160-
$retailerId
161-
)));
162-
}
93+
if ($this->identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_SKU) {
94+
return $product->getId();
95+
} elseif ($this->identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_ID) {
96+
return $product->getSku();
16397
}
164-
165-
return $product;
98+
return false;
16699
}
167100

168101
/**
169102
* Fetch product by retailer id and identifier attribute
170103
*
171104
* @param string|int $retailerId
172-
* @param string $identifierAttr
173105
* @return ProductInterface|bool
174106
*/
175-
private function fetchProduct($retailerId, string $identifierAttr)
107+
public function getProductByFacebookRetailerId($retailerId)
176108
{
177109
try {
178-
if ($identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_SKU) {
179-
return $this->productRepository->get($retailerId);
180-
} elseif ($identifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_ID) {
181-
return $this->productRepository->getById($retailerId);
182-
}
110+
return $this->productRepository->get($retailerId);
183111
} catch (NoSuchEntityException $e) {
184112
// Product not found
185-
return false;
113+
return $this->productRepository->getById($retailerId);
186114
}
187-
return false;
188115
}
189116
}

‎app/code/Meta/Catalog/Plugin/FacebookCatalogDeletePlugin.php‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,9 @@ public function __construct(
8080
*/
8181
public function afterDelete(Product $subject, $result, $object)
8282
{
83-
$identifier = $this->identifier->getMagentoProductRetailerId($object);
84-
if ($identifier === false) {
85-
$this->fbeHelper->log('Deleted Product does not have meta identifier.');
86-
return $result;
87-
}
88-
8983
$catalogDelete = $this->catalogUpdateFactory->create()
9084
->setProductId($object->getId())
91-
->setSku($identifier)
85+
->setSku($object->getSku())
9286
->setMethod('delete');
9387
try {
9488
$this->catalogUpdateResourceModel->deleteUpdateProductEntries($identifier);

‎app/code/Meta/Sales/Model/Order/Shipper.php‎

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,12 @@
2222

2323
use GuzzleHttp\Exception\GuzzleException;
2424
use Magento\Framework\Exception\LocalizedException;
25-
use Magento\Sales\Model\Order\Item as OrderItem;
2625
use Magento\Sales\Model\Order\Shipment;
2726
use Magento\Sales\Model\Order\Shipment\Item;
2827
use Magento\Sales\Model\Order\Shipment\Track;
2928
use Meta\BusinessExtension\Helper\FBEHelper;
3029
use Meta\BusinessExtension\Helper\GraphAPIAdapter;
3130
use Meta\BusinessExtension\Model\System\Config as SystemConfig;
32-
use Meta\Catalog\Model\Config\Source\Product\Identifier as IdentifierConfig;
3331
use Meta\Sales\Helper\OrderHelper;
3432
use Meta\Sales\Helper\ShippingHelper;
3533
use Meta\Sales\Model\FacebookOrder;
@@ -79,7 +77,7 @@ public function __construct(
7977
GraphAPIAdapter $graphAPIAdapter,
8078
ShippingHelper $shippingHelper,
8179
OrderHelper $orderHelper,
82-
FBEHelper $fbeHelper,
80+
FBEHelper $fbeHelper
8381
) {
8482
$this->systemConfig = $systemConfig;
8583
$this->graphAPIAdapter = $graphAPIAdapter;
@@ -99,24 +97,6 @@ public function getOrderShipEvent($storeId = null)
9997
return $this->systemConfig->getOrderShipEvent($storeId);
10098
}
10199

102-
/**
103-
* Get retailer id
104-
*
105-
* @param OrderItem $orderItem
106-
* @return string|int|bool
107-
*/
108-
private function getRetailerId(OrderItem $orderItem)
109-
{
110-
$storeId = $orderItem->getStoreId();
111-
$productIdentifierAttr = $this->systemConfig->getProductIdentifierAttr($storeId);
112-
if ($productIdentifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_SKU) {
113-
return $orderItem->getSku();
114-
} elseif ($productIdentifierAttr === IdentifierConfig::PRODUCT_IDENTIFIER_ID) {
115-
return $orderItem->getProductId();
116-
}
117-
return false;
118-
}
119-
120100
/**
121101
* Get carrier code for facebook
122102
*
@@ -199,12 +179,17 @@ public function markAsShipped(Shipment $shipment)
199179

200180
$storeId = $order->getStoreId();
201181

202-
$itemsToShip = [];
182+
$itemsToShipBySku = [];
183+
$itemsToShipById = [];
203184
/** @var Item $shipmentItem */
204185
foreach ($shipment->getAllItems() as $shipmentItem) {
205186
$orderItem = $shipmentItem->getOrderItem();
206-
$itemsToShip[] = [
207-
'retailer_id' => $this->getRetailerId($orderItem),
187+
$itemsToShipBySku[] = [
188+
'retailer_id' => $orderItem->getSku(),
189+
'quantity' => (int)$shipmentItem->getQty()
190+
];
191+
$itemsToShipById[] = [
192+
'retailer_id' => $orderItem->getId(),
208193
'quantity' => (int)$shipmentItem->getQty()
209194
];
210195
}
@@ -216,14 +201,27 @@ public function markAsShipped(Shipment $shipment)
216201
$fulfillmentAddress['state'] = $this->shippingHelper->getRegionName($fulfillmentAddress['state']);
217202
}
218203

219-
$this->markOrderItemsAsShipped(
220-
(int)$storeId,
221-
$fbOrderId,
222-
$magentoShipmentId,
223-
$itemsToShip,
224-
$trackingInfo,
225-
$fulfillmentAddress
226-
);
204+
try {
205+
$this->markOrderAsShipped(
206+
(int)$storeId,
207+
$fbOrderId,
208+
$shipment->getIncrementId(),
209+
$itemsToShipBySku,
210+
$trackingInfo,
211+
$fulfillmentAddress
212+
);
213+
} catch (\Exception $e) {
214+
// Validated the Meta API will throw if retailer ids provided are invalid
215+
// https://fburl.com/code/p523l7gm
216+
$this->markOrderAsShipped(
217+
(int)$storeId,
218+
$fbOrderId,
219+
$shipment->getIncrementId(),
220+
$itemsToShipById,
221+
$trackingInfo,
222+
$fulfillmentAddress
223+
);
224+
}
227225

228226
if ($track) {
229227
$comment = "Order Marked as Shipped on Meta for {$track->getTitle()}. Tracking #: {$track->getNumber()}";
@@ -292,7 +290,9 @@ public function updateShipmentTracking(Shipment $shipment)
292290

293291
if (count($tracks) == 0) {
294292
// For now, we don't support removing tracking entirely
295-
$this->fbeHelper->log("[updateShipmentTracking] Shipment: {$shipment->getIncrementId()} - Skipping, no tracks");
293+
$this->fbeHelper->log(
294+
"[updateShipmentTracking] Shipment: {$shipment->getIncrementId()} - Skipping, no tracks"
295+
);
296296
return;
297297
}
298298

@@ -307,7 +307,9 @@ public function updateShipmentTracking(Shipment $shipment)
307307

308308
$magentoShipmentId = $shipment->getIncrementId();
309309
if (!FacebookOrder::isSyncedShipmentOutOfSync($order, $magentoShipmentId, $trackingInfo)) {
310-
$this->fbeHelper->log("[updateShipmentTracking] Shipment: {$shipment->getIncrementId()} - Skipping, in sync");
310+
$this->fbeHelper->log(
311+
"[updateShipmentTracking] Shipment: {$shipment->getIncrementId()} - Skipping, in sync"
312+
);
311313
return;
312314
}
313315

@@ -318,7 +320,7 @@ public function updateShipmentTracking(Shipment $shipment)
318320
$trackingInfo,
319321
);
320322

321-
$comment = "Order Shipment Tracking Updated on Meta for {$track->getTitle()}. Tracking #: {$track->getNumber()}";
323+
$comment = "Order Shipment Tracking Updated on Meta for {$track->getTitle()}. Tracking #{$track->getNumber()}";
322324
$order->addCommentToStatusHistory($comment)->save();
323325

324326
$fbOrder = $this->orderHelper->loadFacebookOrderFromMagentoId($order->getId());
@@ -338,7 +340,7 @@ private function updateOrderShipmentTracking(
338340
int $storeId,
339341
string $fbOrderId,
340342
string $magentoShipmentId,
341-
array $trackingInfo,
343+
array $trackingInfo
342344
) {
343345
$this->graphAPIAdapter
344346
->setDebugMode($this->systemConfig->isDebugMode($storeId))

0 commit comments

Comments
 (0)