Alternative to RulesModelBase in DRF to manage dependency on this library.

[sergioisidoro]

I'm working on a Django DRF project, and while implementing the rules, I feel like inheriting from RulesModelBase creates a really heavy dependency on this library.

I'm trying to make it so that the rules dependency only affects the views, and not go all the way to the models.

From what I can gather, the views have access to the current user, and the objects. The predicates get both the user and the permission object as parameters, so it feels like it would be possible.

Has anyone dealt with similar issues than this?

https://github.com/dfunckt/django-rules#permissions-in-django-rest-framework

Of course, it also requires you to declare your models as described in Permissions in models.

[sergioisidoro]

After checking AutoPermissionViewSetMixin it seems that its only dependency is get_perm in:

perm = self.get_queryset().model.get_perm(perm_type)

which simply returns return "%s.%s_%s" % (cls._meta.app_label, perm_type, cls._meta.model_name)

It seems this dependency could be easily isolated into a util class.

def get_permission_from_class(cls, perm_type):
   return "%s.%s_%s" % (cls._meta.app_label, perm_type, cls._meta.model_name)

Which would make AutoPermissionViewSetMixin independent of the Mixin, as long as the rules are loaded (or autoloaded) according to the naming convention. Am I missing something? Would this be a reasonable change?

[dfunckt]

Yes, I'd happily accept such a change. However, is it possible to abstract away the whole self.get_queryset().model.get_perm(perm_type) call? I'd rather not hardcode into Rules how perm names are constructed, but provide hooks for clients to override reasonable defaults instead.

[sergioisidoro]

@dfunckt if we define it in AutoPermissionViewSetMixin, the user can override it by creating:

class MyAutoPermissionViewSetMixin(AutoPermissionViewSetMixin):
   def get_permission_form_model(self, class, perm):
       return "custom permission string"

This would be a breaking change, however, so we need to continue supporting the model mixin. I'm wondering is something like this could work:

        model = self.get_queryset().model
        try:
            use_mixin_method = callable(model.get_perm)
        except AttributeError:
            use_mixin_method = False

        if use_mixin_method:
            perm = model.get_perm(perm_type)
        else:
            perm = self.get_permission_form_model(model, perm_type)

And here there's the issue of precedence. For now the model mixin would have precedence to avoid any breaking changes.

[dfunckt]

Sorry, I must be missing something -- why is it a breaking change?

I'll need to look into the code, but from this discussion I understand that if AutoPermissionViewSetMixin was like:

class AutoPermissionViewSetMixin:
  # ...

  def get_permission_form_model(self, class, perm):
    return self.get_queryset().model.get_perm(perm_type)

existing clients would not break. What do I miss?

[sergioisidoro]

That option still keeps the dependency, although yes, it makes it much easier to move around it

My idea was to take it a step further, and by default, remove the dependency of the Views and DRF Mixins from the RulesModelMixin, and make it fully optional. That could break things if people have already made their custom Model get_perm.

Your suggestion already fulfils my biggest issue, so I would be happy with it :)

---

Heres a bit more detail about the idea I had, with some iterations based on our discussion and being backwards compatible:

# All auto permission mixins (both for DRF and Django views) inherit from this mixin
class BaseAutoPermissionMixin:

  def get_perm(self, class, perm_type):
      return "%s.%s_%s" % (class._meta.app_label, perm_type, class._meta.model_name)

  def get_permission_for_model(self, model, perm_type):
        try:
            use_model_mixin_method = callable(model.get_perm)
        except AttributeError:
            use_model_mixin_method = False

        if use_model_mixin_method:
            perm = model.get_perm(perm_type)
        else:
            perm = self.get_perm(model, perm_type)
         return perm

# For DRF
class AutoPermissionViewSetMixin(BaseAutoPermissionMixin):
  # ...

# For views
class AutoPermissionRequiredMixin(PermissionRequiredMixin, BaseAutoPermissionMixin)

[dfunckt]

Okay, I totally see the use-case and agree in principle that this would be great to have but I need to spend some time to check the existing code and your proposal in more depth. If you're keen to PR stuff in the meantime, feel free and we'll take it from there.

[sergioisidoro]

Sure, I'll post a PR for this. Actually maybe I can make 2:

• One PR, which is the simplest, to abstract the get_permission_form_model making it overridable in the mixin
• Another PR based on the first one with these larger changes.

We can move the discussion there as well, since it's easier to discuss things with concrete code :)

Thanks for taking the time and attention on this! Really appreciate it!

[sergioisidoro]

@dfunckt The PRs should be read for a first review.
Let me know if there is anything else you would like me to do or followup regarding this topic :)
Thanks for your help