Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #11363 from edx/notes-pagination

Browse Source 
Notes pagination
This commit is contained in:
Muzaffar yousaf
2016-03-10 17:26:54 +05:00
parent 656913c716 e1a13a1493
commit 770a45b720
41 changed files with 2053 additions and 589 deletions
Download Patch File Download Diff File Expand all files Collapse all files

227
lms/djangoapps/edxnotes/helpers.py
View File

@@ -1,11 +1,12 @@
"""
Helper methods related to EdxNotes.
"""
import json
import logging
from json import JSONEncoder
from uuid import uuid4
import urlparse
from urllib import urlencode
import requests
from datetime import datetime
@@ -19,7 +20,7 @@ from django.core.urlresolvers import reverse
from django.utils.translation import ugettext as _
from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable
from capa.util import sanitize_html
from edxnotes.plugins import EdxNotesTab
from courseware.views import get_current_child
from courseware.access import has_access
from openedx.core.lib.token_utils import get_id_token
@@ -30,10 +31,10 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
log = logging.getLogger(__name__)
HIGHLIGHT_TAG = "span"
HIGHLIGHT_CLASS = "note-highlight"
# OAuth2 Client name for edxnotes
CLIENT_NAME = "edx-notes"
DEFAULT_PAGE = 1
DEFAULT_PAGE_SIZE = 25
class NoteJSONEncoder(JSONEncoder):
@@ -63,22 +64,33 @@ def get_token_url(course_id):
})
def send_request(user, course_id, path="", query_string=None):
def send_request(user, course_id, page, page_size, path="", text=None):
"""
Sends a request with appropriate parameters and headers.
Sends a request to notes api with appropriate parameters and headers.
Arguments:
user: Current logged in user
course_id: Course id
page: requested or default page number
page_size: requested or default page size
path: `search` or `annotations`. This is used to calculate notes api endpoint.
text: text to search.
Returns:
Response received from notes api
"""
url = get_internal_endpoint(path)
params = {
"user": anonymous_id_for_user(user, None),
"course_id": unicode(course_id).encode("utf-8"),
"page": page,
"page_size": page_size,
}
if query_string:
if text:
params.update({
"text": query_string,
"highlight": True,
"highlight_tag": HIGHLIGHT_TAG,
"highlight_class": HIGHLIGHT_CLASS,
"text": text,
"highlight": True
})
try:
@@ -90,6 +102,7 @@ def send_request(user, course_id, path="", query_string=None):
params=params
)
except RequestException:
log.error("Failed to connect to edx-notes-api: url=%s, params=%s", url, str(params))
raise EdxNotesServiceUnavailable(_("EdxNotes Service is unavailable. Please try again in a few minutes."))
return response
@@ -125,15 +138,13 @@ def preprocess_collection(user, course, collection):
store = modulestore()
filtered_collection = list()
cache = {}
include_path_info = ('course_structure' not in settings.NOTES_DISABLED_TABS)
with store.bulk_operations(course.id):
for model in collection:
update = {
u"text": sanitize_html(model["text"]),
u"quote": sanitize_html(model["quote"]),
u"updated": dateutil_parse(model["updated"]),
}
if "tags" in model:
update[u"tags"] = [sanitize_html(tag) for tag in model["tags"]]
model.update(update)
usage_id = model["usage_id"]
if usage_id in cache:
@@ -160,42 +171,46 @@ def preprocess_collection(user, course, collection):
log.debug("Unit not found: %s", usage_key)
continue
section = unit.get_parent()
if not section:
log.debug("Section not found: %s", usage_key)
continue
if section in cache:
usage_context = cache[section]
usage_context.update({
"unit": get_module_context(course, unit),
})
model.update(usage_context)
cache[usage_id] = cache[unit] = usage_context
filtered_collection.append(model)
continue
if include_path_info:
section = unit.get_parent()
if not section:
log.debug("Section not found: %s", usage_key)
continue
if section in cache:
usage_context = cache[section]
usage_context.update({
"unit": get_module_context(course, unit),
})
model.update(usage_context)
cache[usage_id] = cache[unit] = usage_context
filtered_collection.append(model)
continue
chapter = section.get_parent()
if not chapter:
log.debug("Chapter not found: %s", usage_key)
continue
if chapter in cache:
usage_context = cache[chapter]
usage_context.update({
"unit": get_module_context(course, unit),
"section": get_module_context(course, section),
})
model.update(usage_context)
cache[usage_id] = cache[unit] = cache[section] = usage_context
filtered_collection.append(model)
continue
chapter = section.get_parent()
if not chapter:
log.debug("Chapter not found: %s", usage_key)
continue
if chapter in cache:
usage_context = cache[chapter]
usage_context.update({
"unit": get_module_context(course, unit),
"section": get_module_context(course, section),
})
model.update(usage_context)
cache[usage_id] = cache[unit] = cache[section] = usage_context
filtered_collection.append(model)
continue
usage_context = {
"unit": get_module_context(course, unit),
"section": get_module_context(course, section),
"chapter": get_module_context(course, chapter),
"section": get_module_context(course, section) if include_path_info else {},
"chapter": get_module_context(course, chapter) if include_path_info else {},
}
model.update(usage_context)
cache[usage_id] = cache[unit] = cache[section] = cache[chapter] = usage_context
if include_path_info:
cache[section] = cache[chapter] = usage_context
cache[usage_id] = cache[unit] = usage_context
filtered_collection.append(model)
return filtered_collection
@@ -239,39 +254,97 @@ def get_index(usage_key, children):
return children.index(usage_key)
def search(user, course, query_string):
def construct_pagination_urls(request, course_id, api_next_url, api_previous_url):
"""
Returns search results for the `query_string(str)`.
Construct next and previous urls for LMS. `api_next_url` and `api_previous_url`
are returned from notes api but we need to transform them according to LMS notes
views by removing and replacing extra information.
Arguments:
request: HTTP request object
course_id: course id
api_next_url: notes api next url
api_previous_url: notes api previous url
Returns:
next_url: lms notes next url
previous_url: lms notes previous url
"""
response = send_request(user, course.id, "search", query_string)
try:
content = json.loads(response.content)
collection = content["rows"]
except (ValueError, KeyError):
log.warning("invalid JSON: %s", response.content)
raise EdxNotesParseError(_("Server error. Please try again in a few minutes."))
def lms_url(url):
"""
Create lms url from api url.
"""
if url is None:
return None
content.update({
"rows": preprocess_collection(user, course, collection)
})
keys = ('page', 'page_size', 'text')
parsed = urlparse.urlparse(url)
query_params = urlparse.parse_qs(parsed.query)
return json.dumps(content, cls=NoteJSONEncoder)
encoded_query_params = urlencode({key: query_params.get(key)[0] for key in keys if key in query_params})
return "{}?{}".format(request.build_absolute_uri(base_url), encoded_query_params)
base_url = reverse("notes", kwargs={"course_id": course_id})
next_url = lms_url(api_next_url)
previous_url = lms_url(api_previous_url)
return next_url, previous_url
def get_notes(user, course):
def get_notes(request, course, page=DEFAULT_PAGE, page_size=DEFAULT_PAGE_SIZE, text=None):
"""
Returns all notes for the user.
Returns paginated list of notes for the user.
Arguments:
request: HTTP request object
course: Course descriptor
page: requested or default page number
page_size: requested or default page size
text: text to search. If None then return all results for the current logged in user.
Returns:
Paginated dictionary with these key:
start: start of the current page
current_page: current page number
next: url for next page
previous: url for previous page
count: total number of notes available for the sent query
num_pages: number of pages available
results: list with notes info dictionary. each item in this list will be a dict
"""
response = send_request(user, course.id, "annotations")
path = 'search' if text else 'annotations'
response = send_request(request.user, course.id, page, page_size, path, text)
try:
collection = json.loads(response.content)
except ValueError:
return None
log.error("Invalid JSON response received from notes api: response_content=%s", response.content)
raise EdxNotesParseError(_("Invalid JSON response received from notes api."))
if not collection:
return None
# Verify response dict structure
expected_keys = ['total', 'rows', 'num_pages', 'start', 'next', 'previous', 'current_page']
keys = collection.keys()
if not keys or not all(key in expected_keys for key in keys):
log.error("Incorrect data received from notes api: collection_data=%s", str(collection))
raise EdxNotesParseError(_("Incorrect data received from notes api."))
return json.dumps(preprocess_collection(user, course, collection), cls=NoteJSONEncoder)
filtered_results = preprocess_collection(request.user, course, collection['rows'])
# Notes API is called from:
# 1. The annotatorjs in courseware. It expects these attributes to be named "total" and "rows".
# 2. The Notes tab Javascript proxied through LMS. It expects these attributes to be called "count" and "results".
collection['count'] = collection['total']
del collection['total']
collection['results'] = filtered_results
del collection['rows']
collection['next'], collection['previous'] = construct_pagination_urls(
request,
course.id,
collection['next'],
collection['previous']
)
return collection
def get_endpoint(api_url, path=""):
@@ -354,26 +427,6 @@ def generate_uid():
def is_feature_enabled(course):
"""
Returns True if Student Notes feature is enabled for the course,
False otherwise.
In order for the application to be enabled it must be:
1) enabled globally via FEATURES.
2) present in the course tab configuration.
3) Harvard Annotation Tool must be disabled for the course.
Returns True if Student Notes feature is enabled for the course, False otherwise.
"""
return (settings.FEATURES.get("ENABLE_EDXNOTES")
and [t for t in course.tabs if t["type"] == "edxnotes"] # tab found
and not is_harvard_notes_enabled(course))
def is_harvard_notes_enabled(course):
"""
Returns True if Harvard Annotation Tool is enabled for the course,
False otherwise.
Checks for 'textannotation', 'imageannotation', 'videoannotation' in the list
of advanced modules of the course.
"""
modules = set(['textannotation', 'imageannotation', 'videoannotation'])
return bool(modules.intersection(course.advanced_modules))
return EdxNotesTab.is_enabled(course)

