Skip to content

Commit

Permalink
feat: sync full metadata for restricted courses
Browse files Browse the repository at this point in the history
ENT-9597
  • Loading branch information
iloveagent57 committed Oct 21, 2024
1 parent 5ea87f4 commit f92b203
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 13 deletions.
59 changes: 52 additions & 7 deletions enterprise_catalog/apps/api/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
FORCE_INCLUSION_METADATA_TAG_KEY,
LEARNER_PATHWAY,
PROGRAM,
QUERY_FOR_RESTRICTED_RUNS,
REINDEX_TASK_BATCH_SIZE,
TASK_BATCH_SIZE,
VIDEO,
Expand All @@ -49,6 +50,7 @@
from enterprise_catalog.apps.catalog.models import (
CatalogQuery,
ContentMetadata,
RestrictedCourseMetadata,
create_course_associated_programs,
update_contentmetadata_from_discovery,
)
Expand All @@ -73,7 +75,7 @@
EXPLORE_CATALOG_TITLES = ['A la carte', 'Subscription']


def _fetch_courses_by_keys(course_keys):
def _fetch_courses_by_keys(course_keys, extra_query_params=None):
"""
Fetches course data from discovery's /api/v1/courses endpoint for the provided course keys.
Expand All @@ -82,7 +84,7 @@ def _fetch_courses_by_keys(course_keys):
Returns:
list of dict: Returns a list of dictionaries where each dictionary represents the course data from discovery.
"""
return DiscoveryApiClient().fetch_courses_by_keys(course_keys)
return DiscoveryApiClient().fetch_courses_by_keys(course_keys, extra_query_params=extra_query_params)


def _fetch_programs_by_keys(program_keys):
Expand Down Expand Up @@ -283,6 +285,14 @@ def _update_full_content_metadata_course(content_keys, dry_run=False):
)
modified_content_metadata_records.append(modified_course_record)

program_content_keys = create_course_associated_programs(
course_metadata_dict.get('programs', []),
modified_course_record,
)
_update_full_content_metadata_program(program_content_keys, dry_run)

_update_full_restricted_course_metadata(modified_course_record, course_review, dry_run)

if dry_run:
logger.info('dry_run=True, not updating course metadata')
else:
Expand All @@ -307,6 +317,46 @@ def _update_full_content_metadata_course(content_keys, dry_run=False):
)


def _update_full_restricted_course_metadata(modified_metadata_record, course_review, dry_run):
"""
For all restricted courses whose parent is ``modified_metadata_record``, does a full
update of metadata and restricted run relationships.
"""
restricted_courses = list(modified_metadata_record.restricted_courses.all())
if not restricted_courses:
return

# Fetch from /api/v1/courses with restricted content included
metadata_list = _fetch_courses_by_keys(
[modified_metadata_record.content_key],
extra_query_params=QUERY_FOR_RESTRICTED_RUNS,
)
if not metadata_list:
raise Exception(
f'No restricted course metadata could be fetched for {modified_metadata_record.content_key}'
)

restricted_metadata = metadata_list[0]

for restricted_course in restricted_courses:
# First, run the "standard" transformations below to update with the full
# course metadata, do normalization, etc.
_update_single_full_course_record(
restricted_metadata, restricted_course, course_review, dry_run,
)
restricted_course.save()
# Second, update the restricted course record to filter out
# restricted runs for the restricted course and ensure the proper
# course <-> run relationships.
# restricted_metadata.content_key will be the same as restricted_course.content_key.
if restricted_course.catalog_query:
RestrictedCourseMetadata.store_record_with_query(
restricted_metadata, restricted_course.catalog_query,
)
else:
RestrictedCourseMetadata.store_canonical_record(restricted_metadata)


