Bugfix for UpdateItem event firing multiple times on add to cart #244
Conversation
|
AWESOME ! 🎉 , waiting for tests to pass once passing ill merge. |
|
Seems travis is having issues with composer install. |
|
I see, but I didn't changed anything to composer. |
|
Will look at it tonight to see if i can fix it. |
|
I fixed travis , but there are two errors we need to fix before moving on : |
tests/ItemsTest.php
Outdated
| Event::assertDispatched('laracart.addItem', function ($e, $item) use ($cartItem) { | ||
| return $item === $cartItem; | ||
| }); | ||
| Event::assertDispatched('laracart.addItem', 1); |
There was a problem hiding this comment.
This is causing errors, did you mean to add this line?
There was a problem hiding this comment.
Event::assertDispatched('laracart.addItem', 1)
tests/ItemsTest.php
Outdated
| Event::assertDispatched('laracart.updateItem', function ($e, $eventItem) use ($item) { | ||
| return $eventItem['item'] === $item && $eventItem['newHash'] === $item->getHash(); | ||
| }); | ||
| Event::assertDispatched('laracart.updateItem', 1); |
There was a problem hiding this comment.
This is causing errors, did you mean to add this line?
There was a problem hiding this comment.
Event::assertDispatched('laracart.updateItem', 1);
There was a problem hiding this comment.
Yes this line is checking if the event is fired once. I was checking this as we speak. I think this line is supported from Laravel 5.6+ and your package is requiring Laravel 5.4.
There was a problem hiding this comment.
Will update and remove those versions from being supported
There was a problem hiding this comment.
Founded another solution. :) I'll commit it in a few seconds.
|
Thnx! |
|
Thank you! |
Made this bug fix with tests to prevent UpdateItem to firing multiple times.
The reason was the magic setter which triggered the generateHash method for every new created property. The generateHash method is firing the update event.
I made a check in the magic setter if the property already exists and if so, if the property is changed.
The generateHash method will now only be triggered when a property is changed.
Some tests are failing, but this has nothing to do with my changes.