Merge lp:~zyga/django-restricted-resource/fix-is-owned-by-for-group-owners into lp:django-restricted-resource

[Zygmunt Krynicki]

Fix resource.is_owned_by(user) when user belongs to resource.group owner.

[Michael Hudson-Doyle (mwhudson)]

Looks fine, but I find the comments a little confusing.

+ # If the principal is an User then this object is owned by that user or
+ # the group the user belongs to.

I think if this was phrased as a question it would be more clear: "is this object owned by that user or a group that the user belongs to?" Similarly for the next comment down.

review: Approve

[Paul Larson (pwlars)]

review: Approve

=== modified file 'django_restricted_resource/models.py'
--- django_restricted_resource/models.py	2011-06-23 09:59:46 +0000
+++ django_restricted_resource/models.py	2011-09-09 17:39:26 +0000
@@ -117,10 +117,22 @@
         """
         if principal is None:
             return False
-        if not isinstance(principal,
-                          (User, Group, AnonymousUser, type(None))):
+        # If the principal is an User then this object is owned by that user or
+        # the group the user belongs to.
+        if isinstance(principal, (User, AnonymousUser, type(None))):
+            user = filter_bogus_users(principal)
+            if user is None:
+                return False
+            if self.user is not None:
+                return self.user == user
+            else:
+                return self.group in user.groups.all()
+        # If the principal is a Group then this object is owned by that group
+        elif isinstance(principal, Group):
+            group = principal
+            return self.group == group
+        else:
             raise TypeError("Expected User or Group instance as argument")
-        return self.user == principal or self.group == principal
 
     def get_access_type(self, principal):
         """

=== modified file 'django_restricted_resource/tests.py'
--- django_restricted_resource/tests.py	2011-05-23 16:41:17 +0000
+++ django_restricted_resource/tests.py	2011-09-09 17:39:26 +0000
@@ -165,6 +165,19 @@
         self.assertTrue(resource.is_owned_by(self.owner))
 
 
+class GroupMemberOwnsResource(FixtureHelper):
+
+    def test(self):
+        """
+        RestrictedResource.is_owned_by() returns True for owning group members
+        """
+        group = self.getUniqueGroup()
+        user = self.getUniqueUser()
+        user.groups.add(group)
+        resource = self.getUniqueResource(owner=group)
+        self.assertTrue(resource.is_owned_by(user))
+
+
 class ResourceManagerOwnerSetFindsNoMatchesForOthers(
     FixtureHelper, TestCaseWithInvariants):
     """