Revise view permissions

Detail view now accessible to any authenticated user.
Delete and TogglePrimary views now accessible only to owner.

Resolves #12

.coveragerc

@@ -1,7 +1,8 @@
 [run]
-source = pinax
-omit = pinax/images/tests/*,pinax/images/admin.py
+source = pinax/images
+omit = pinax/images/*/tests, pinax/images/admin.py
 branch = 1
 
 [report]
-omit = pinax/images/tests/*,pinax/images/admin.py
+omit = pinax/images/*/tests, pinax/images/admin.py
+

docs/changelog.md

@@ -1,5 +1,17 @@
 # Change Log
 
+## 2.0.0
+
+* Revise access permissions for some views:
+
+  * ImageSet detail view now accessible by any authenticated user
+  * Image delete view now accessible only by image owner.
+  * Image "toggle primary" view now accessible only by image owner.
+
+## 1.0.0
+
+* Update version for Pinax 16.04 release
+
 ## 0.2.1
 
 * Improve documentation

pinax/images/tests/tests.py

@@ -1,14 +1,33 @@
+import mock
+
+from django.core.files import File
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.core.urlresolvers import reverse
 
 from .test import TestCase
+from ..models import Image
 
 
-class Tests(TestCase):
+class ImageSetUploadView(TestCase):
 
     def setUp(self):
         self.user = self.make_user("arthur")
+        self.image_file = SimpleUploadedFile(
+            name='foo.gif',
+            content=b'GIF87a\x01\x00\x01\x00\x80\x01\x00\x00\x00\x00ccc,\x00\x00\x00\x00\x01\x00\x01\x00\x00\x02\x02D\x01\x00'
+        )
+
+        self.mock_file = mock.Mock(spec=File)
+        self.mock_file.read.return_value = "fake file contents"
+
+    def test_upload_by_anonymous(self):
+        """
+        Ensure anonymous user is redirected
+        """
+        self.get("pinax_images:imageset_new_upload")
+        self.response_302()  # user should be redirected
 
-    def test_get_imageset_upload(self):
+    def test_upload_get(self):
         """
         Ensure GET action returns error code
         """
@@ -17,10 +36,179 @@ def test_get_imageset_upload(self):
             self.get(path)
             self.response_405()
 
