Merge pull request #68 from edx/clintonb/catalog-user-api

Added support for setting Catalog viewers via API

course_discovery/apps/api/serializers.py

+from django.contrib.auth import get_user_model
 from django.utils.translation import ugettext_lazy as _
 from rest_framework import serializers
 
@@ -6,6 +7,8 @@ from course_discovery.apps.course_metadata.models import (
     Course, CourseRun, Image, Organization, Person, Prerequisite, Seat, Subject, Video
 )
 
+User = get_user_model()
+
 
 class TimestampModelSerializer(serializers.ModelSerializer):
     modified = serializers.DateTimeField()
@@ -85,9 +88,21 @@ class OrganizationSerializer(serializers.ModelSerializer):
 
 
 class CatalogSerializer(serializers.ModelSerializer):
+    courses_count = serializers.IntegerField(read_only=True, help_text=_('Number of courses contained in this catalog'))
+    viewers = serializers.SlugRelatedField(slug_field='username', queryset=User.objects.all(), many=True,
+                                           allow_null=True, allow_empty=True, required=False,
+                                           help_text=_('Usernames of users with explicit access to view this catalog'))
+
+    def create(self, validated_data):
+        # Set viewers after the model has been saved
+        viewers = validated_data.pop('viewers')
+        instance = super(CatalogSerializer, self).create(validated_data)
+        instance.viewers = viewers
+        return instance
+
     class Meta(object):
         model = Catalog
-        fields = ('id', 'name', 'query', 'courses_count',)
+        fields = ('id', 'name', 'query', 'courses_count', 'viewers')
 
 
 class CourseRunSerializer(TimestampModelSerializer):

course_discovery/apps/api/tests/test_serializers.py

@@ -9,6 +9,7 @@ from course_discovery.apps.api.serializers import(
     PersonSerializer,
 )
 from course_discovery.apps.catalogs.tests.factories import CatalogFactory
+from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.course_metadata.tests.factories import (
     CourseFactory, CourseRunFactory, SubjectFactory, PrerequisiteFactory,
     ImageFactory, VideoFactory, OrganizationFactory, PersonFactory, SeatFactory
@@ -21,7 +22,8 @@ def json_date_format(datetime_obj):
 
 class CatalogSerializerTests(TestCase):
     def test_data(self):
-        catalog = CatalogFactory(query='*:*')  # We intentionally use a query for all Courses.
+        user = UserFactory()
+        catalog = CatalogFactory(query='*:*', viewers=[user])  # We intentionally use a query for all Courses.
         courses = CourseFactory.create_batch(10)
         serializer = CatalogSerializer(catalog)
 
@@ -29,7 +31,8 @@ class CatalogSerializerTests(TestCase):
             'id': catalog.id,
             'name': catalog.name,
             'query': catalog.query,
-            'courses_count': len(courses)
+            'courses_count': len(courses),
+            'viewers': [user.username]
         }
         self.assertDictEqual(serializer.data, expected)
 

course_discovery/apps/api/v1/tests/test_views/test_catalogs.py

@@ -35,9 +35,11 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi
     def assert_catalog_created(self, **headers):
         name = 'The Kitchen Sink'
         query = '*.*'
+        viewer = UserFactory()
         data = {
             'name': name,
-            'query': query
+            'query': query,
+            'viewers': [viewer.username]
         }
 
         response = self.client.post(self.catalog_list_url, data, format='json', **headers)
@@ -47,6 +49,7 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi
         self.assertDictEqual(response.data, self.serialize_catalog(catalog))
         self.assertEqual(catalog.name, name)
         self.assertEqual(catalog.query, query)
+        self.assertListEqual(list(catalog.viewers), [viewer])
 
     def grant_catalog_permission_to_user(self, user, action, catalog=None):
         """ Grant the user access to view `self.catalog`. """

course_discovery/apps/api/v1/views.py

@@ -39,7 +39,16 @@ class CatalogViewSet(viewsets.ModelViewSet):
         return super(CatalogViewSet, self).destroy(request, *args, **kwargs)
 
     def list(self, request, *args, **kwargs):
-        """ Retrieve a list of all catalogs. """
+        """ Retrieve a list of all catalogs.
+        ---
+        parameters:
+            - name: username
+              description: User whose catalogs should be retrieved.
+              required: false
+              type: string
+              paramType: query
+              multiple: false
+        """
         return super(CatalogViewSet, self).list(request, *args, **kwargs)
 
     def partial_update(self, request, *args, **kwargs):

course_discovery/apps/catalogs/models.py

+from collections import Iterable
+
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 from django_extensions.db.models import TimeStampedModel
+from guardian.shortcuts import get_users_with_perms
 from haystack.query import SearchQuerySet
 
 from course_discovery.apps.core.mixins import ModelPermissionsMixin
@@ -8,6 +11,7 @@ from course_discovery.apps.course_metadata.models import Course
 
 
 class Catalog(ModelPermissionsMixin, TimeStampedModel):
+    VIEW_PERMISSION = 'view_catalog'
     name = models.CharField(max_length=255, null=False, blank=False, help_text=_('Catalog name'))
     query = models.TextField(null=False, blank=False, help_text=_('Query to retrieve catalog contents'))
 
@@ -52,6 +56,52 @@ class Catalog(ModelPermissionsMixin, TimeStampedModel):
 
         return contains
 
