Support for tables indirectly linked to the tenant via intermediary table(s)

[gausam]

Was discussing with @marktopper around how we can have multi-tenancy support in Voyager when we recalled Landlord. Out of the box the two can work together with one not even knowing about the existence of the other, but looking at a practical application's schema from what I'm working on at the moment, the requirement below makes things a bit difficult for those coming in to Landlord after they have their schema laid out already:

This package assumes that you have at least one column on all of your Tenant scoped tables that references which tenant each row belongs to.

Some tables will obviously have direct tenant references, e.g Students table will link to Class table via class_id in the students table; but Grades will usually link to the student via student_id and then establish a relationship with the Class from there on ($grade->student->class).

Would you consider adding support for such scenarios, which may be quite common? As to the actual best implementation, that is up to you of course - but off the top of my head (hence crude, but bear with me) I was thinking that even something basic like "If the model has a scope tenant() then run with that, otherwise look for $tenantColumns".

Carrying from the example earlier on, Grade would have:

<?php

use Illuminate\Database\Eloquent\Model;
use HipsterJazzbo\Landlord\BelongsToTenants;

class Grade extends Model
{
    use BelongsToTenants;

    /**
     * @param int $tenantId Current tenant Id
     * @return Illuminate\Database\Eloquent\Builder
     */
    public function scopeTenant($tenantId)
    {
        return $query->whereHas('student.class', function($query) use ($tenantId) {
            $query->where('id', $tenantId);
        });
   }
}

What do you think? Here's hoping :pray: you'll consider this as it'll allow greater flexibility and more universal support confidence when multi-tenancy deployment documentation is added to Voyager.

[gausam]

@HipsterJazzbo :wave: Any thoughts on this?

[hipsterjazzbo]

Hi @samtheson,

Have you run into any instances where this doesn't currently work? I'm using Landlord in production in a number of pretty complex apps, and distant relationships work fine.

It may be that you're regarding the wrong thing as the Tenant — the Tenant ought to be some entity that relates to a real-world person or organisation whose data should not be shared with other people or organisations using the system. In your example, I'd suspect a more likely Tenant would be for School, not Class, and all the Classes belong to a School as well; they're not the end of the chain so to speak.

Does that make sense, or have I totally missed your point? :P

[gausam]

@HipsterJazzbo Thanks Caleb,

Oh so Landlord's able to automatically scope Grade::all() to the current School tenant, to which it's only linked via a belongsTo?

Heh just to make sure we're on the same page: My example was a theoretical multi-tenant student grade management system. So the School is the tenant, and in the system there's a section for a school to view all students. That's easy because Student has school_id. But there's also a section to list all grades, and Grade has no school_id but links to its School via the Student. That's the edge case I'm inquiring about.

Thanks again!

[hipsterjazzbo]

I guess I still mightn't totally understand the example. If Grade contains markings for particular students of a particular school, why not just throw a school_id column on there? It doesn't always have to reflect a relationship in the Model, it could be there solely for the purposes of data integrity.

[gausam]

We're now actually on the very same page :+1:

That's the request we have, in light of this hard requirement:

This package assumes that you have at least one column on all of your Tenant scoped tables that references which tenant each row belongs to.

In the majority of applications, not every table has a direct relationship with the main table from which we determine the tenancy. So coming in to Landlord afterwards, and esp in our case where people find Landlord through Voyager, it's quite a steep demand to have them modify their entire perfectly normalized schema to have Landlord multi-tenancy going. In one example application for a large global enterprise, we have dozens of tables - and to request such a change would be a monster of a request and hard to justify in front of a team who are yet to fully appreciate what Landlord can do for them.

So that's why after a bit of back and forth in the community we decided we'd ask what you think about adding a bit more flexibility around how child tables relate to the main tenant table. It won't even be a lot of code to do it, I think, because the hardest sell so far when I introduce people (with already-made projects)) to Landlord is This package assumes that you have at least one column on all of your Tenant scoped tables that references which tenant each row belongs to.. That would a non-issue, I think, if there was a more flexible way to define the relationship.

For example if I want to use Landlord in a project with an existing massive database, I'd have to add the tenancy column to tables that do not have it, and then I must write a script to populate that column for all the tables.

Hope that lays out my story quite.

So... What do you think?

[gausam]

For straight forward relationships we could allow defining the tenancy relationship with a simple string (student.class.school in the case of our Grade example), and then for more complex scenarios the user can define their custom query scope upon which Voyager builds.

[gausam]

Hi again @HipsterJazzbo :), put together quickly a working app to demonstrate.

The fork has only modifications for reads, but a similar mechanism would apply I guess for creation of new tenant-scoped models. You can view the small change required in the diff.

And in the Laravel application, you can see this applied in the Grade class. Student and School work normally as Landlord would expect.

Aside: Will add a docs PR to demonstrate the Landlord middleware approach, could help a couple of people.

[hipsterjazzbo]

Hmmm. So, here's the problem from my perspective: Landlord is designed as much as possible to make it impossible for data to bleed across tenants. This is the reason for the tenant column requirement — no matter how the data is accessed, it can be scoped.

I'm hesitant to do something like this without some very thorough understanding of the repercussions in other circumstances, outside of your examples.

I'd actually be inclined to say that adding tenant columns and populating them is exactly what you should do. It may be painful, but it only needs to be done once, and it makes a huge improvement to data integrity, Landlord or no.

Thoughts?

[NineToeNerd]

In your example I would use Grade::has('student')->all(); to get all grades in that Class (provided everything else is set up correctly). That's what I'm using as a workaround for a similar situation.

[gausam]

Respected your recommendation @HipsterJazzbo, and finished the project well a few months ago 👍