wagnerwagner / merx

Merx is a plugin to create online shops with Kirby.
https://merx.wagnerwagner.de
104 stars 10 forks source link

updateItem with empty array element does not update #34

Closed plagasul closed 3 years ago

plagasul commented 3 years ago

Hello,

I have this product in cart:

Array
(
    [id] => cactus
    [quantity] => 1
    [title] => Cactus
    [price] => 12.99
    [taxRate] => 0
    [tax] => 0
    [sum] => 12.99
    [sumTax] => 0
    [frames] => Array
        (
            [0] => frames-options/wood-white
        )

)

[frames] may contain one or several items, and I want to add or remove those.

Let's say I want to remove the single item actually present in [frames]

$product = 'cactus';
$frame = 'frames-options/wood-white';

/* Get frames array */
$frames = cart()->find($product)['frames'] ?? []; 

/* Remove frame from array */
unset($frames[array_search($frame, $frames)]);

/* Build product */
/* Note that $frames is an empty array now */
$updatedProduct = [
    'id' => $product,
    'frames' => $frames,                                        
];

/* Update product */
cart()->updateItem($updatedProduct);

I would expect this to update the product with an empty array but it does not seem to do so, this is the product on the cart after updating:

Array
(
    [id] => cactus
    [quantity] => 1
    [title] => Cactus
    [price] => 12.99
    [taxRate] => 0
    [tax] => 0
    [sum] => 12.99
    [sumTax] => 0
    [frames] => Array
        (
            [0] => frames-options/wood-white
        )
)

...exactly the same.

Something similar happens when updating the frames array with an array whose length is shorter:

Array
(
    [id] => axe
    [quantity] => 2
    [frames] => Array
        (
            [0] => eine
            [1] => zwei
        )
    ...
)
cart()->update([[
    'id' => 'axe',
    'frames' => ['mango']
]]);

results in

Array
(
    [id] => axe
    [quantity] => 2
    [frames] => Array
        (
            [0] => mango
            [1] => vier
        )
    ...
)

At first this was unexpected but after much testing it seems this is the expected behaviour of A::MERGE_OVERWRITE , which you @tobiasfabian probably use so the properties of a product that we are not providing in our updated product array are left untouched.

Unfortunately this translates to my nested array, and lefts me unable to remove items in this manner.

I need to think about how best to do this, but any ideas would be greatly appreciated

Thank you

tobiasfabian commented 3 years ago

Thanks for your feedback / issue.

A::MERGE_OVERWRITE: elements are overwritten [recursively], keys are preserved
https://getkirby.com/docs/reference/tools/a/merge

I think A::merge() works as intended – as you’ve written. https://github.com/wagnerwagner/merx/blob/43e9b8ddf195b1a6f540319dd49cda1de074a1b7/src/productList.php#L66

Option A:
You could store a copy of the cart item in a variable. Remove the cart item from the cart. Update the copied cart item as you’d like to and add it again to the cart. But this would change the order of the cart items.

Option B:
You can try to use $cart->set() directly which is used here https://github.com/wagnerwagner/merx/blob/43e9b8ddf195b1a6f540319dd49cda1de074a1b7/src/productList.php#L67

But I'm not sure how well that will work.

plagasul commented 3 years ago

Thanks @tobiasfabian

I can't get set() to work. As far as I understand it expects the ID of a product which may or may not be present in the cart, and an array of properties for that product.

I have this product int he cart:

Array
(
    [id] => bananas-a
    [quantity] => 1
    [title] => Five Bananas Two Apples
    [price] => 10
    [taxRate] => 0
    [tax] => 0
    [sum] => 10
    [sumTax] => 0
    [frames] => Array
        (
            [0] => frames-options/wood-white-includes-passepartout
        )

)

And I attempt to update/set it with this code

// Define product and frame
$product = 'bananas-a';
$frame = 'frames-options/wood-white-includes-passepartout';

// Build product
// Note that $frames is an empty array now
$updatedProduct = [
    'id' => $product,
    'frames' => [],                                     
];

// Update/Set product
cart()->set($product, $updatedProduct);

The result is the same:

Array
(
    [id] => bananas-a
    [quantity] => 1
    [title] => Five Bananas Two Apples
    [price] => 10
    [taxRate] => 0
    [tax] => 0
    [sum] => 10
    [sumTax] => 0
    [frames] => Array
        (
            [0] => frames-options/wood-white-includes-passepartout
        )

)

Am I using set() right ?

Alternatively, is there any way to completely remove a single property from a product ? I could copy [frames], delete it, then add it again on one swoop, but this is a similar problem isn't it ?

Danke

plagasul commented 3 years ago

@tobiasfabian it seems that using cart()->set() directly DOES return the correctly modified cart, but it does NOT modify the session's cart... is this expected ?

I mean if I have:

Array
(
    [id] => bananas-a
    [quantity] => 1
    [title] => Five Bananas Two Apples
    [price] => 10
    [taxRate] => 0
    [tax] => 0
    [sum] => 10
    [sumTax] => 0
    [frames] => Array
        (
            [0] => frames-options/wood-white-includes-passepartout
        )

)

and I do:

$updatedProduct = [
    'id' => 'bananas-a',
    'frames' => [], 
    'price' => 2222,
    'blah' => 'bleh'                                    
];

$updatedCart = cart()->set($product, $updatedProduct);

dump($updatedCart->first());
dump(cart()->first());

I get:

Array
(
    [id] => bananas-a
    [frames] => Array
        (
        )

    [price] => 2222
    [blah] => bleh
    [quantity] => 1
    [title] => Five Bananas Two Apples
    [taxRate] => 0
    [tax] => 0
    [sum] => 2222
    [sumTax] => 0
)

Array
(
    [id] => bananas-a
    [quantity] => 1
    [title] => Five Bananas Two Apples
    [price] => 10
    [taxRate] => 0
    [tax] => 0
    [sum] => 10
    [sumTax] => 0
    [frames] => Array
        (
            [0] => frames-options/wood-white-includes-passepartout
        )

)

So, yes, the first dump is correct, so set() works, but it does not set the session's cart as mentioned.

Perhaps it is a basic, but I am unsure if this is expected, and if I am missing any step in order to make the changes to the session's cart.

Cheers.

plagasul commented 3 years ago

I've just noticed save(), as a private function of cart()

The following seemingly works, using cart() constructor to fully replace the cart's contents.

    $updatedCart = cart()->set($product, $updatedProduct);
    cart($updatedCart->toArray());

Does seem a bit like hunting flies with bazookas tho. Replacing the whole session cart just in order to replace an item in an array in a product.

Does it seem to you like a bad idea ?

I can also save directly to session :

    $updatedCart = cart()->set($product, $updatedProduct);
    kirby()->session()->set('ww.merx.cartItems', $updatedCart->toArray());

Danke

plagasul commented 3 years ago

I am going to close the issue as there is probably not much to add than what I say in the previous post.