diff --git a/lms/djangoapps/notes/api.py b/lms/djangoapps/notes/api.py index 43dc2805c8..68b144dd9a 100644 --- a/lms/djangoapps/notes/api.py +++ b/lms/djangoapps/notes/api.py @@ -28,6 +28,7 @@ API_SETTINGS = { #----------------------------------------------------------------------# # API requests are routed through api_request() using the resource map. + def api_enabled(request, course_id): ''' Returns True if the api is enabled for the course, otherwise False. @@ -35,9 +36,10 @@ def api_enabled(request, course_id): course = _get_course(request, course_id) return notes_enabled_for_course(course) + @login_required def api_request(request, course_id, **kwargs): - ''' + ''' Routes API requests to the appropriate action method and returns JSON. Raises a 404 if the requested resource does not exist or notes are disabled for the course. @@ -49,7 +51,7 @@ def api_request(request, course_id, **kwargs): raise Http404 # Locate the requested resource - resource_map = API_SETTINGS.get('RESOURCE_MAP', {}) + resource_map = API_SETTINGS.get('RESOURCE_MAP', {}) resource_name = kwargs.pop('resource') resource_method = request.method resource = resource_map.get(resource_name) @@ -59,7 +61,7 @@ def api_request(request, course_id, **kwargs): raise Http404 if resource_method not in resource.keys(): - log.debug('Resource "{0}" does not support method "{1}"'.format(resource_name, resource_method)) + log.debug('Resource "{0}" does not support method "{1}"'.format(resource_name, resource_method)) raise Http404 # Execute the action associated with the resource @@ -75,7 +77,7 @@ def api_request(request, course_id, **kwargs): # Format and output the results data = None response = result[0] - if len(result) == 2: + if len(result) == 2: data = result[1] formatted = api_format(request, response, data) @@ -86,10 +88,11 @@ def api_request(request, course_id, **kwargs): return response + def api_format(request, response, data): - ''' - Returns a two-element list containing the content type and content. - ''' + ''' + Returns a two-element list containing the content type and content. + ''' content_type = 'application/json' if data is None: content = '' @@ -97,6 +100,7 @@ def api_format(request, response, data): content = json.dumps(data) return [content_type, content] + def _get_course(request, course_id): ''' Helper function to load and return a user's course. @@ -106,20 +110,22 @@ def _get_course(request, course_id): #----------------------------------------------------------------------# # API actions exposed via the resource map. + def index(request, course_id): - ''' - Returns a list of annotation objects. + ''' + Returns a list of annotation objects. ''' MAX_LIMIT = API_SETTINGS.get('MAX_NOTE_LIMIT') notes = Note.objects.order_by('id').filter(course_id=course_id, - user=request.user)[:MAX_LIMIT] + user=request.user)[:MAX_LIMIT] return [HttpResponse(), [note.as_dict() for note in notes]] + def create(request, course_id): - ''' - Receives an annotation object to create and returns a 303 with the read location. + ''' + Receives an annotation object to create and returns a 303 with the read location. ''' note = Note(course_id=course_id, user=request.user) @@ -135,9 +141,10 @@ def create(request, course_id): return [response, None] + def read(request, course_id, note_id): - ''' - Returns a single annotation object. + ''' + Returns a single annotation object. ''' try: note = Note.objects.get(id=note_id) @@ -149,9 +156,10 @@ def read(request, course_id, note_id): return [HttpResponse(), note.as_dict()] + def update(request, course_id, note_id): - ''' - Updates an annotation object and returns a 303 with the read location. + ''' + Updates an annotation object and returns a 303 with the read location. ''' try: note = Note.objects.get(id=note_id) @@ -174,8 +182,9 @@ def update(request, course_id, note_id): return [response, None] + def delete(request, course_id, note_id): - ''' + ''' Deletes the annotation object and returns a 204 with no content. ''' try: @@ -190,12 +199,13 @@ def delete(request, course_id, note_id): return [HttpResponse('', status=204), None] + def search(request, course_id): - ''' + ''' Returns a subset of annotation objects based on a search query. ''' MAX_LIMIT = API_SETTINGS.get('MAX_NOTE_LIMIT') - + # search parameters offset = request.GET.get('offset', '') limit = request.GET.get('limit', '') @@ -222,7 +232,7 @@ def search(request, course_id): # retrieve notes notes = Note.objects.order_by('id').filter(**filters) total = notes.count() - rows = notes[offset:offset+limit] + rows = notes[offset:offset + limit] result = { 'total': total, 'rows': [note.as_dict() for note in rows] @@ -230,8 +240,9 @@ def search(request, course_id): return [HttpResponse(), result] + def root(request, course_id): - ''' - Returns version information about the API. + ''' + Returns version information about the API. ''' return [HttpResponse(), API_SETTINGS.get('META')] diff --git a/lms/djangoapps/notes/models.py b/lms/djangoapps/notes/models.py index fe8a708391..c07995b8d7 100644 --- a/lms/djangoapps/notes/models.py +++ b/lms/djangoapps/notes/models.py @@ -2,12 +2,12 @@ from django.db import models from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.core.exceptions import ValidationError - import json import logging log = logging.getLogger(__name__) + class Note(models.Model): user = models.ForeignKey(User, db_index=True) course_id = models.CharField(max_length=255, db_index=True) @@ -18,7 +18,7 @@ class Note(models.Model): range_start_offset = models.IntegerField() range_end = models.CharField(max_length=2048) range_end_offset = models.IntegerField() - tags = models.TextField(default="") # comma-separated string + tags = models.TextField(default="") # comma-separated string created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) updated = models.DateTimeField(auto_now=True, db_index=True) @@ -68,4 +68,4 @@ class Note(models.Model): 'tags': self.tags.split(","), 'created': str(self.created), 'updated': str(self.updated) - } \ No newline at end of file + } diff --git a/lms/djangoapps/notes/tests.py b/lms/djangoapps/notes/tests.py index 6417ad6965..d84b5d1026 100644 --- a/lms/djangoapps/notes/tests.py +++ b/lms/djangoapps/notes/tests.py @@ -14,11 +14,12 @@ import logging from . import utils, api, models + class UtilsTest(TestCase): - def setUp(self): - ''' + def setUp(self): + ''' Setup a dummy course-like object with a tabs field that can be - accessed via attribute lookup. + accessed via attribute lookup. ''' self.course = collections.namedtuple('DummyCourse', ['tabs']) self.course.tabs = [] @@ -35,20 +36,20 @@ class UtilsTest(TestCase): Tests that notes are enabled when the course tab configuration contains a tab with type "notes." ''' - self.course.tabs = [ - {'type': 'foo'}, - {'name': 'My Notes', 'type': 'notes'}, - {'type':'bar'}] + self.course.tabs = [{'type': 'foo'}, + {'name': 'My Notes', 'type': 'notes'}, + {'type': 'bar'}] self.assertTrue(utils.notes_enabled_for_course(self.course)) + class ApiTest(TestCase): def setUp(self): self.client = Client() # Mocks - api.api_enabled = self.mock_api_enabled(True) + api.api_enabled = self.mock_api_enabled(True) # Create two accounts self.password = 'abc' @@ -57,16 +58,16 @@ class ApiTest(TestCase): self.instructor = User.objects.create_user('instructor', 'instructor@test.com', self.password) self.course_id = 'HarvardX/CB22x/The_Ancient_Greek_Hero' self.note = { - 'user':self.student, - 'course_id':self.course_id, - 'uri':'/', - 'text':'foo', - 'quote':'bar', - 'range_start':0, - 'range_start_offset':0, - 'range_end':100, - 'range_end_offset':0, - 'tags':'a,b,c' + 'user': self.student, + 'course_id': self.course_id, + 'uri': '/', + 'text': 'foo', + 'quote': 'bar', + 'range_start': 0, + 'range_start_offset': 0, + 'range_end': 100, + 'range_end_offset': 0, + 'tags': 'a,b,c' } def mock_api_enabled(self, is_enabled): @@ -84,7 +85,7 @@ class ApiTest(TestCase): self.client.login(username=username, password=password) def url(self, name, args={}): - args.update({'course_id':self.course_id}) + args.update({'course_id': self.course_id}) return reverse(name, kwargs=args) def create_notes(self, num_notes, create=True): @@ -100,12 +101,12 @@ class ApiTest(TestCase): self.login() resp = self.client.get(self.url('notes_api_root')) - self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.status_code, 200) self.assertNotEqual(resp.content, '') content = json.loads(resp.content) - self.assertEqual(set(('name','version')), set(content.keys())) + self.assertEqual(set(('name', 'version')), set(content.keys())) self.assertIsInstance(content['version'], int) self.assertEqual(content['name'], 'Notes API') @@ -135,7 +136,7 @@ class ApiTest(TestCase): def test_index_max_notes(self): self.login() - MAX_LIMIT = api.API_SETTINGS.get('MAX_NOTE_LIMIT') + MAX_LIMIT = api.API_SETTINGS.get('MAX_NOTE_LIMIT') num_notes = MAX_LIMIT + 1 self.create_notes(num_notes) @@ -155,11 +156,12 @@ class ApiTest(TestCase): note_dict = notes[0].as_dict() excluded_fields = ['id', 'user_id', 'created', 'updated'] - note = dict([(k, v) for k,v in note_dict.items() if k not in excluded_fields]) + note = dict([(k, v) for k, v in note_dict.items() if k not in excluded_fields]) - resp = self.client.post(self.url('notes_api_notes'), json.dumps(note), - content_type='application/json', - HTTP_X_REQUESTED_WITH='XMLHttpRequest') + resp = self.client.post(self.url('notes_api_notes'), + json.dumps(note), + content_type='application/json', + HTTP_X_REQUESTED_WITH='XMLHttpRequest') self.assertEqual(resp.status_code, 303) self.assertEqual(len(resp.content), 0) @@ -168,9 +170,10 @@ class ApiTest(TestCase): self.login() for empty_test in [None, [], '']: - resp = self.client.post(self.url('notes_api_notes'), json.dumps(empty_test), - content_type='application/json', - HTTP_X_REQUESTED_WITH='XMLHttpRequest') + resp = self.client.post(self.url('notes_api_notes'), + json.dumps(empty_test), + content_type='application/json', + HTTP_X_REQUESTED_WITH='XMLHttpRequest') self.assertEqual(resp.status_code, 500) def test_create_note_missing_ranges(self): @@ -181,14 +184,15 @@ class ApiTest(TestCase): note_dict = notes[0].as_dict() excluded_fields = ['id', 'user_id', 'created', 'updated'] + ['ranges'] - note = dict([(k, v) for k,v in note_dict.items() if k not in excluded_fields]) + note = dict([(k, v) for k, v in note_dict.items() if k not in excluded_fields]) - resp = self.client.post(self.url('notes_api_notes'), json.dumps(note), - content_type='application/json', - HTTP_X_REQUESTED_WITH='XMLHttpRequest') + resp = self.client.post(self.url('notes_api_notes'), + json.dumps(note), + content_type='application/json', + HTTP_X_REQUESTED_WITH='XMLHttpRequest') self.assertEqual(resp.status_code, 500) - def test_read_note(self): + def test_read_note(self): self.login() notes = self.create_notes(3) @@ -220,12 +224,14 @@ class ApiTest(TestCase): # set the student id to a different student (not the one that created the notes) self.login(as_student=self.student2) - resp = self.client.get(self.url('notes_api_note', { 'note_id': note.id})) + resp = self.client.get(self.url('notes_api_note', {'note_id': note.id})) self.assertEqual(resp.status_code, 403) self.assertEqual(resp.content, '') @unittest.skip("skipping update test stub") - def test_update_note(self): pass + def test_update_note(self): + pass @unittest.skip("skipping search test stub") - def test_search_note(self): pass + def test_search_note(self): + pass diff --git a/lms/djangoapps/notes/urls.py b/lms/djangoapps/notes/urls.py index 7811a5f044..6abe92253a 100644 --- a/lms/djangoapps/notes/urls.py +++ b/lms/djangoapps/notes/urls.py @@ -1,9 +1,10 @@ from django.conf.urls import patterns, url + id_regex = r"(?P[0-9A-Fa-f]+)" urlpatterns = patterns('notes.api', - url(r'^api$', 'api_request', {'resource':'root'}, name='notes_api_root'), - url(r'^api/annotations$', 'api_request', {'resource':'notes'}, name='notes_api_notes'), - url(r'^api/annotations/' + id_regex + r'$', 'api_request', {'resource':'note'}, name='notes_api_note'), - url(r'^api/search', 'api_request', {'resource':'search'}, name='notes_api_search') -) + url(r'^api$', 'api_request', {'resource': 'root'}, name='notes_api_root'), + url(r'^api/annotations$', 'api_request', {'resource': 'notes'}, name='notes_api_notes'), + url(r'^api/annotations/' + id_regex + r'$', 'api_request', {'resource': 'note'}, name='notes_api_note'), + url(r'^api/search', 'api_request', {'resource': 'search'}, name='notes_api_search') + ) diff --git a/lms/djangoapps/notes/utils.py b/lms/djangoapps/notes/utils.py index 80a872da4e..e6e784ce49 100644 --- a/lms/djangoapps/notes/utils.py +++ b/lms/djangoapps/notes/utils.py @@ -1,9 +1,17 @@ +from django.conf import settings + + def notes_enabled_for_course(course): - ''' - Returns True if the notes app is enabled for the course, False otherwise. + ''' - # TODO: create a separate policy setting to enable/disable notes - tab_type = 'notes' - tabs = course.tabs - tab_found = next((True for t in tabs if t['type'] == tab_type), False) - return tab_found + Returns True if the notes app is enabled for the course, False otherwise. + + In order for the app to be enabled it must be: + 1) enabled globally via MITX_FEATURES. + 2) present in the course tab configuration. + ''' + + tab_found = next((True for t in course.tabs if t['type'] == 'notes'), False) + feature_enabled = settings.MITX_FEATURES.get('ENABLE_STUDENT_NOTES') + + return feature_enabled and tab_found diff --git a/lms/djangoapps/notes/views.py b/lms/djangoapps/notes/views.py index ba47540071..cf125781d0 100644 --- a/lms/djangoapps/notes/views.py +++ b/lms/djangoapps/notes/views.py @@ -6,6 +6,7 @@ from notes.models import Note from notes.utils import notes_enabled_for_course import json + @login_required def notes(request, course_id): ''' Displays the student's notes. ''' @@ -13,7 +14,7 @@ def notes(request, course_id): course = get_course_with_access(request.user, course_id, 'load') if not notes_enabled_for_course(course): raise Http404 - + notes = Note.objects.filter(course_id=course_id, user=request.user).order_by('-created', 'uri') json_notes = json.dumps([n.as_dict() for n in notes]) context = {