Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Commit

Permalink
refactor: Remove EdxRestApiClient usage in edx-analytics-dashboard
Browse files Browse the repository at this point in the history
  • Loading branch information
UvgenGen committed Apr 7, 2022
1 parent 1ec5e93 commit 1851a55
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 70 deletions.
17 changes: 13 additions & 4 deletions analytics_dashboard/core/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
4 changes: 2 additions & 2 deletions analytics_dashboard/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
14 changes: 11 additions & 3 deletions analytics_dashboard/courses/presenters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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. """
Expand All @@ -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
Expand Down
19 changes: 13 additions & 6 deletions analytics_dashboard/courses/presenters/performance.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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']]
Expand Down Expand Up @@ -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 {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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']))

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)

Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -977,15 +978,17 @@ 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)

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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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"]}},
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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'))
Expand Down
Loading

0 comments on commit 1851a55

Please sign in to comment.