fix(multitenancy.mdx): role should be on user not organization

[johncantrell97]

I was reading through the multitenancy docs and I'm pretty sure the example schema definition has the global role defined in the wrong place. Seems like it belongs on the User and not the Organization to be consistent with the rest of the doc. It also makes more sense to me that the User and their Membership have roles and not the Organization itself.

Only other nit is I prefer to call "GlobalRole" "UserRole" since it's on the user and then you end up with User and Member roles. Thoughts?

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/blitzjs/blitzjs-com/CkmgBqVcdd3ZHradM8RQPr15RWy9
✅ Preview: https://blitzjs-com-git-fork-johncantrell97-fix-multitenancy-4e2d9f.vercel.app

[johncantrell97]

The schema also has a unique constraint on organizationId, invitedEmail on the Membership table but then says it will clear invitedEmail once the User is set. Won't this cause a problem with the unique constraint?

Probably better to just not clear it?

In my implementation I also use a Token similar to the forgotPassword Token to handle authentication from the email. I guess you could just send along the invitedEmail in the query param to match on but that's easier to forge/guess

[johncantrell97]

Also noticed the type for roles in session publicData is incorrect. Needs to be union of UserRole and GlobalRole instead of an intersection.

[johncantrell97]

Had a question about this:

const project = await db.project.update({
      where: {
        id,
        // Filter by organizationId
        organizationId: ctx.session.orgId,
      },
      data,
  })

To get this to work I assume we would have to add an index on [id, organizationId] for the Project model in our schema?

[johncantrell97]

In a multitenant setup would it make sense to create a combined authorize resolver instead of writing it all over the place?

Something like:

const authorize = resolver.pipe( resolver.authorize(), setDefaultOrganizationId, enforceSuperAdminIfNotCurrentOrganization)

and then use this in queries?

[JuanM04]

Nice catch, and thank you for all your observations!

Only other nit is I prefer to call "GlobalRole" "UserRole" since it's on the user and then you end up with User and Member roles. Thoughts?

Yeah, it makes more sense. I like UserRole.

The schema also has a unique constraint on organizationId, invitedEmail on the Membership table but then says it will clear invitedEmail once the User is set. Won't this cause a problem with the unique constraint?

It's a weird constraint. Maybe we could change it to @@unique([organizationId, userId, invitedEmail]).

Also noticed the type for roles in session publicData is incorrect. Needs to be union of UserRole and GlobalRole instead of an intersection.

You're totally right. Could you change it?

we would have to add an index on [id, organizationId] for the Project model in our schema?

Yes/no. We should but we don't have to. You can add it if you want, seems like a nice addition.

would it make sense to create a combined authorize resolver instead of writing it all over the place?

It depends of the app. In this case, an example, I think it's not necessary.

[tlindener]

I tried to follow the docs today including the fixes mentioned in this PR, unfortunately I didn't get it to work with type issues when creating sessions. May be it would be useful to have a working example together with the docs explaining what's happening there?

[JuanM04]

I tried to follow the docs today including the fixes mentioned in this PR, unfortunately I didn't get it to work with type issues when creating sessions. May be it would be useful to have a working example together with the docs explaining what's happening there?

Yes, that would be very useful. If you want, you can try to make it :wink:

[tlindener]

I tried to follow the docs today including the fixes mentioned in this PR, unfortunately I didn't get it to work with type issues when creating sessions. May be it would be useful to have a working example together with the docs explaining what's happening there?

Yes, that would be very useful. If you want, you can try to make it 😉

I'm completely new to blitzjs. Love the idea and would love to contribute once I got the hang of it but multi-tenancy is one of the requirements for all my projects so first figuring this out is currently not priority given that I have a working project with multi-tenancy already working :/

[maciekChmura]

Had a question about this:

const project = await db.project.update({
      where: {
        id,
        // Filter by organizationId
        organizationId: ctx.session.orgId,
      },
      data,
  })

To get this to work I assume we would have to add an index on [id, organizationId] for the Project model in our schema?

Hello, Yesterday I was asking about this on Discord. (https://discord.com/channels/802917734999523368/814627789180239893/882027330564943922) @flybayer solved my problem by changing the update to updateMany 😃 I made a PR for it also: https://github.com/blitz-js/blitzjs.com/pull/545