citusdata / django-multitenant

Python/Django support for distributed multi-tenant databases like Postgres+Citus
MIT License
730 stars 118 forks source link

ManyToMany field between the TenantModel class and a Non Tenant model crashes on .add() #163

Open KrYpTeD974 opened 1 year ago

KrYpTeD974 commented 1 year ago

It seems that having a ManyToMany field between the TenantModel class and a Non Tenant model results in a crash when trying to add items in the collection. This easily reproductible from a fresh Django install:

models.py

from django.contrib.auth.models import User
from django.db import models
from django_multitenant.models import TenantModel

# Create your models here.
class Tenant(TenantModel):
    name = models.CharField(max_length=50)
    users = models.ManyToManyField(User,
                                   related_name="tenants")

    class TenantMeta:
        tenant_field_name = "id"

test.py

from django.contrib.auth.models import User
from django.test import TestCase
from django_multitenant.utils import set_current_tenant

from multitenancy.models import Tenant

# Create your tests here.

class TestTenant(TestCase):
    def test_tenant_users(self):
        tenant = Tenant.objects.create(name="tenant")       
        set_current_tenant(tenant)
        user = User.objects.create(username="test", email="test")
        tenant.users.add(user)
        self.assertEqual(user.tenants.first(), tenant)

The error is :

AttributeError: 'Tenant_users' object has no attribute 'tenant_field'

I don't think the link table betweek User and Tenant should have a tenant_field.

denys-chura commented 1 year ago

Same issue, different error:

ValueError: Model_many_to_many_field is not an instance or a subclass of TenantModel or does not inherit from TenantMixin

Looks like this caused due the fact that "through" Model_many_to_many_field table was created behind the scenes by Django, so it is non tenant model.

models.py

class Model(TenantModel):
    ...
    many_to_many_field = models.ManyToManyField("OtherModel")
tuannguyen-groove commented 1 year ago

Same here, have to downgrade to 3.0 which is working fine

gurkanindibay commented 1 year ago

Hey @KrYpTeD974 @denys-chura @tuannguyen-groove I fixed the issue. Just waiting for the review. I will publish a new release after approval PR is https://github.com/citusdata/django-multitenant/pull/165 Thanks for using the extension and reporting the issues

KrYpTeD974 commented 1 year ago

Thank you for fixing this so quickly ! I have tested the PR on the +1600 tests of my project, and it seems to work great !

gurkanindibay commented 1 year ago

Thanks @KrYpTeD974 for your quick response and tests. With your support, I hope we will make django_multitenant more stable