Nexmo / nexmo-laravel

Add Vonage functionality such as SMS and voice calling to your Laravel app with this Laravel Service Provider.
MIT License
317 stars 79 forks source link

Using Nexmo in Laravel with Multi-tenancy #59

Closed cverster closed 3 years ago

cverster commented 3 years ago

I'm currently using Spatie's multi-tenancy package to implement multi-tenancy on my laravel app. Before migrating to multi-tenancy, my Nexmo SMS implementation was working fine, but after implementing Spatie's multi-tenancy package (and consequently starting to use mutliple databases per tenant), I am no longer able to send SMSs. I don't get any error messages, but the SMS just doesn't send. I didn't expect this as I don't believe Nexmo uses my database..

Expected Behavior

The SMS to send using Nexmo and using my code as implemented prior to migrating to mutli-tenancy. Below is an example of a Nexmo call in a notification. Also note that I have both NEXMO_KEY and NEXMO_SECRET set in .env:

<?php

namespace App\Notifications;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Notifications\Messages\MailMessage;
use Illuminate\Notifications\Notification;
use Illuminate\Notifications\Messages\NexmoMessage;
use Nexmo\Laravel\Facade\Nexmo;

class ShiftTwoHourSMSNotification extends Notification
{
    use Queueable;

    public $content;
    public $phone_number;

    /**
     * Create a new notification instance.
     *
     * @return void
     */
    public function __construct($content, $phone_number)
    {
        $this->content = $content;
        $this->phone_number = $phone_number;
    }

    /**
     * Get the notification's delivery channels.
     *
     * @param  mixed  $notifiable
     * @return array
     */
    public function via($notifiable)
    {
        return ['nexmo'];
    }

    /**
     * Get the Nexmo / SMS representation of the notification.
     *
     * @param  mixed  $notifiable
     * @return NexmoMessage
     */
    public function toNexmo($notifiable)
    {
        return Nexmo::message()->send([
            'to'   => $this->phone_number,
            'from' => '[my_number]',
            'text' => $this->content,
        ]);
    }

    /**
     * Get the array representation of the notification.
     *
     * @param  mixed  $notifiable
     * @return array
     */
    public function toArray($notifiable)
    {
        return [
            //
        ];
    }
}

Current Behavior

No SMS is being sent, no error is appearing, and I have nothing coming up in laravel log. I can currently send an SMS using normal Nexmo code as described in the Get Started section of the docs, but my feeling is that maybe this has something to do with the Laravel Nexmo Facade?

Possible Solution

I don't even know where to start, as I have limited understanding of the Nexmo laravel package under the hood.

Steps to Reproduce (for bugs)

Implement Spatie's multi-tenancy package and use a mutliple database setup. Then try to send an sms with nexmo

Context

Your Environment

dragonmantank commented 3 years ago

There are two potential issues. One is that the multi-tenancy package is causing an issue with notifications. I would make sure that the toNexmo() method is even being called before going any further. I am not familiar with the multi-tenancy package so I'm not sure how it affects everything.

The second is the actual code in toNexmo(). Our facade does not return a NexmoMessage object, but instead returns a Collection object from our SDK. The SMS should still send as you are invoking the send functionality directly, but the return value is not what Laravel is expecting and will cause issues. If you are using notifications I would use the Nexmo Notification Channel package instead.

dragonmantank commented 3 years ago

Closing due to inactivity. If you get some more information on this feel free to re-open and we'll see what we can do!