bavix / laravel-wallet

It's easy to work with a virtual wallet
https://bavix.github.io/laravel-wallet/
MIT License
1.12k stars 223 forks source link

Events are not dispatching #903

Closed GeorgeInteli closed 6 months ago

GeorgeInteli commented 6 months ago

Describe the bug The events from the package are not dispatching.

To Reproduce Steps to reproduce the behavior:

  1. Create user with wallet
  2. Deposit amount on wallet
  3. Expect to receive Balance Updated event
  4. No event received

Expected behavior After a deposit I expect to receive event on Balance change and Transaction created.

Screenshots Here is sample test image

Server:

rez1dent3 commented 6 months ago

@GeorgeInteli Hello. All events are sent when a transaction is committed. Add a check to your transaction level test, it must be 0 for the event to be sent.

I'm sure the test will fail, because... pest starts a transaction before the test starts and ends it after the test ends.

You need to call commit yourself before checking.

GeorgeInteli commented 6 months ago

Hello @rez1dent3 , I can confirm that even on transaction level 0 the events are not dispaching. It was level 1 inside pest, it is going to level 0 after commit, but the test still fails. The most confusing thing about all this is that balanceInt is actually being updated, but the event is not fired.

Note that even if the events did fire i dont think reverting or commiting something outside the scope of the function is a solution. I am sure this was discussed before. Any best practices or solutions you might recommend? And what is the reasoning behind transaction level 0. I see it hard coded inside TransactionCommittedListener.php (i read #412 and #455)

rez1dent3 commented 6 months ago

@GeorgeInteli The laravel architecture is such that if there is no listener, then the event is not sent.

Here you can see the tests of the package: https://github.com/bavix/laravel-wallet/blob/e8c70cc385319a2119fd1c4e2426ec1dddefff56/tests/Units/Domain/EventTest.php#L73-L88

https://github.com/bavix/laravel-wallet/blob/master/tests/Units/Domain/EventTest.php

rez1dent3 commented 6 months ago

And what is the reasoning behind transaction level 0

The transaction can be canceled and the event sent.

rez1dent3 commented 6 months ago

I checked locally again. Events are sent. Need more data.

rez1dent3 commented 6 months ago

@GeorgeInteli I didn't notice at first, but you're testing the implementation, not the interface. You need to check the contract itself, the trigger for it.

GeorgeInteli commented 6 months ago

image

Hmmm i got nothing. Not seeing the event at all inside the dispatcher events stack.

I am seeing however our own Laravel defined events. There is something specific to this package events.

rez1dent3 commented 6 months ago

As I already wrote above, you need to determine the listener.

rez1dent3 commented 6 months ago

@GeorgeInteli Hello. I took some time and started debugging. The flush method is not implemented for Fake in laravel. You will not be able to implement your check.

https://github.com/laravel/framework/blob/0525a8819db53bc66c878413829a4368d63c3d47/src/Illuminate/Support/Testing/Fakes/EventFake.php#L278-L287

GeorgeInteli commented 6 months ago

Thank you @rez1dent3 Now we know what the issue is :) We will just have incomplete test for the moment.

rez1dent3 commented 6 months ago

@GeorgeInteli You can use it in a similar way to the tests in the package itself. Those,

this will give you the ability to write tests.

Look at the tests in the project: https://github.com/bavix/laravel-wallet/blob/master/tests/Units/Domain/EventTest.php