Merge lp:~salgado/offspring/more-piston-handlers into lp:~linaro-automation/offspring/private-builds

[Guilherme Salgado]

This one updates BuildRequestHandler to include private objects the user is allowed to see.

[James Tunnicliffe (dooferlad)]

Looks good.

[James Tunnicliffe (dooferlad)]

Last comment wasn't an approve one. Who knows if I need to do this...

review: Approve

=== modified file 'lib/offspring/web/queuemanager/handlers.py'
--- lib/offspring/web/queuemanager/handlers.py	2011-10-13 23:23:41 +0000
+++ lib/offspring/web/queuemanager/handlers.py	2011-10-17 17:30:29 +0000
@@ -154,7 +154,6 @@
         return results
 
 
-# TODO: Must be made privacy-aware.
 class BuildRequestHandler(BaseHandler):
     allowed_methods = ('GET', 'POST', 'DELETE')
     model = BuildRequest
@@ -162,10 +161,10 @@
 
     def read(self, request, projectName=None, request_id=None):
         try:
+            requests = BuildRequest.all_objects.accessible_by_user(
+                request.user)
             if projectName is not None:
-                requests = BuildRequest.objects.filter(project__name=projectName)
-            else:
-                requests = BuildRequest.objects
+                requests = requests.filter(project__name=projectName)
         except:
             return rc.NOT_FOUND
         if request_id is not None:
@@ -177,12 +176,12 @@
     def delete(self, request, projectName=None, request_id=None):
         if request.user.has_perm("queuemanager.can_build"):
             try:
+                requests = BuildRequest.all_objects.accessible_by_user(
+                    request.user)
                 if projectName is not None:
-                    requests = BuildRequest.objects.filter(project__name=projectName)
-                else:
-                    requests = BuildRequest.objects
+                    requests = requests.filter(project__name=projectName)
             except:
-                return RC.NOT_FOUND
+                return rc.NOT_FOUND
             if request_id is not None:
                 try:
                     requests.get(pk=request_id).delete()
@@ -202,7 +201,8 @@
     def create(self, request, projectName):
         if request.user.has_perm("queuemanager.can_build"):
             try:
-                p = Project.objects.get(pk=projectName)
+                p = Project.all_objects.accessible_by_user(request.user).get(
+                    pk=projectName)
                 if request.POST.has_key('build_reason'):
                     request = BuildRequest.queue_build(p, request.user, reason=request.POST['build_reason'], scoreBonus=25)
                 else:

=== added file 'lib/offspring/web/queuemanager/tests/helpers.py'
--- lib/offspring/web/queuemanager/tests/helpers.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/tests/helpers.py	2011-10-17 17:30:29 +0000
@@ -0,0 +1,14 @@
+from django.contrib.auth.models import (
+    Group,
+    Permission,
+    )
+
+
+def grant_permission_to_user(user, permission):
+    group = Group(name=user.username)
+    group.save()
+    group.permissions.add(Permission.objects.get(codename=permission))
+    group.save()
+    user.groups.add(group)
+    user.save()
+    return user

=== modified file 'lib/offspring/web/queuemanager/tests/test_handlers.py'
--- lib/offspring/web/queuemanager/tests/test_handlers.py	2011-10-13 23:16:02 +0000
+++ lib/offspring/web/queuemanager/tests/test_handlers.py	2011-10-17 17:30:29 +0000
@@ -6,13 +6,18 @@
 from django.test import TestCase
 from django.contrib.sessions.middleware import SessionMiddleware
 
+from piston.utils import rc
+
+from offspring.web.queuemanager.models import BuildRequest
 from offspring.web.queuemanager.handlers import (
     BuildManifestComparisonHandler,
     BuildManifestHandler,
+    BuildRequestHandler,
     BuildResultHandler,
     ReleaseHandler,
     )
 from offspring.web.queuemanager.tests.factory import factory
+from offspring.web.queuemanager.tests.helpers import grant_permission_to_user
 
 
 class BaseHandlerTestCase(TestCase):
@@ -97,6 +102,88 @@
         self.assertEqual(0, results.count())
 
 
