diff --git a/course_discovery/apps/core/api_client/lms.py b/course_discovery/apps/core/api_client/lms.py index 259fe1851b..74f92f71e2 100644 --- a/course_discovery/apps/core/api_client/lms.py +++ b/course_discovery/apps/core/api_client/lms.py @@ -2,8 +2,10 @@ API Client for LMS. """ import logging -from urllib.parse import urljoin +from typing import Optional, Union +from urllib.parse import urlencode, urljoin +from django.conf import settings from django.core.cache import cache from edx_django_utils.cache import get_cache_key from requests.exceptions import RequestException @@ -51,7 +53,7 @@ def get_api_access_request(self, user): "contacted": True } """ - resource = 'api-admin/api/v1/api_access_request/' + resource = settings.LMS_API_URLS['api_access_request'] query_parameters = { 'user__username': user.username } @@ -89,3 +91,121 @@ def get_api_access_request(self, user): exception.__class__.__name__, user.username) return api_access_request + + def _get_blocks_data( + self, + item_id: str, + cache_key: str, + query_parameters: Union[str, dict], + resource: str, + response_root_key: Optional[str] = None, + ): + """ + Helper function to fetch blocks based on given resourse and item_id. + + Args: + item_id (str): course_id or block_id + cache_key (str): cache key + query_parameters (Union[str, dict]): query parameters for the request + resource (str): resource url + + Returns: + (dict): dict with xblock data + """ + cached_blocks = cache.get(cache_key) + + if cached_blocks is SENTINEL_NO_RESULT: + return None + + if cached_blocks: + return cached_blocks + + blocks = None + try: + resource_url = urljoin(self.lms_url, resource) + response = self.client.get(resource_url, params=query_parameters) + response.raise_for_status() + blocks = response.json() + if response_root_key: + blocks = blocks[response_root_key] + if blocks: + cache.set(cache_key, blocks, ONE_HOUR) + else: + cache.set(cache_key, SENTINEL_NO_RESULT, ONE_HOUR) + logger.info('No blocks found for [%s].', item_id) + + except (RequestException, KeyError) as exception: + cache.set(cache_key, SENTINEL_NO_RESULT, ONE_MINUTE) + logger.exception('%s: Failed to fetch blocks from LMS for [%s].', + exception.__class__.__name__, item_id) + + return blocks + + def get_course_blocks_data(self, course_id: str, **kwargs): + """ + Get all xblocks under a given course. + + Args: + course_id (str): course key + **kwargs: Can be used to pass additional query params to api + + Returns: + (dict): dict with xblock data + """ + resource = settings.LMS_API_URLS['blocks'] + query_parameters = { + 'course_id': course_id, + 'all_blocks': True, + 'depth': 'all', + 'requested_fields': 'children', + **kwargs, + } + encoded_query_parameters = urlencode(query_parameters, safe=':') + cache_key = get_cache_key(course_id=course_id, resource=resource) + return self._get_blocks_data( + course_id, + cache_key, + encoded_query_parameters, + resource, + response_root_key='blocks', + ) + + def get_blocks_data(self, block_id: str, **kwargs): + """ + Get xblock data for given block_id or all blocks for given course_id. + + Args: + block_id (str): usage key + **kwargs: Can be used to pass additional query params to api + + Returns: + (dict): dict with xblock data + """ + resource = settings.LMS_API_URLS['blocks'] + block_id + query_parameters = { + 'all_blocks': True, + 'depth': 'all', + 'requested_fields': 'children', + **kwargs, + } + cache_key = get_cache_key(block_id=block_id, resource=resource) + return self._get_blocks_data(block_id, cache_key, query_parameters, resource, response_root_key='blocks') + + def get_blocks_metadata(self, block_id: str, **kwargs): + """ + Get xblock metadata for given block_id. + + Args: + block_id (str): usage key + **kwargs: Can be used to pass additional query params to api + + Returns: + (dict): dict with xblock data + """ + resource = settings.LMS_API_URLS['block_metadata'] + block_id + query_parameters = { + 'include': 'index_dictionary', + **kwargs, + } + cache_key = get_cache_key(block_id=block_id, resource=resource) + return self._get_blocks_data(block_id, cache_key, query_parameters, resource) diff --git a/course_discovery/apps/core/tests/mixins.py b/course_discovery/apps/core/tests/mixins.py index a1ce1a2fb9..6766cca661 100644 --- a/course_discovery/apps/core/tests/mixins.py +++ b/course_discovery/apps/core/tests/mixins.py @@ -124,3 +124,91 @@ def mock_api_access_request_with_invalid_data(self, lms_url, user, status=200, r content_type='application/json', status=status, ) + + def mock_blocks_data_request(self, lms_url, override_blocks=None, status=200): + """ + Mock the blocks data requests endpoint response of the LMS. + """ + data = { + 'root': 'block-v1:edX+DemoX+Demo_Course+type@course+block@course', + 'blocks': { + 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4': { + 'id': 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4', + 'block_id': '030e35c4756a4ddc8d40b95fbbfff4d4', + 'type': 'html', + 'display_name': 'Blank HTML Page', + }, + 'block-v1:edX+DemoX+Demo_Course+type@video+block@0b9e39477cf34507a7a48f74be381fdd': { + 'id': 'block-v1:edX+DemoX+Demo_Course+type@video+block@0b9e39477cf34507a7a48f74be381fdd', + 'block_id': '0b9e39477cf34507a7a48f74be381fdd', + 'type': 'video', + 'display_name': 'Welcome!', + }, + 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical_0270f6de40fc': { + 'id': 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical_0270f6de40fc', + 'block_id': 'vertical_0270f6de40fc', + 'type': 'vertical', + 'display_name': 'Introduction: Video and Sequences', + 'children': [ + 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4', + 'block-v1:edX+DemoX+Demo_Course+type@video+block@0b9e39477cf34507a7a48f74be381fdd' + ] + }, + } + } + if override_blocks is not None: + data['blocks'] = override_blocks + + responses.add( + responses.GET, + lms_url, + body=json.dumps(data), + content_type='application/json', + status=status, + ) + return data + + def mock_block_metadata_request(self, base_url, status=200): + data = { + 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4': { + 'id': 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4', + 'type': 'html', + 'index_dictionary': { + 'content': { + 'display_name': 'Blank HTML Page', + 'html_content': 'Welcome to the Open edX Demo Course Introduction.' + }, + 'content_type': 'Text' + } + }, + 'block-v1:edX+DemoX+Demo_Course+type@video+block@0b9e39477cf34507a7a48f74be381fdd': { + 'id': 'block-v1:edX+DemoX+Demo_Course+type@video+block@0b9e39477cf34507a7a48f74be381fdd', + 'type': 'video', + 'index_dictionary': { + 'content': { + 'display_name': 'Welcome!', + 'transcript_en': ' ERIC: Hi, and welcome to the edX demonstration course.' + }, + 'content_type': 'Video' + } + }, + 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical_0270f6de40fc': { + 'id': 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical_0270f6de40fc', + 'type': 'vertical', + 'index_dictionary': { + 'content': { + 'display_name': 'Introduction: Video and Sequences' + }, + 'content_type': 'Sequence' + }, + } + } + for block_id, block_body in data.items(): + responses.add( + responses.GET, + base_url + block_id, + body=json.dumps(block_body), + content_type='application/json', + status=status, + ) + return data diff --git a/course_discovery/apps/core/tests/test_api_clients.py b/course_discovery/apps/core/tests/test_api_clients.py index 40c763e377..f044bb75e5 100644 --- a/course_discovery/apps/core/tests/test_api_clients.py +++ b/course_discovery/apps/core/tests/test_api_clients.py @@ -1,6 +1,8 @@ import logging +from urllib.parse import urljoin import responses +from django.conf import settings from django.test import TestCase from course_discovery.apps.core.api_client import lms @@ -41,6 +43,11 @@ def setUp(self): 'site': 1, 'contacted': True } + self.block_id = 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical_0270f6de40fc' + resource = settings.LMS_API_URLS['blocks'] + self.block_resource_url = urljoin(self.partner.lms_url, resource + self.block_id) + self.course_resource_url = urljoin(self.partner.lms_url, resource) + self.block_metadata_base_url = urljoin(self.partner.lms_url, settings.LMS_API_URLS['block_metadata']) @responses.activate def test_get_api_access_request(self): @@ -185,3 +192,48 @@ def test_get_api_access_request_with_multiple_records(self): assert self.lms.get_api_access_request(self.user)['company_name'] == 'Test Company' assert 'Multiple ApiAccessRequest models returned from LMS API for user [%s].' % self.user.username in \ self.log_messages['warning'] + + @responses.activate + def test_get_course_blocks_data(self): + """ + Verify that `get_course_blocks_data` returns correct value. + """ + data = self.mock_blocks_data_request(self.course_resource_url) + assert self.lms.get_course_blocks_data('dummy-course-id') == data['blocks'] + + @responses.activate + def test_get_blocks_data(self): + """ + Verify that `get_blocks_data` returns correct value. + """ + data = self.mock_blocks_data_request(self.block_resource_url) + assert self.lms.get_blocks_data(self.block_id) == data['blocks'] + + @responses.activate + def test_get_block_metadata(self): + """ + Verify that `get_blocks_metadata` returns correct value. + """ + data = self.mock_block_metadata_request(self.block_metadata_base_url) + assert self.lms.get_blocks_metadata(self.block_id) == data[self.block_id] + + @responses.activate + def test_get_blocks_data_with_no_results(self): + """ + Verify that `get_blocks_data` returns None when + API returns no results. + """ + self.mock_blocks_data_request(self.block_resource_url, override_blocks={}) + assert not self.lms.get_blocks_data(self.block_id) + assert 'No blocks found for [%s].' % self.block_id in self.log_messages['info'] + + @responses.activate + def test_get_blocks_data_cache_hit(self): + """ + Verify that `get_blocks_data` returns the correct value and then + returns the cached results on another call with the same block_id. + """ + data = self.mock_blocks_data_request(self.block_resource_url) + assert self.lms.get_blocks_data(self.block_id) == data['blocks'] + assert self.lms.get_blocks_data(self.block_id) == data['blocks'] + assert len(responses.calls) == 1 diff --git a/course_discovery/apps/taxonomy_support/providers.py b/course_discovery/apps/taxonomy_support/providers.py index ca817fc49d..c7e5149101 100644 --- a/course_discovery/apps/taxonomy_support/providers.py +++ b/course_discovery/apps/taxonomy_support/providers.py @@ -13,9 +13,13 @@ For a more detailed explanation of the implementation and thinking behind this provider can be found at https://openedx.atlassian.net/wiki/spaces/SOL/pages/1814922129/Platform+Agnostic+Implementation+of+Taxonomy+Application """ +from django.conf import settings +from django.utils.translation import gettext_lazy as _ from edx_django_utils.db import chunked_queryset -from taxonomy.providers import CourseMetadataProvider, ProgramMetadataProvider +from taxonomy.providers import CourseMetadataProvider, ProgramMetadataProvider, XBlockContent, XBlockMetadataProvider +from course_discovery.apps.core.api_client.lms import LMSAPIClient +from course_discovery.apps.core.models import Partner from course_discovery.apps.course_metadata.contentful_utils import ( fetch_and_transform_bootcamp_contentful_data, fetch_and_transform_degree_contentful_data, get_aggregated_data_from_contentful_data @@ -31,7 +35,7 @@ class DiscoveryCourseMetadataProvider(CourseMetadataProvider): @staticmethod def get_courses(course_ids): # lint-amnesty, pylint: disable=arguments-differ """ - Get list of courses matching the given course UUIDs and return then in the form of a dict. + Get list of courses matching the given course UUIDs and return them in the form of a dict. """ courses = Course.everything.filter(uuid__in=course_ids).distinct() contentful_data = fetch_and_transform_bootcamp_contentful_data() @@ -74,7 +78,7 @@ class DiscoveryProgramMetadataProvider(ProgramMetadataProvider): @staticmethod def get_programs(program_ids): # lint-amnesty, pylint: disable=arguments-differ """ - Get list of programs matching the given program UUIDs and return then in the form of a dict. + Get list of programs matching the given program UUIDs and return them in the form of a dict. """ programs = Program.objects.filter(uuid__in=program_ids).distinct() contentful_data = fetch_and_transform_degree_contentful_data() @@ -106,3 +110,84 @@ def get_all_programs(): # lint-amnesty, pylint: disable=arguments-differ program.overview ), } + + +class DiscoveryXBlockMetadataProvider(XBlockMetadataProvider): + """ + Discovery xblock provider. + """ + + def __init__(self): + """ + Get lms client with default partner object. + """ + if settings.DEFAULT_PARTNER_ID: + partner = Partner.objects.filter(id=settings.DEFAULT_PARTNER_ID).first() + else: + partner = Partner.objects.first() + if partner is None: + raise KeyError(_('No partner object found!')) + self.client = LMSAPIClient(partner) + + def _get_block_content(self, block_id: str) -> list: + """ + Fetches block metadata i.e. `index_dictionary` using lms api and + returns content values as unique list. + """ + block_metadata = self.client.get_blocks_metadata(block_id) or {} + content = block_metadata.get('index_dictionary', {}).get('content', {}) + content_list = [] + for content_text in content.values(): + content_text = str(content_text).strip() if content_text else None + if content_text: + content_list.append(content_text) + return content_list + + def _combine_text_data(self, cur_block, all_blocks): + """ + Recursively combines content values in all children blocks. + """ + block_id = cur_block.get('id') + content_list = [] + if block_id: + content_list = self._get_block_content(block_id) + for child in cur_block.get('children', []): + child_block = all_blocks.get(child) + if child_block: + content_list.extend(self._combine_text_data(child_block, all_blocks)) + # return ordered unique list of content. + return list(dict.fromkeys(content_list)) + + def get_xblocks(self, xblock_ids): + """ + Get list of xblocks matching the given xblock UUIDs and return them in + the form of XBlockContent. + """ + + blocks_data = set() + for block_id in xblock_ids: + all_blocks = self.client.get_blocks_data(block_id) or {} + for block in all_blocks.values(): + if block['type'] in settings.TAXONOMY_XBLOCK_SUPPORTED_TYPES: + content = '\n'.join(self._combine_text_data(block, all_blocks)) + blocks_data.add(XBlockContent( + key=block.get('id'), + content_type=block.get('type'), + content=content, + )) + return list(blocks_data) + + def get_all_xblocks_in_course(self, course_id: str): + """ + Get iterator for all unit/video xblocks in course + """ + blocks = self.client.get_course_blocks_data(course_id) or {} + + for block in blocks.values(): + if block['type'] in settings.TAXONOMY_XBLOCK_SUPPORTED_TYPES: + content = '\n'.join(self._combine_text_data(block, blocks)) + yield XBlockContent( + key=block.get('id'), + content_type=block.get('type'), + content=content, + ) diff --git a/course_discovery/apps/taxonomy_support/tests/test_providers.py b/course_discovery/apps/taxonomy_support/tests/test_providers.py index 08179bc5f2..c22976db17 100644 --- a/course_discovery/apps/taxonomy_support/tests/test_providers.py +++ b/course_discovery/apps/taxonomy_support/tests/test_providers.py @@ -2,8 +2,8 @@ Validate taxonomy integration. This file validates the following - 1. Make sure the provider specified by `TAXONOMY_COURSE_METADATA_PROVIDER` and `TAXONOMY_PROGRAM_METADATA_PROVIDER` - implements all the abstract methods + 1. Make sure the provider specified by `TAXONOMY_COURSE_METADATA_PROVIDER`, `TAXONOMY_PROGRAM_METADATA_PROVIDER` + and `TAXONOMY_XBLOCK_METADATA_PROVIDER` implements all the abstract methods 2. Make sure the signature of all the methods match with the interfaces of the abstract class 3. Make sure the data returned and the structure of the data matches with the definitions inside the interface. @@ -17,6 +17,7 @@ Note: Course Metadata validator will use the provider pointed by the `TAXONOMY_COURSE_METADATA_PROVIDER` django setting. Note: Program Metadata validator will use the provider pointed by the `TAXONOMY_PROGRAM_METADATA_PROVIDER` django setting. +Note: XBlock Metadata validator will use the provider pointed by the `TAXONOMY_XBLOCK_METADATA_PROVIDER` django setting. Reason behind keeping the validator a part of taxonomy-connector is to keep the provider and its validation logic in the same repository, so whenever a new dependency (e.g. a new method or a new field in the returned data) is added in the @@ -24,14 +25,20 @@ discovery and its interface in taxonomy are always in sync. """ from unittest import mock +from urllib.parse import urljoin +from django.conf import settings from django.test import TestCase -from taxonomy.validators import CourseMetadataProviderValidator, ProgramMetadataProviderValidator +from taxonomy.validators import ( + CourseMetadataProviderValidator, ProgramMetadataProviderValidator, XBlockMetadataProviderValidator +) +from course_discovery.apps.core.tests.factories import PartnerFactory +from course_discovery.apps.core.tests.mixins import LMSAPIClientMixin from course_discovery.apps.course_metadata.tests.factories import CourseFactory, ProgramFactory -class TaxonomyIntegrationTests(TestCase): +class TaxonomyIntegrationTests(TestCase, LMSAPIClientMixin): """ Validate integration of taxonomy_support and metadata providers. """ @@ -62,3 +69,24 @@ def test_validate_program_metadata(self, _contentful_data): # Run all the validations, note that an assertion error will be raised if any of the validation fail. program_metadata_validator.validate() + + def test_validate_xblock_metadata(self): + """ + Validate that there are no integration issues in xblock provider. + """ + self.mock_access_token() + partner = PartnerFactory.create(id=settings.DEFAULT_PARTNER_ID, lms_url='http://127.0.0.1:8000') + block_ids = [ + 'block-v1:edX+DemoX+Demo_Course+type@video+block@0b9e39477cf34507a7a48f74be381fdd', + 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical_0270f6de40fc', + ] + resource = settings.LMS_API_URLS['blocks'] + for block_id in block_ids: + block_resource_url = urljoin(partner.lms_url, resource + block_id) + block_metadata_url = urljoin(partner.lms_url, 'api/courses/v1/block_metadata/') + self.mock_blocks_data_request(block_resource_url) + self.mock_block_metadata_request(block_metadata_url) + xblock_metadata_validator = XBlockMetadataProviderValidator(block_ids) + + # Run all the validations, note that an assertion error will be raised if any of the validation fail. + xblock_metadata_validator.validate() diff --git a/course_discovery/settings/base.py b/course_discovery/settings/base.py index e8f72ee9fb..de85f18f15 100644 --- a/course_discovery/settings/base.py +++ b/course_discovery/settings/base.py @@ -593,6 +593,8 @@ # Settings related to the taxonomy_support TAXONOMY_COURSE_METADATA_PROVIDER = 'course_discovery.apps.taxonomy_support.providers.DiscoveryCourseMetadataProvider' TAXONOMY_PROGRAM_METADATA_PROVIDER = 'course_discovery.apps.taxonomy_support.providers.DiscoveryProgramMetadataProvider' +TAXONOMY_XBLOCK_METADATA_PROVIDER = 'course_discovery.apps.taxonomy_support.providers.DiscoveryXBlockMetadataProvider' +TAXONOMY_XBLOCK_SUPPORTED_TYPES = ['video', 'vertical'] # Settings related to the EMSI client EMSI_API_ACCESS_TOKEN_URL = 'https://auth.emsicloud.com/connect/token' @@ -679,3 +681,8 @@ CONTENTFUL_SPACE_ID = None CONTENTFUL_CONTENT_DELIVERY_API_KEY = None CONTENTFUL_ENVIRONMENT = None +LMS_API_URLS = { + 'api_access_request': 'api-admin/api/v1/api_access_request/', + 'blocks': 'api/courses/v1/blocks/', + 'block_metadata': 'api/courses/v1/block_metadata/', +}