+    @property
+    def viewers(self):
+        """ Returns a QuerySet of users who have been granted explicit access to view this Catalog.
+
+        Returns:
+            QuerySet
+        """
+        # NOTE (CCB): This method actually returns any individual User with *any* permission on the object. It is
+        # safe to assume that those who can create/modify the model can also view it. If that assumption changes,
+        # change this code!
+        return get_users_with_perms(self, with_superusers=False, with_group_users=False)
+
+    @viewers.setter
+    def viewers(self, value):
+        """ Sets the viewers of this model.
+
+        This method utilizes Django permissions to set access. Existing user-specific access permissions will be
+        overwritten. Group permissions will not be affected.
+
+        Args:
+            value (Iterable): Collection of `User` objects.
+
+        Raises:
+            TypeError: The given value is not iterable, or is a string.
+
+        Returns:
+            None
+        """
+        if isinstance(value, str) or not isinstance(value, Iterable):
+            raise TypeError('Viewers must be a non-string iterable containing User objects.')
+
+        new = set(value)
+        existing = set(self.viewers)
+
+        # Remove users who no longer have access
+        to_be_removed = existing - new
+
+        for user in to_be_removed:
+            user.del_obj_perm(self.VIEW_PERMISSION, self)
+
+        # Add new users
+        new = new - existing
+
+        for user in new:
+            user.add_obj_perm(self.VIEW_PERMISSION, self)
+
     class Meta(TimeStampedModel.Meta):
         abstract = False
         permissions = (

course_discovery/apps/catalogs/tests/factories.py

@@ -10,3 +10,8 @@ class CatalogFactory(factory.DjangoModelFactory):
 
     name = FuzzyText(prefix='catalog-name-')
     query = '*:*'
+
+    @factory.post_generation
+    def viewers(self, create, extracted, **kwargs):  # pylint: disable=method-hidden,unused-argument
+        if create and extracted:
+            self.viewers = extracted

course_discovery/apps/catalogs/tests/test_models.py

+import ddt
 from django.test import TestCase
 
+from course_discovery.apps.catalogs.models import Catalog
 from course_discovery.apps.catalogs.tests import factories
+from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin
 from course_discovery.apps.course_metadata.tests.factories import CourseFactory
 
 
+@ddt.ddt
 class CatalogTests(ElasticsearchTestMixin, TestCase):
     """ Catalog model tests. """
 
@@ -43,3 +47,44 @@ class CatalogTests(ElasticsearchTestMixin, TestCase):
         CourseFactory()
         CourseFactory(title='ABCDEF')
         self.assertEqual(self.catalog.courses_count, 2)
+
+    def test_get_viewers(self):
+        """ Verify the method returns a QuerySet of individuals with explicit permission to view a Catalog. """
+        catalog = self.catalog
+        self.assertFalse(catalog.viewers.exists())  # pylint:disable=no-member
+
+        user = UserFactory()
+        user.add_obj_perm(Catalog.VIEW_PERMISSION, catalog)
+        self.assertListEqual(list(catalog.viewers), [user])
+
+    def test_set_viewers(self):
+        """ Verify the method updates the set of users with permission to view a Catalog. """
+        users = UserFactory.create_batch(2)
+        permission = 'catalogs.' + Catalog.VIEW_PERMISSION
+
+        for user in users:
+            self.assertFalse(user.has_perm(permission, self.catalog))
+
+        # Verify a list of users can be added as viewers
+        self.catalog.viewers = users
+        for user in users:
+            self.assertTrue(user.has_perm(permission, self.catalog))
+
+        # Verify existing users, not in the list, have their access revoked.
+        permitted = users[0]
+        revoked = users[1]
+        self.catalog.viewers = [permitted]
+        self.assertTrue(permitted.has_perm(permission, self.catalog))
+        self.assertFalse(revoked.has_perm(permission, self.catalog))
+
+        # Verify all users have their access revoked when passing in an empty list
+        self.catalog.viewers = []
+        for user in users:
+            self.assertFalse(user.has_perm(permission, self.catalog))
+
+    @ddt.data(None, 35, 'a')
+    def test_set_viewers_with_invalid_argument(self, viewers):
+        """ Verify the method raises a `TypeError` if the passed value is not iterable, or is a string. """
+        with self.assertRaises(TypeError) as context:
+            self.catalog.viewers = viewers
+        self.assertEqual(context.exception.args[0], 'Viewers must be a non-string iterable containing User objects.')

course_discovery/apps/core/mixins.py

@@ -7,6 +7,7 @@ class ModelPermissionsMixin:
     Inheriting models should have the default add, change, and delete permissions, as well as the
     custom "view" permission.
     """
+
     @classmethod
     def get_permission(cls, action):
         """
@@ -46,6 +47,14 @@ class ModelPermissionsMixin:
 
     @authenticated_users
     @allow_staff_or_superuser
+    def has_object_create_permission(self, request):  # pragma: no cover
+        # NOTE (CCB): This method is solely here to ensure object creation and permissions behave appropriately
+        # when using the Browseable API. This is not called when making a JSON request.
+        perm = self.get_permission('add')
+        return request.user.has_perm(perm, self)
+
+    @authenticated_users
+    @allow_staff_or_superuser
     def has_object_destroy_permission(self, request):
         perm = self.get_permission('delete')
         return request.user.has_perm(perm, self)