Skip to content

Conversation

miguelmarcondesf
Copy link
Contributor

feature: Easy associations between tenanted and untenanted models

enhance active record associations to detect if they're between a tenanted model and an untenanted model, and in that case do the extra work to make sure the tenant is respected.

Checklist:

  • has_one
  • has_many
  • belongs_to
  • Fixtures double check

@flavorjones
Copy link
Member

Hi! Thanks for opening this draft. I'll dig in and review as soon as I can find time!

@flavorjones
Copy link
Member

Sorry, it's been a busy week. I will definitely get to this soon!

Copy link
Member

@flavorjones flavorjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right direction. I know in the original description I suggested something like .where(tenant_id: tenant) but that was meant to be illustrative.

The general problem that I personally need to solve is for a model where the tenant ID is not in the database.

@flavorjones
Copy link
Member

Hmm. Please ignore my review for now, I'm still wrapping my head around the problem and this approach.

Copy link
Member

@flavorjones flavorjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good start on a very complex feature. I'm not at all sure how belongs_to might be implemented, but this seems like a promising direction for the has_one and has_many side of the association.


class_methods do
# I think we can have more configs later
def cross_tenant_config(**config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I want to have a general cross-tenant config on a model. I think it would be better to have the config per-association.

For example, if we want to send messages between users we might do something like:

class Message < UntenantedRecord
  belongs_to :sender, class_name: "User", foreign_key: "sender_id"
  belongs_to :recipient, class_name: "User", foreign_key: "recipient_id"
end

class User < TenantedRecord
  has_many :sent_messages, class_name: "Message", inverse_of: :sender, foreign_key: "sender_id"
  has_many :received_messages, class_name: "Message", inverse_of: :recipient, foreign_key: "recipient_id"
end

In this case, I would want separate tenant columns for the 'sender' and 'recipient' associations.

And so it might make more sense for this to be customized on the association like:

belongs_to :sender, class_name: "User",
                    foreign_key: "sender_id",
                    tenant_key: "sender_tenant"

and

  has_many :sent_messages,
           class_name: "Message",
           inverse_of: :sender,
           foreign_key: "sender_id",
           tenant_key: "sender_tenant"

test "has_one automatically scopes by tenant_id" do
TenantedApplicationRecord.create_tenant("foo") do
user = User.create!(email: "[email protected]")
Announcement.create!(message: "Foo announcement", tenant_id: "foo", user: user)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I'd like to be able to do this in an untenanted scope, but it looks like AR does some validation that requires a tenanted connection ...

I'd also like to be able to do this without explicitly setting the tenant id in Announcement, that is:

Announcement.create! user: user

should set the tenant column automatically.

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

Successfully merging this pull request may close these issues.

2 participants