Merge pull request #40 from edx/ekafeel/notes-max-limit-error

[TNL-4029] user can't add more than maximum number of allowed notes

notesapi/v1/tests/test_views.py

@@ -18,6 +18,7 @@ from rest_framework.test import APITestCase
 from .helpers import get_id_token
 
 TEST_USER = "test_user_id"
+TEST_OTHER_USER = "test_other_user_id"
 
 if not settings.ES_DISABLED:
     import haystack
@@ -54,7 +55,7 @@ class BaseAnnotationViewTests(APITestCase):
             "tags": ["pink", "lady"]
         }
 
-    def _create_annotation(self, **kwargs):
+    def _create_annotation(self, expected_status=status.HTTP_201_CREATED, **kwargs):
         """
         Create annotation
         """
@@ -62,7 +63,7 @@ class BaseAnnotationViewTests(APITestCase):
         opts.update(kwargs)
         url = reverse('api:v1:annotations')
         response = self.client.post(url, opts, format='json')
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(response.status_code, expected_status)
         return response.data.copy()
 
     def _do_annotation_update(self, data, updated_fields):
@@ -346,6 +347,38 @@ class AnnotationListViewTests(BaseAnnotationViewTests):
         del annotation['created']
         self.assertEqual(annotation, note)
 
+    @patch('django.conf.settings.MAX_NOTES_PER_COURSE', 5)
+    def test_create_maximum_allowed(self):
+        """
+        Tests if user can create more than maximum allowed notes per course
+        Also test if other user can create notes and Same user can create notes in other course
+        """
+        for i in xrange(5):
+            kwargs = {'text': 'Foo_{}'.format(i)}
+            self._create_annotation(**kwargs)
+
+        # Creating more notes should result in 400 error
+        kwargs = {'text': 'Foo_{}'.format(6)}
+        response = self._create_annotation(expected_status=status.HTTP_400_BAD_REQUEST, **kwargs)
+        self.assertEqual(
+            response['error_msg'],
+            u'You can create up to {0} notes.'
+            u' You must remove some notes before you can add new ones.'.format(settings.MAX_NOTES_PER_COURSE)
+        )
+
+        # if user tries to create note in a different course it should succeed
+        kwargs = {'course_id': 'test-course-id-2'}
+        response = self._create_annotation(**kwargs)
+        self.assertTrue('id' in response)
+
+        # if another user to tries to create note in first course it should succeed
+        token = get_id_token(TEST_OTHER_USER)
+        self.client.credentials(HTTP_X_ANNOTATOR_AUTH_TOKEN=token)
+        self.headers = {'user': TEST_OTHER_USER}
+        kwargs = {'user': TEST_OTHER_USER}
+        response = self._create_annotation(**kwargs)
+        self.assertTrue('id' in response)
+
     def test_read_all_no_annotations(self):
         """
         Tests list all annotations endpoint when no annotations are present in database.

notesapi/v1/views.py

 import logging
 import json
+import newrelic.agent
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.core.exceptions import ValidationError
 from django.db.models import Q
+from django.utils.translation import ugettext as _
 
 from rest_framework import status
 from rest_framework.generics import GenericAPIView
@@ -22,6 +24,13 @@ if not settings.ES_DISABLED:
 log = logging.getLogger(__name__)
 
 
+class AnnotationsLimitReachedError(Exception):
+    """
+    Exception when trying to create more than allowed annotations
+    """
+    pass
+
+
 class AnnotationSearchView(GenericAPIView):
     """
     **Use Case**
@@ -275,20 +284,41 @@ class AnnotationListView(GenericAPIView):
 
         Returns 400 request if bad payload is sent or it was empty object.
         """
-        if 'id' in self.request.data:
+        if not self.request.data or 'id' in self.request.data:
             return Response(status=status.HTTP_400_BAD_REQUEST)
 
         try:
+            total_notes = Note.objects.filter(
+                    user_id=self.request.data['user'], course_id=self.request.data['course_id']
+            ).count()
+            if total_notes >= settings.MAX_NOTES_PER_COURSE:
+                raise AnnotationsLimitReachedError
+
             note = Note.create(self.request.data)
             note.full_clean()
+
+            # Gather metrics for New Relic so we can slice data in New Relic Insights
+            newrelic.agent.add_custom_parameter('notes.count', total_notes)
         except ValidationError as error:
             log.debug(error, exc_info=True)
             return Response(status=status.HTTP_400_BAD_REQUEST)
+        except AnnotationsLimitReachedError:
+            error_message = _(
+                u'You can create up to {max_num_annotations_per_course} notes.'
+                u' You must remove some notes before you can add new ones.'
+            ).format(max_num_annotations_per_course=settings.MAX_NOTES_PER_COURSE)
+            log.info(
+                u'Attempted to create more than %s annotations',
+                settings.MAX_NOTES_PER_COURSE
+            )
+
+            return Response({
+                'error_msg': error_message
+            }, status=status.HTTP_400_BAD_REQUEST)
 
         note.save()
 
         location = reverse('api:v1:annotations_detail', kwargs={'annotation_id': note.id})
-
         serializer = NoteSerializer(note)
         return Response(serializer.data, status=status.HTTP_201_CREATED, headers={'Location': location})
 

notesserver/settings/common.py

@@ -83,3 +83,6 @@ TEMPLATE_DIRS = (
 )
 
 DEFAULT_NOTES_PAGE_SIZE = 25
+
+### Maximum number of allowed notes for each student per course ###
+MAX_NOTES_PER_COURSE = 500

requirements/base.txt

@@ -10,3 +10,4 @@ MySQL-python==1.2.5  # GPL License
 gunicorn==19.1.1     # MIT
 path.py==3.0.1
 python-dateutil==2.4.0
+newrelic==2.40.0.34            # New Relic

requirements/optional.txt

-newrelic==2.40.0.34            # New Relic