Merge pull request #8336 from edx/dan-f/expose-edxnotes-interface-setting

Add support for public/internal edx-notes-api endpoints

lms/djangoapps/edxnotes/decorators.py

@@ -4,7 +4,7 @@ Decorators related to edXNotes.
 from django.conf import settings
 import json
 from edxnotes.helpers import (
-    get_endpoint,
+    get_public_endpoint,
     get_id_token,
     get_token_url,
     generate_uid,
@@ -45,7 +45,7 @@ def edxnotes(cls):
                     "courseId": unicode(self.runtime.course_id).encode("utf-8"),
                     "token": get_id_token(self.runtime.get_real_user(self.runtime.anonymous_student_id)),
                     "tokenUrl": get_token_url(self.runtime.course_id),
-                    "endpoint": get_endpoint(),
+                    "endpoint": get_public_endpoint(),
                     "debug": settings.DEBUG,
                     "eventStringLimit": settings.TRACK_MAX_EVENT / 6,
                 },

lms/djangoapps/edxnotes/helpers.py

@@ -79,7 +79,7 @@ def send_request(user, course_id, path="", query_string=None):
     """
     Sends a request with appropriate parameters and headers.
     """
-    url = get_endpoint(path)
+    url = get_internal_endpoint(path)
     params = {
         "user": anonymous_id_for_user(user, None),
         "course_id": unicode(course_id).encode("utf-8"),
@@ -286,14 +286,19 @@ def get_notes(user, course):
     return json.dumps(preprocess_collection(user, course, collection), cls=NoteJSONEncoder)
 
 
-def get_endpoint(path=""):
+def get_endpoint(api_url, path=""):
     """
     Returns edx-notes-api endpoint.
+
+    Arguments:
+        api_url (str): base url to the notes api
+        path (str): path to the resource
+    Returns:
+        str: full endpoint to the notes api
     """
     try:
-        url = settings.EDXNOTES_INTERFACE['url']
-        if not url.endswith("/"):
-            url += "/"
+        if not api_url.endswith("/"):
+            api_url += "/"
 
         if path:
             if path.startswith("/"):
@@ -301,11 +306,21 @@ def get_endpoint(path=""):
             if not path.endswith("/"):
                 path += "/"
 
-        return url + path
+        return api_url + path
     except (AttributeError, KeyError):
         raise ImproperlyConfigured(_("No endpoint was provided for EdxNotes."))
 
 
+def get_public_endpoint(path=""):
+    """Get the full path to a resource on the public notes API."""
+    return get_endpoint(settings.EDXNOTES_PUBLIC_API, path)
+
+
+def get_internal_endpoint(path=""):
+    """Get the full path to a resource on the private notes API."""
+    return get_endpoint(settings.EDXNOTES_INTERNAL_API, path)
+
+
 def get_course_position(course_module):
     """
     Return the user's current place in the course.

lms/djangoapps/edxnotes/tests.py

 """
 Tests for the EdxNotes app.
 """
+from contextlib import contextmanager
+import ddt
 import json
 import jwt
 from mock import patch, MagicMock
@@ -15,6 +17,7 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.core.exceptions import ImproperlyConfigured
 from django.test.client import RequestFactory
+from django.test.utils import override_settings
 from oauth2_provider.tests.factories import ClientFactory
 from provider.oauth2.models import Client
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -76,7 +79,7 @@ class EdxNotesDecoratorTest(ModuleStoreTestCase):
         self.problem = TestProblem(self.course)
 
     @patch.dict("django.conf.settings.FEATURES", {'ENABLE_EDXNOTES': True})
-    @patch("edxnotes.decorators.get_endpoint")
+    @patch("edxnotes.decorators.get_public_endpoint")
     @patch("edxnotes.decorators.get_token_url")
     @patch("edxnotes.decorators.get_id_token")
     @patch("edxnotes.decorators.generate_uid")
@@ -141,6 +144,7 @@ class EdxNotesDecoratorTest(ModuleStoreTestCase):
 
 
 @skipUnless(settings.FEATURES["ENABLE_EDXNOTES"], "EdxNotes feature needs to be enabled.")
+@ddt.ddt
 class EdxNotesHelpersTest(ModuleStoreTestCase):
     """
     Tests for EdxNotes helpers.
@@ -232,29 +236,44 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
                             {"type": "bar"}]
         self.assertTrue(helpers.is_feature_enabled(self.course))
 
-    def test_get_endpoint(self):
-        """
-        Tests that storage_url method returns appropriate values.
-        """
+    @ddt.data(
+        helpers.get_public_endpoint,
+        helpers.get_internal_endpoint,
+    )
+    def test_get_endpoints(self, get_endpoint_function):
+        """
+        Test that the get_public_endpoint and get_internal_endpoint functions
+        return appropriate values.
+        """
+        @contextmanager
+        def patch_edxnotes_api_settings(url):
+            """
+            Convenience function for patching both EDXNOTES_PUBLIC_API and
+            EDXNOTES_INTERNAL_API.
+            """
+            with override_settings(EDXNOTES_PUBLIC_API=url):
+                with override_settings(EDXNOTES_INTERNAL_API=url):
+                    yield
+
         # url ends with "/"
-        with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com/"}):
-            self.assertEqual("http://example.com/", helpers.get_endpoint())
+        with patch_edxnotes_api_settings("http://example.com/"):
+            self.assertEqual("http://example.com/", get_endpoint_function())
 
         # url doesn't have "/" at the end
-        with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}):
-            self.assertEqual("http://example.com/", helpers.get_endpoint())
+        with patch_edxnotes_api_settings("http://example.com"):
+            self.assertEqual("http://example.com/", get_endpoint_function())
 
         # url with path that starts with "/"
-        with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}):
-            self.assertEqual("http://example.com/some_path/", helpers.get_endpoint("/some_path"))
+        with patch_edxnotes_api_settings("http://example.com"):
+            self.assertEqual("http://example.com/some_path/", get_endpoint_function("/some_path"))
 
         # url with path without "/"
-        with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}):
-            self.assertEqual("http://example.com/some_path/", helpers.get_endpoint("some_path/"))
+        with patch_edxnotes_api_settings("http://example.com"):
+            self.assertEqual("http://example.com/some_path/", get_endpoint_function("some_path/"))
 
         # url is not configured
-        with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": None}):
-            self.assertRaises(ImproperlyConfigured, helpers.get_endpoint)
+        with patch_edxnotes_api_settings(None):
+            self.assertRaises(ImproperlyConfigured, get_endpoint_function)
 
     @patch("edxnotes.helpers.requests.get")
     def test_get_notes_correct_data(self, mock_get):
@@ -669,7 +688,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
             helpers.get_module_context(self.course, self.chapter_2)
         )
 
-    @patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"})
+    @override_settings(EDXNOTES_PUBLIC_API="http://example.com")
+    @override_settings(EDXNOTES_INTERNAL_API="http://example.com")
     @patch("edxnotes.helpers.anonymous_id_for_user")
     @patch("edxnotes.helpers.get_id_token")
     @patch("edxnotes.helpers.requests.get")
@@ -697,7 +717,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
             }
         )
 
-    @patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"})
+    @override_settings(EDXNOTES_PUBLIC_API="http://example.com")
+    @override_settings(EDXNOTES_INTERNAL_API="http://example.com")
     @patch("edxnotes.helpers.anonymous_id_for_user")
     @patch("edxnotes.helpers.get_id_token")
     @patch("edxnotes.helpers.requests.get")

lms/envs/aws.py

@@ -629,4 +629,5 @@ else:
 
 # EdxNotes config
 
-EDXNOTES_INTERFACE = ENV_TOKENS.get('EDXNOTES_INTERFACE', EDXNOTES_INTERFACE)
+EDXNOTES_PUBLIC_API = ENV_TOKENS.get('EDXNOTES_PUBLIC_API', EDXNOTES_PUBLIC_API)
+EDXNOTES_INTERNAL_API = ENV_TOKENS.get('EDXNOTES_INTERNAL_API', EDXNOTES_INTERNAL_API)

lms/envs/bok_choy.py

@@ -71,7 +71,8 @@ XQUEUE_INTERFACE['url'] = 'http://localhost:8040'
 OPEN_ENDED_GRADING_INTERFACE['url'] = 'http://localhost:8041/'
 
 # Configure the LMS to use our stub EdxNotes implementation
-EDXNOTES_INTERFACE['url'] = 'http://localhost:8042/api/v1'
+EDXNOTES_PUBLIC_API = 'http://localhost:8042/api/v1'
+EDXNOTES_INTERNAL_API = 'http://localhost:8042/api/v1'
 
 # Enable django-pipeline and staticfiles
 STATIC_ROOT = (TEST_ROOT / "staticfiles").abspath()

lms/envs/common.py

@@ -1038,9 +1038,8 @@ MOCK_STAFF_GRADING = False
 ################################# EdxNotes config  #########################
 
 # Configure the LMS to use our stub EdxNotes implementation
-EDXNOTES_INTERFACE = {
-    'url': 'http://localhost:8120/api/v1',
-}
+EDXNOTES_PUBLIC_API = 'http://localhost:8120/api/v1'
+EDXNOTES_INTERNAL_API = 'http://localhost:8120/api/v1'
 
 ########################## Parental controls config  #######################