Commit 2838dba1 by Arthur Barrett

fix pep8 violations

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