def _get_course_records_by_key(fetched_course_keys):
"""
Helper to fetch a dict of course `ContentMetadata` records by content key.
Expand Down Expand Up @@ -344,11 +394,6 @@ def _update_single_full_course_record(course_metadata_dict, metadata_record, cou
metadata_record.content_key, json.dumps(metadata_record.json_metadata)
))

program_content_keys = create_course_associated_programs(
course_metadata_dict.get('programs', []),
metadata_record,
)
_update_full_content_metadata_program(program_content_keys, dry_run)
return metadata_record


Expand Down
71 changes: 67 additions & 4 deletions enterprise_catalog/apps/api/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
FORCE_INCLUSION_METADATA_TAG_KEY,
LEARNER_PATHWAY,
PROGRAM,
QUERY_FOR_RESTRICTED_RUNS,
)
from enterprise_catalog.apps.catalog.models import CatalogQuery, ContentMetadata
from enterprise_catalog.apps.catalog.serializers import (
Expand Down Expand Up @@ -2377,6 +2378,7 @@ def test_index_algolia_dry_run(self, mock_search_client):
@mock.patch('enterprise_catalog.apps.api.tasks.ContentMetadata.objects.filter')
@mock.patch('enterprise_catalog.apps.api.tasks.create_course_associated_programs')
@mock.patch('enterprise_catalog.apps.api.tasks._update_full_content_metadata_program')
@override_settings(SHOULD_FETCH_RESTRICTED_COURSE_RUNS=True)
def test_update_full_content_metadata_course(
self,
mock_update_content_metadata_program,
Expand All @@ -2388,8 +2390,18 @@ def test_update_full_content_metadata_course(
# Mock data
content_keys = ['course1', 'course2']
full_course_dicts = [
{'key': 'course1', 'title': 'Course 1', FORCE_INCLUSION_METADATA_TAG_KEY: True},
{'key': 'course2', 'title': 'Course 2'}
{
'key': 'course1',
'title': 'Course 1',
FORCE_INCLUSION_METADATA_TAG_KEY: True,
'course_runs': [
{'key': 'course-run-1'},
]
},
{
'key': 'course2',
'title': 'Course 2',
},
]
reviews_for_courses_dict = {
'course1': {'reviews_count': 10, 'avg_course_rating': 4.5},
Expand All @@ -2399,15 +2411,53 @@ def test_update_full_content_metadata_course(
content_metadata_2 = ContentMetadataFactory(content_type=COURSE, content_key='course2')
metadata_records_for_fetched_keys = [content_metadata_1, content_metadata_2]

restricted_course_dicts = [
{
'key': 'course1',
'title': 'Course 1',
'other': 'stuff',
FORCE_INCLUSION_METADATA_TAG_KEY: True,
'course_runs': [
{'key': 'course-run-1'},
{'key': 'course-run-1-restricted'},
]
},
]
restricted_course = RestrictedCourseMetadataFactory(
unrestricted_parent=content_metadata_1,
content_type=COURSE,
content_key='course1',
_json_metadata={
'course_runs': [
{'key': 'course-run-1'},
{'key': 'course-run-1-restricted'},
],
}
)
restricted_run = ContentMetadataFactory(
content_type=COURSE_RUN,
content_key='course-run-1-restricted',
)
restricted_relationship = RestrictedRunAllowedForRestrictedCourseFactory(
course=restricted_course,
run=restricted_run,
)

# Configure mock objects
mock_fetch_courses_by_keys.return_value = full_course_dicts
mock_fetch_courses_by_keys.side_effect = [
full_course_dicts,
restricted_course_dicts,
]
mock_get_course_reviews.return_value = reviews_for_courses_dict
mock_filter.return_value = metadata_records_for_fetched_keys

# Call the function
tasks._update_full_content_metadata_course(content_keys) # pylint: disable=protected-access

mock_fetch_courses_by_keys.assert_called_once_with(content_keys)
mock_fetch_courses_by_keys.assert_has_calls([
mock.call(content_keys),
mock.call([restricted_course.content_key], extra_query_params=QUERY_FOR_RESTRICTED_RUNS),
])
mock_get_course_reviews.assert_called_once_with(['course1', 'course2'])
mock_filter.assert_called_once_with(content_key__in=['course1', 'course2'])

Expand All @@ -2421,6 +2471,19 @@ def test_update_full_content_metadata_course(
self.assertEqual(mock_update_content_metadata_program.call_count, 2)
self.assertEqual(mock_create_course_associated_programs.call_count, 2)

restricted_course.refresh_from_db()
actual_relationships = list(
restricted_course.restricted_run_allowed_for_restricted_course.all(),
)
self.assertEqual(1, len(actual_relationships))
self.assertEqual(actual_relationships[0], restricted_relationship)
self.assertEqual(actual_relationships[0].run, restricted_run)
self.assertEqual('stuff', restricted_course.json_metadata['other'])
self.assertEqual(
{run['key'] for run in restricted_course.json_metadata['course_runs']},
{'course-run-1', 'course-run-1-restricted'},
)

@mock.patch('enterprise_catalog.apps.api.tasks._fetch_courses_by_keys')
@mock.patch('enterprise_catalog.apps.api.tasks.DiscoveryApiClient.get_course_reviews')
@mock.patch('enterprise_catalog.apps.api.tasks.ContentMetadata.objects.filter')
Expand Down
4 changes: 3 additions & 1 deletion enterprise_catalog/apps/api_client/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ def get_programs(self, query_params=None):

return programs

def fetch_courses_by_keys(self, course_keys):
def fetch_courses_by_keys(self, course_keys, extra_query_params=None):
"""
Fetches course data from discovery's /api/v1/courses endpoint for the provided course keys.
Expand All @@ -529,6 +529,8 @@ def fetch_courses_by_keys(self, course_keys):
for course_keys_chunk in batched_course_keys:
# Discovery expects the keys param to be in the format ?keys=course1,course2,...
query_params = {'keys': ','.join(course_keys_chunk)}
if extra_query_params:
query_params.update(extra_query_params)
courses.extend(self.get_courses(query_params=query_params))

return courses
Expand Down
2 changes: 1 addition & 1 deletion enterprise_catalog/apps/catalog/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ def _store_record(cls, course_metadata_dict, catalog_query=None):
record cannot be found.
"""
content_type = course_metadata_dict.get('content_type')
if content_type != COURSE:
if content_type and (content_type != COURSE):
raise Exception('Can only store RestrictedContentMetadata with content type of course')

course_key = course_metadata_dict['key']
Expand Down

0 comments on commit f92b203

Please sign in to comment.