merx
merx copied to clipboard
Custom cart fields cannot be updated
Hi Tobias, thank you for this awesome plugin, I love it!
I am not sure if this is bug or a feature, but I found an inconsistence between default and custom cart fields behavior. Please see below.
Steps to reproduce
Setup
- Set custom cart field
shipping
in optionww.merx.cart.fields
- Have a product page:
Id: test
----
Title: Test product
----
Price: 50
----
Shipping: 20
- Add (or update) item with custom values for price and shipping
$cart->add([
'id' => 'test',
'price' => 99,
'shipping' => 33
]);
Expected behavior
- price should be 99
- shipping should be 33
Current behavior
- price is 99
- shipping is 20
The difference
Default fields have this check so they are fetched only once when item is added to cart and you did not provide any custom value. https://github.com/wagnerwagner/merx/blob/ec51526d1fba7233ac632b2c953397abee864416/src/ProductList.php#L102-L104
Custom fields are missing this feature and they are fetched from product page on every cart initialization, cart update or item update, making basically impossible to have any custom value in custom fields from different source than product page model. https://github.com/wagnerwagner/merx/blob/ec51526d1fba7233ac632b2c953397abee864416/src/ProductList.php#L125-L135
I would suggest to make this behavior consistent by adding the same check to custom fields too. The behavior will slightly change. I can imagine someone having setup based on page model methods and being dependent on this feature, so I am not sure if it should be done. But again, it is inconsistent with the default fields.
foreach (option('ww.merx.cart.fields', []) as $fieldName) {
if (!isset($value[$fieldName])) { // <- add check if the field is present in updated data
$field = $page->{$fieldName}();
if (is_a($field, '\Kirby\Cms\Field') && $field->isNotEmpty()) {
$value[$fieldName] = $field->toString();
} elseif (
$field === null ||
is_scalar($field) ||
is_string($field) ||
(is_object($field) && method_exists($field, '__toString'))
) {
$value[$fieldName] = (string)$field;
}
}
}
What do you think?
Thank you, all the best, Jakub