19
lms/djangoapps/edxnotes/plugins.py
View File

@@ -1,9 +1,8 @@
"""
Registers the "edX Notes" feature for the edX platform.
"""
from django.conf import settings
from django.utils.translation import ugettext_noop
from courseware.tabs import EnrolledTab
@@ -27,4 +26,20 @@ class EdxNotesTab(EnrolledTab):
"""
if not super(EdxNotesTab, cls).is_enabled(course, user=user):
return False
if not settings.FEATURES.get("ENABLE_EDXNOTES") or is_harvard_notes_enabled(course):
return False
return course.edxnotes
def is_harvard_notes_enabled(course):
"""
Returns True if Harvard Annotation Tool is enabled for the course,
False otherwise.
Checks for 'textannotation', 'imageannotation', 'videoannotation' in the list
of advanced modules of the course.
"""
modules = set(['textannotation', 'imageannotation', 'videoannotation'])
return bool(modules.intersection(course.advanced_modules))

514
lms/djangoapps/edxnotes/tests.py
View File

@@ -8,11 +8,13 @@ import jwt
from mock import patch, MagicMock
from unittest import skipUnless
from datetime import datetime
import urlparse
from edxmako.shortcuts import render_to_string
from edxnotes import helpers
from edxnotes.decorators import edxnotes
from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable
from edxnotes.plugins import EdxNotesTab
from django.conf import settings
from django.core.urlresolvers import reverse
from django.core.exceptions import ImproperlyConfigured
@@ -31,6 +33,29 @@ from courseware.tabs import get_course_tab_list
from student.tests.factories import UserFactory, CourseEnrollmentFactory
FEATURES = settings.FEATURES.copy()
NOTES_API_EMPTY_RESPONSE = {
"total": 0,
"rows": [],
"current_page": 1,
"start": 0,
"next": None,
"previous": None,
"num_pages": 0,
}
NOTES_VIEW_EMPTY_RESPONSE = {
"count": 0,
"results": [],
"current_page": 1,
"start": 0,
"next": None,
"previous": None,
"num_pages": 0,
}
def enable_edxnotes_for_the_course(course, user_id):
"""
Enable EdxNotes for the course.
@@ -118,6 +143,7 @@ class EdxNotesDecoratorTest(ModuleStoreTestCase):
Tests that get_html is wrapped when feature flag is on, but edxnotes are
disabled for the course.
"""
self.course.edxnotes = False
self.assertEqual("original_get_html", self.problem.get_html())
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": False})
@@ -191,6 +217,9 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
self.user = UserFactory.create(username="Joe", email="joe@example.com", password="edx")
self.client.login(username=self.user.username, password="edx")
self.request = RequestFactory().request()
self.request.user = self.user
def _get_unit_url(self, course, chapter, section, position=1):
"""
Returns `jump_to_id` url for the `vertical`.
@@ -202,14 +231,6 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
'position': position,
})
def test_edxnotes_not_enabled(self):
"""
Tests that edxnotes are disabled when the course tab configuration does NOT
contain a tab with type "edxnotes."
"""
self.course.tabs = []
self.assertFalse(helpers.is_feature_enabled(self.course))
def test_edxnotes_harvard_notes_enabled(self):
"""
Tests that edxnotes are disabled when Harvard Annotation Tool is enabled.
@@ -226,15 +247,17 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
self.course.advanced_modules = ["textannotation", "videoannotation", "imageannotation"]
self.assertFalse(helpers.is_feature_enabled(self.course))
def test_edxnotes_enabled(self):
@ddt.unpack
@ddt.data(
{'_edxnotes': True},
{'_edxnotes': False}
)
def test_is_feature_enabled(self, _edxnotes):
"""
Tests that edxnotes are enabled when the course tab configuration contains
a tab with type "edxnotes."
Tests that is_feature_enabled shows correct behavior.
"""
self.course.tabs = [{"type": "foo"},
{"name": "Notes", "type": "edxnotes"},
{"type": "bar"}]
self.assertTrue(helpers.is_feature_enabled(self.course))
self.course.edxnotes = _edxnotes
self.assertEqual(helpers.is_feature_enabled(self.course), _edxnotes)
@ddt.data(
helpers.get_public_endpoint,
@@ -280,71 +303,91 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
"""
Tests the result if correct data is received.
"""
mock_get.return_value.content = json.dumps([
mock_get.return_value.content = json.dumps(
{
u"quote": u"quote text",
u"text": u"text",
u"usage_id": unicode(self.html_module_1.location),
u"updated": datetime(2014, 11, 19, 8, 5, 16, 00000).isoformat(),
},
{
u"quote": u"quote text",
u"text": u"text",
u"usage_id": unicode(self.html_module_2.location),
u"updated": datetime(2014, 11, 19, 8, 6, 16, 00000).isoformat(),
"total": 2,
"current_page": 1,
"start": 0,
"next": None,
"previous": None,
"num_pages": 1,
"rows": [
{
u"quote": u"quote text",
u"text": u"text",
u"usage_id": unicode(self.html_module_1.location),
u"updated": datetime(2014, 11, 19, 8, 5, 16, 00000).isoformat(),
},
{
u"quote": u"quote text",
u"text": u"text",
u"usage_id": unicode(self.html_module_2.location),
u"updated": datetime(2014, 11, 19, 8, 6, 16, 00000).isoformat(),
}
]
}
])
)
self.assertItemsEqual(
[
{
u"quote": u"quote text",
u"text": u"text",
u"chapter": {
u"display_name": self.chapter.display_name_with_default_escaped,
u"index": 0,
u"location": unicode(self.chapter.location),
u"children": [unicode(self.sequential.location)]
{
"count": 2,
"current_page": 1,
"start": 0,
"next": None,
"previous": None,
"num_pages": 1,
"results": [
{
u"quote": u"quote text",
u"text": u"text",
u"chapter": {
u"display_name": self.chapter.display_name_with_default_escaped,
u"index": 0,
u"location": unicode(self.chapter.location),
u"children": [unicode(self.sequential.location)]
},
u"section": {
u"display_name": self.sequential.display_name_with_default_escaped,
u"location": unicode(self.sequential.location),
u"children": [
unicode(self.vertical.location), unicode(self.vertical_with_container.location)
]
},
u"unit": {
u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
u"display_name": self.vertical.display_name_with_default_escaped,
u"location": unicode(self.vertical.location),
},
u"usage_id": unicode(self.html_module_2.location),
u"updated": "Nov 19, 2014 at 08:06 UTC",
},
u"section": {
u"display_name": self.sequential.display_name_with_default_escaped,
u"location": unicode(self.sequential.location),
u"children": [unicode(self.vertical.location), unicode(self.vertical_with_container.location)]
{
u"quote": u"quote text",
u"text": u"text",
u"chapter": {
u"display_name": self.chapter.display_name_with_default_escaped,
u"index": 0,
u"location": unicode(self.chapter.location),
u"children": [unicode(self.sequential.location)]
},
u"section": {
u"display_name": self.sequential.display_name_with_default_escaped,
u"location": unicode(self.sequential.location),
u"children": [
unicode(self.vertical.location),
unicode(self.vertical_with_container.location)]
},
u"unit": {
u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
u"display_name": self.vertical.display_name_with_default_escaped,
u"location": unicode(self.vertical.location),
},
u"usage_id": unicode(self.html_module_1.location),
u"updated": "Nov 19, 2014 at 08:05 UTC",
},
u"unit": {
u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
u"display_name": self.vertical.display_name_with_default_escaped,
u"location": unicode(self.vertical.location),
},
u"usage_id": unicode(self.html_module_2.location),
u"updated": "Nov 19, 2014 at 08:06 UTC",
},
{
u"quote": u"quote text",
u"text": u"text",
u"chapter": {
u"display_name": self.chapter.display_name_with_default_escaped,
u"index": 0,
u"location": unicode(self.chapter.location),
u"children": [unicode(self.sequential.location)]
},
u"section": {
u"display_name": self.sequential.display_name_with_default_escaped,
u"location": unicode(self.sequential.location),
u"children": [
unicode(self.vertical.location),
unicode(self.vertical_with_container.location)]
},
u"unit": {
u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
u"display_name": self.vertical.display_name_with_default_escaped,
u"location": unicode(self.vertical.location),
},
u"usage_id": unicode(self.html_module_1.location),
u"updated": "Nov 19, 2014 at 08:05 UTC",
},
],
json.loads(helpers.get_notes(self.user, self.course))
]
},
helpers.get_notes(self.request, self.course)
)
@patch("edxnotes.helpers.requests.get", autospec=True)
@@ -353,15 +396,15 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
Tests the result if incorrect json is received.
"""
mock_get.return_value.content = "Error"
self.assertIsNone(helpers.get_notes(self.user, self.course))
self.assertRaises(EdxNotesParseError, helpers.get_notes, self.request, self.course)
@patch("edxnotes.helpers.requests.get", autospec=True)
def test_get_notes_empty_collection(self, mock_get):
"""
Tests the result if an empty collection is received.
Tests the result if an empty response is received.
"""
mock_get.return_value.content = json.dumps([])
self.assertIsNone(helpers.get_notes(self.user, self.course))
mock_get.return_value.content = json.dumps({})
self.assertRaises(EdxNotesParseError, helpers.get_notes, self.request, self.course)
@patch("edxnotes.helpers.requests.get", autospec=True)
def test_search_correct_data(self, mock_get):
@@ -370,6 +413,11 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
"""
mock_get.return_value.content = json.dumps({
"total": 2,
"current_page": 1,
"start": 0,
"next": None,
"previous": None,
"num_pages": 1,
"rows": [
{
u"quote": u"quote text",
@@ -388,8 +436,13 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
self.assertItemsEqual(
{
"total": 2,
"rows": [
"count": 2,
"current_page": 1,
"start": 0,
"next": None,
"previous": None,
"num_pages": 1,
"results": [
{
u"quote": u"quote text",
u"text": u"text",
@@ -440,7 +493,7 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
},
]
},
json.loads(helpers.search(self.user, self.course, "test"))
helpers.get_notes(self.request, self.course)
)
@patch("edxnotes.helpers.requests.get", autospec=True)
@@ -449,7 +502,7 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
Tests the result if incorrect json is received.
"""
mock_get.return_value.content = "Error"
self.assertRaises(EdxNotesParseError, helpers.search, self.user, self.course, "test")
self.assertRaises(EdxNotesParseError, helpers.get_notes, self.request, self.course)
@patch("edxnotes.helpers.requests.get", autospec=True)
def test_search_wrong_data_format(self, mock_get):
@@ -457,60 +510,17 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
Tests the result if incorrect data structure is received.
"""
mock_get.return_value.content = json.dumps({"1": 2})
self.assertRaises(EdxNotesParseError, helpers.search, self.user, self.course, "test")
self.assertRaises(EdxNotesParseError, helpers.get_notes, self.request, self.course)
@patch("edxnotes.helpers.requests.get", autospec=True)
def test_search_empty_collection(self, mock_get):
"""
Tests no results.
"""
mock_get.return_value.content = json.dumps({
"total": 0,
"rows": []
})
mock_get.return_value.content = json.dumps(NOTES_API_EMPTY_RESPONSE)
self.assertItemsEqual(
{
"total": 0,
"rows": []
},
json.loads(helpers.search(self.user, self.course, "test"))
)
def test_preprocess_collection_escaping(self):
"""
Tests the result if appropriate module is not found.
"""
initial_collection = [{
u"quote": u"test <script>alert('test')</script>",
u"text": u"text \"<>&'",
u"usage_id": unicode(self.html_module_1.location),
u"updated": datetime(2014, 11, 19, 8, 5, 16, 00000).isoformat()
}]
self.assertItemsEqual(
[{
u"quote": u"test &lt;script&gt;alert('test')&lt;/script&gt;",
u"text": u'text "&lt;&gt;&amp;\'',
u"chapter": {
u"display_name": self.chapter.display_name_with_default_escaped,
u"index": 0,
u"location": unicode(self.chapter.location),
u"children": [unicode(self.sequential.location)]
},
u"section": {
u"display_name": self.sequential.display_name_with_default_escaped,
u"location": unicode(self.sequential.location),
u"children": [unicode(self.vertical.location), unicode(self.vertical_with_container.location)]
},
u"unit": {
u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
u"display_name": self.vertical.display_name_with_default_escaped,
u"location": unicode(self.vertical.location),
},
u"usage_id": unicode(self.html_module_1.location),
u"updated": datetime(2014, 11, 19, 8, 5, 16, 00000),
}],
helpers.preprocess_collection(self.user, self.course, initial_collection)
NOTES_VIEW_EMPTY_RESPONSE,
helpers.get_notes(self.request, self.course)
)
def test_preprocess_collection_no_item(self):
@@ -625,6 +635,59 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
[], helpers.preprocess_collection(self.user, self.course, initial_collection)
)
@override_settings(NOTES_DISABLED_TABS=['course_structure', 'tags'])
def test_preprocess_collection_with_disabled_tabs(self, ):
"""
Tests that preprocess collection returns correct data if `course_structure` and `tags` are disabled.
"""
initial_collection = [
{
u"quote": u"quote text1",
u"text": u"text1",
u"usage_id": unicode(self.html_module_1.location),
u"updated": datetime(2016, 01, 26, 8, 5, 16, 00000).isoformat(),
},
{
u"quote": u"quote text2",
u"text": u"text2",
u"usage_id": unicode(self.html_module_2.location),
u"updated": datetime(2016, 01, 26, 9, 6, 17, 00000).isoformat(),
},
]
self.assertItemsEqual(
[
{
'section': {},
'chapter': {},
"unit": {
u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
u"display_name": self.vertical.display_name_with_default_escaped,
u"location": unicode(self.vertical.location),
},
u'text': u'text1',
u'quote': u'quote text1',
u'usage_id': unicode(self.html_module_1.location),
u'updated': datetime(2016, 01, 26, 8, 5, 16)
},
{
'section': {},
'chapter': {},
"unit": {
u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
u"display_name": self.vertical.display_name_with_default_escaped,
u"location": unicode(self.vertical.location),
},
u'text': u'text2',
u'quote': u'quote text2',
u'usage_id': unicode(self.html_module_2.location),
u'updated': datetime(2016, 01, 26, 9, 6, 17)
}
],
helpers.preprocess_collection(self.user, self.course, initial_collection)
)
def test_get_parent_unit(self):
"""
Tests `get_parent_unit` method for the successful result.
@@ -693,14 +756,19 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
@patch("edxnotes.helpers.anonymous_id_for_user", autospec=True)
@patch("edxnotes.helpers.get_edxnotes_id_token", autospec=True)
@patch("edxnotes.helpers.requests.get", autospec=True)
def test_send_request_with_query_string(self, mock_get, mock_get_id_token, mock_anonymous_id_for_user):
def test_send_request_with_text_param(self, mock_get, mock_get_id_token, mock_anonymous_id_for_user):
"""
Tests that requests are send with correct information.
"""
mock_get_id_token.return_value = "test_token"
mock_anonymous_id_for_user.return_value = "anonymous_id"
helpers.send_request(
self.user, self.course.id, path="test", query_string="text"
self.user,
self.course.id,
path="test",
text="text",
page=helpers.DEFAULT_PAGE,
page_size=helpers.DEFAULT_PAGE_SIZE
)
mock_get.assert_called_with(
"http://example.com/test/",
@@ -712,8 +780,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
"course_id": unicode(self.course.id),
"text": "text",
"highlight": True,
"highlight_tag": "span",
"highlight_class": "note-highlight",
'page': 1,
'page_size': 25,
}
)
@@ -722,14 +790,14 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
@patch("edxnotes.helpers.anonymous_id_for_user", autospec=True)
@patch("edxnotes.helpers.get_edxnotes_id_token", autospec=True)
@patch("edxnotes.helpers.requests.get", autospec=True)
def test_send_request_without_query_string(self, mock_get, mock_get_id_token, mock_anonymous_id_for_user):
def test_send_request_without_text_param(self, mock_get, mock_get_id_token, mock_anonymous_id_for_user):
"""
Tests that requests are send with correct information.
"""
mock_get_id_token.return_value = "test_token"
mock_anonymous_id_for_user.return_value = "anonymous_id"
helpers.send_request(
self.user, self.course.id, path="test"
self.user, self.course.id, path="test", page=1, page_size=25
)
mock_get.assert_called_with(
"http://example.com/test/",
@@ -739,6 +807,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
params={
"user": "anonymous_id",
"course_id": unicode(self.course.id),
'page': helpers.DEFAULT_PAGE,
'page_size': helpers.DEFAULT_PAGE_SIZE,
}
)
@@ -808,8 +878,69 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
self.assertEqual(0, helpers.get_index(unicode(self.vertical.location), children))
self.assertEqual(1, helpers.get_index(unicode(self.vertical_with_container.location), children))
@ddt.unpack
@ddt.data(
{'previous_api_url': None, 'next_api_url': None},
{'previous_api_url': None, 'next_api_url': 'edxnotes/?course_id=abc&page=2&page_size=10&user=123'},
{'previous_api_url': 'edxnotes.org/?course_id=abc&page=2&page_size=10&user=123', 'next_api_url': None},
{
'previous_api_url': 'edxnotes.org/?course_id=abc&page_size=10&user=123',
'next_api_url': 'edxnotes.org/?course_id=abc&page=3&page_size=10&user=123'
},
{
'previous_api_url': 'edxnotes.org/?course_id=abc&page=2&page_size=10&text=wow&user=123',
'next_api_url': 'edxnotes.org/?course_id=abc&page=4&page_size=10&text=wow&user=123'
},
)
def test_construct_url(self, previous_api_url, next_api_url):
"""
Verify that `construct_url` works correctly.
"""
# make absolute url
# pylint: disable=no-member
if self.request.is_secure():
host = 'https://' + self.request.get_host()
else:
host = 'http://' + self.request.get_host()
notes_url = host + reverse("notes", args=[unicode(self.course.id)])
def verify_url(constructed, expected):
"""
Verify that constructed url is correct.
"""
# if api url is None then constructed url should also be None
if expected is None:
self.assertEqual(expected, constructed)
else:
# constructed url should startswith notes view url instead of api view url
self.assertTrue(constructed.startswith(notes_url))
# constructed url should not contain extra params
self.assertNotIn('user', constructed)
# constructed url should only has these params if present in api url
allowed_params = ('page', 'page_size', 'text')
# extract query params from constructed url
parsed = urlparse.urlparse(constructed)
params = urlparse.parse_qs(parsed.query)
# verify that constructed url has only correct params and params have correct values
for param, value in params.items():
self.assertIn(param, allowed_params)
self.assertIn('{}={}'.format(param, value[0]), expected)
next_url, previous_url = helpers.construct_pagination_urls(
self.request,
self.course.id,
next_api_url, previous_api_url
)
verify_url(next_url, next_api_url)
verify_url(previous_url, previous_api_url)
@skipUnless(settings.FEATURES["ENABLE_EDXNOTES"], "EdxNotes feature needs to be enabled.")
@ddt.ddt
class EdxNotesViewsTest(ModuleStoreTestCase):
"""
Tests for EdxNotes views.
@@ -822,7 +953,7 @@ class EdxNotesViewsTest(ModuleStoreTestCase):
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
self.client.login(username=self.user.username, password="edx")
self.notes_page_url = reverse("edxnotes", args=[unicode(self.course.id)])
self.search_url = reverse("search_notes", args=[unicode(self.course.id)])
self.notes_url = reverse("notes", args=[unicode(self.course.id)])
self.get_token_url = reverse("get_token", args=[unicode(self.course.id)])
self.visibility_url = reverse("edxnotes_visibility", args=[unicode(self.course.id)])
@@ -858,14 +989,14 @@ class EdxNotesViewsTest(ModuleStoreTestCase):
# pylint: disable=unused-argument
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True})
@patch("edxnotes.views.get_notes", return_value=[])
@patch("edxnotes.views.get_notes", return_value={'results': []})
def test_edxnotes_view_is_enabled(self, mock_get_notes):
"""
Tests that appropriate view is received if EdxNotes feature is enabled.
"""
enable_edxnotes_for_the_course(self.course, self.user.id)
response = self.client.get(self.notes_page_url)
self.assertContains(response, 'Highlights and notes you\'ve made in course content')
self.assertContains(response, 'Highlights and notes you&#39;ve made in course content')
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": False})
def test_edxnotes_view_is_disabled(self):
@@ -877,74 +1008,38 @@ class EdxNotesViewsTest(ModuleStoreTestCase):
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True})
@patch("edxnotes.views.get_notes", autospec=True)
def test_edxnotes_view_404_service_unavailable(self, mock_get_notes):
"""
Tests that 404 status code is received if EdxNotes service is unavailable.
"""
mock_get_notes.side_effect = EdxNotesServiceUnavailable
enable_edxnotes_for_the_course(self.course, self.user.id)
response = self.client.get(self.notes_page_url)
self.assertEqual(response.status_code, 404)
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True})
@patch("edxnotes.views.search", autospec=True)
def test_search_notes_successfully_respond(self, mock_search):
"""
Tests that `search_notes` successfully respond if EdxNotes feature is enabled.
Tests that search notes successfully respond if EdxNotes feature is enabled.
"""
mock_search.return_value = json.dumps({
"total": 0,
"rows": [],
})
mock_search.return_value = NOTES_VIEW_EMPTY_RESPONSE
enable_edxnotes_for_the_course(self.course, self.user.id)
response = self.client.get(self.search_url, {"text": "test"})
self.assertEqual(json.loads(response.content), {
"total": 0,
"rows": [],
})
response = self.client.get(self.notes_url, {"text": "test"})
self.assertEqual(json.loads(response.content), NOTES_VIEW_EMPTY_RESPONSE)
self.assertEqual(response.status_code, 200)
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": False})
@patch("edxnotes.views.search", autospec=True)
def test_search_notes_is_disabled(self, mock_search):
def test_search_notes_is_disabled(self):
"""
Tests that 404 status code is received if EdxNotes feature is disabled.
"""
mock_search.return_value = json.dumps({
"total": 0,
"rows": [],
})
response = self.client.get(self.search_url, {"text": "test"})
response = self.client.get(self.notes_url, {"text": "test"})
self.assertEqual(response.status_code, 404)
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True})
@patch("edxnotes.views.search", autospec=True)
def test_search_404_service_unavailable(self, mock_search):
@patch("edxnotes.views.get_notes", autospec=True)
def test_search_500_service_unavailable(self, mock_search):
"""
Tests that 404 status code is received if EdxNotes service is unavailable.
Tests that 500 status code is received if EdxNotes service is unavailable.
"""
mock_search.side_effect = EdxNotesServiceUnavailable
enable_edxnotes_for_the_course(self.course, self.user.id)
response = self.client.get(self.search_url, {"text": "test"})
response = self.client.get(self.notes_url, {"text": "test"})
self.assertEqual(response.status_code, 500)
self.assertIn("error", response.content)
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True})
@patch("edxnotes.views.search", autospec=True)
def test_search_notes_without_required_parameters(self, mock_search):
"""
Tests that 400 status code is received if the required parameters were not sent.
"""
mock_search.return_value = json.dumps({
"total": 0,
"rows": [],
})
enable_edxnotes_for_the_course(self.course, self.user.id)
response = self.client.get(self.search_url)
self.assertEqual(response.status_code, 400)
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True})
@patch("edxnotes.views.search", autospec=True)
@patch("edxnotes.views.get_notes", autospec=True)
def test_search_notes_exception(self, mock_search):
"""
Tests that 500 status code is received if invalid data was received from
@@ -952,7 +1047,7 @@ class EdxNotesViewsTest(ModuleStoreTestCase):
"""
mock_search.side_effect = EdxNotesParseError
enable_edxnotes_for_the_course(self.course, self.user.id)
response = self.client.get(self.search_url, {"text": "test"})
response = self.client.get(self.notes_url, {"text": "test"})
self.assertEqual(response.status_code, 500)
self.assertIn("error", response.content)
@@ -1022,3 +1117,50 @@ class EdxNotesViewsTest(ModuleStoreTestCase):
content_type="application/json",
)
self.assertEqual(response.status_code, 400)
@skipUnless(settings.FEATURES["ENABLE_EDXNOTES"], "EdxNotes feature needs to be enabled.")
@ddt.ddt
class EdxNotesPluginTest(ModuleStoreTestCase):
"""
EdxNotesTab tests.
"""
def setUp(self):
super(EdxNotesPluginTest, self).setUp()
self.course = CourseFactory.create(edxnotes=True)
self.user = UserFactory.create(username="ma", email="ma@ma.info", password="edx")
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
def test_edxnotes_tab_with_unauthorized_user(self):
"""
Verify EdxNotesTab visibility when user is unauthroized.
"""
user = UserFactory.create(username="ma1", email="ma1@ma1.info", password="edx")
self.assertFalse(EdxNotesTab.is_enabled(self.course, user=user))
@ddt.unpack
@ddt.data(
{'enable_edxnotes': False},
{'enable_edxnotes': True}
)
def test_edxnotes_tab_with_feature_flag(self, enable_edxnotes):
"""
Verify EdxNotesTab visibility when ENABLE_EDXNOTES feature flag is enabled/disabled.
"""
FEATURES['ENABLE_EDXNOTES'] = enable_edxnotes
with override_settings(FEATURES=FEATURES):
self.assertEqual(EdxNotesTab.is_enabled(self.course), enable_edxnotes)
@ddt.unpack
@ddt.data(
{'harvard_notes_enabled': False},
{'harvard_notes_enabled': True}
)
def test_edxnotes_tab_with_harvard_notes(self, harvard_notes_enabled):
"""
Verify EdxNotesTab visibility when harvard notes feature is enabled/disabled.
"""
with patch("edxnotes.plugins.is_harvard_notes_enabled") as mock_harvard_notes_enabled:
mock_harvard_notes_enabled.return_value = harvard_notes_enabled
self.assertEqual(EdxNotesTab.is_enabled(self.course), not harvard_notes_enabled)

2
lms/djangoapps/edxnotes/urls.py
View File

@@ -7,7 +7,7 @@ from django.conf.urls import patterns, url
urlpatterns = patterns(
"edxnotes.views",
url(r"^/$", "edxnotes", name="edxnotes"),
url(r"^/search/$", "search_notes", name="search_notes"),
url(r"^/notes/$", "notes", name="notes"),
url(r"^/token/$", "get_token", name="get_token"),
url(r"^/visibility/$", "edxnotes_visibility", name="edxnotes_visibility"),
)

116
lms/djangoapps/edxnotes/views.py
View File

@@ -7,6 +7,7 @@ from django.contrib.auth.decorators import login_required
from django.http import HttpResponse, HttpResponseBadRequest, Http404
from django.conf import settings
from django.core.urlresolvers import reverse
from django.views.decorators.http import require_GET
from edxmako.shortcuts import render_to_response
from opaque_keys.edx.keys import CourseKey
from courseware.courses import get_course_with_access
@@ -18,8 +19,10 @@ from edxnotes.helpers import (
get_edxnotes_id_token,
get_notes,
is_feature_enabled,
search,
get_course_position,
DEFAULT_PAGE,
DEFAULT_PAGE_SIZE,
NoteJSONEncoder,
)
@@ -30,6 +33,13 @@ log = logging.getLogger(__name__)
def edxnotes(request, course_id):
"""
Displays the EdxNotes page.
Arguments:
request: HTTP request object
course_id: course id
Returns:
Rendered HTTP response.
"""
course_key = CourseKey.from_string(course_id)
course = get_course_with_access(request.user, "load", course_key)
@@ -37,20 +47,20 @@ def edxnotes(request, course_id):
if not is_feature_enabled(course):
raise Http404
try:
notes = get_notes(request.user, course)
except EdxNotesServiceUnavailable:
raise Http404
notes_info = get_notes(request, course)
has_notes = (len(notes_info.get('results')) > 0)
context = {
"course": course,
"search_endpoint": reverse("search_notes", kwargs={"course_id": course_id}),
"notes": notes,
"debug": json.dumps(settings.DEBUG),
"notes_endpoint": reverse("notes", kwargs={"course_id": course_id}),
"notes": notes_info,
"page_size": DEFAULT_PAGE_SIZE,
"debug": settings.DEBUG,
'position': None,
'disabled_tabs': settings.NOTES_DISABLED_TABS,
'has_notes': has_notes,
}
if not notes:
if not has_notes:
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
course.id, request.user, course, depth=2
)
@@ -66,27 +76,97 @@ def edxnotes(request, course_id):
return render_to_response("edxnotes/edxnotes.html", context)
@require_GET
@login_required
def search_notes(request, course_id):
def notes(request, course_id):
"""
Handles search requests.
Notes view to handle list and search requests.
Query parameters:
page: page number to get
page_size: number of items in the page
text: text string to search. If `text` param is missing then get all the
notes for the current user for this course else get only those notes
which contain the `text` value.
Arguments:
request: HTTP request object
course_id: course id
Returns:
Paginated response as JSON. A sample response is below.
{
"count": 101,
"num_pages": 11,
"current_page": 1,
"results": [
{
"chapter": {
"index": 4,
"display_name": "About Exams and Certificates",
"location": "i4x://org/course/category/name@revision",
"children": [
"i4x://org/course/category/name@revision"
]
},
"updated": "Dec 09, 2015 at 09:31 UTC",
"tags": ["shadow","oil"],
"quote": "foo bar baz",
"section": {
"display_name": "edX Exams",
"location": "i4x://org/course/category/name@revision",
"children": [
"i4x://org/course/category/name@revision",
"i4x://org/course/category/name@revision",
]
},
"created": "2015-12-09T09:31:17.338305Z",
"ranges": [
{
"start": "/div[1]/p[1]",
"end": "/div[1]/p[1]",
"startOffset": 0,
"endOffset": 6
}
],
"user": "50cf92f9a3d8489df95e583549b919df",
"text": "first angry height hungry structure",
"course_id": "edx/DemoX/Demo",
"id": "1231",
"unit": {
"url": "/courses/edx%2FDemoX%2FDemo/courseware/1414ffd5143b4b508f739b563ab468b7/workflow/1",
"display_name": "EdX Exams",
"location": "i4x://org/course/category/name@revision"
},
"usage_id": "i4x://org/course/category/name@revision"
} ],
"next": "http://0.0.0.0:8000/courses/edx%2FDemoX%2FDemo/edxnotes/notes/?page=2&page_size=10",
"start": 0,
"previous": null
}
"""
course_key = CourseKey.from_string(course_id)
course = get_course_with_access(request.user, "load", course_key)
course = get_course_with_access(request.user, 'load', course_key)
if not is_feature_enabled(course):
raise Http404
if "text" not in request.GET:
return HttpResponseBadRequest()
page = request.GET.get('page') or DEFAULT_PAGE
page_size = request.GET.get('page_size') or DEFAULT_PAGE_SIZE
text = request.GET.get('text')
query_string = request.GET["text"]
try:
search_results = search(request.user, course, query_string)
notes_info = get_notes(
request,
course,
page=page,
page_size=page_size,
text=text
)
except (EdxNotesParseError, EdxNotesServiceUnavailable) as err:
return JsonResponseBadRequest({"error": err.message}, status=500)
return HttpResponse(search_results)
return HttpResponse(json.dumps(notes_info, cls=NoteJSONEncoder), content_type="application/json")
# pylint: disable=unused-argument