Add UpdateProductInViewModel#71
Conversation
| string unitId = settings.UseUnitPrices ? productForRequest.DefaultUnitId : null; | ||
| var productSelection = productForRequest.GetPriceProductSelection(1, unitId); | ||
|
|
||
| var productInfo = ProductManager.GetProductInfo(productForRequest, settings, user); |
There was a problem hiding this comment.
High: cache lookup/fetch is not currency-aware. GetProductInfo(productForRequest, settings, user) ignores the current currency, and FetchProductInfos(..., false, ...) can reuse another currency's cached ProductInfo. Pass the context/unit to GetProductInfo and enable currency checking on fetch.
| if (productInfo == null) | ||
| return; | ||
|
|
||
| ProductManager.ProductProvider.FillProductValues(productInfo, loadedProduct, settings, productSelection.Quantity, context); |
There was a problem hiding this comment.
Medium: this mutates loadedProduct, not the ProductViewModel passed in. If ProductFields or price lazy properties were already evaluated, the caller still sees stale values. Rebuild/assign the affected view-model properties or return a refreshed ProductViewModel.
| var logger = new Logging.Logger(settings); | ||
| bool showVariantDefault = SystemConfiguration.Instance.GetBoolean("/Globalsettings/Ecom/Product/ShowVariantDefault"); | ||
| var productForRequest = showVariantDefault ? ProductManager.ProductProvider.GetProductFromVariantComboId(loadedProduct, logger) : loadedProduct; | ||
| var context = new LiveContext(Helpers.GetCurrentCurrency(), user, null); |
There was a problem hiding this comment.
Medium: new LiveContext(GetCurrentCurrency(), user, null) leaves both Shop and PriceContext null. Because PriceContext is null, FillProductValues (line 165) writes the price into a throwaway new PriceContext(...) (see ProductProviderBase.FillProductValues/GetPriceInfoCore) that is then discarded — so the price computation here has no lasting effect, independent of the view-model issue noted on line 165. The live-price hook does the opposite: ProductPriceProvider.PreparePrices builds new LiveContext(context) so the request's real PriceContext (and shop) is used. Thread the caller's PriceContext/shop in here too. Also note settings/shopId come from Global.CurrentShopId, but the context shop is null, so the fetch runs shop-less while the settings are shop-scoped.
|
|
||
| var logger = new Logging.Logger(settings); | ||
| bool showVariantDefault = SystemConfiguration.Instance.GetBoolean("/Globalsettings/Ecom/Product/ShowVariantDefault"); | ||
| var productForRequest = showVariantDefault ? ProductManager.ProductProvider.GetProductFromVariantComboId(loadedProduct, logger) : loadedProduct; |
There was a problem hiding this comment.
Medium: with ShowVariantDefault on, productForRequest is the variant-combo product and drives GetProductInfo, FetchProductInfos, and productSelection (lines 148-159), but FillProductValues is then applied to loadedProduct (the base product), not productForRequest (line 165). ProductPriceProvider.PreparePrices/FindPriceInfo use the same product throughout. Please confirm this cross-apply is intended — otherwise the base product is filled from the combo product's info (or from none, if the identifiers differ).
|
|
||
| // Update Product Custom Fields | ||
| if (settings.AddProductFieldsToRequest && product.ProductFieldValues.Count > 0) | ||
| if (settings.AddProductFieldsToRequest) |
There was a problem hiding this comment.
Low: dropping && product.ProductFieldValues.Count > 0 has no functional effect here — FillProductFieldValues only foreaches over product.ProductFieldValues, so an empty collection is already a no-op. If the intent was to populate/send fields when the in-memory collection is empty, this change doesn't achieve it; if it's just simplification, it's harmless but doesn't change behaviour.
| AddChildXmlNode(itemNode, "Quantity", productWithQuantity.Quantity.ToIntegrationString(currentSettings, logger)); | ||
|
|
||
| if (settings.AddProductFieldsToRequest && product.ProductFieldValues.Count > 0) | ||
| if (settings.AddProductFieldsToRequest) |
There was a problem hiding this comment.
Low: same as the ProductProviderBase change — AppendProductFields only iterates product.ProductFieldValues, so dropping the Count > 0 guard produces identical XML (no custom-field nodes when the collection is empty). No-op unless the goal was to emit something for the empty case, which this doesn't do.
2 participants
No description provided.