Merge lp:~salgado/django-group-access/refactor into lp:django-group-access

Proposed by Guilherme Salgado
Status: Merged
Merged at revision: 4
Proposed branch: lp:~salgado/django-group-access/refactor
Merge into: lp:django-group-access
Diff against target: 50 lines (+16/-14)
1 file modified
models.py (+16/-14)
To merge this branch: bzr merge lp:~salgado/django-group-access/refactor
Reviewer Review Type Date Requested Status
Nicola Heald Pending
Review via email: mp+76075@code.launchpad.net

Description of the change

This one refactors accessible_by_user() so that subclasses can add new filters.

This way I can write a custom AccessManager which lets anyone see non-private objects. Something like

class CustomObjectsManager(AccessManager):

    def _get_accessible_by_user_filter_rules(self, user):
        rules = super(
            CustomObjectsManager, self)._get_accessible_by_user_filter_rules(
                user)
        return rules | models.Q(is_private=False)

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'models.py'
2--- models.py 2011-09-19 15:04:00 +0000
3+++ models.py 2011-09-19 17:22:26 +0000
4@@ -8,9 +8,7 @@
5 def get_for_owner(self, user):
6 return self.filter(owner=user)
7
8- def accessible_by_user(self, user):
9- if AccessGroup.objects.filter(members=user, supergroup=True).count():
10- return self.all()
11+ def _get_accessible_by_user_filter_rules(self, user):
12 if hasattr(self.model, 'access_relation'):
13 acr = getattr(self.model, 'access_relation')
14 k = '%s__access_groups__in' % acr
15@@ -19,20 +17,24 @@
16 no_related_records = {k: True}
17 k = '%s__owner' % acr
18 direct_owner_dict = {k: user}
19- available = self.filter(
20+ return (
21 models.Q(**access_groups_dict) |
22 models.Q(**direct_owner_dict) |
23- models.Q(**no_related_records)).distinct()
24- # Although this extra .filter() call seems redundant it turns out
25- # to be a huge performance optimization. Without it the ORM will
26- # join on the related tables and .distinct() them, which killed
27- # performance in HEXR leading to 30+ seconds to load a page.
28- return self.filter(pk__in=available)
29+ models.Q(**no_related_records))
30 else:
31- available = self.filter(
32- access_groups__in=AccessGroup.objects.filter(members=user))
33- return self.filter(
34- models.Q(pk__in=available) | models.Q(owner=user))
35+ user_groups = AccessGroup.objects.filter(members=user)
36+ return (models.Q(access_groups__in=user_groups) |
37+ models.Q(owner=user))
38+
39+ def accessible_by_user(self, user):
40+ if AccessGroup.objects.filter(members=user, supergroup=True).count():
41+ return self.all()
42+ rules = self._get_accessible_by_user_filter_rules(user)
43+ # Although this extra .filter() call seems redundant it turns out
44+ # to be a huge performance optimization. Without it the ORM will
45+ # join on the related tables and .distinct() them, which killed
46+ # performance in HEXR leading to 30+ seconds to load a page.
47+ return self.filter(pk__in=self.filter(rules).distinct())
48
49
50 class AccessGroup(models.Model):

Subscribers

People subscribed via source and target branches