Merge pull request #12786 from edx/clintonb/catalog-admin-csv

Added CSV catalog downloads

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'

lms/templates/api_admin/catalogs/detail.html

-## mako
-<%page expression_filter="h"/>
-<%inherit file="../../main.html"/>
-<%!
-from django.core.urlresolvers import reverse
-from django.utils.translation import ugettext as _
-%>
-
-<%block name="pagetitle">${catalog.name}</%block>
-
-<%block name="content">
-<div id="api-access-wrapper">
-  <h1 id="api-header">${catalog.name}</h1>
-  <p class="api-copy-body">${catalog.query}</p>
-  <p class="api-copy-body"><a href="${edit_link}">${_("Edit or delete this catalog.")}</a></p>
-  <p class="api-copy-body"><a href="${preview_link}">${_("See a preview of this catalog's contents.")}</a></p>
-</div>
-</%block>

lms/templates/api_admin/catalogs/edit.html

@@ -23,6 +23,12 @@ from django.utils.translation import ugettext as _
 <div id="api-access-wrapper">
   <h1 id="api-header">${catalog.name}</h1>
 
+  <p>
+    <a href="${'{root}/{id}/csv/'.format(root=catalog_api_catalog_endpoint, id=catalog.id)}" target="_blank">
+      ${_("Download CSV")}
+    </a>
+  </p>
+
   <div class="catalog-body">
     <div class="api-form-container">
       <form class="api-form" id="catalog-update" action="${reverse('api_admin:catalog-edit', args=(catalog.id,))}" method="post">

lms/templates/api_admin/catalogs/list.html

@@ -25,7 +25,10 @@ CatalogPreviewFactory({
   <ul>
     % for catalog in catalogs:
     <li>
-      <a href="${reverse('api_admin:catalog-edit', args=(catalog.id,))}">${catalog.name}</a>
+      <a href="${reverse('api_admin:catalog-edit', args=(catalog.id,))}">${catalog.name}</a>&nbsp;
+      (<a
+          href="${'{root}/{id}/csv/'.format(root=catalog_api_catalog_endpoint, id=catalog.id)}"
+          target="_blank">${_("Download CSV")}</a>)
     </li>
     % endfor
   </ul>

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):
     """
@@ -44,10 +41,9 @@ def get_id_token(user):
     }
     secret_key = helpers.get_value('JWT_AUTH', settings.JWT_AUTH)['JWT_SECRET_KEY']
 
-    return jwt.encode(payload, secret_key)
+    return jwt.encode(payload, secret_key).decode('utf-8')
 
 
 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

@@ -6,7 +6,6 @@ from django.contrib.sites.shortcuts import get_current_site
 from django.core.urlresolvers import reverse_lazy, reverse
 from django.http.response import JsonResponse
 from django.shortcuts import redirect
-from django.utils.translation import ugettext as _
 from django.views.generic import View
 from django.views.generic.base import TemplateView
 from django.views.generic.edit import CreateView
@@ -144,75 +143,77 @@ 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 []
 
+    def get_context_data(self, client, username, form):
+        """ Retrieve context data for the template. """
+
+        return {
+            'username': username,
+            'catalogs': self._get_catalogs(client, username),
+            'form': form,
+            'preview_url': reverse('api_admin:catalog-preview'),
+            'catalog_api_catalog_endpoint': client.catalogs.url().rstrip('/'),
+            'catalog_api_url': client.courses.url(),
+        }
+
     def get(self, request, username):
         """Display a list of a user's catalogs."""
         client = course_discovery_api_client(request.user)
-        catalogs = self._get_catalogs(client, username)
-        return render_to_response(self.template, {
-            'username': username,
-            'catalogs': catalogs,
-            'form': CatalogForm(initial={'viewers': [username]}),
-            'preview_url': reverse('api_admin:catalog-preview'),
-            'catalog_api_url': client.api.v1.courses.url(),
-        })
+        form = CatalogForm(initial={'viewers': [username]})
+        return render_to_response(self.template, self.get_context_data(client, username, form))
 
     def post(self, request, username):
         """Create a new catalog for a user."""
         form = CatalogForm(request.POST)
         client = course_discovery_api_client(request.user)
         if not form.is_valid():
-            catalogs = self._get_catalogs(client, username)
-            return render_to_response(self.template, {
-                'form': form,
-                'catalogs': catalogs,
-                'username': username,
-                'preview_url': reverse('api_admin:catalog-preview'),
-                'catalog_api_url': client.api.v1.courses.url(),
-            }, status=400)
+            return render_to_response(self.template, self.get_context_data(client, username, form), 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']}))
 
 
 class CatalogEditView(View):
     """View to edit an individual catalog."""
 
+    template_name = 'api_admin/catalogs/edit.html'
+
+    def get_context_data(self, catalog, form, client):
+        """ Retrieve context data for the template. """
+
+        return {
+            'catalog': catalog,
+            'form': form,
+            'preview_url': reverse('api_admin:catalog-preview'),
+            'catalog_api_url': client.courses.url(),
+            'catalog_api_catalog_endpoint': client.catalogs.url().rstrip('/'),
+        }
+
     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(),
-        })
+        return render_to_response(self.template_name, self.get_context_data(catalog, form, client))
 
     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(),
-            }, status=400)
-        catalog = client.api.v1.catalogs(catalog_id).patch(form.instance.attributes)
+            return render_to_response(self.template_name, self.get_context_data(catalog, form, client), status=400)
+        catalog = client.catalogs(catalog_id).patch(form.instance.attributes)
         return redirect(reverse('api_admin:catalog-edit', kwargs={'catalog_id': catalog['id']}))
 
 
@@ -227,7 +228,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}