diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index e0a7bc5df6..5564f0c420 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -797,7 +797,6 @@ class CourseMetadataEditingTest(CourseTestCase): self.fullcourse = CourseFactory.create() self.course_setting_url = get_url(self.course.id, 'advanced_settings_handler') self.fullcourse_setting_url = get_url(self.fullcourse.id, 'advanced_settings_handler') - self.notes_tab = {"type": "notes", "name": "My Notes"} self.request = RequestFactory().request() self.user = UserFactory() @@ -1127,60 +1126,6 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertIn('advertised_start', test_model, 'Missing revised advertised_start metadata field') self.assertEqual(test_model['advertised_start']['value'], 'start B', "advertised_start not expected value") - def test_advanced_components_munge_tabs(self): - """ - Test that adding and removing specific advanced components adds and removes tabs. - """ - # First ensure that none of the tabs are visible - self.assertNotIn(self.notes_tab, self.course.tabs) - - # Now enable student notes and verify that the "My Notes" tab has been added - self.client.ajax_post(self.course_setting_url, { - 'advanced_modules': {"value": ["notes"]} - }) - course = modulestore().get_course(self.course.id) - self.assertIn(self.notes_tab, course.tabs) - - # Disable student notes and verify that the "My Notes" tab is gone - self.client.ajax_post(self.course_setting_url, { - 'advanced_modules': {"value": [""]} - }) - course = modulestore().get_course(self.course.id) - self.assertNotIn(self.notes_tab, course.tabs) - - def test_advanced_components_munge_tabs_validation_failure(self): - with patch('contentstore.views.course._refresh_course_tabs', side_effect=InvalidTabsException): - resp = self.client.ajax_post(self.course_setting_url, { - 'advanced_modules': {"value": ["notes"]} - }) - self.assertEqual(resp.status_code, 400) - - error_msg = [ - { - 'message': 'An error occurred while trying to save your tabs', - 'model': {'display_name': 'Tabs Exception'} - } - ] - self.assertEqual(json.loads(resp.content.decode('utf-8')), error_msg) - - # verify that the course wasn't saved into the modulestore - course = modulestore().get_course(self.course.id) - self.assertNotIn("notes", course.advanced_modules) - - @ddt.data( - [{'type': 'course_info'}, {'type': 'courseware'}, {'type': 'wiki', 'is_hidden': True}], - [{'type': 'course_info', 'name': 'Home'}, {'type': 'courseware', 'name': 'Course'}], - ) - def test_course_tab_configurations(self, tab_list): - self.course.tabs = tab_list - modulestore().update_item(self.course, self.user.id) - self.client.ajax_post(self.course_setting_url, { - 'advanced_modules': {"value": ["notes"]} - }) - course = modulestore().get_course(self.course.id) - tab_list.append(self.notes_tab) - self.assertEqual(tab_list, course.tabs) - @patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True}) @patch('xmodule.util.xmodule_django.get_current_request') def test_post_settings_with_staff_not_enrolled(self, mock_request): diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 343d12653a..c9d930a1cd 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -214,16 +214,16 @@ class PrimitiveTabEdit(ModuleStoreTestCase): def test_insert(self): """Test primitive tab insertion.""" course = CourseFactory.create() - tabs.primitive_insert(course, 2, 'notes', 'aname') - self.assertEquals(course.tabs[2], {'type': 'notes', 'name': 'aname'}) + tabs.primitive_insert(course, 2, 'pdf_textbooks', 'aname') + self.assertEquals(course.tabs[2], {'type': 'pdf_textbooks', 'name': 'aname'}) with self.assertRaises(ValueError): - tabs.primitive_insert(course, 0, 'notes', 'aname') + tabs.primitive_insert(course, 0, 'pdf_textbooks', 'aname') with self.assertRaises(ValueError): tabs.primitive_insert(course, 3, 'static_tab', 'aname') def test_save(self): """Test course saving.""" course = CourseFactory.create() - tabs.primitive_insert(course, 3, 'notes', 'aname') + tabs.primitive_insert(course, 3, 'pdf_textbooks', 'aname') course2 = modulestore().get_course(course.id) - self.assertEquals(course2.tabs[3], {'type': 'notes', 'name': 'aname'}) + self.assertEquals(course2.tabs[3], {'type': 'pdf_textbooks', 'name': 'aname'}) diff --git a/common/djangoapps/util/tests/test_db.py b/common/djangoapps/util/tests/test_db.py index 3df590fc95..536a8ec3da 100644 --- a/common/djangoapps/util/tests/test_db.py +++ b/common/djangoapps/util/tests/test_db.py @@ -222,6 +222,8 @@ class MigrationTests(TestCase): """ Tests for migrations. """ + + @unittest.skip("Migration will delete the Note model. Need to ship not referencing it first. DEPR-18.") @override_settings(MIGRATION_MODULES={}) def test_migrations_are_in_sync(self): """ diff --git a/lms/djangoapps/courseware/tests/test_navigation.py b/lms/djangoapps/courseware/tests/test_navigation.py index 3ada71ac6c..4a6f666e3e 100644 --- a/lms/djangoapps/courseware/tests/test_navigation.py +++ b/lms/djangoapps/courseware/tests/test_navigation.py @@ -68,7 +68,7 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): display_name='fullchrome', chrome='accordion,tabs') cls.tabtest = ItemFactory.create(parent=cls.chapterchrome, - display_name='progress_tab', + display_name='pdf_textbooks_tab', default_tab='progress') cls.staff_user = GlobalStaffFactory() @@ -132,7 +132,7 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): response = self.client.get(reverse('courseware_section', kwargs={ 'course_id': text_type(self.course.id), 'chapter': 'Chrome', - 'section': 'progress_tab', + 'section': 'pdf_textbooks_tab', })) self.assertTabActive('progress', response) diff --git a/lms/djangoapps/notes/README.md b/lms/djangoapps/notes/README.md deleted file mode 100644 index 07bcfa3d22..0000000000 --- a/lms/djangoapps/notes/README.md +++ /dev/null @@ -1,56 +0,0 @@ -Notes Django App -================ - -This is a django application that stores and displays notes that students make while reading static HTML book(s) in their courseware. Note taking functionality in the static HTML book(s) is handled by a wrapper script around [annotator.js](http://okfnlabs.org/annotator/), which interfaces with the API provided by this application to store and retrieve notes. - -Usage ------ - -To use this application, course staff must opt-in by doing the following: - -* Login to [Studio](http://studio.edx.org/). -* Go to *Course Settings* -> *Advanced Settings* -* Find the ```advanced_modules``` policy key and in the policy value field, add ```"notes"``` to the list. -* Save the course settings. - -The result of following these steps is that you should see a new tab appear in the courseware named *My Notes*. This will display a journal of notes that the student has created in the static HTML book(s). Second, when you highlight text in the static HTML book(s), a dialog will appear. You can enter some notes and tags and save it. The note will appear highlighted in the text and will also be saved to the journal. - -To disable the *My Notes* tab and notes in the static HTML book(s), simply reverse the above steps (i.e. remove ```"notes"``` from the ```advanced_modules``` policy setting). - -### Caveats and Limitations - -* Notes are private to each student. -* Sharing and replying to notes is not supported. -* The student *My Notes* interface is very limited. -* There is no instructor interface to view student notes. - -Developer Overview ------------------- - -### Quickstart - -``` -$ ./manage.py lms syncdb --migrate -``` - -Then follow the steps above to enable the *My Notes* tab or manually add a tab to the policy tab configuration with ```{"type": "notes", "name": "My Notes"}```. - -### App Directory Structure: - -lms/djangoapps/notes: - -* api.py - API used by annotator.js on the frontend -* models.py - Contains note model for storing notes -* tests.py - Unit tests -* views.py - View to display the journal of notes (i.e. *My Notes* tab) -* urls.py - Maps the API and View routes. -* utils.py - Contains method for checking if the course has this app enabled. Intended to be public to other modules. - -Also requires: - -* lms/static/js/notes.js -- wrapper around annotator.js -* lms/templates/notes.html -- used by views.py to display the notes - -Interacts with: - -* lms/djangoapps/staticbook - the html static book checks to see if notes is enabled and has some logic to enable/disable accordingly diff --git a/lms/djangoapps/notes/api.py b/lms/djangoapps/notes/api.py deleted file mode 100644 index 602202cb8c..0000000000 --- a/lms/djangoapps/notes/api.py +++ /dev/null @@ -1,257 +0,0 @@ -from __future__ import absolute_import - -import collections -import json -import logging - -import six -from django.contrib.auth.decorators import login_required -from django.core.exceptions import ValidationError -from django.http import Http404, HttpResponse -from opaque_keys.edx.keys import CourseKey - -from courseware.courses import get_course_with_access -from notes.models import Note -from notes.utils import notes_enabled_for_course - -log = logging.getLogger(__name__) - -API_SETTINGS = { - 'META': {'name': 'Notes API', 'version': 1}, - - # Maps resources to HTTP methods and actions - 'RESOURCE_MAP': { - 'root': {'GET': 'root'}, - 'notes': {'GET': 'index', 'POST': 'create'}, - 'note': {'GET': 'read', 'PUT': 'update', 'DELETE': 'delete'}, - 'search': {'GET': 'search'}, - }, - - # Cap the number of notes that can be returned in one request - 'MAX_NOTE_LIMIT': 1000, -} - -# Wrapper class for HTTP response and data. All API actions are expected to return this. -ApiResponse = collections.namedtuple('ApiResponse', ['http_response', 'data']) - -#----------------------------------------------------------------------# -# API requests are routed through api_request() using the resource map. - - -def api_enabled(request, course_key): - ''' - Returns True if the api is enabled for the course, otherwise False. - ''' - course = _get_course(request, course_key) - 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. - ''' - assert isinstance(course_id, six.string_types) - course_key = CourseKey.from_string(course_id) - - # Verify that the api should be accessible to this course - if not api_enabled(request, course_key): - log.debug(u'Notes are disabled for course: {0}'.format(course_id)) - raise Http404 - - # Locate the requested resource - resource_map = API_SETTINGS.get('RESOURCE_MAP', {}) - resource_name = kwargs.pop('resource') - resource_method = request.method - resource = resource_map.get(resource_name) - - if resource is None: - log.debug(u'Resource "{0}" does not exist'.format(resource_name)) - raise Http404 - - if resource_method not in list(resource.keys()): - log.debug(u'Resource "{0}" does not support method "{1}"'.format(resource_name, resource_method)) - raise Http404 - - # Execute the action associated with the resource - func = resource.get(resource_method) - module = globals() - if func not in module: - log.debug(u'Function "{0}" does not exist for request {1} {2}'.format(func, resource_method, resource_name)) - raise Http404 - - log.debug(u'API request: {0} {1}'.format(resource_method, resource_name)) - - api_response = module[func](request, course_key, **kwargs) - http_response = api_format(api_response) - - return http_response - - -def api_format(api_response): - ''' - Takes an ApiResponse and returns an HttpResponse. - ''' - http_response = api_response.http_response - content_type = 'application/json' - content = '' - - # not doing a strict boolean check on data becuase it could be an empty list - if api_response.data is not None and api_response.data != '': - content = json.dumps(api_response.data) - - http_response['Content-type'] = content_type - http_response.content = content - - log.debug(u'API response type: {0} content: {1}'.format(content_type, content)) - - return http_response - - -def _get_course(request, course_key): - ''' - Helper function to load and return a user's course. - ''' - return get_course_with_access(request.user, 'load', course_key) - -#----------------------------------------------------------------------# -# API actions exposed via the resource map. - - -def index(request, course_key): - ''' - Returns a list of annotation objects. - ''' - MAX_LIMIT = API_SETTINGS.get('MAX_NOTE_LIMIT') - - notes = Note.objects.order_by('id').filter(course_id=course_key, - user=request.user)[:MAX_LIMIT] - - return ApiResponse(http_response=HttpResponse(), data=[note.as_dict() for note in notes]) - - -def create(request, course_key): - ''' - Receives an annotation object to create and returns a 303 with the read location. - ''' - note = Note(course_id=course_key, user=request.user) - - try: - note.clean(request.body) - except ValidationError as e: - log.debug(e) - return ApiResponse(http_response=HttpResponse('', status=400), data=None) - - note.save() - response = HttpResponse('', status=303) - response['Location'] = note.get_absolute_url() - - return ApiResponse(http_response=response, data=None) - - -def read(request, _course_key, note_id): - ''' - Returns a single annotation object. - ''' - try: - note = Note.objects.get(id=note_id) - except Note.DoesNotExist: - return ApiResponse(http_response=HttpResponse('', status=404), data=None) - - if note.user.id != request.user.id: - return ApiResponse(http_response=HttpResponse('', status=403), data=None) - - return ApiResponse(http_response=HttpResponse(), data=note.as_dict()) - - -def update(request, course_key, note_id): # pylint: disable=unused-argument - ''' - Updates an annotation object and returns a 303 with the read location. - ''' - try: - note = Note.objects.get(id=note_id) - except Note.DoesNotExist: - return ApiResponse(http_response=HttpResponse('', status=404), data=None) - - if note.user.id != request.user.id: - return ApiResponse(http_response=HttpResponse('', status=403), data=None) - - try: - note.clean(request.body) - except ValidationError as e: - log.debug(e) - return ApiResponse(http_response=HttpResponse('', status=400), data=None) - - note.save() - - response = HttpResponse('', status=303) - response['Location'] = note.get_absolute_url() - - return ApiResponse(http_response=response, data=None) - - -def delete(request, course_id, note_id): - ''' - Deletes the annotation object and returns a 204 with no content. - ''' - try: - note = Note.objects.get(id=note_id) - except Note.DoesNotExist: - return ApiResponse(http_response=HttpResponse('', status=404), data=None) - - if note.user.id != request.user.id: - return ApiResponse(http_response=HttpResponse('', status=403), data=None) - - note.delete() - - return ApiResponse(http_response=HttpResponse('', status=204), data=None) - - -def search(request, course_key): - ''' - 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', '') - uri = request.GET.get('uri', '') - - # validate search parameters - if offset.isdigit(): - offset = int(offset) - else: - offset = 0 - - if limit.isdigit(): - limit = int(limit) - if limit == 0 or limit > MAX_LIMIT: - limit = MAX_LIMIT - else: - limit = MAX_LIMIT - - # set filters - filters = {'course_id': course_key, 'user': request.user} - if uri != '': - filters['uri'] = uri - - # retrieve notes - notes = Note.objects.order_by('id').filter(**filters) - total = notes.count() - rows = notes[offset:offset + limit] - result = { - 'total': total, - 'rows': [note.as_dict() for note in rows] - } - - return ApiResponse(http_response=HttpResponse(), data=result) - - -def root(request, course_key): # pylint: disable=unused-argument - ''' - Returns version information about the API. - ''' - return ApiResponse(http_response=HttpResponse(), data=API_SETTINGS.get('META')) diff --git a/lms/djangoapps/notes/models.py b/lms/djangoapps/notes/models.py index f22856d3da..07fff11508 100644 --- a/lms/djangoapps/notes/models.py +++ b/lms/djangoapps/notes/models.py @@ -2,99 +2,3 @@ Notes models """ from __future__ import absolute_import - -import json - -from django.contrib.auth.models import User -from django.core.exceptions import ValidationError -from django.db import models -from django.urls import reverse -from django.utils.html import strip_tags -from opaque_keys.edx.django.models import CourseKeyField -from six import text_type - - -class Note(models.Model): - """ - Stores user Notes for the LMS local Notes service. - - .. pii: Legacy model for an app that edx.org hasn't used since 2013 - .. pii_types: other - .. pii_retirement: retained - """ - - user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) - course_id = CourseKeyField(max_length=255, db_index=True) - uri = models.CharField(max_length=255, db_index=True) - text = models.TextField(default="") - quote = models.TextField(default="") - range_start = models.CharField(max_length=2048) # xpath string - range_start_offset = models.IntegerField() - range_end = models.CharField(max_length=2048) # xpath string - range_end_offset = models.IntegerField() - 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) - - class Meta: - app_label = 'notes' - - def clean(self, json_body): - """ - Cleans the note object or raises a ValidationError. - """ - if json_body is None: - raise ValidationError('Note must have a body.') - - body = json.loads(json_body) - if not isinstance(body, dict): - raise ValidationError('Note body must be a dictionary.') - - # NOTE: all three of these fields should be considered user input - # and may be output back to the user, so we need to sanitize them. - # These fields should only contain _plain text_. - self.uri = strip_tags(body.get('uri', '')) - self.text = strip_tags(body.get('text', '')) - self.quote = strip_tags(body.get('quote', '')) - - ranges = body.get('ranges') - if ranges is None or len(ranges) != 1: - raise ValidationError('Note must contain exactly one range.') - - self.range_start = ranges[0]['start'] - self.range_start_offset = ranges[0]['startOffset'] - self.range_end = ranges[0]['end'] - self.range_end_offset = ranges[0]['endOffset'] - - self.tags = "" - tags = [strip_tags(tag) for tag in body.get('tags', [])] - if len(tags) > 0: - self.tags = ",".join(tags) - - def get_absolute_url(self): - """ - Returns the absolute url for the note object. - """ - kwargs = {'course_id': text_type(self.course_id), 'note_id': str(self.pk)} - return reverse('notes_api_note', kwargs=kwargs) - - def as_dict(self): - """ - Returns the note object as a dictionary. - """ - return { - 'id': self.pk, - 'user_id': self.user.pk, - 'uri': self.uri, - 'text': self.text, - 'quote': self.quote, - 'ranges': [{ - 'start': self.range_start, - 'startOffset': self.range_start_offset, - 'end': self.range_end, - 'endOffset': self.range_end_offset - }], - 'tags': self.tags.split(","), - 'created': str(self.created), - 'updated': str(self.updated) - } diff --git a/lms/djangoapps/notes/tests.py b/lms/djangoapps/notes/tests.py deleted file mode 100644 index 5d35138e51..0000000000 --- a/lms/djangoapps/notes/tests.py +++ /dev/null @@ -1,452 +0,0 @@ -""" -Unit tests for the notes app. -""" - -from __future__ import absolute_import - -import json - -import six -from django.contrib.auth.models import User -from django.core.exceptions import ValidationError -from django.test import RequestFactory, TestCase -from django.test.client import Client -from django.urls import reverse -from mock import Mock, patch -from opaque_keys.edx.locator import CourseLocator -from six import text_type -from six.moves import range - -from courseware.tabs import CourseTab, get_course_tab_list -from notes import api, models, utils -from student.tests.factories import CourseEnrollmentFactory, UserFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory - - -class UtilsTest(ModuleStoreTestCase): - """ Tests for the notes utils. """ - def setUp(self): - ''' - Setup a dummy course-like object with a tabs field that can be - accessed via attribute lookup. - ''' - super(UtilsTest, self).setUp() - self.course = CourseFactory.create() - - def test_notes_not_enabled(self): - ''' - Tests that notes are disabled when the course tab configuration does NOT - contain a tab with type "notes." - ''' - self.assertFalse(utils.notes_enabled_for_course(self.course)) - - def test_notes_enabled(self): - ''' - Tests that notes are enabled when the course tab configuration contains - a tab with type "notes." - ''' - with self.settings(FEATURES={'ENABLE_STUDENT_NOTES': True}): - self.course.advanced_modules = ["notes"] - self.assertTrue(utils.notes_enabled_for_course(self.course)) - - -class CourseTabTest(ModuleStoreTestCase): - """ - Test that the course tab shows up the way we expect. - """ - def setUp(self): - ''' - Setup a dummy course-like object with a tabs field that can be - accessed via attribute lookup. - ''' - super(CourseTabTest, self).setUp() - self.course = CourseFactory.create() - self.user = UserFactory() - CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) - - def enable_notes(self): - """Enable notes and add the tab to the course.""" - self.course.tabs.append(CourseTab.load("notes")) - self.course.advanced_modules = ["notes"] - - def has_notes_tab(self, course, user): - """ Returns true if the current course and user have a notes tab, false otherwise. """ - request = RequestFactory().request() - request.user = user - all_tabs = get_course_tab_list(request, course) - return any([tab.name == u'My Notes' for tab in all_tabs]) - - def test_course_tab_not_visible(self): - # module not enabled in the course - self.assertFalse(self.has_notes_tab(self.course, self.user)) - - with self.settings(FEATURES={'ENABLE_STUDENT_NOTES': False}): - # setting not enabled and the module is not enabled - self.assertFalse(self.has_notes_tab(self.course, self.user)) - - # module is enabled and the setting is not enabled - self.course.advanced_modules = ["notes"] - self.assertFalse(self.has_notes_tab(self.course, self.user)) - - def test_course_tab_visible(self): - self.enable_notes() - self.assertTrue(self.has_notes_tab(self.course, self.user)) - self.course.advanced_modules = [] - self.assertFalse(self.has_notes_tab(self.course, self.user)) - - -class ApiTest(TestCase): - - def setUp(self): - super(ApiTest, self).setUp() - self.client = Client() - - # Mocks - patcher = patch.object(api, 'api_enabled', Mock(return_value=True)) - patcher.start() - self.addCleanup(patcher.stop) - - # 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_key = CourseLocator('HarvardX', 'CB22x', 'The_Ancient_Greek_Hero') - self.note = { - 'user': self.student, - 'course_id': self.course_key, - 'uri': '/', - 'text': 'foo', - 'quote': 'bar', - 'range_start': 0, - 'range_start_offset': 0, - 'range_end': 100, - 'range_end_offset': 0, - 'tags': 'a,b,c' - } - - # Make sure no note with this ID ever exists for testing purposes - self.NOTE_ID_DOES_NOT_EXIST = 99999 - - def login(self, as_student=None): - username = None - password = self.password - - 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': text_type(self.course_key)}) - return reverse(name, kwargs=args) - - def create_notes(self, num_notes, create=True): - notes = [] - for __ in range(num_notes): - note = models.Note(**self.note) - if create: - note.save() - notes.append(note) - return notes - - def test_root(self): - self.login() - - resp = self.client.get(self.url('notes_api_root')) - self.assertEqual(resp.status_code, 200) - self.assertNotEqual(resp.content, '') - - content = json.loads(resp.content.decode('utf-8')) - - self.assertEqual(set(('name', 'version')), set(content.keys())) - self.assertIsInstance(content['version'], int) - self.assertEqual(content['name'], 'Notes API') - - def test_index_empty(self): - self.login() - - resp = self.client.get(self.url('notes_api_notes')) - self.assertEqual(resp.status_code, 200) - self.assertNotEqual(resp.content, '') - - content = json.loads(resp.content.decode('utf-8')) - self.assertEqual(len(content), 0) - - def test_index_with_notes(self): - num_notes = 3 - self.login() - self.create_notes(num_notes) - - resp = self.client.get(self.url('notes_api_notes')) - self.assertEqual(resp.status_code, 200) - self.assertNotEqual(resp.content, '') - - content = json.loads(resp.content.decode('utf-8')) - self.assertIsInstance(content, list) - self.assertEqual(len(content), num_notes) - - def test_index_max_notes(self): - self.login() - - MAX_LIMIT = api.API_SETTINGS.get('MAX_NOTE_LIMIT') - num_notes = MAX_LIMIT + 1 - self.create_notes(num_notes) - - resp = self.client.get(self.url('notes_api_notes')) - self.assertEqual(resp.status_code, 200) - self.assertNotEqual(resp.content, '') - - content = json.loads(resp.content.decode('utf-8')) - 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, 400) - - 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, 400) - - 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.pk})) - self.assertEqual(resp.status_code, 200) - self.assertNotEqual(resp.content, '') - - content = json.loads(resp.content.decode('utf-8')) - self.assertEqual(content['id'], note.pk) - self.assertEqual(content['user_id'], note.user_id) - - def test_note_doesnt_exist_to_read(self): - self.login() - resp = self.client.get(self.url('notes_api_note', { - 'note_id': self.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.pk})) - self.assertEqual(resp.status_code, 403) - self.assertEqual(resp.content, '') - - def test_delete_note(self): - self.login() - - notes = self.create_notes(1) - self.assertEqual(len(notes), 1) - note = notes[0] - - resp = self.client.delete(self.url('notes_api_note', { - 'note_id': note.pk - })) - self.assertEqual(resp.status_code, 204) - self.assertEqual(resp.content, '') - - with self.assertRaises(models.Note.DoesNotExist): - models.Note.objects.get(pk=note.pk) - - def test_note_does_not_exist_to_delete(self): - self.login() - - resp = self.client.delete(self.url('notes_api_note', { - 'note_id': self.NOTE_ID_DOES_NOT_EXIST - })) - self.assertEqual(resp.status_code, 404) - self.assertEqual(resp.content, '') - - def test_student_doesnt_have_permission_to_delete_note(self): - notes = self.create_notes(1) - self.assertEqual(len(notes), 1) - note = notes[0] - - self.login(as_student=self.student2) - resp = self.client.delete(self.url('notes_api_note', { - 'note_id': note.pk - })) - self.assertEqual(resp.status_code, 403) - self.assertEqual(resp.content, '') - - try: - models.Note.objects.get(pk=note.pk) - except models.Note.DoesNotExist: - self.fail('note should exist and not be deleted because the student does not have permission to do so') - - def test_update_note(self): - notes = self.create_notes(1) - note = notes[0] - - updated_dict = note.as_dict() - updated_dict.update({ - 'text': 'itchy and scratchy', - 'tags': ['simpsons', 'cartoons', 'animation'] - }) - - self.login() - resp = self.client.put(self.url('notes_api_note', {'note_id': note.pk}), - json.dumps(updated_dict), - content_type='application/json', - HTTP_X_REQUESTED_WITH='XMLHttpRequest') - self.assertEqual(resp.status_code, 303) - self.assertEqual(resp.content, '') - - actual = models.Note.objects.get(pk=note.pk) - actual_dict = actual.as_dict() - for field in ['text', 'tags']: - self.assertEqual(actual_dict[field], updated_dict[field]) - - def test_search_note_params(self): - self.login() - - total = 3 - notes = self.create_notes(total) - invalid_uri = ''.join([note.uri for note in notes]) - - tests = [{'limit': 0, 'offset': 0, 'expected_rows': total}, - {'limit': 0, 'offset': 2, 'expected_rows': total - 2}, - {'limit': 0, 'offset': total, 'expected_rows': 0}, - {'limit': 1, 'offset': 0, 'expected_rows': 1}, - {'limit': 2, 'offset': 0, 'expected_rows': 2}, - {'limit': total, 'offset': 2, 'expected_rows': 1}, - {'limit': total, 'offset': total, 'expected_rows': 0}, - {'limit': total + 1, 'offset': total + 1, 'expected_rows': 0}, - {'limit': total + 1, 'offset': 0, 'expected_rows': total}, - {'limit': 0, 'offset': 0, 'uri': invalid_uri, 'expected_rows': 0, 'expected_total': 0}] - - for test in tests: - params = dict([(k, str(test[k])) - for k in ('limit', 'offset', 'uri') - if k in test]) - resp = self.client.get(self.url('notes_api_search'), - params, - content_type='application/json', - HTTP_X_REQUESTED_WITH='XMLHttpRequest') - - self.assertEqual(resp.status_code, 200) - self.assertNotEqual(resp.content, '') - - content = json.loads(resp.content.decode('utf-8')) - - for expected_key in ('total', 'rows'): - self.assertIn(expected_key, content) - - if 'expected_total' in test: - self.assertEqual(content['total'], test['expected_total']) - else: - self.assertEqual(content['total'], total) - - self.assertEqual(len(content['rows']), test['expected_rows']) - - for row in content['rows']: - self.assertIn('id', row) - - -class NoteTest(TestCase): - def setUp(self): - super(NoteTest, self).setUp() - - self.password = 'abc' - self.student = User.objects.create_user('student', 'student@test.com', self.password) - self.course_key = CourseLocator('HarvardX', 'CB22x', 'The_Ancient_Greek_Hero') - self.note = { - 'user': self.student, - 'course_id': self.course_key, - 'uri': '/', - 'text': 'foo', - 'quote': 'bar', - 'range_start': 0, - 'range_start_offset': 0, - 'range_end': 100, - 'range_end_offset': 0, - 'tags': 'a,b,c' - } - - def test_clean_valid_note(self): - reference_note = models.Note(**self.note) - body = reference_note.as_dict() - - note = models.Note(course_id=self.course_key, user=self.student) - try: - note.clean(json.dumps(body)) - self.assertEqual(note.uri, body['uri']) - self.assertEqual(note.text, body['text']) - self.assertEqual(note.quote, body['quote']) - self.assertEqual(note.range_start, body['ranges'][0]['start']) - self.assertEqual(note.range_start_offset, body['ranges'][0]['startOffset']) - self.assertEqual(note.range_end, body['ranges'][0]['end']) - self.assertEqual(note.range_end_offset, body['ranges'][0]['endOffset']) - self.assertEqual(note.tags, ','.join(body['tags'])) - except ValidationError: - self.fail('a valid note should not raise an exception') - - def test_clean_invalid_note(self): - note = models.Note(course_id=self.course_key, user=self.student) - for empty_type in (None, '', 0, []): - with self.assertRaises(ValidationError): - note.clean(None) - - with self.assertRaises(ValidationError): - note.clean(json.dumps({ - 'text': 'foo', - 'quote': 'bar', - 'ranges': [{} for __ in range(10)] # too many ranges - })) - - def test_as_dict(self): - note = models.Note(course_id=self.course_key, user=self.student) - d = note.as_dict() - self.assertNotIsInstance(d, six.string_types) - self.assertEqual(d['user_id'], self.student.id) - self.assertNotIn('course_id', d) diff --git a/lms/djangoapps/notes/urls.py b/lms/djangoapps/notes/urls.py deleted file mode 100644 index 4e4623a229..0000000000 --- a/lms/djangoapps/notes/urls.py +++ /dev/null @@ -1,17 +0,0 @@ -""" -URL definitions for the notes app -""" - -from __future__ import absolute_import - -from django.conf.urls import url - -from notes.api import api_request - -id_regex = r"(?P[0-9A-Fa-f]+)" -urlpatterns = [ - 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 deleted file mode 100644 index 6ce8344725..0000000000 --- a/lms/djangoapps/notes/utils.py +++ /dev/null @@ -1,22 +0,0 @@ -""" -Notes utilities -""" -from __future__ import absolute_import - -from django.conf import settings - - -def notes_enabled_for_course(course): - - ''' - 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 FEATURES. - 2) present in the course tab configuration. - ''' - - tab_found = "notes" in course.advanced_modules - feature_enabled = settings.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 deleted file mode 100644 index 6489689b63..0000000000 --- a/lms/djangoapps/notes/views.py +++ /dev/null @@ -1,56 +0,0 @@ -""" -Views to support the edX Notes feature. -""" - -from __future__ import absolute_import - -from django.conf import settings -from django.contrib.auth.decorators import login_required -from django.http import Http404 -from django.utils.translation import ugettext_noop -from opaque_keys.edx.keys import CourseKey - -from courseware.courses import get_course_with_access -from courseware.tabs import EnrolledTab -from edxmako.shortcuts import render_to_response -from notes.models import Note -from notes.utils import notes_enabled_for_course - - -@login_required -def notes(request, course_id): - ''' Displays the student's notes. ''' - course_key = CourseKey.from_string(course_id) - course = get_course_with_access(request.user, 'load', course_key) - if not notes_enabled_for_course(course): - raise Http404 - - notes = Note.objects.filter(course_id=course_key, user=request.user).order_by('-created', 'uri') - - student = request.user - storage = course.annotation_storage_url - context = { - 'course': course, - 'notes': notes, - 'student': student, - 'storage': storage, - 'token': None, - 'default_tab': 'myNotes', - } - - return render_to_response('notes.html', context) - - -class NotesTab(EnrolledTab): - """ - A tab for the course notes. - """ - type = 'notes' - title = ugettext_noop("My Notes") - view_name = "notes" - - @classmethod - def is_enabled(cls, course, user=None): - if not super(NotesTab, cls).is_enabled(course, user): - return False - return settings.FEATURES.get('ENABLE_STUDENT_NOTES') and "notes" in course.advanced_modules diff --git a/lms/templates/notes.html b/lms/templates/notes.html deleted file mode 100644 index 266f8afca0..0000000000 --- a/lms/templates/notes.html +++ /dev/null @@ -1,234 +0,0 @@ -<%page expression_filter="h"/> -<%inherit file="main.html" /> -<%namespace name='static' file='static_content.html'/> -${static.css(group='style-vendor-tinymce-content', raw=True)} -${static.css(group='style-vendor-tinymce-skin', raw=True)} -${static.css(group='style-xmodule-annotations', raw=True)} -<%! -from django.utils.translation import ugettext as _ -from openedx.core.djangolib.js_utils import js_escaped_string -from django.urls import reverse -%> - -<%block name="headextra"> -<%static:css group='style-course-vendor'/> -<%static:css group='style-course'/> -<%static:js group='courseware'/> - - - - - -<%block name="js_extra"> - - - -<%include file="/courseware/course_navigation.html" args="active_page='notes'" /> - -
-
-