-    def test_get_imageset_detail(self):
+    def test_upload_new_imageset(self):
+        """
+        Upload image and store in new ImageSet.
+        """
+        url = "pinax_images:imageset_new_upload"
+        post_data = {"files": self.image_file}
+        with self.login(self.user):
+            self.post(url, data=post_data)
+            self.response_200()
+            self.assertEqual(self.user.image_sets.count(), 1)
+            self.assertEqual(self.user.images.count(), 1)
+            self.assertEqual(self.user.images.get().image_set, self.user.image_sets.get())
+
+    def test_upload_image(self):
+        """
+        Upload image to existing ImageSet.
+        """
+        image_set = self.user.image_sets.create()
+        post_data = {"files": self.image_file}
+        url = "pinax_images:imageset_upload"
+        with self.login(self.user):
+            self.post(url, image_set.pk, data=post_data)
+            self.response_200()
+            self.assertEqual(self.user.images.count(), 1)
+            self.assertEqual(self.user.images.get().image_set, image_set)
+
+
+class ImageSetMixin(object):
+
+    def setUp(self):
+        self.user = self.make_user("arthur")
+        # create imageset for Arthur
+        self.image_set = self.user.image_sets.create()
+        # create image in imageset
+        self.image = Image.objects.create(
+            image_set=self.image_set,
+            image=SimpleUploadedFile(
+                name='foo.gif',
+                content=b'GIF87a\x01\x00\x01\x00\x80\x01\x00\x00\x00\x00ccc,\x00\x00\x00\x00\x01\x00\x01\x00\x00\x02\x02D\x01\x00'
+            ),
+            original_filename="foo.gif",
+            created_by=self.user
+        )
+
+
+class ImageSetDetailView(ImageSetMixin, TestCase):
+
+    def setUp(self):
+        super(ImageSetDetailView, self).setUp()
+        self.view_url = "pinax_images:imageset_detail"
+
+    def test_detail_by_anonymous(self):
+        """
+        Ensure anonymous user is redirected
+        """
+        self.get(self.view_url, self.image_set.pk)
+        self.response_302()  # user should be redirected
+
+    def test_detail_by_owner(self):
+        """
+        Ensure imageset owner can see image
+        """
+        with self.login(self.user):
+            self.assertGoodView(self.view_url, self.image_set.pk)
+
+    def test_detail_by_other(self):
+        """
+        Ensure any authenticated user can see image
+        """
+        other_user = self.make_user("other")
+        with self.login(other_user):
+            self.assertGoodView(self.view_url, self.image_set.pk)
+
+    def test_detail_bad_pk(self):
+        """
+        Ensure 404 error with bad imageset PK
+        """
+        with self.login(self.user):
+            self.get(self.view_url, pk=555)
+            self.response_404()
+
+
+class ImageDeleteView(ImageSetMixin, TestCase):
+
+    def setUp(self):
+        super(ImageDeleteView, self).setUp()
+        self.view_url = "pinax_images:image_delete"
+
+    def test_delete_by_anonymous(self):
+        """
+        Ensure anonymous user is redirected
+        """
+        self.post(self.view_url, self.image.pk)
+        self.response_302()  # user should be redirected
+
+    def test_delete_by_owner(self):
+        """
+        Ensure imageset owner can delete image
+        """
+        with self.login(self.user):
+            self.post(self.view_url, self.image.pk)
+            self.response_200()
+            # Ensure image is not available
+            self.assertFalse(Image.objects.filter(pk=self.image.pk))
+
+    def test_delete_by_other(self):
+        """
+        Ensure non-owner cannot delete image
+        """
+        other_user = self.make_user("other")
+        with self.login(other_user):
+            self.post(self.view_url, self.image.pk)
+            self.response_404()
+
+    def test_delete_bad_pk(self):
+        """
+        Ensure POST with invalid ImageSet PK fails.
+        """
+        with self.login(self.user):
+            self.post(self.view_url, 555)
+            self.response_404()
+
+
+class ImageTogglePrimaryView(ImageSetMixin, TestCase):
+
+    def setUp(self):
+        super(ImageTogglePrimaryView, self).setUp()
+        self.view_url = "pinax_images:image_make_primary"
+
+    def test_toggle_by_anonymous(self):
+        """
+        Ensure anonymous user is redirected
+        """
+        self.post(self.view_url, self.image.pk)
+        self.response_302()  # user should be redirected
+
+    def test_toggle_by_owner(self):
+        """
+        Ensure imageset owner can toggle primary image
+        """
+        # Ensure image_set does not have a primary image
+        self.assertEqual(self.image_set.primary_image, None)
+
+        with self.login(self.user):
+            # Set primary image
+            self.post(self.view_url, self.image.pk)
+            self.response_200()
+
+            # Ensure image is now primary image for image_set
+            self.image_set.refresh_from_db()
+            self.assertEqual(self.image_set.primary_image, self.image)
+
+            # Reset primary image
+            self.post(self.view_url, self.image.pk)
+            self.response_200()
+
+            # Ensure image is no longer primary image for image_set
+            self.image_set.refresh_from_db()
+            self.assertEqual(self.image_set.primary_image, None)
+
+    def test_toggle_by_other(self):
+        """
+        Ensure non-owner cannot toggle primary image
+        """
+        other_user = self.make_user("other")
+        with self.login(other_user):
+            self.post(self.view_url, self.image.pk)
+            self.response_404()
+
+    def test_toggle_bad_pk(self):
         """
-        Ensure GET with invalid ImageSet PK fails.
+        Ensure POST with invalid Image PK fails.
         """
         with self.login(self.user):
-            self.get("pinax_images:imageset_detail", pk=555)
+            self.post(self.view_url, 555)
             self.response_404()

pinax/images/views.py

@@ -8,57 +8,82 @@
 
 
 class ImageSetUploadView(LoginRequiredMixin, View):
