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

Unable to return no phones from routeNotificationForSmscru #26

Closed
Finesse opened this issue Jun 5, 2018 · 1 comment
Closed

Unable to return no phones from routeNotificationForSmscru #26

Finesse opened this issue Jun 5, 2018 · 1 comment

Comments

@Finesse
Copy link
Contributor

Finesse commented Jun 5, 2018

If I return null, an empty string or an empty array from the routeNotificationForSmscru method of a Notifiable, I get a CouldNotSendNotification exception. I think this is incorrect because all the built-in Laravel channels just do nothing if no recipient is set (examples: 1, 2, 3).

To workaround it I have to do the following in my notification classes:

public function via($notifiable)
{
    if ($notifiable->routeNotificationFor('smscru', $this)) {
        return [SmscRuChannel::class];
    } else {
        return [];
    }
}

There is one more problem. Notifications objects are not passed to the routeNotificationForSmscru method so I can't decide what number to send an SMS to depending on the notification class or object.

Also it would be great if I can return either a string or an array of phone numbers from routeNotificationForSmscru method. I know I can return multiple phones in a single string separating them by comma, but the array would be a more convenient and Laravel-style way.

If you agree, I can implement all the features and make a pull request.

@jhaoda
Copy link
Member

jhaoda commented Jun 5, 2018

Thanks. I agree and I'm waiting for a PR.

jhaoda pushed a commit that referenced this issue Jun 10, 2018
Also the notification instance is passed to routeNotificationForSmscru() in Laravel v5.6+

Resolves #26
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