Merge lp:~salgado/offspring/lexbuilder-without-current-job-is-public into lp:offspring

[Guilherme Salgado]

Make sure Lexbuilders with no current_job are considered public

[lp:~salgado/offspring/lexbuilder-without-current-job-is-public]

113. By Guilherme Salgado on 2012-01-11

merge sidebars branch

[Kevin McDermott (bigkevmcd)]

Looks good

+ builder = factory.make_lexbuilder()
+ builder.current_job = None
+ builder.save()

this could be tidied up with kwargs on the make_lexbuilder to pass in current_job ?

review: Approve

[Guilherme Salgado (salgado)]

On 12/01/12 09:21, Kevin McDermott wrote:
> Review: Approve
>
> Looks good
>
> + builder = factory.make_lexbuilder()
> + builder.current_job = None
> + builder.save()
>
> this could be tidied up with kwargs on the make_lexbuilder to pass in current_job ?

make_lexbuilder() already takes a current_job argument but when it's
None it creates an arbitrary one. A bunch of tests rely on that so I
think it's easier to just keep this one as is, maybe with a comment like

# Must manually set current_job to None because the above call will
# create an arbitrary BuildResult and assign it to the newly created
# builder's current_job.
builder.current_job = None

[Kevin McDermott (bigkevmcd)]

Oh aye, fine with me :-)

=== modified file 'lib/offspring/web/queuemanager/models.py'
--- lib/offspring/web/queuemanager/models.py	2012-01-11 18:22:25 +0000
+++ lib/offspring/web/queuemanager/models.py	2012-01-11 18:22:25 +0000
@@ -171,11 +171,18 @@
         return privacy_attr
 
     def _get_access_controlled_object(self):
+        """Return the object associated to this one that is access controlled.
+
+        In some cases the relationship between this model and the one which is
+        access controlled is NULLable (e.g. Lexbuilder.current_job), and if
+        that's the case and there's no access-controlled object linked to this
+        one, we return None.
+        """
         if getattr(self, 'access_relation', None) is None:
             return self
         names = self.access_relation.split('__')
         obj = self
-        while len(names):
+        while len(names) and obj is not None:
             name = names.pop(0)
             obj = getattr(obj, name)
         return obj
@@ -183,11 +190,15 @@
     @property
     def is_private(self):
         obj = self._get_access_controlled_object()
+        if obj is None:
+            return False
         return getattr(obj, self._privacy_attr_name)
 
     def is_visible_to(self, user):
-        is_visible_to = getattr(self._get_access_controlled_object(),
-                                self._is_visible_method_name)
+        obj = self._get_access_controlled_object()
+        if obj is None:
+            return True
+        is_visible_to = getattr(obj, self._is_visible_method_name)
         return is_visible_to(user)
 
 

=== modified file 'lib/offspring/web/queuemanager/tests/test_models.py'
--- lib/offspring/web/queuemanager/tests/test_models.py	2012-01-11 18:22:25 +0000
+++ lib/offspring/web/queuemanager/tests/test_models.py	2012-01-11 18:22:25 +0000
@@ -309,6 +309,20 @@
         return factory.make_lexbuilder(
             current_job=factory.make_build_result(project=project))
 
+    def test_lexbuilder_without_current_job(self):
+        """
+        A Lexbuilder with no current_job is not considered private and thus
+        is visible to anyone.
+        """
+        builder = factory.make_lexbuilder()
+        builder.current_job = None
+        builder.save()
+        user = factory.make_user()
+        self.assertEqual(False, builder.is_private)
+        self.assertEqual(True, builder.is_visible_to(user))
+        self.assertEqual(
+            [builder], list(self.model.all_objects.accessible_by_user(user)))
+
 
 class BuildResultTests(
         TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):