From b3d272de9d84078627bbc856995a0d41e783b766 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Tue, 15 Dec 2020 10:53:27 +0100
Subject: [PATCH] Suggestions

---
 arkindex/project/mixins.py | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/arkindex/project/mixins.py b/arkindex/project/mixins.py
index 84c3562bc2..2ec9f0a1d4 100644
--- a/arkindex/project/mixins.py
+++ b/arkindex/project/mixins.py
@@ -1,6 +1,7 @@
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
 from django.db.models import IntegerField, Q, Value, functions
+from django.db.models.query_utils import DeferredAttribute
 from django.shortcuts import get_object_or_404
 from django.views.decorators.cache import cache_page
 from rest_framework.exceptions import APIException, ValidationError
@@ -28,6 +29,14 @@ class ACLMixin(object):
     def user(self):
         return self._user or self.request.user
 
+    def _check_level(self, level):
+        assert type(level) is int, 'An integer level is required to compare access rights.'
+        assert level >= 1, 'Level integer should be greater than or equal to 1.'
+        assert level <= 100, 'level integer should be lower than or equal to 100'
+
+    def _has_public_field(self, model):
+        return type(getattr(model, 'public', None)) is DeferredAttribute
+
     def get_public_instances(self, model, default_level):
         return model.objects \
             .filter(public=True) \
@@ -37,14 +46,16 @@ class ACLMixin(object):
         """
         Return a model queryset matching a given access level for this user.
         """
+        self._check_level(level)
+        include_public = level <= Role.Guest.value and self._has_public_field(model)
+
         # Handle specific cases (i.e. admin or anonymous user)
         if self.user.is_admin or self.user.is_internal:
             return model.objects.all().annotate(max_level=Value(Role.Admin.value))
         elif self.user.is_anonymous:
-            if not public:
+            if not include_public:
                 return model.objects.none()
-            else:
-                return self.get_public_instances(model, Role.Guest.value)
+            return self.get_public_instances(model, Role.Guest.value)
 
         # Filter users rights and annotate the resulting level for those rights
         queryset = model.objects \
@@ -65,13 +76,15 @@ class ACLMixin(object):
         queryset = queryset.filter(max_level__gte=level)
 
         # Use a join to add public instances as this is the more elegant solution
-        if public and level <= Role.Guest.value:
+        if include_public:
             queryset = queryset.union(self.get_public_instances(model, Role.Guest.value))
 
         # Return distinct corpus with the max right level among matching rights
         return queryset.order_by('id', '-max_level').distinct('id')
 
     def has_access(self, instance, level):
+        self._check_level(level)
+
         if self.user.is_admin or self.user.is_internal:
             return True
         return instance.memberships.filter(
@@ -113,7 +126,7 @@ class NewCorpusACLMixin(ACLMixin):
 
     @property
     def readable_corpora(self):
-        return self.rights_filter(Corpus, Role.Guest.value, public=True)
+        return self.rights_filter(Corpus, Role.Guest.value)
 
     @property
     def writable_corpora(self):
-- 
GitLab