tenancy / multi-tenant

Run multiple websites using the same Laravel installation while keeping tenant specific data separated for fully independent multi-domain setups, previously github.com/hyn/multi-tenant
https://tenancy.dev
MIT License
2.56k stars 393 forks source link

Laravel notifications not working with tenancy? #685

Closed DefaultLGTM closed 5 years ago

DefaultLGTM commented 5 years ago

Description

..


Actual behavior

Returns NULL

Expected behavior

Count of all unread notifications


Information


tenancy.php config

webserver.php config


Error log

Hi. I have a working application being refactored to accommodate multi tenancy. My question is this:

Why does this return NULL... Auth::user()->unreadNotifications; where I should be expecting a non-null return value?

Refactored code to Users::find(Auth::user()->id)->unreadNotifications and then it works.

Applied UsesTenantConnection trait to User model. For some reason, I have both Users and User model. User is used by Laravel's default login route.

Just to make sure, should all tenancy-specific models have UsesTenantConnection implemented?

DefaultLGTM commented 5 years ago

One more thing... when accessing 1 Notification, of which is assigned to $notification, and outputting it thru dd(), it's there. But, if dd($notification->id) is invoked, 0 is returned when it should not be that. What is happening?

luceos commented 5 years ago

Seems like you expect all models to be on the tenant connection without explicitly configuring them as such.

For some reason, I have both Users and User model. User is used by Laravel's default login route.

This doesn't make sense. You can configure the Model used in the auth.php configuration file. Make sure you don't duplicate models unnecessarily.

Just to make sure, should all tenancy-specific models have UsesTenantConnection implemented?

Yes, when they have that trait applied they will use the tenant connection.

One more thing... when accessing 1 Notification, of which is assigned to $notification, and outputting it thru dd(), it's there. But, if dd($notification->id) is invoked, 0 is returned when it should not be that.

Can you explain what a $notification is? Is that a model?

juhasev commented 5 years ago

I just run into this also... He is talking about Laravel Notifications. Almost always you would queue these because email, sms etc... sending can take time... The package could be extended with new trait TenantAwareNotification for non queued notifications. However if you do queue the Notification you can just use TenantAwareJob trait like this and things will work fine.

namespace App\Notifications;

use Illuminate\Bus\Queueable;
use Illuminate\Notifications\Notification;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Notifications\Messages\BroadcastMessage;
use Hyn\Tenancy\Queue\TenantAwareJob;

class Announcement extends Notification implements ShouldQueue
{
    use Queueable;
    use TenantAwareJob;

    // Your code here

}
luceos commented 5 years ago

Notification can also be added to the override models key in the tenancy.php configuration. This was confirmed by another user.