+class BuildRequestHandlerTests(BaseHandlerTestCase):
+
+    def test_public_build_request_is_visible_to_anyone(self):
+        job = factory.makeBuildRequest(
+            project=factory.makeProject(is_private=False))
+        results = BuildRequestHandler().read(
+            self.request, projectName=job.project.name, request_id=job.id)
+        self.assertEqual(job, results)
+
+    def test_private_build_request_is_visible_to_owner(self):
+        job = factory.makeBuildRequest(
+            project=factory.makeProject(is_private=True))
+        self._login(job.project.owner)
+        results = BuildRequestHandler().read(
+            self.request, projectName=job.project.name, request_id=job.id)
+        self.assertEqual(job, results)
+
+    def test_private_build_request_is_not_visible_to_others(self):
+        job = factory.makeBuildRequest(
+            project=factory.makeProject(is_private=True))
+        self._login(factory.makeUser())
+        self.assertRaises(
+            BuildRequest.DoesNotExist,
+            BuildRequestHandler().read,
+            self.request, projectName=job.project.name, request_id=job.id)
+
+    def test_new_build_request_on_public_project(self):
+        project = factory.makeProject(is_private=False)
+        self.assertCreateReturnCode(
+            project, factory.makeUser(), rc.CREATED.status_code)
+
+    def test_owner_requests_new_build_on_private_project(self):
+        project = factory.makeProject(is_private=True)
+        self.assertCreateReturnCode(
+            project, project.owner, rc.CREATED.status_code)
+
+    def test_other_user_requests_new_build_on_private_project(self):
+        project = factory.makeProject(is_private=True)
+        self.assertCreateReturnCode(
+            project, factory.makeUser(), rc.BAD_REQUEST.status_code)
+
+    def test_delete_build_request_on_public_project(self):
+        job = factory.makeBuildRequest(
+            project=factory.makeProject(is_private=False))
+        self.assertDeleteReturnCode(
+            job, factory.makeUser(), rc.DELETED.status_code)
+
+    def test_owner_deletes_build_request_on_private_project(self):
+        job = factory.makeBuildRequest(
+            project=factory.makeProject(is_private=True))
+        self.assertDeleteReturnCode(
+            job, job.project.owner, rc.DELETED.status_code)
+
+    def test_other_user_deletes_build_request_on_private_project(self):
+        job = factory.makeBuildRequest(
+            project=factory.makeProject(is_private=True))
+        self.assertDeleteReturnCode(
+            job, factory.makeUser(), rc.NOT_FOUND.status_code)
+
+    def assertCreateReturnCode(self, project, user, return_code):
+        """Assert that BuildRequestHandler.create() returns the given code.
+
+        Also grant the given user the 'can_build' permission and logs it in.
+        """
+        grant_permission_to_user(user, 'can_build')
+        self._login(user)
+        result = BuildRequestHandler().create(
+            self.request, projectName=project.name)
+        self.assertEqual(return_code, result.status_code)
+
+    def assertDeleteReturnCode(self, job, user, return_code):
+        """Assert that BuildRequestHandler.delete() returns the given code.
+
+        Also grant the given user the 'can_build' permission and logs it in.
+        """
+        grant_permission_to_user(user, 'can_build')
+        self._login(user)
+        result = BuildRequestHandler().delete(
+            self.request, projectName=job.project.name, request_id=job.id)
+        self.assertEqual(return_code, result.status_code)
+
+
 class ReleaseHandlerTests(BaseHandlerTestCase):
 
     def test_public_project_release(self):

=== modified file 'lib/offspring/web/queuemanager/tests/test_views.py'
--- lib/offspring/web/queuemanager/tests/test_views.py	2011-10-10 21:15:48 +0000
+++ lib/offspring/web/queuemanager/tests/test_views.py	2011-10-17 17:30:29 +0000
@@ -1,9 +1,5 @@
 from django.core.urlresolvers import reverse
-from django.contrib.auth.models import (
-    AnonymousUser,
-    Group,
-    Permission,
-    )
+from django.contrib.auth.models import AnonymousUser
 from django.http import Http404
 from django.test import TestCase
 
@@ -13,6 +9,7 @@
     )
 from offspring.web.queuemanager.views import get_possibly_private_object
 from offspring.web.queuemanager.tests.factory import factory
+from offspring.web.queuemanager.tests.helpers import grant_permission_to_user
 
 
 class Test_get_possibly_private_object(TestCase):
@@ -309,14 +306,3 @@
 def make_user_and_login(client):
     user = factory.makeUser()
     return client.login(username=user.username, password=user.username)
-
-
-def grant_permission_to_user(user, permission):
-    group = Group(name=user.username)
-    group.save()
-    group.permissions.add(Permission.objects.get(codename=permission))
-    group.save()
-    user.groups.add(group)
-    user.save()
-    return user
-