Custom cart fields cannot be updated

[jimkrutor]

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

1. Set custom cart field shipping in option ww.merx.cart.fields
2. Have a product page:

Id: test
----
Title: Test product
----
Price: 50
----
Shipping: 20

3. 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.

merx/src/ProductList.php

Lines 102 to 104 in ec51526

	if (!isset($value['title'])) {
	$value['title'] = $page->title()->toString();
	}

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.

merx/src/ProductList.php

Lines 125 to 135 in ec51526

	foreach (option('ww.merx.cart.fields', []) as $fieldName) {
	$field = $page->{$fieldName}();
	if (
	$field === null ||
	is_scalar($field) ||
	is_string($field) ||
	(is_object($field) && method_exists($field, '__toString'))
	) {
	$value[$fieldName] = (string)$field;
	}
	}

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

[tobiasfabian]

Thank you for your detailed description.

I updated the code and added a check for !array_key_exists($fieldName, $value) instead of isset(). This allows you to set a null value via $cart->add([…, 'shipping' => null]). (see 843238e)

I think is_a($field, '\Kirby\Cms\Field') && $field->isNotEmpty() is not necessary because every Kirby\Cms\Field has a __toString method.

Let me know if this works for you.

This change is published in v1.8.1