diff --git a/analytics_dashboard/core/tests/test_utils.py b/analytics_dashboard/core/tests/test_utils.py index ce39522da..d25433972 100644 --- a/analytics_dashboard/core/tests/test_utils.py +++ b/analytics_dashboard/core/tests/test_utils.py @@ -158,11 +158,20 @@ class CourseStructureApiClientTests(TestCase): """ def test_default_timeout(self): - client = CourseStructureApiClient('http://example.com/', 'arbitrary_access_token') + client = CourseStructureApiClient( + settings.BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL, + settings.BACKEND_SERVICE_EDX_OAUTH2_KEY, + settings.BACKEND_SERVICE_EDX_OAUTH2_SECRET, + ) # pylint: disable=protected-access - self.assertEqual(client._store['session'].timeout, settings.LMS_DEFAULT_TIMEOUT) + self.assertEqual(client._timeout, settings.LMS_DEFAULT_TIMEOUT) def test_explicit_timeout(self): - client = CourseStructureApiClient('http://example.com/', 'arbitrary_access_token', timeout=2.5) + client = CourseStructureApiClient( + settings.BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL, + settings.BACKEND_SERVICE_EDX_OAUTH2_KEY, + settings.BACKEND_SERVICE_EDX_OAUTH2_SECRET, + timeout=(2.05, 4), + ) # pylint: disable=protected-access - self.assertEqual(client._store['session'].timeout, 2.5) + self.assertEqual(client._timeout, (2.05, 4)) diff --git a/analytics_dashboard/core/utils.py b/analytics_dashboard/core/utils.py index 94f64b384..3fdf05d99 100644 --- a/analytics_dashboard/core/utils.py +++ b/analytics_dashboard/core/utils.py @@ -29,8 +29,8 @@ class CourseStructureApiClient(clients.CourseStructureApiClient): A very thin wrapper around `common.clients.CourseStructureApiClient`, which defaults the client timeout to `settings.LMS_DEFAULT_TIMEOUT`. """ - def __init__(self, url, access_token, timeout=settings.LMS_DEFAULT_TIMEOUT): - super().__init__(url, access_token=access_token, timeout=timeout) + def __init__(self, base_url, client_id, client_secret, timeout=settings.LMS_DEFAULT_TIMEOUT): + super().__init__(base_url, client_id, client_secret, timeout=timeout) def translate_dict_values(items, keys): diff --git a/analytics_dashboard/courses/presenters/__init__.py b/analytics_dashboard/courses/presenters/__init__.py index 7296e90bf..b63b67287 100644 --- a/analytics_dashboard/courses/presenters/__init__.py +++ b/analytics_dashboard/courses/presenters/__init__.py @@ -2,6 +2,7 @@ import datetime import logging from collections import OrderedDict +from urllib.parse import urljoin from analyticsclient.client import Client from django.conf import settings @@ -60,9 +61,13 @@ class CourseAPIPresenterMixin(metaclass=abc.ABCMeta): _last_updated = None - def __init__(self, access_token, course_id, analytics_client): + def __init__(self, course_id, analytics_client): super().__init__(course_id, analytics_client) - self.course_api_client = CourseStructureApiClient(settings.COURSE_API_URL, access_token) + self.course_api_client = CourseStructureApiClient( + settings.BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL, + settings.BACKEND_SERVICE_EDX_OAUTH2_KEY, + settings.BACKEND_SERVICE_EDX_OAUTH2_SECRET, + ) def _get_structure(self): """ Retrieves course structure from the course API. """ @@ -76,7 +81,10 @@ def _get_structure(self): 'all_blocks': 'true', 'requested_fields': 'children,format,graded', } - structure = self.course_api_client.blocks().get(**blocks_kwargs) + structure = self.course_api_client.get( + urljoin(settings.COURSE_API_URL + '/', 'blocks/'), + params=blocks_kwargs + ).json() cache.set(key, structure) return structure diff --git a/analytics_dashboard/courses/presenters/performance.py b/analytics_dashboard/courses/presenters/performance.py index 65db6f7f7..2a5e3d0a5 100644 --- a/analytics_dashboard/courses/presenters/performance.py +++ b/analytics_dashboard/courses/presenters/performance.py @@ -1,12 +1,13 @@ import logging from collections import OrderedDict, namedtuple +from urllib.parse import urljoin from analyticsclient.exceptions import NotFoundError from django.conf import settings from django.core.cache import cache from django.urls import reverse from django.utils.translation import ugettext_lazy as _ -from edx_rest_api_client.exceptions import HttpClientError +from requests.exceptions import HTTPError from slugify import slugify from common.course_structure import CourseStructure @@ -41,9 +42,13 @@ class CoursePerformancePresenter(CourseAPIPresenterMixin, CoursePresenter): # minimum screen space a grading policy bar will take up (even if a policy is 0%, display some bar) MIN_POLICY_DISPLAY_PERCENT = 5 - def __init__(self, access_token, course_id, analytics_client): - super().__init__(access_token, course_id, analytics_client) - self.grading_policy_client = CourseStructureApiClient(settings.GRADING_POLICY_API_URL, access_token) + def __init__(self, course_id, analytics_client): + super().__init__(course_id, analytics_client) + self.grading_policy_client = CourseStructureApiClient( + settings.BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL, + settings.BACKEND_SERVICE_EDX_OAUTH2_KEY, + settings.BACKEND_SERVICE_EDX_OAUTH2_SECRET, + ) def course_module_data(self): try: @@ -183,7 +188,9 @@ def grading_policy(self): if not grading_policy: logger.debug('Retrieving grading policy for course: %s', self.course_id) - grading_policy = self.grading_policy_client.policy.courses(self.course_id).get() + grading_policy = self.grading_policy_client.get( + urljoin(settings.GRADING_POLICY_API_URL + '/', f'policy/courses/{self.course_id}'), + ).json() # Remove empty assignment types as they are not useful and will cause issues downstream. grading_policy = [item for item in grading_policy if item['assignment_type']] @@ -495,7 +502,7 @@ def _get_course_module_data(self): try: logger.debug("Retrieving tags distribution for course: %s", self.course_id) return self._course_module_data() - except HttpClientError as e: + except HTTPError as e: logger.error("Unable to retrieve tags distribution info for %s: %s", self.course_id, e) return {} diff --git a/analytics_dashboard/courses/tests/test_presenters/test_presenters.py b/analytics_dashboard/courses/tests/test_presenters/test_presenters.py index 89cb9f930..77ea6cf06 100644 --- a/analytics_dashboard/courses/tests/test_presenters/test_presenters.py +++ b/analytics_dashboard/courses/tests/test_presenters/test_presenters.py @@ -7,7 +7,6 @@ from analyticsclient.constants import activity_types as AT from analyticsclient.constants import enrollment_modes from ddt import data, ddt, unpack -from django.conf import settings from django.core.cache import cache from django.test import TestCase, override_settings from django.urls import reverse @@ -214,7 +213,7 @@ class CourseEngagementVideoPresenterTests(TestCase): def setUp(self): super().setUp() self.course_id = 'this/course/id' - self.presenter = CourseEngagementVideoPresenter(settings.COURSE_API_KEY, self.course_id, Client('base_url')) + self.presenter = CourseEngagementVideoPresenter(self.course_id, Client('base_url')) def test_default_block_data(self): self.assertDictEqual(self.presenter.default_block_data, { @@ -263,8 +262,8 @@ def test_graded_modes(self, grade_status): course_structure_fixtures = self._create_graded_and_ungraded_course_structure_fixtures() course_fixture = course_structure_fixtures['course'] chapter_fixture = course_structure_fixtures['chapter'] - - with mock.patch('slumber.Resource.get', mock.Mock(return_value=course_fixture.course_structure())): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = course_fixture.course_structure() with mock.patch('analyticsclient.course.Course.videos', mock.Mock(return_value=utils.get_mock_video_data(course_fixture))): # check that we get results for both graded and ungraded @@ -595,7 +594,8 @@ def test_sibling(self, fixture, start_block, offset, expected_sibling_block): with mock.patch( 'analyticsclient.course.Course.videos', mock.Mock(return_value=utils.get_mock_video_data(fixture)) ): - with mock.patch('slumber.Resource.get', mock.Mock(return_value=fixture.course_structure())): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = fixture.course_structure() sibling = self.presenter.sibling_block(utils.get_encoded_module_id(start_block['id']), offset) if expected_sibling_block is None: self.assertIsNone(sibling) @@ -628,7 +628,8 @@ def test_sibling_no_data(self): 'analyticsclient.course.Course.videos', mock.Mock(return_value=utils.get_mock_video_data(fixture, excluded_module_ids=[self.VIDEO_2['id']])) ): - with mock.patch('slumber.Resource.get', mock.Mock(return_value=fixture.course_structure())): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = fixture.course_structure() sibling = self.presenter.sibling_block(utils.get_encoded_module_id(self.VIDEO_1['id']), 1) self.assertEqual(sibling['id'], utils.get_encoded_module_id(self.VIDEO_3['id'])) @@ -809,7 +810,7 @@ def setUp(self): cache.clear() self.course_id = PERFORMER_PRESENTER_COURSE_ID self.problem_id = 'i4x://edX/DemoX.1/problem/05d289c5ad3d47d48a77622c4a81ec36' - self.presenter = CoursePerformancePresenter(settings.COURSE_API_KEY, self.course_id, Client('base_url')) + self.presenter = CoursePerformancePresenter(self.course_id, Client('base_url')) self.factory = CoursePerformanceDataFactory() # First and last response counts were added, insights can handle both types of API responses at the moment. @@ -900,14 +901,14 @@ def assertAnswerDistribution(self, expected_problem_parts, expected_questions, a self.assertListEqual(answer_distribution_entry.answer_distribution_limited, expected_answer_distribution[:12]) - @mock.patch('slumber.Resource.get', mock.Mock(return_value=CoursePerformanceDataFactory.grading_policy)) - def test_grading_policy(self): + @mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') + def test_grading_policy(self, api_client_get_mock): """ Verify the presenter returns the correct grading policy. Empty (non-truthy) assignment types should be dropped. """ - + api_client_get_mock.return_value.json.return_value = CoursePerformanceDataFactory.grading_policy grading_policy = self.presenter.grading_policy() self.assertListEqual(grading_policy, self.factory.presented_grading_policy) @@ -927,8 +928,8 @@ def test_assignments(self): """ Verify the presenter returns the correct assignments and sets the last updated date. """ self.assertIsNone(self.presenter.last_updated) - - with mock.patch('slumber.Resource.get', mock.Mock(return_value=self.factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = self.factory.structure with mock.patch('analyticsclient.course.Course.problems', self.factory.problems): # With no assignment type set, the method should return all assignment types. assignments = self.presenter.assignments() @@ -964,8 +965,8 @@ def test_problem(self): """ Verify the presenter returns a specific problem. """ problem = self.factory.presented_assignments[0]['children'][0] _id = problem['id'] - - with mock.patch('slumber.Resource.get', mock.Mock(return_value=self.factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = self.factory.structure actual = self.presenter.block(_id) expected = { 'id': _id, @@ -977,7 +978,8 @@ def test_problem(self): def test_sections(self): """ Verify the presenter returns a specific assignment. """ ungraded_problems = self.factory.problems(False) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=self.factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = self.factory.structure with mock.patch('analyticsclient.course.Course.problems', mock.Mock(return_value=ungraded_problems)): expected = self.factory.presented_sections self.assertListEqual(self.presenter.sections(), expected) @@ -985,7 +987,8 @@ def test_sections(self): def test_section(self): """ Verify the presenter returns a specific assignment. """ ungraded_problems = self.factory.problems(False) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=self.factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = self.factory.structure with mock.patch('analyticsclient.course.Course.problems', mock.Mock(return_value=ungraded_problems)): # The method should return None if the assignment does not exist. self.assertIsNone(self.presenter.section(None)) @@ -996,7 +999,8 @@ def test_section(self): def test_subsections(self): """ Verify the presenter returns a specific assignment. """ ungraded_problems = self.factory.problems(False) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=self.factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = self.factory.structure with mock.patch('analyticsclient.course.Course.problems', mock.Mock(return_value=ungraded_problems)): # The method should return None if the assignment does not exist. self.assertIsNone(self.presenter.subsections(None)) @@ -1008,7 +1012,8 @@ def test_subsections(self): def test_subsection(self): """ Verify the presenter returns a specific assignment. """ ungraded_problems = self.factory.problems(False) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=self.factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = self.factory.structure with mock.patch('analyticsclient.course.Course.problems', mock.Mock(return_value=ungraded_problems)): # The method should return None if the assignment does not exist. self.assertIsNone(self.presenter.subsection(None, None)) @@ -1021,7 +1026,8 @@ def test_subsection(self): def test_subsection_problems(self): """ Verify the presenter returns a specific assignment. """ ungraded_problems = self.factory.problems(False) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=self.factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = self.factory.structure with mock.patch('analyticsclient.course.Course.problems', mock.Mock(return_value=ungraded_problems)): # The method should return None if the assignment does not exist. self.assertIsNone(self.presenter.subsection_children(None, None)) @@ -1039,7 +1045,7 @@ class TagsDistributionPresenterTests(TestCase): def setUp(self): cache.clear() self.course_id = PERFORMER_PRESENTER_COURSE_ID - self.presenter = TagsDistributionPresenter(settings.COURSE_API_KEY, self.course_id, Client('base_url')) + self.presenter = TagsDistributionPresenter(self.course_id, Client('base_url')) @data(annotated([{"total_submissions": 21, "correct_submissions": 5, "tags": {"difficulty": ["Hard"]}}, @@ -1071,7 +1077,8 @@ def setUp(self): def test_available_tags(self, init_tags_data): factory = TagsDistributionDataFactory(init_tags_data) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = factory.structure with mock.patch('analyticsclient.course.Course.problems_and_tags', mock.Mock(return_value=factory.problems_and_tags)): available_tags = self.presenter.get_available_tags() @@ -1130,7 +1137,8 @@ def test_available_tags(self, init_tags_data): def test_tags_distribution(self, init_tags_data): factory = TagsDistributionDataFactory(init_tags_data) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = factory.structure with mock.patch('analyticsclient.course.Course.problems_and_tags', mock.Mock(return_value=factory.problems_and_tags)): tags_distribution = self.presenter.get_tags_distribution('learning_outcome') @@ -1185,7 +1193,8 @@ def test_tags_distribution(self, init_tags_data): def test_modules_marked_with_tag(self, init_tags_data): factory = TagsDistributionDataFactory(init_tags_data) - with mock.patch('slumber.Resource.get', mock.Mock(return_value=factory.structure)): + with mock.patch('analytics_dashboard.core.utils.CourseStructureApiClient.get') as api_client_get_mock: + api_client_get_mock.return_value.json.return_value = factory.structure with mock.patch('analyticsclient.course.Course.problems_and_tags', mock.Mock(return_value=factory.problems_and_tags)): modules = self.presenter.get_modules_marked_with_tag('learning_outcome', slugify('Learned nothing')) diff --git a/analytics_dashboard/courses/tests/test_views/__init__.py b/analytics_dashboard/courses/tests/test_views/__init__.py index 644d1789b..8b6f9ca98 100644 --- a/analytics_dashboard/courses/tests/test_views/__init__.py +++ b/analytics_dashboard/courses/tests/test_views/__init__.py @@ -1,8 +1,8 @@ import json import logging -from unittest import mock - from unittest.mock import Mock, patch +from urllib.parse import urljoin + import httpretty from analyticsclient.exceptions import NotFoundError from ddt import data, ddt, unpack @@ -36,7 +36,7 @@ class CourseAPIMixin: COURSE_BLOCKS_API_TEMPLATE = \ settings.COURSE_API_URL + \ 'blocks/?course_id={course_id}&requested_fields=children,graded&depth=all&all_blocks=true' - GRADING_POLICY_API_TEMPLATE = settings.GRADING_POLICY_API_URL + '/policy/courses/{course_id}/' + GRADING_POLICY_API_TEMPLATE = settings.GRADING_POLICY_API_URL + '/policy/courses/{course_id}' def mock_course_api(self, url, body=None, **kwargs): """ @@ -64,11 +64,31 @@ def mock_course_api(self, url, body=None, **kwargs): httpretty.register_uri(httpretty.GET, url, **default_kwargs) logger.debug('Mocking Course API URL: %s', url) + def mock_oauth_api(self, body=None, **kwargs): + # Avoid developer confusion when httpretty is not active and fail the test now. + if not httpretty.is_enabled(): + self.fail('httpretty is not enabled. The mock will not be used!') + + url = urljoin(settings.BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL + '/', f'access_token') + body = body or { + 'access_token': 'test_access_tocken', + 'expires_in': 10, + } + default_kwargs = { + 'body': kwargs.get('body', json.dumps(body)), + 'content_type': 'application/json' + } + default_kwargs.update(kwargs) + + httpretty.register_uri(httpretty.POST, url, **default_kwargs) + logger.debug('Mocking OAuth API URL: %s', url) + def mock_course_detail(self, course_id, extra=None): - path = f'{settings.COURSE_API_URL}courses/{course_id}/' + path = urljoin(settings.COURSE_API_URL + '/', f'courses/{course_id}') body = {'id': course_id, 'name': mock_course_name(course_id)} if extra: body.update(extra) + self.mock_oauth_api() self.mock_course_api(path, body) @@ -217,7 +237,7 @@ class CourseEnrollmentViewTestMixin(CourseViewTestMixin): @override_switch('enable_course_api', active=True) @override_switch('display_course_name_in_nav', active=True) def test_valid_course(self, course_id, age_available): - with mock.patch('analytics_dashboard.courses.views.enrollment.age_available', return_value=age_available): + with patch('analytics_dashboard.courses.views.enrollment.age_available', return_value=age_available): # we have to rebuild nav according to setting since navs are on the class EnrollmentTemplateView.secondary_nav_items = _enrollment_secondary_nav() EnrollmentDemographicsTemplateView.tertiary_nav_items = _enrollment_tertiary_nav() diff --git a/analytics_dashboard/courses/views/__init__.py b/analytics_dashboard/courses/views/__init__.py index 02686504c..9457f68ca 100644 --- a/analytics_dashboard/courses/views/__init__.py +++ b/analytics_dashboard/courses/views/__init__.py @@ -3,6 +3,7 @@ import logging import re from datetime import datetime +from urllib.parse import urljoin import requests from analyticsclient.client import Client @@ -19,11 +20,8 @@ from django.utils.translation import ugettext_noop from django.views import View from django.views.generic import TemplateView -from edx_rest_api_client.client import EdxRestApiClient -from edx_rest_api_client.exceptions import ( - HttpClientError, - SlumberBaseException, -) +from requests.exceptions import HTTPError +from requests.exceptions import RequestException from opaque_keys.edx.keys import CourseKey from waffle import switch_is_active @@ -64,13 +62,11 @@ def dispatch(self, request, *args, **kwargs): self.course_api_enabled = switch_is_active('enable_course_api') if self.course_api_enabled and request.user.is_authenticated: - self.access_token = settings.COURSE_API_KEY or EdxRestApiClient.get_and_cache_jwt_oauth_access_token( + self.course_api = CourseStructureApiClient( settings.BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL, settings.BACKEND_SERVICE_EDX_OAUTH2_KEY, settings.BACKEND_SERVICE_EDX_OAUTH2_SECRET, - timeout=(3.05, 55), - )[0] - self.course_api = CourseStructureApiClient(settings.COURSE_API_URL, self.access_token) + ) return super().dispatch(request, *args, **kwargs) @@ -92,9 +88,11 @@ def get_course_info(self, course_id): if not info: try: logger.debug("Retrieving detail for course: %s", course_id) - info = self.course_api.courses(course_id).get() + info = self.course_api.get( + urljoin(settings.COURSE_API_URL + '/', f'courses/{course_id}') + ).json() cache.set(key, info) - except HttpClientError as e: + except HTTPError as e: logger.error("Unable to retrieve course info for %s: %s", course_id, e) info = {} @@ -113,7 +111,13 @@ def get_courses(self): while page: try: logger.debug('Retrieving page %d of course info...', page) - response = self.course_api.courses.get(page=page, page_size=100) + response = self.course_api.get( + urljoin(settings.COURSE_API_URL + '/', 'courses/'), + params={ + 'page': page, + 'page_size': 100, + } + ).json() course_details = response['results'] # Cache the information so that it doesn't need to be retrieved later. @@ -129,7 +133,7 @@ def get_courses(self): else: page = None logger.debug('Completed retrieval of course info. Retrieved info for %d courses.', len(courses)) - except HttpClientError as e: + except HTTPError as e: logger.error("Unable to retrieve course data: %s", e) page = None break @@ -828,7 +832,7 @@ class CourseStructureExceptionMixin: def dispatch(self, request, *args, **kwargs): try: return super().dispatch(request, *args, **kwargs) - except SlumberBaseException as e: + except RequestException as e: # Return the appropriate response if a 404 occurred. response = getattr(e, 'response') if response is not None: diff --git a/analytics_dashboard/courses/views/engagement.py b/analytics_dashboard/courses/views/engagement.py index 1097b2d28..987d1b5b5 100644 --- a/analytics_dashboard/courses/views/engagement.py +++ b/analytics_dashboard/courses/views/engagement.py @@ -102,7 +102,7 @@ class EngagementVideoContentTemplateView(AnalyticsV0Mixin, CourseStructureMixin, no_data_message = _('Looks like no one has watched any videos in these sections.') def get_context_data(self, **kwargs): - self.presenter = CourseEngagementVideoPresenter(self.access_token, self.course_id, self.analytics_client) + self.presenter = CourseEngagementVideoPresenter(self.course_id, self.analytics_client) context = super().get_context_data(**kwargs) context.update({ 'sections': self.presenter.sections(), diff --git a/analytics_dashboard/courses/views/performance.py b/analytics_dashboard/courses/views/performance.py index 1a7392751..1eb6b47a8 100644 --- a/analytics_dashboard/courses/views/performance.py +++ b/analytics_dashboard/courses/views/performance.py @@ -77,7 +77,7 @@ def get_context_data(self, **kwargs): translate_dict_values(self.secondary_nav_items, ('text',)) context_data = super().get_context_data(**kwargs) - self.presenter = CoursePerformancePresenter(self.access_token, self.course_id, self.analytics_client) + self.presenter = CoursePerformancePresenter(self.course_id, self.analytics_client) context_data['no_data_message'] = self.no_data_message context_data['js_data']['course'].update({ @@ -381,7 +381,7 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) self.selected_tag_value = kwargs.get('tag_value', None) - self.tags_presenter = TagsDistributionPresenter(self.access_token, self.course_id, self.analytics_client) + self.tags_presenter = TagsDistributionPresenter(self.course_id, self.analytics_client) first_level_content_nav, first_selected_item = self.tags_presenter.get_tags_content_nav( 'learning_outcome', self.selected_tag_value) diff --git a/analytics_dashboard/settings/base.py b/analytics_dashboard/settings/base.py index 3067afe1c..accfe908a 100644 --- a/analytics_dashboard/settings/base.py +++ b/analytics_dashboard/settings/base.py @@ -419,7 +419,7 @@ # before giving up. These values should be overridden in # configuration. ANALYTICS_API_DEFAULT_TIMEOUT = 10 -LMS_DEFAULT_TIMEOUT = 5 +LMS_DEFAULT_TIMEOUT = (3.05, 5) ########## END EXTERNAL SERVICE TIMEOUTS _ = lambda s: s diff --git a/common/clients.py b/common/clients.py index 2da994dab..46a377be8 100644 --- a/common/clients.py +++ b/common/clients.py @@ -1,12 +1,14 @@ import logging +from urllib.parse import urljoin -from edx_rest_api_client.client import EdxRestApiClient -from edx_rest_api_client.exceptions import HttpClientError +from django.conf import settings +from edx_rest_api_client.client import OAuthAPIClient +from requests.exceptions import HTTPError logger = logging.getLogger(__name__) -class CourseStructureApiClient(EdxRestApiClient): +class CourseStructureApiClient(OAuthAPIClient): """ This class is a sub-class of the edX Rest API Client (https://github.com/edx/edx-rest-api-client). @@ -16,9 +18,6 @@ class CourseStructureApiClient(EdxRestApiClient): """ DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%SZ" - def __init__(self, url, access_token, timeout): - super().__init__(url, jwt=access_token, timeout=timeout) - @property def all_courses(self): courses = [] @@ -27,19 +26,30 @@ def all_courses(self): while page: try: logger.debug('Retrieving page %d of course info...', page) - response = self.courses.get(page=page, page_size=100) + response = self.get( + urljoin(settings.COURSE_API_URL + '/', 'courses/'), + params={ + 'page': page, + 'page_size': 100 + } + ).json() course_details = response['results'] courses += course_details - if response['next']: + if response['pagination']['next']: page += 1 else: page = None logger.debug('Completed retrieval of course info. Retrieved info for %d courses.', len(courses)) - except HttpClientError as e: + except (KeyError, HTTPError) as e: logger.error("Unable to retrieve course data: %s", e) page = None break return courses + + def request(self, method, url, **kwargs): # pylint: disable=arguments-differ + response = super().request(method, url, **kwargs) + response.raise_for_status() + return response