Version 2.1 change breaks current object permissions

[justinask7]

Good day.

Thank you very much for the library, we're using this in our company for a while! Will try to share our findings and suggestions as we use it.

What we've spotted after the latest release is that while releasing 2.0 version (3582a6f) check for object permissions from ObjectType class was deleted. Currently I can spot the following issues:

1. check_object_permissions function is still there, but as far as I understand it's not used anymore, right? (https://github.com/0soft/graphene-django-plus/blob/master/graphene_django_plus/types.py#L181) This creates a little confusion and even more with the description of this function
2. Was it really necessary to delete this function usage? Because it was quite useful for extra business logic permissions when trying to retrieve the object

What I'd suggest is since you're already checking permissions of a GuardianModel, maybe this function can be left as a placeholder for extra permission logic? As same as before_save and after_save methods from graphene library with a simple pass line?

What are your ideas on this guys? Does that make sense?

[bellini666]

Hi @justinask7 ,

The change was made because, actually, it would never return False (at least not after v1.4). The problem is, based on the fact that GuardedModelManager.for_user should return a queryset that can only contain models that the user can see, the get_node function would already filter any object that doesn't have permissions out, so the object there will be None (as you can see in this NOTE: https://github.com/0soft/graphene-django-plus/blob/master/graphene_django_plus/types.py#L148).

You are saying in point number 2 that it was useful for some extra business logic. What version were you using before upgrading to this one? There was an issue before v1.4 that was making optimized models not use the type's queryset but the base queryset instead (optimizer passes cls._meta.model.objects instead of c.get_queryset, which is oposed to what the default get_node does), which I fixed in this commit.

But having said all that, I can add a call to it back so that one can do some extra checking in it. It will not affect the default implementation because of the reasons I explained in the first paragraph.

Thanks for pointing that out and glad to see other people using this library :)

[bellini666]

Hi @justinask7 , I just released a new version with the change and it should be hitting pypi's repository in the next minutes.

Feel free to open any other issues regarding problems you find or even suggestions for the lib!