${_('My Notes')}

-
-
-
${_('You do not have any notes.')}
-
- -
-
- diff --git a/lms/urls.py b/lms/urls.py index 36269c671d..f2b4ce6f88 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -27,7 +27,6 @@ from lms.djangoapps.instructor.views import coupons as instructor_coupons_views from lms.djangoapps.instructor.views import instructor_dashboard as instructor_dashboard_views from lms.djangoapps.instructor.views import registration_codes as instructor_registration_codes_views from lms.djangoapps.instructor_task import views as instructor_task_views -from notes import views as notes_views from openedx.core.djangoapps.auth_exchange.views import LoginWithAccessTokenView from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.common_views.xblock import xblock_resource @@ -577,19 +576,6 @@ urlpatterns += [ verified_track_content_views.cohorting_settings, name='verified_track_cohorting', ), - url( - r'^courses/{}/notes$'.format( - settings.COURSE_ID_PATTERN, - ), - notes_views.notes, - name='notes', - ), - url( - r'^courses/{}/notes/'.format( - settings.COURSE_ID_PATTERN, - ), - include('notes.urls') - ), # LTI endpoints listing url( diff --git a/setup.py b/setup.py index 6b58c7df51..316ba828f5 100644 --- a/setup.py +++ b/setup.py @@ -27,7 +27,6 @@ setup( "external_link = lms.djangoapps.courseware.tabs:ExternalLinkCourseTab", "html_textbooks = lms.djangoapps.courseware.tabs:HtmlTextbookTabs", "instructor = lms.djangoapps.instructor.views.instructor_dashboard:InstructorDashboardTab", - "notes = lms.djangoapps.notes.views:NotesTab", "pdf_textbooks = lms.djangoapps.courseware.tabs:PDFTextbookTabs", "progress = lms.djangoapps.courseware.tabs:ProgressTab", "static_tab = xmodule.tabs:StaticTab",