-    image_set = None
+    """
+    Add one or more images to an ImageSet owned by current user.
+    """
+    model = ImageSet
+
+    def get_queryset(self):
+        """
+        Return QuerySet of all ImageSets related to user.
+        """
+        return self.request.user.image_sets.all()
 
     def get_image_set(self):
+        """
+        Obtain existing ImageSet if `pk` is specified, otherwise
+        create a new ImageSet for the user.
+        """
         image_set_pk = self.kwargs.get("pk", None)
         if image_set_pk is None:
             return self.request.user.image_sets.create()
-        return get_object_or_404(self.request.user.image_sets.all(), pk=image_set_pk)
+        return get_object_or_404(self.get_queryset(), pk=image_set_pk)
 
     def post(self, request, *args, **kwargs):
-        self.image_set = self.get_image_set()
+        image_set = self.get_image_set()
         for file in request.FILES.getlist("files"):
-            self.image_set.images.create(
+            image_set.images.create(
                 image=file,
                 original_filename=file.name,
                 created_by=request.user
             )
-        return JsonResponse(self.image_set.image_data())
+        return JsonResponse(image_set.image_data())
 
 
 class ImageSetDetailView(LoginRequiredMixin, SingleObjectMixin, View):
+    """
+    Show ImageSet information.
+    """
     model = ImageSet
 
-    def get_object(self, queryset=None):
-        return get_object_or_404(
-            self.request.user.image_sets.all(),
-            pk=self.kwargs.get(self.pk_url_kwarg)
-        )
-
     def get(self, request, *args, **kwargs):
         self.object = self.get_object()
         return JsonResponse(self.object.image_data())
 
 
-class ImageDeleteView(LoginRequiredMixin, SingleObjectMixin, View):
+class UserImageMixin(LoginRequiredMixin):
+    """
+    Constrains user to her own Images.
+    """
+    def get_queryset(self):
+        """
+        Return QuerySet of all Images related to user.
+        """
+        return self.request.user.images.all()
+
+
+class ImageDeleteView(UserImageMixin, SingleObjectMixin, View):
+    """
+    Delete the specified image.
+    """
     model = Image
 
     def post(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        self.object.delete()
-        return JsonResponse(self.object.image_set.image_data())
+        image = self.get_object()
+        image_set = image.image_set  # save for later reference
+        image.delete()
+        return JsonResponse(image_set.image_data())
 
 
-class ImageTogglePrimaryView(LoginRequiredMixin, SingleObjectMixin, View):
+class ImageTogglePrimaryView(UserImageMixin, SingleObjectMixin, View):
     """
-    Make the specified image "primary" for the ImageSet if not already,
-    or reset ImageSet primary image to None if specified image is currently
-    set as the primary image.
+    Make the specified image "primary" for it's ImageSet if not already set.
+    Reset the related ImageSet primary image to None if specified image
+    is currently set as primary.
     """
     model = Image
 
     def post(self, request, *args, **kwargs):
-        self.object = self.get_object()
-        self.object.toggle_primary()
-        return JsonResponse(self.object.image_set.image_data())
+        image = self.get_object()
+        image.toggle_primary()
+        return JsonResponse(image.image_set.image_data())

setup.py

@@ -16,7 +16,7 @@ def read(*parts):
     description="an app for managing collections of images associated with a content object",
     name="pinax-images",
     long_description=read("README.md"),
-    version="1.0.0",
+    version="2.0.0",
     url="http://github.com/pinax/pinax-images/",
     license="MIT",
     packages=find_packages(),
@@ -26,13 +26,14 @@ def read(*parts):
     test_suite="runtests.runtests",
     tests_require=[
         "django-test-plus>=1.0.11",
+        "mock>=2.0.0",
     ],
     install_requires=[
         "django-appconf>=1.0.1",
         "django-imagekit>=3.2.7",
         "pilkit>=1.1.13",
-        "pillow>=3.1.1",
-        "pytz>=2015.6",
+        "pillow>=3.3.0",
+        "pytz>=2016.6.1",
     ],
     classifiers=[
         "Development Status :: 4 - Beta",