diff --git a/lms/djangoapps/notes/api.py b/lms/djangoapps/notes/api.py index ea334faf10..43dc2805c8 100644 --- a/lms/djangoapps/notes/api.py +++ b/lms/djangoapps/notes/api.py @@ -13,7 +13,7 @@ API_SETTINGS = { # Version 'META': {'name': 'Notes API', 'version': 1}, - # Maps resources to HTTP methods + # Maps resources to HTTP methods and actions 'RESOURCE_MAP': { 'root': {'GET': 'root'}, 'notes': {'GET': 'index', 'POST': 'create'}, @@ -43,12 +43,12 @@ def api_request(request, course_id, **kwargs): disabled for the course. ''' - # Verify that notes are enabled for the course + # Verify that the api should be accessible to this course if not api_enabled(request, course_id): log.debug('Notes not enabled for course') raise Http404 - # Locate and validate the requested resource + # Locate the requested resource resource_map = API_SETTINGS.get('RESOURCE_MAP', {}) resource_name = kwargs.pop('resource') resource_method = request.method @@ -62,7 +62,7 @@ def api_request(request, course_id, **kwargs): log.debug('Resource "{0}" does not support method "{1}"'.format(resource_name, resource_method)) raise Http404 - # Find the associated function definition and execute the request + # Execute the action associated with the resource func = resource.get(resource_method) module = globals() if func not in module: diff --git a/lms/djangoapps/notes/tests.py b/lms/djangoapps/notes/tests.py index c48202dcc2..6417ad6965 100644 --- a/lms/djangoapps/notes/tests.py +++ b/lms/djangoapps/notes/tests.py @@ -1,5 +1,5 @@ """ -Unit tests for the notes API and model. +Unit tests for the notes app. """ from django.test import TestCase @@ -7,8 +7,8 @@ from django.test.client import Client from django.core.urlresolvers import reverse from django.contrib.auth.models import User -from collections import namedtuple -from random import random +import collections +import unittest import json import logging @@ -20,7 +20,7 @@ class UtilsTest(TestCase): Setup a dummy course-like object with a tabs field that can be accessed via attribute lookup. ''' - self.course = namedtuple('DummyCourse', ['tabs']) + self.course = collections.namedtuple('DummyCourse', ['tabs']) self.course.tabs = [] def test_notes_not_enabled(self): @@ -48,11 +48,12 @@ class ApiTest(TestCase): self.client = Client() # Mocks - api.api_enabled = (lambda request, course_id: True) + api.api_enabled = self.mock_api_enabled(True) # Create two accounts self.password = 'abc' self.student = User.objects.create_user('student', 'student@test.com', self.password) + self.student2 = User.objects.create_user('student2', 'student2@test.com', self.password) self.instructor = User.objects.create_user('instructor', 'instructor@test.com', self.password) self.course_id = 'HarvardX/CB22x/The_Ancient_Greek_Hero' self.note = { @@ -68,15 +69,31 @@ class ApiTest(TestCase): 'tags':'a,b,c' } - def login(self): - self.client.login(username=self.student.username, password=self.password) + def mock_api_enabled(self, is_enabled): + return (lambda request, course_id: is_enabled) - def url(self, name): - return reverse(name, kwargs={'course_id':self.course_id}) + def login(self, as_student=None): + username = None + password = self.password - def create_notes(self, num_notes): - notes = [ models.Note(**self.note) for n in range(num_notes) ] - models.Note.objects.bulk_create(notes) + if as_student is None: + username = self.student.username + else: + username = as_student.username + + self.client.login(username=username, password=password) + + def url(self, name, args={}): + args.update({'course_id':self.course_id}) + return reverse(name, kwargs=args) + + def create_notes(self, num_notes, create=True): + notes = [] + for n in range(num_notes): + note = models.Note(**self.note) + if create: + note.save() + notes.append(note) return notes def test_root(self): @@ -103,7 +120,7 @@ class ApiTest(TestCase): self.assertEqual(len(content), 0) def test_index_with_notes(self): - num_notes = 7 + num_notes = 3 self.login() self.create_notes(num_notes) @@ -112,6 +129,7 @@ class ApiTest(TestCase): self.assertNotEqual(resp.content, '') content = json.loads(resp.content) + self.assertIsInstance(content, list) self.assertEqual(len(content), num_notes) def test_index_max_notes(self): @@ -126,4 +144,88 @@ class ApiTest(TestCase): self.assertNotEqual(resp.content, '') content = json.loads(resp.content) + self.assertIsInstance(content, list) self.assertEqual(len(content), MAX_LIMIT) + + def test_create_note(self): + self.login() + + notes = self.create_notes(1) + self.assertEqual(len(notes), 1) + + 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]) + + 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) + + def test_create_empty_notes(self): + 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') + self.assertEqual(resp.status_code, 500) + + def test_create_note_missing_ranges(self): + self.login() + + notes = self.create_notes(1) + self.assertEqual(len(notes), 1) + 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]) + + 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): + self.login() + + notes = self.create_notes(3) + self.assertEqual(len(notes), 3) + + for note in notes: + resp = self.client.get(self.url('notes_api_note', {'note_id': note.id})) + self.assertEqual(resp.status_code, 200) + self.assertNotEqual(resp.content, '') + + content = json.loads(resp.content) + self.assertEqual(content['id'], note.id) + self.assertEqual(content['user_id'], note.user_id) + + def test_note_doesnt_exist_to_read(self): + NOTE_ID_DOES_NOT_EXIST = 99999 + + self.login() + resp = self.client.get(self.url('notes_api_note', { + 'note_id': NOTE_ID_DOES_NOT_EXIST + })) + self.assertEqual(resp.status_code, 404) + self.assertEqual(resp.content, '') + + def test_student_doesnt_have_permission_to_read_note(self): + notes = self.create_notes(1) + self.assertEqual(len(notes), 1) + note = notes[0] + + # 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})) + self.assertEqual(resp.status_code, 403) + self.assertEqual(resp.content, '') + + @unittest.skip("skipping update test stub") + def test_update_note(self): pass + + @unittest.skip("skipping search test stub") + def test_search_note(self): pass