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

[8.x] Notification assertions respect shouldSend method on notification #38979

Conversation

romalytvynenko
Copy link
Contributor

@romalytvynenko romalytvynenko commented Sep 27, 2021

Support for notification's shouldSend method were added in PR #37930. This method allows users to add some logic to cancel sending notification based on given notifiable and channel.

Problem

However, testing methods assertSentTo and assertNotSentTo weren't aware about this feature and didn't behave as expected (reported here as well #38909).

For example, take this code:

// somewhere in user's code
Notification::send($user, new SomeNotification);

// implementation
class SomeNotification extends Notification
{
    public function via($notifiable)
    {
        return ['mail'];
    }

    public function shouldSend($notifiable, $channel)
    {
        return false;
    }
}

Now, when testing whether notification hasn't been sent, we won't get expected assertion result:

Notification::fake();

// some code triggering notification send

Notification::assertNotSentTo($user, SomeNotification::class); // will fail, but should pass

Solution

This PR adds support of shouldSend to notification's assertSentTo and assertNotSentTo methods. So now when shouldSend is presented on notification class and returns false for some notifiable, assertSentTo and assertNotSentTo can be used to check that behavior.

Fixes #38909

@taylorotwell taylorotwell merged commit 314de9c into laravel:8.x Sep 27, 2021
victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
…tion (laravel#38979)

* added `shouldSend` support to notification assertions

* changed count to empty
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

Successfully merging this pull request may close these issues.

shouldSend does not get called or change outcome of "assertNotSent" or other notification assertions
2 participants