jazzband / django-invitations

Generic invitations app for Django
GNU General Public License v3.0
567 stars 166 forks source link

Custom `INVITATION_MODEL` with unique_together fields #203

Open lardissone opened 2 years ago

lardissone commented 2 years ago

I'm working on a multi-tenant app using django-tenants and django-tenant-users, the custom user model is build around this abstract model. Everything works great.

Now I need to implement invitations, but in my case, the invitations should be unique by email+tenant because a user can be invited to the system and to different tenants (I would figure the logic to make this happen in the future). So I tried to create a custom INVITATION_MODEL this way:

users/models.py:

# ...
from tenant_users.tenants.models import UserProfile

class User(UserProfile):
    ROLES = [
        ('ADMIN', 'Administrator'),
        ('SPECIALIST', 'Specialist'),
        ('ASSISTANT', 'Assistant'),
    ]
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    role = models.CharField(max_length=30, choices=ROLES)

    def __str__(self):
        return self.email

class Invitation(AbstractBaseInvitation):
    email = models.EmailField()
    tenant = models.ForeignKey(   # <--- this is the tenant to which the invitation should be tied to
        Tenant,
        on_delete=models.CASCADE,
        editable=False
    )
    created = models.DateTimeField(default=timezone.now)

    class Meta:
        unique_together = ['email', 'tenant']  # <--- unique together

    @classmethod
    def create(cls, email, inviter=None, **kwargs):
        # ...

    def key_expired(self):
        # ...

    def send_invitation(self, request, **kwargs):
        # ...

    def __str__(self):
        return f"Invite: {self.tenant.name} - {self.email}"

Pretty much the same as the default model but just adding the extra tenant field. Added the INVITATION_MODEL to the settings.

I tried manage.py migrate and/or manage.py makemigration but I get this error:

invitations.Invitation.inviter: (fields.E304) Reverse accessor 'User.invitation_set' for 'invitations.Invitation.inviter' clashes with reverse accessor for 'users.Invitation.inviter'.
    HINT: Add or change a related_name argument to the definition for 'invitations.Invitation.inviter' or 'users.Invitation.inviter'.
users.Invitation.inviter: (fields.E304) Reverse accessor 'User.invitation_set' for 'users.Invitation.inviter' clashes with reverse accessor for 'invitations.Invitation.inviter'.
    HINT: Add or change a related_name argument to the definition for 'users.Invitation.inviter' or 'invitations.Invitation.inviter'.

Any idea how can I fix it? Or you know a better approach (or lib) to implement invitations in a multi-tenancy app?

montaro commented 1 year ago

This happened to me because I had messy migrations because of the multiple trials. Drop the DB tables of the previous invitations tables (under your app or the django-invitations app) and it should work later

ruphoff commented 1 year ago

The same thing happened to me! It seems the problem is, that even when you are setting your invitation model via. INVITATIONS_INVITATION_MODEL, invitation.Invitation is NOT excluded from the migration. After adding this modelfield I was able to create the migration file:

 inviter = models.ForeignKey(
        settings.AUTH_USER_MODEL,
        null=True,
        blank=True,
        on_delete=models.CASCADE,
        related_name='+'
 )

But when running manage.py migrate, the invitations default migrations are applied as well. Any help?

nitsujri commented 1 year ago

You can prevent the migrations from being installed by:

https://docs.djangoproject.com/en/dev/ref/settings/#migration-modules

MIGRATION_MODULES = {
    "invitations": None,
}

But the problem is it screws up pytest because it's always trying to do a DB reference with the Invitation model via migration. The only way I've been able to keep it out is use pytest --no-migrations which takes the models as-defined and builds a schema.

In theory this shouldn't make a difference, running through the migrations or simply using the models to build up the DB, but it feels wrong to be forced into this position.

mdrie commented 1 year ago

The best would be to exclude invitations.Invitation when INVITATIONS_INVITATION_MODEL is set to something else, but I do not know, if that is possible.

The next best would be to allow both classes to coexist without overwriting inviter. (Which is a nice work-around. Thanx, @ruphoff!) That could be achieved (as described here: https://docs.djangoproject.com/en/4.2/topics/db/models/#abstract-related-name) with:

class AbstractBaseInvitation(models.Model):
    [...]
    inviter = models.ForeignKey(
        settings.AUTH_USER_MODEL,
        null=True,
        blank=True,
        on_delete=models.CASCADE,
        related_name="%(app_label)s_%(class)ss",
        related_query_name="%(app_label)s_%(class)s",
    [...]