diff --git a/enterprise_catalog/apps/catalog/admin.py b/enterprise_catalog/apps/catalog/admin.py index 9504824b8..20623bee4 100644 --- a/enterprise_catalog/apps/catalog/admin.py +++ b/enterprise_catalog/apps/catalog/admin.py @@ -193,10 +193,25 @@ def get_restricted_runs_allowed(self, obj): ).filter( course=obj, ) - restricted_runs = (relationship.run for relationship in restricted_runs_allowed_for_restricted_course) + restricted_runs = [ + relationship.run for relationship in restricted_runs_allowed_for_restricted_course + if relationship.run + ] return _html_list_from_objects(restricted_runs, "admin:catalog_contentmetadata_change") +@admin.register(RestrictedRunAllowedForRestrictedCourse) +class RestrictedRunAllowedForRestrictedCourseAdmin(UnchangeableMixin): + """ + Admin class to show restricted course <-> run relationships. + """ + list_display = ( + 'id', + 'course', + 'run', + ) + + @admin.register(CatalogQuery) class CatalogQueryAdmin(UnchangeableMixin): """ Admin configuration for the custom CatalogQuery model. """ diff --git a/enterprise_catalog/apps/catalog/models.py b/enterprise_catalog/apps/catalog/models.py index 44217d8f2..c13b65e3c 100644 --- a/enterprise_catalog/apps/catalog/models.py +++ b/enterprise_catalog/apps/catalog/models.py @@ -865,15 +865,13 @@ def _store_record(cls, course_metadata_dict, catalog_query=None): course_key = course_metadata_dict['key'] parent_record = ContentMetadata.objects.get(content_key=course_key, content_type=COURSE) + record, _ = cls.objects.update_or_create( content_key=course_key, - content_uuid=course_metadata_dict['uuid'], content_type=COURSE, unrestricted_parent=parent_record, catalog_query=catalog_query, - defaults={ - '_json_metadata': course_metadata_dict, - }, + defaults=_restricted_content_defaults(course_metadata_dict), ) return record @@ -884,27 +882,38 @@ def store_canonical_record(cls, course_metadata_dict): @classmethod def store_record_with_query(cls, course_metadata_dict, catalog_query): filtered_metadata = cls.filter_restricted_runs(course_metadata_dict, catalog_query) - return cls._store_record(filtered_metadata, catalog_query) + course_record = cls._store_record(filtered_metadata, catalog_query) + for course_run_dict in cls.restricted_runs_for_course(filtered_metadata, catalog_query): + course_run_record, _ = ContentMetadata.objects.get_or_create( + content_key=course_run_dict['key'], + content_type=COURSE_RUN, + defaults=_restricted_content_defaults(course_run_dict), + ) + RestrictedRunAllowedForRestrictedCourse.objects.get_or_create( + course=course_record, run=course_run_record, + ) + + return course_record @classmethod def filter_restricted_runs(cls, course_metadata_dict, catalog_query): """ Returns a copy of ``course_metadata_dict`` whose course_runs list contains only unrestricted runs and restricted runs that are allowed - by the provided ``catalog_query``. + by the provided ``catalog_query``, and whose ``course_runs_keys``, + ``course_run_statuses``, and ``first_enrollable_paid_seat_price`` items + are updated to take only these allowed runs into account. """ filtered_metadata = copy.deepcopy(course_metadata_dict) - allowed_restricted_runs = catalog_query.restricted_runs_allowed.get(course_metadata_dict['key'], []) allowed_runs = [] allowed_statuses = set() allowed_keys = [] - for run in filtered_metadata['course_runs']: - if run.get(COURSE_RUN_RESTRICTION_TYPE_KEY) is None or run['key'] in allowed_restricted_runs: - allowed_runs.append(run) - allowed_statuses.add(run['status']) - allowed_keys.append(run['key']) + for run in cls.allowed_runs_for_course(filtered_metadata, catalog_query): + allowed_runs.append(run) + allowed_statuses.add(run['status']) + allowed_keys.append(run['key']) filtered_metadata['course_runs'] = allowed_runs filtered_metadata['course_run_keys'] = allowed_keys @@ -915,6 +924,33 @@ def filter_restricted_runs(cls, course_metadata_dict, catalog_query): return filtered_metadata + @staticmethod + def allowed_runs_for_course(course_metadata_dict, catalog_query): + """ + Given a ``course_metadata_dict``, returns a filtered list of ``course_runs`` + containing only unrestricted runs and restricted runs that are allowed by + the provided ``catalog_query``. + """ + restricted_runs = RestrictedCourseMetadata.restricted_runs_for_course(course_metadata_dict, catalog_query) + unrestricted_runs = [ + run for run in course_metadata_dict['course_runs'] + if run.get(COURSE_RUN_RESTRICTION_TYPE_KEY) is None + ] + return unrestricted_runs + restricted_runs + + @staticmethod + def restricted_runs_for_course(course_metadata_dict, catalog_query): + """ + Given a ``course_metadata_dict``, returns a filtered list of ``course_runs`` + containing only restricted runs that are allowed by + the provided ``catalog_query``. + """ + allowed_restricted_runs = catalog_query.restricted_runs_allowed.get(course_metadata_dict['key'], []) + return [ + run for run in course_metadata_dict['course_runs'] + if run['key'] in allowed_restricted_runs + ] + class RestrictedRunAllowedForRestrictedCourse(TimeStampedModel): """ @@ -955,6 +991,17 @@ def content_metadata_with_type_course(): return content_metadata +def _restricted_content_defaults(entry): + """ + Helper to populate the update_or_create() ``defaults`` + for restricted content. + """ + defaults = {'_json_metadata': entry} + if content_uuid := entry.get('uuid'): + defaults['content_uuid'] = content_uuid + return defaults + + def _get_defaults_from_metadata(entry, exists=False): """ Given a metadata entry from course-discovery's /search/all API endpoint, this function determines the @@ -1440,7 +1487,7 @@ def synchronize_restricted_content(catalog_query, dry_run=False): restricted_course_keys = list(catalog_query.restricted_runs_allowed.keys()) content_filter = { - 'content_type': 'course', + 'content_type': COURSE, 'key': restricted_course_keys, } discovery_client = DiscoveryApiClient() @@ -1448,7 +1495,7 @@ def synchronize_restricted_content(catalog_query, dry_run=False): content_filter, QUERY_FOR_RESTRICTED_RUNS, ) - restricted_course_keys = [] + results = [] for course_dict in course_payload: LOGGER.info('Storing restricted course %s for catalog_query %s', course_dict.get('key'), catalog_query.id) if dry_run: @@ -1458,9 +1505,24 @@ def synchronize_restricted_content(catalog_query, dry_run=False): restricted_course_record = RestrictedCourseMetadata.store_record_with_query( course_dict, catalog_query, ) - restricted_course_keys.append(restricted_course_record.content_key) + results.append(restricted_course_record.content_key) - return restricted_course_keys + restricted_course_run_keys = list(catalog_query.restricted_courses_by_run_key.keys()) + run_content_filter = { + 'content_type': COURSE_RUN, + 'key': restricted_course_run_keys, + } + course_run_payload = discovery_client.retrieve_metadata_for_content_filter( + run_content_filter, QUERY_FOR_RESTRICTED_RUNS, + ) + for course_run_dict in course_run_payload: + course_run_record, _ = ContentMetadata.objects.update_or_create( + content_key=course_run_dict['key'], + content_type=COURSE_RUN, + defaults=_restricted_content_defaults(course_run_dict), + ) + results.append(course_run_record.content_key) + return results class CatalogUpdateCommandConfig(ConfigurationModel): diff --git a/enterprise_catalog/apps/catalog/tests/test_models.py b/enterprise_catalog/apps/catalog/tests/test_models.py index 62abc470c..0fabd04f6 100644 --- a/enterprise_catalog/apps/catalog/tests/test_models.py +++ b/enterprise_catalog/apps/catalog/tests/test_models.py @@ -1238,18 +1238,21 @@ def test_store_record_with_query(self): 'key': 'course-v1:edX+course+run1', 'is_restricted': False, 'status': 'published', + 'uuid': str(uuid4()), }, { 'key': 'course-v1:edX+course+run2', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'unpublished', + 'uuid': str(uuid4()), }, { 'key': 'course-v1:edX+course+run3', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'other', + 'uuid': str(uuid4()), }, ], } @@ -1270,12 +1273,14 @@ def test_store_record_with_query(self): 'key': 'course-v1:edX+course+run1', 'is_restricted': False, 'status': 'published', + 'uuid': content_metadata_dict['course_runs'][0]['uuid'], }, { 'key': 'course-v1:edX+course+run2', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'unpublished', + 'uuid': content_metadata_dict['course_runs'][1]['uuid'] }, ], ) @@ -1292,6 +1297,14 @@ def test_store_record_with_query(self): self.assertEqual(record.content_type, content_metadata_dict['content_type']) self.assertEqual(record.unrestricted_parent, parent_record) self.assertEqual(record.catalog_query, catalog_query) + self.assertEqual( + list(record.restricted_run_allowed_for_restricted_course.all().select_related( + 'run', + ).values_list( + 'run__content_key', flat=True, + )), + ['course-v1:edX+course+run2'], + ) @override_settings(SHOULD_FETCH_RESTRICTED_COURSE_RUNS=False) @mock.patch('enterprise_catalog.apps.catalog.models.DiscoveryApiClient') @@ -1325,6 +1338,7 @@ def test_synchronize_restricted_content(self, mock_client): 'restricted_runs_allowed': { 'course:edX+course': [ 'course-v1:edX+course+run2', + 'course-v1:edX+course+run3', ], }, }, @@ -1338,47 +1352,91 @@ def test_synchronize_restricted_content(self, mock_client): 'key': 'course-v1:edX+course+run1', 'is_restricted': False, 'status': 'published', + 'uuid': str(uuid4()), }, { 'key': 'course-v1:edX+course+run2', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'unpublished', + 'uuid': str(uuid4()), }, { 'key': 'course-v1:edX+course+run3', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'other', + 'uuid': str(uuid4()), }, ], } + course_run_results = [ + { + 'key': 'course-v1:edX+course+run2', + 'is_restricted': True, + COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, + 'status': 'unpublished', + 'uuid': str(uuid4()), + 'other': 'stuff', + }, + { + 'key': 'course-v1:edX+course+run3', + 'is_restricted': True, + COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, + 'status': 'other', + 'uuid': str(uuid4()), + 'other': 'things', + }, + ] parent_record = factories.ContentMetadataFactory.create( content_key='edX+course', content_type=COURSE, ) mock_retrieve = mock_client.return_value.retrieve_metadata_for_content_filter - mock_retrieve.return_value = [ - content_metadata_dict, + mock_retrieve.side_effect = [ + [content_metadata_dict], + course_run_results, ] result = synchronize_restricted_content(catalog_query) - mock_retrieve.assert_called_once_with( - { - 'content_type': 'course', - 'key': ['edX+course'], - }, - QUERY_FOR_RESTRICTED_RUNS, - ) - self.assertEqual(result, [content_metadata_dict['key']]) + mock_retrieve.assert_has_calls([ + mock.call( + { + 'content_type': 'course', + 'key': ['edX+course'], + }, + QUERY_FOR_RESTRICTED_RUNS, + ), + mock.call( + { + 'content_type': 'courserun', + 'key': ['course-v1:edX+course+run2', 'course-v1:edX+course+run3'], + }, + QUERY_FOR_RESTRICTED_RUNS, + ), + ]) + self.assertEqual(result, ['edX+course', 'course-v1:edX+course+run2', 'course-v1:edX+course+run3']) self.assertIsNotNone(RestrictedCourseMetadata.objects.get( content_key=content_metadata_dict['key'], unrestricted_parent=parent_record, catalog_query=None, )) - self.assertIsNotNone(RestrictedCourseMetadata.objects.get( + + restricted_course = RestrictedCourseMetadata.objects.get( content_key=content_metadata_dict['key'], unrestricted_parent=parent_record, catalog_query=catalog_query, - )) + ) + restricted_run_relationships = list( + restricted_course.restricted_run_allowed_for_restricted_course.all().select_related( + 'run', + ).order_by( + 'run__content_key', + )) + self.assertEqual( + [relationship.run.content_key for relationship in restricted_run_relationships], + ['course-v1:edX+course+run2', 'course-v1:edX+course+run3'], + ) + self.assertEqual(restricted_run_relationships[0].run.json_metadata['other'], 'stuff') + self.assertEqual(restricted_run_relationships[1].run.json_metadata['other'], 'things') diff --git a/enterprise_catalog/apps/curation/models.py b/enterprise_catalog/apps/curation/models.py index 435f9ba48..2206ba728 100644 --- a/enterprise_catalog/apps/curation/models.py +++ b/enterprise_catalog/apps/curation/models.py @@ -206,7 +206,7 @@ def title(self): """ if not self.content_metadata: return None - return self.content_metadata.json_metadata.get('title') # pylint: disable=no-member + return self.content_metadata.json_metadata.get('title') @property def course_run_statuses(self): @@ -215,7 +215,7 @@ def course_run_statuses(self): """ if not self.content_metadata: return None - return self.content_metadata.json_metadata.get('course_run_statuses') # pylint: disable=no-member + return self.content_metadata.json_metadata.get('course_run_statuses') @property def card_image_url(self): @@ -232,14 +232,13 @@ def card_image_url(self): # aside: pylint doesn't know that self.content_metadata.json_metadata is dict-like, so we have to silence all # the warnings. if content_type == COURSE: - return self.content_metadata.json_metadata.get('image_url') # pylint: disable=no-member + return self.content_metadata.json_metadata.get('image_url') if content_type == COURSE_RUN: - return self.content_metadata.json_metadata.get('image_url') # pylint: disable=no-member + return self.content_metadata.json_metadata.get('image_url') elif content_type == PROGRAM: - return self.content_metadata.json_metadata.get('card_image_url') # pylint: disable=no-member + return self.content_metadata.json_metadata.get('card_image_url') elif content_type == LEARNER_PATHWAY: try: - # pylint: disable=invalid-sequence-index return self.content_metadata.json_metadata['card_image']['card']['url'] except (KeyError, TypeError): # KeyError covers the case where any of the keys along the path are missing, @@ -271,9 +270,9 @@ def authoring_organizations(self): content_type = self.content_type owners = [] if content_type == COURSE: - owners = self.content_metadata.json_metadata.get('owners') # pylint: disable=no-member + owners = self.content_metadata.json_metadata.get('owners') elif content_type == PROGRAM: - owners = self.content_metadata.json_metadata.get('authoring_organizations') # pylint: disable=no-member + owners = self.content_metadata.json_metadata.get('authoring_organizations') return [ {