merx icon indicating copy to clipboard operation
merx copied to clipboard

Custom cart fields cannot be updated

Open jimkrutor opened this issue 10 months ago • 0 comments

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

jimkrutor avatar Apr 08 '24 09:04 jimkrutor