Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using HasRelationshipObservables trait in multiple models causes double events to be fired in tests #40

Open
BoyeMagnus opened this issue Sep 16, 2020 · 7 comments

Comments

@BoyeMagnus
Copy link

When adding the HasRelationshipObservables trait to multiple models, the tests often fail, because of observers receiving the event twice.

For eg. a user and a group model with HasRelationshipObservables. With two observers that subscribe to belongsToManyAttached both.

trait HasRelationshipObservables
{
    /**
     * @var array
     */
    protected static $relationshipObservables = [];

    /**
     * Initialize relationship observables.
     *
     * @return void
     */
    public static function bootHasRelationshipObservables()
    {
         ......gets the methods

        static::mergeRelationshipObservables($methods);
    }

Static properties will most of the time in tests never be reset, because of how PHPUnit works. When booting HasRelationshipObservables, then it merges the $methods with the static $relationshipObservables. This causes the observer to be fire multiple times in tests where the static property is never reset.

A solution would be to set $relationshipObservables to an empty array in the bootHasRelationshipObservables. Anyone expiring the same issues?

@chelout
Copy link
Owner

chelout commented Sep 16, 2020

Thanks, @BoyeMagnus
Could you please provide a test that fails?

@BoyeMagnus
Copy link
Author

Yes I'll provide one asap 😄

@chelout
Copy link
Owner

chelout commented Sep 16, 2020

Thanks again, i'll look into it tomorrow.

@chelout
Copy link
Owner

chelout commented Sep 22, 2020

ping @BoyeMagnus

@BoyeMagnus
Copy link
Author

Sorry for the delay, had a pretty rough sprint.
I've made a branch with a test job that is being run every time the event should be happening (In the UserObserver). As you can see the job is being fired more than one time if you have test cases in PHPUnit.

Adding static::$relationshipObservables = []; to line 28 in Chelout\RelationshipEvents\Traits\HasRelationshipObservables fixes the problem.
The problem is that the User model is booted multiple times during the tests, and as the package appends the "methods" in the static $relationshipObservables variable, then it keeps growing for each test.

I've created a PR with the test here: #41

@BoyeMagnus
Copy link
Author

@chelout ping

@chelout
Copy link
Owner

chelout commented Oct 24, 2020

@BoyeMagnus sorry for the huge delay, working on MVP project for days and nights 🤯
I would accept PR with described changes with provided tests if you have a little more time then me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants