Skip to content

E
edx-platform
Commit 88049309 by Clinton Blackburn Committed by Clinton Blackburn
Options

Corrected Course Catalog API URL 

Retrieving the URL from settings rather than an OAuth client that might not exist.

ECOM-4729
parent 5f159591
Showing with 50 additions and 59 deletions
lms/envs/aws.py
......@@ -726,6 +726,8 @@ ECOMMERCE_API_URL = ENV_TOKENS.get('ECOMMERCE_API_URL', ECOMMERCE_API_URL)
ECOMMERCE_API_SIGNING_KEY = AUTH_TOKENS.get('ECOMMERCE_API_SIGNING_KEY', ECOMMERCE_API_SIGNING_KEY)
ECOMMERCE_API_TIMEOUT = ENV_TOKENS.get('ECOMMERCE_API_TIMEOUT', ECOMMERCE_API_TIMEOUT)
COURSE_CATALOG_API_URL = ENV_TOKENS.get('COURSE_CATALOG_API_URL', COURSE_CATALOG_API_URL)
##### Custom Courses for EdX #####
if FEATURES.get('CUSTOM_COURSES_EDX'):
INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon')
......
lms/envs/common.py
......@@ -2731,6 +2731,8 @@ ECOMMERCE_API_SIGNING_KEY = None
ECOMMERCE_API_TIMEOUT = 5
ECOMMERCE_SERVICE_WORKER_USERNAME = 'ecommerce_worker'
COURSE_CATALOG_API_URL = None
# Reverification checkpoint name pattern
CHECKPOINT_PATTERN = r'(?P<checkpoint_name>[^/]+)'
......
lms/envs/test.py
......@@ -585,3 +585,5 @@ TransformerRegistry.USE_PLUGIN_MANAGER = False
# Set the default Oauth2 Provider Model so that migrations can run in
# verbose mode
OAUTH2_PROVIDER_APPLICATION_MODEL = 'oauth2_provider.Application'
COURSE_CATALOG_API_URL = 'https://catalog.example.com/api/v1'
openedx/core/djangoapps/api_admin/tests/test_views.py
#pylint: disable=missing-docstring
import unittest
""" Tests for the api_admin app's views. """
import json
from urlparse import urljoin
import unittest
import ddt
import httpretty
from django.conf import settings
from django.core.urlresolvers import reverse
from django.test import TestCase
from django.test.utils import override_settings
from edx_oauth2_provider.tests.factories import ClientFactory
import httpretty
from oauth2_provider.models import get_application_model
from openedx.core.djangoapps.api_admin.models import ApiAccessRequest, ApiAccessConfig
......@@ -19,14 +18,10 @@ from openedx.core.djangoapps.api_admin.tests.factories import (
from openedx.core.djangoapps.api_admin.tests.utils import VALID_DATA
from student.tests.factories import UserFactory
Application = get_application_model() # pylint: disable=invalid-name
MOCK_CATALOG_API_URL_ROOT = 'https://api.example.com/'
class ApiAdminTest(TestCase):
def setUp(self):
super(ApiAdminTest, self).setUp()
ApiAccessConfig(enabled=True).save()
......@@ -34,7 +29,6 @@ class ApiAdminTest(TestCase):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class ApiRequestViewTest(ApiAdminTest):
def setUp(self):
super(ApiRequestViewTest, self).setUp()
self.url = reverse('api_admin:api-request')
......@@ -103,7 +97,6 @@ class ApiRequestViewTest(ApiAdminTest):
@override_settings(PLATFORM_NAME='edX')
@ddt.ddt
class ApiRequestStatusViewTest(ApiAdminTest):
def setUp(self):
super(ApiRequestStatusViewTest, self).setUp()
password = 'abc123'
......@@ -207,7 +200,6 @@ class ApiRequestStatusViewTest(ApiAdminTest):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class ApiTosViewTest(ApiAdminTest):
def test_get_api_tos(self):
"""Verify that the terms of service can be read."""
url = reverse('api_admin:api-tos')
......@@ -217,20 +209,23 @@ class ApiTosViewTest(ApiAdminTest):
class CatalogTest(ApiAdminTest):
def setUp(self):
super(CatalogTest, self).setUp()
password = 'abc123'
self.user = UserFactory(password=password, is_staff=True)
self.client.login(username=self.user.username, password=password)
ClientFactory(user=self.user, name='course-discovery', url=MOCK_CATALOG_API_URL_ROOT)
def mock_catalog_api(self, url, data, method=httpretty.GET, status_code=200):
def mock_catalog_endpoint(self, data, catalog_id=None, method=httpretty.GET, status_code=200):
""" Mock the Course Catalog API's catalog endpoint. """
self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Catalog API calls.')
httpretty.reset()
url = '{root}/catalogs/'.format(root=settings.COURSE_CATALOG_API_URL.rstrip('/'))
if catalog_id:
url += '{id}/'.format(id=catalog_id)
httpretty.register_uri(
method,
urljoin(MOCK_CATALOG_API_URL_ROOT, url),
url,
body=json.dumps(data),
content_type='application/json',
status=status_code
......@@ -239,7 +234,6 @@ class CatalogTest(ApiAdminTest):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class CatalogSearchViewTest(CatalogTest):
def setUp(self):
super(CatalogSearchViewTest, self).setUp()
self.url = reverse('api_admin:catalog-search')
......@@ -251,7 +245,7 @@ class CatalogSearchViewTest(CatalogTest):
@httpretty.activate
def test_post(self):
catalog_user = UserFactory()
self.mock_catalog_api('api/v1/catalogs/', {'results': []})
self.mock_catalog_endpoint({'results': []})
response = self.client.post(self.url, {'username': catalog_user.username})
self.assertRedirects(response, reverse('api_admin:catalog-list', kwargs={'username': catalog_user.username}))
......@@ -262,7 +256,6 @@ class CatalogSearchViewTest(CatalogTest):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class CatalogListViewTest(CatalogTest):
def setUp(self):
super(CatalogListViewTest, self).setUp()
self.catalog_user = UserFactory()
......@@ -271,9 +264,7 @@ class CatalogListViewTest(CatalogTest):
@httpretty.activate
def test_get(self):
catalog = CatalogFactory(viewers=[self.catalog_user.username])
self.mock_catalog_api('api/v1/catalogs/', {
'results': [catalog.attributes]
})
self.mock_catalog_endpoint({'results': [catalog.attributes]})
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200)
self.assertIn(catalog.name, response.content.decode('utf-8'))
......@@ -281,7 +272,7 @@ class CatalogListViewTest(CatalogTest):
@httpretty.activate
def test_get_no_catalogs(self):
"""Verify that the view works when no catalogs are set up."""
self.mock_catalog_api('api/v1/catalogs/', {}, status_code=404)
self.mock_catalog_endpoint({}, status_code=404)
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200)
......@@ -293,18 +284,16 @@ class CatalogListViewTest(CatalogTest):
'viewers': [self.catalog_user.username]
}
catalog_id = 123
self.mock_catalog_api('api/v1/catalogs/', dict(catalog_data, id=catalog_id), method=httpretty.POST)
self.mock_catalog_endpoint(dict(catalog_data, id=catalog_id), method=httpretty.POST)
response = self.client.post(self.url, catalog_data)
self.assertEqual(httpretty.last_request().method, 'POST')
self.mock_catalog_api('api/v1/catalogs/{}/'.format(catalog_id), CatalogFactory().attributes)
self.mock_catalog_endpoint(CatalogFactory().attributes, catalog_id=catalog_id)
self.assertRedirects(response, reverse('api_admin:catalog-edit', kwargs={'catalog_id': catalog_id}))
@httpretty.activate
def test_post_invalid(self):
catalog = CatalogFactory(viewers=[self.catalog_user.username])
self.mock_catalog_api('api/v1/catalogs/', {
'results': [catalog.attributes]
})
self.mock_catalog_endpoint({'results': [catalog.attributes]})
response = self.client.post(self.url, {
'name': '',
'query': '*',
......@@ -317,7 +306,6 @@ class CatalogListViewTest(CatalogTest):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class CatalogEditViewTest(CatalogTest):
def setUp(self):
super(CatalogEditViewTest, self).setUp()
self.catalog_user = UserFactory()
......@@ -326,17 +314,17 @@ class CatalogEditViewTest(CatalogTest):
@httpretty.activate
def test_get(self):
self.mock_catalog_api('api/v1/catalogs/{}/'.format(self.catalog.id), self.catalog.attributes)
self.mock_catalog_endpoint(self.catalog.attributes, catalog_id=self.catalog.id)
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200)
self.assertIn(self.catalog.name, response.content.decode('utf-8'))
@httpretty.activate
def test_delete(self):
self.mock_catalog_api(
'api/v1/catalogs/{}/'.format(self.catalog.id),
self.mock_catalog_endpoint(
self.catalog.attributes,
method=httpretty.DELETE
method=httpretty.DELETE,
catalog_id=self.catalog.id
)
response = self.client.post(self.url, {'delete-catalog': 'on'})
self.assertRedirects(response, reverse('api_admin:catalog-search'))
......@@ -349,18 +337,15 @@ class CatalogEditViewTest(CatalogTest):
@httpretty.activate
def test_edit(self):
self.mock_catalog_api(
'api/v1/catalogs/{}/'.format(self.catalog.id),
self.catalog.attributes, method=httpretty.PATCH
)
self.mock_catalog_endpoint(self.catalog.attributes, method=httpretty.PATCH, catalog_id=self.catalog.id)
new_attributes = dict(self.catalog.attributes, **{'delete-catalog': 'off', 'name': 'changed'})
response = self.client.post(self.url, new_attributes)
self.mock_catalog_api('api/v1/catalogs/{}/'.format(self.catalog.id), new_attributes)
self.mock_catalog_endpoint(new_attributes, catalog_id=self.catalog.id)
self.assertRedirects(response, reverse('api_admin:catalog-edit', kwargs={'catalog_id': self.catalog.id}))
@httpretty.activate
def test_edit_invalid(self):
self.mock_catalog_api('api/v1/catalogs/{}/'.format(self.catalog.id), self.catalog.attributes)
self.mock_catalog_endpoint(self.catalog.attributes, catalog_id=self.catalog.id)
new_attributes = dict(self.catalog.attributes, **{'delete-catalog': 'off', 'name': ''})
response = self.client.post(self.url, new_attributes)
self.assertEqual(response.status_code, 400)
......@@ -370,7 +355,6 @@ class CatalogEditViewTest(CatalogTest):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class CatalogPreviewViewTest(CatalogTest):
def setUp(self):
super(CatalogPreviewViewTest, self).setUp()
self.url = reverse('api_admin:catalog-preview')
......@@ -378,7 +362,12 @@ class CatalogPreviewViewTest(CatalogTest):
@httpretty.activate
def test_get(self):
data = {'count': 1, 'results': ['test data'], 'next': None, 'prev': None}
self.mock_catalog_api('api/v1/courses/', data)
httpretty.register_uri(
httpretty.GET,
'{root}/courses/'.format(root=settings.COURSE_CATALOG_API_URL.rstrip('/')),
body=json.dumps(data),
content_type='application/json'
)
response = self.client.get(self.url, {'q': '*'})
self.assertEqual(response.status_code, 200)
self.assertEqual(json.loads(response.content), data)
......
openedx/core/djangoapps/api_admin/utils.py
""" Course Discovery API Service. """
import datetime
import jwt
from django.conf import settings
from edx_rest_api_client.client import EdxRestApiClient
import jwt
from openedx.core.djangoapps.theming import helpers
from provider.oauth2.models import Client
from student.models import UserProfile, anonymous_id_for_user
CLIENT_NAME = 'course-discovery'
def get_id_token(user):
"""
......@@ -49,5 +46,4 @@ def get_id_token(user):
def course_discovery_api_client(user):
""" Returns a Course Discovery API client setup with authentication for the specified user. """
course_discovery_client = Client.objects.get(name=CLIENT_NAME)
return EdxRestApiClient(course_discovery_client.url, jwt=get_id_token(user))
return EdxRestApiClient(settings.COURSE_CATALOG_API_URL, jwt=get_id_token(user))
openedx/core/djangoapps/api_admin/views.py
......@@ -144,7 +144,7 @@ class CatalogListView(View):
def _get_catalogs(self, client, username):
"""Retrieve catalogs for a user. Returns the empty list if none are found."""
try:
response = client.api.v1.catalogs.get(username=username)
response = client.catalogs.get(username=username)
return [Catalog(attributes=catalog) for catalog in response['results']]
except HttpNotFoundError:
return []
......@@ -158,7 +158,7 @@ class CatalogListView(View):
'catalogs': catalogs,
'form': CatalogForm(initial={'viewers': [username]}),
'preview_url': reverse('api_admin:catalog-preview'),
'catalog_api_url': client.api.v1.courses.url(),
'catalog_api_url': client.courses.url(),
})
def post(self, request, username):
......@@ -172,11 +172,11 @@ class CatalogListView(View):
'catalogs': catalogs,
'username': username,
'preview_url': reverse('api_admin:catalog-preview'),
'catalog_api_url': client.api.v1.courses.url(),
'catalog_api_url': client.courses.url(),
}, status=400)
attrs = form.instance.attributes
catalog = client.api.v1.catalogs.post(attrs)
catalog = client.catalogs.post(attrs)
return redirect(reverse('api_admin:catalog-edit', kwargs={'catalog_id': catalog['id']}))
......@@ -186,33 +186,33 @@ class CatalogEditView(View):
def get(self, request, catalog_id):
"""Display a form to edit this catalog."""
client = course_discovery_api_client(request.user)
response = client.api.v1.catalogs(catalog_id).get()
response = client.catalogs(catalog_id).get()
catalog = Catalog(attributes=response)
form = CatalogForm(instance=catalog)
return render_to_response('api_admin/catalogs/edit.html', {
'catalog': catalog,
'form': form,
'preview_url': reverse('api_admin:catalog-preview'),
'catalog_api_url': client.api.v1.courses.url(),
'catalog_api_url': client.courses.url(),
})
def post(self, request, catalog_id):
"""Update or delete this catalog."""
client = course_discovery_api_client(request.user)
if request.POST.get('delete-catalog') == 'on':
client.api.v1.catalogs(catalog_id).delete()
client.catalogs(catalog_id).delete()
return redirect(reverse('api_admin:catalog-search'))
form = CatalogForm(request.POST)
if not form.is_valid():
response = client.api.v1.catalogs(catalog_id).get()
response = client.catalogs(catalog_id).get()
catalog = Catalog(attributes=response)
return render_to_response('api_admin/catalogs/edit.html', {
'catalog': catalog,
'form': form,
'preview_url': reverse('api_admin:catalog-preview'),
'catalog_api_url': client.api.v1.courses.url(),
'catalog_api_url': client.courses.url(),
}, status=400)
catalog = client.api.v1.catalogs(catalog_id).patch(form.instance.attributes)
catalog = client.catalogs(catalog_id).patch(form.instance.attributes)
return redirect(reverse('api_admin:catalog-edit', kwargs={'catalog_id': catalog['id']}))
......@@ -227,7 +227,7 @@ class CatalogPreviewView(View):
client = course_discovery_api_client(request.user)
# Just pass along the request params including limit/offset pagination
if 'q' in request.GET:
results = client.api.v1.courses.get(**request.GET)
results = client.courses.get(**request.GET)
# Ensure that we don't just return all the courses if no query is given
else:
results = {'count': 0, 'results': [], 'next': None, 'prev': None}
......
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