diff --git a/enterprise_catalog/apps/api/tasks.py b/enterprise_catalog/apps/api/tasks.py index 001051f2..2ac33754 100644 --- a/enterprise_catalog/apps/api/tasks.py +++ b/enterprise_catalog/apps/api/tasks.py @@ -39,6 +39,7 @@ FORCE_INCLUSION_METADATA_TAG_KEY, LEARNER_PATHWAY, PROGRAM, + QUERY_FOR_RESTRICTED_RUNS, REINDEX_TASK_BATCH_SIZE, TASK_BATCH_SIZE, VIDEO, @@ -73,7 +74,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. @@ -82,7 +83,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): @@ -283,6 +284,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: @@ -307,6 +316,44 @@ 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}' + ) + + full_restricted_metadata = metadata_list[0] + + for restricted_course in restricted_courses: + # First, update the restricted course record's json metadata to use the full metadata + # from above. + restricted_course.update_metadata(full_restricted_metadata, is_full_update=True) + # Second, if it's not the canonical copy of the restricted course, + # we have to reset the restricted course *run* relationships. + if restricted_course.catalog_query: + restricted_course.update_course_run_relationships() + + # Last, run the "standard" transformations below to update with the full + # course metadata, do normalization, etc. + _update_single_full_course_record( + full_restricted_metadata, restricted_course, course_review, dry_run, skip_json_metadata_update=True, + ) + restricted_course.save() + + def _get_course_records_by_key(fetched_course_keys): """ Helper to fetch a dict of course `ContentMetadata` records by content key. @@ -320,14 +367,17 @@ def _get_course_records_by_key(fetched_course_keys): } -def _update_single_full_course_record(course_metadata_dict, metadata_record, course_review, dry_run): +def _update_single_full_course_record( + course_metadata_dict, metadata_record, course_review, dry_run, skip_json_metadata_update=False, +): """ Given a fetched dictionary of course content metadata and an option `course_review` record, updates an existing course `ContentMetadata` instance with the "full" dictionary of `json_metadata` for that course. """ - # Merge the full metadata from discovery's /api/v1/courses into the local metadata object. - metadata_record._json_metadata.update(course_metadata_dict) # pylint: disable=protected-access + if not skip_json_metadata_update: + # Merge the full metadata from discovery's /api/v1/courses into the local metadata object. + metadata_record._json_metadata.update(course_metadata_dict) # pylint: disable=protected-access _normalize_metadata_record(metadata_record) @@ -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 diff --git a/enterprise_catalog/apps/api/tests/test_tasks.py b/enterprise_catalog/apps/api/tests/test_tasks.py index 7a7d5137..f490276b 100644 --- a/enterprise_catalog/apps/api/tests/test_tasks.py +++ b/enterprise_catalog/apps/api/tests/test_tasks.py @@ -19,10 +19,12 @@ from enterprise_catalog.apps.catalog.constants import ( COURSE, COURSE_RUN, + COURSE_RUN_RESTRICTION_TYPE_KEY, EXEC_ED_2U_COURSE_TYPE, 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 ( @@ -2377,6 +2379,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, @@ -2386,10 +2389,29 @@ def test_update_full_content_metadata_course( mock_fetch_courses_by_keys ): # Mock data + course_1_uuid = uuid.uuid4() + course_run_1_uuid = uuid.uuid4() + restricted_run_1_uuid = uuid.uuid4() + restricted_run_2_uuid = uuid.uuid4() 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', + 'uuid': course_1_uuid, + 'aggregation_key': 'course:course1', + 'title': 'Course 1', + FORCE_INCLUSION_METADATA_TAG_KEY: True, + 'course_runs': [ + { + 'key': 'course-run-1', 'uuid': course_1_uuid, + 'aggregation_key': 'courserun:course1', + }, + ] + }, + { + 'key': 'course2', + 'title': 'Course 2', + }, ] reviews_for_courses_dict = { 'course1': {'reviews_count': 10, 'avg_course_rating': 4.5}, @@ -2399,15 +2421,91 @@ 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', + 'uuid': course_1_uuid, + 'title': 'Course 1', + 'other': 'stuff', + FORCE_INCLUSION_METADATA_TAG_KEY: True, + 'course_runs': [ + {'key': 'course-run-1', 'uuid': course_run_1_uuid, 'aggregation_key': 'courserun:course-run-1'}, + { + 'key': 'course-run-1-restricted', COURSE_RUN_RESTRICTION_TYPE_KEY: 'anything', + 'aggregation_key': 'courserun:course1', 'uuid': restricted_run_1_uuid, + }, + { + 'key': 'another-restricted-run', COURSE_RUN_RESTRICTION_TYPE_KEY: 'anything', + 'aggregation_key': 'courserun:course1', 'uuid': restricted_run_2_uuid, + }, + ], + }, + ] + catalog_query = CatalogQueryFactory( + content_filter={ + 'restricted_runs_allowed': { + 'course:course1': [ + 'course-run-1-restricted', + ], + }, + }, + ) + restricted_course = RestrictedCourseMetadataFactory( + unrestricted_parent=content_metadata_1, + catalog_query=catalog_query, + content_type=COURSE, + content_key='course1', + _json_metadata={ + 'course_runs': [ + { + 'key': 'course-run-1', 'uuid': course_run_1_uuid, + 'aggregation_key': 'courserun:course1', + }, + { + 'key': 'course-run-1-restricted', COURSE_RUN_RESTRICTION_TYPE_KEY: 'anything', + 'aggregation_key': 'courserun:course1', 'uuid': restricted_run_1_uuid, + }, + ] + } + ) + canonical_restricted_course = RestrictedCourseMetadataFactory( + unrestricted_parent=content_metadata_1, + catalog_query=None, + content_type=COURSE, + content_key='course1', + _json_metadata={ + 'course_runs': [ + { + 'key': 'course-run-1', 'uuid': course_run_1_uuid, + 'aggregation_key': 'courserun:course1', + }, + { + 'key': 'course-run-1-restricted', COURSE_RUN_RESTRICTION_TYPE_KEY: 'anything', + 'aggregation_key': 'courserun:course1', 'uuid': restricted_run_1_uuid, + }, + { + 'key': 'another-restricted-run', COURSE_RUN_RESTRICTION_TYPE_KEY: 'anything', + 'aggregation_key': 'courserun:course1', 'uuid': restricted_run_2_uuid, + }, + ], + } + ) + # 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']) @@ -2421,6 +2519,33 @@ 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) + # Test that the restricted course for our catalog query contains + # only the one restricted run, because that's all the query definition allows. + restricted_course.refresh_from_db() + restricted_run = ContentMetadata.objects.get( + content_type=COURSE_RUN, + parent_content_key='course1', + content_key='course-run-1-restricted', + ) + related_run = restricted_course.restricted_run_allowed_for_restricted_course.filter( + content_key=restricted_run.content_key, + ).first() + self.assertEqual(1, restricted_course.restricted_run_allowed_for_restricted_course.all().count()) + self.assertEqual(related_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'}, + ) + # The canonical restricted course record should contain two restricted runs and a non-restricted run + canonical_restricted_course.refresh_from_db() + self.assertEqual( + {run['key'] for run in canonical_restricted_course.json_metadata['course_runs']}, + {'course-run-1', 'course-run-1-restricted', 'another-restricted-run'}, + ) + # But the canonical course will *not* have any direct relationship to restricted run ContentMetadtata records. + self.assertEqual(0, canonical_restricted_course.restricted_run_allowed_for_restricted_course.all().count()) + @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') diff --git a/enterprise_catalog/apps/api_client/discovery.py b/enterprise_catalog/apps/api_client/discovery.py index e7d4a433..65e8c8b8 100644 --- a/enterprise_catalog/apps/api_client/discovery.py +++ b/enterprise_catalog/apps/api_client/discovery.py @@ -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. @@ -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 diff --git a/enterprise_catalog/apps/catalog/algolia_utils.py b/enterprise_catalog/apps/catalog/algolia_utils.py index 33b89212..300bbc6c 100644 --- a/enterprise_catalog/apps/catalog/algolia_utils.py +++ b/enterprise_catalog/apps/catalog/algolia_utils.py @@ -1131,6 +1131,7 @@ def _get_course_run(course, course_run): 'content_price': normalized_content_metadata.get('content_price'), 'is_active': _get_is_active_course_run(course_run), 'is_late_enrollment_eligible': _get_is_late_enrollment_eligible(course_run), + 'restriction_type': course_run.get('restriction_type'), } return course_run diff --git a/enterprise_catalog/apps/catalog/migrations/0043_restricted_course_m2m_field.py b/enterprise_catalog/apps/catalog/migrations/0043_restricted_course_m2m_field.py new file mode 100644 index 00000000..a714221b --- /dev/null +++ b/enterprise_catalog/apps/catalog/migrations/0043_restricted_course_m2m_field.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.16 on 2024-10-23 15:22 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('catalog', '0042_alter_restrictedcoursemetadata_display_name'), + ] + + operations = [ + migrations.AddField( + model_name='restrictedcoursemetadata', + name='restricted_run_allowed_for_restricted_course', + field=models.ManyToManyField(through='catalog.RestrictedRunAllowedForRestrictedCourse', to='catalog.contentmetadata'), + ), + migrations.AlterField( + model_name='restrictedrunallowedforrestrictedcourse', + name='course', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='catalog.restrictedcoursemetadata'), + ), + ] diff --git a/enterprise_catalog/apps/catalog/models.py b/enterprise_catalog/apps/catalog/models.py index b999e6b3..2b7d8fbb 100644 --- a/enterprise_catalog/apps/catalog/models.py +++ b/enterprise_catalog/apps/catalog/models.py @@ -842,6 +842,11 @@ class Meta: related_name='restricted_content_metadata', on_delete=models.deletion.SET_NULL, ) + restricted_run_allowed_for_restricted_course = models.ManyToManyField( + ContentMetadata, + through='RestrictedRunAllowedForRestrictedCourse', + through_fields=('course', 'run'), + ) history = HistoricalRecords() def __str__(self): @@ -851,8 +856,123 @@ def __str__(self): catalog_query_id = self.catalog_query.id if self.catalog_query else None return f"<{self.__class__.__name__} for '{self.content_key}' and CatalogQuery ({catalog_query_id})>" + @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 + ] + + @property + def restricted_run_dicts(self): + return self.restricted_runs_for_course(self.json_metadata, self.catalog_query) + + def update_metadata(self, course_metadata_dict, is_full_update=False): + """ + Updates and saves the json_metadata for this restricted course. + Params: + course_metadata_dict: A dictionary of course metadata + fetched from either /api/v1/search/all *or* /api/v1/courses (course-discovery). + is_full_update: Whether the course metadata provided is fetched from /api/v1/courses. + If so, we fully update the json_metadata of this record with those contents, otherwise + the update is trimmed down by `_get_defaults_from_metadata()`. Defaults to False. + """ + if self.catalog_query: + filtered_metadata = self.filter_restricted_runs(course_metadata_dict, self.catalog_query) + else: + filtered_metadata = course_metadata_dict + + if is_full_update: + self._json_metadata.update(filtered_metadata) + else: + # We care about preserving the values of the "plucked" + # fields iterated through in `_get_defaults_from_metadata()` + # when we're doing a non-full update + # (i.e. an update from a course-discovery api/v1/search/all payload) + self._json_metadata.update( + _get_defaults_from_metadata(filtered_metadata)['_json_metadata'], + ) + self.save() + + def update_course_run_relationships(self): + """ + Updates the relationships between restricted course runs, this + restricted course, and this restricted course's catalog_query. + """ + existing_relationships = list(self.catalog_query.contentmetadata_set.filter( + parent_content_key=self.content_key, + ).values_list('content_key', flat=True)) + LOGGER.info( + '%s has existing course run relationships %s prior to updating', + self, existing_relationships, + ) + + restricted_runs = [] + + for course_run_dict in self.restricted_run_dicts: + course_run_record = self.update_or_create_run( + course_run_key=get_content_key(course_run_dict), + parent_content_key=self.content_key, + course_run_dict=course_run_dict, + ) + restricted_runs.append(course_run_record) + + # We use a set() here, with clear=True, to clear and then reset the related allowed runs + # for this restricted course. This is necessary in the case that a previously-allowed + # run becomes unassociated from the restricted course. + self.catalog_query.contentmetadata_set.set(restricted_runs, clear=True) + LOGGER.info('Updated restricted runs on catalog query of %s to %s', self, restricted_runs) + self.restricted_run_allowed_for_restricted_course.set(restricted_runs, clear=True) + LOGGER.info('Updated restricted runs of %s to %s', self, restricted_runs) + self.refresh_from_db() + @classmethod - def _store_record(cls, course_metadata_dict, catalog_query=None): + def update_or_create_run(cls, course_run_key, parent_content_key, course_run_dict): + """ + Helper to create a course run ContentMetadata record, provided a content key, + a parent course key, and a dictionary of course run metadata. + """ + defaults = _get_defaults_from_metadata(course_run_dict) + # We have to pop these fields from defaults to keep them from + # being used in the UPDATE statement, because the nested course run + # data from the /api/v1/search/all payload *does not* include the + # course run uuid or aggregation key, which are used by + # _get_defaults_from_metadata() to compute the content type and parent key. + # We additionally pop the content_key field because it's another primary + # identifier used to uniquely identify the record in the non-defaults arguments + # in update_or_create() below. + for key in ['content_key', 'parent_content_key', 'content_type']: + defaults.pop(key, None) + course_run_record, _ = ContentMetadata.objects.update_or_create( + content_key=course_run_key, + content_type=COURSE_RUN, + parent_content_key=parent_content_key, + defaults=defaults, + ) + return course_run_record + + @classmethod + def _store_record(cls, course_metadata_dict, catalog_query=None, is_full_update=False): """ Given a course metadata dictionary, stores a corresponding ``RestrictedContentMetadata`` record. Raises if the content key @@ -866,40 +986,37 @@ 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( + defaults = {} + if content_uuid := get_content_uuid(course_metadata_dict): + defaults['content_uuid'] = content_uuid + + record, _ = cls.objects.get_or_create( content_key=course_key, content_type=COURSE, unrestricted_parent=parent_record, catalog_query=catalog_query, - defaults=_restricted_content_defaults(course_metadata_dict), + defaults=defaults, ) + record.update_metadata(course_metadata_dict, is_full_update=is_full_update) return record @classmethod - def store_canonical_record(cls, course_metadata_dict): - return cls._store_record(course_metadata_dict) + def store_canonical_record(cls, course_metadata_dict, is_full_update=False): + """ + Stores the canonical copy of this record, which will include all existing + course runs for a course, regardless of their restriction status. The canonical + record will *not* have a related catalog query. + """ + return cls._store_record(course_metadata_dict, is_full_update=is_full_update) @classmethod - def store_record_with_query(cls, course_metadata_dict, catalog_query): - filtered_metadata = cls.filter_restricted_runs(course_metadata_dict, catalog_query) - course_record = cls._store_record(filtered_metadata, catalog_query) - relationships = [] - - 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), - ) - relationship, _ = RestrictedRunAllowedForRestrictedCourse.objects.get_or_create( - course=course_record, run=course_run_record, - ) - relationships.append(relationship) - - # We use a set() here, with clear=True, to clear and then reset the related allowed runs - # for this restricted course. This is necessary in the case that a previously-allowed - # run becomes unassociated from the restricted course. - course_record.restricted_run_allowed_for_restricted_course.set(relationships, clear=True) + def store_record_with_query(cls, course_metadata_dict, catalog_query, is_full_update=False): + """ + Stores a restricted course record containing only unrestricted course runs + and the restricted course runs explicitly allowed by the provided catalog query. + """ + course_record = cls._store_record(course_metadata_dict, catalog_query, is_full_update=is_full_update) + course_record.update_course_run_relationships() return course_record @classmethod @@ -919,7 +1036,7 @@ def filter_restricted_runs(cls, course_metadata_dict, catalog_query): for run in cls.allowed_runs_for_course(filtered_metadata, catalog_query): allowed_runs.append(run) - allowed_statuses.add(run['status']) + allowed_statuses.add(run.get('status')) allowed_keys.append(run['key']) filtered_metadata['course_runs'] = allowed_runs @@ -931,33 +1048,6 @@ 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): """ @@ -973,7 +1063,6 @@ class RestrictedRunAllowedForRestrictedCourse(TimeStampedModel): RestrictedCourseMetadata, blank=False, null=True, - related_name='restricted_run_allowed_for_restricted_course', on_delete=models.deletion.SET_NULL, ) run = models.ForeignKey( @@ -1523,10 +1612,20 @@ def synchronize_restricted_content(catalog_query, dry_run=False): 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), + course_run_key = get_content_key(course_run_dict) + LOGGER.info( + 'Storing restricted course run %s for catalog_query %s', + course_run_dict.get('key'), catalog_query.id, + ) + if dry_run: + continue + + # These are "top-level" course run dictionaries, which have aggregation_keys + # from which we can determine the parent course key. + course_run_record = RestrictedCourseMetadata.update_or_create_run( + course_run_key=course_run_key, + parent_content_key=get_parent_content_key(course_run_dict), + course_run_dict=course_run_dict, ) results.append(course_run_record.content_key) return results diff --git a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py index ad4c0733..65e79074 100644 --- a/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py +++ b/enterprise_catalog/apps/catalog/tests/test_algolia_utils.py @@ -9,10 +9,12 @@ from enterprise_catalog.apps.catalog.constants import ( ALGOLIA_DEFAULT_TIMESTAMP, COURSE, + COURSE_RUN_RESTRICTION_TYPE_KEY, EXEC_ED_2U_COURSE_TYPE, EXEC_ED_2U_READABLE_COURSE_TYPE, LEARNER_PATHWAY, PROGRAM, + RESTRICTION_FOR_B2B, ) from enterprise_catalog.apps.catalog.tests.factories import ( ContentMetadataFactory, @@ -338,6 +340,7 @@ def test_get_course_subjects(self, course_metadata, expected_subjects): 'is_active': True, 'is_late_enrollment_eligible': False, 'content_price': 0.0, + 'restriction_type': None, }, ), ( @@ -397,6 +400,7 @@ def test_get_course_subjects(self, course_metadata, expected_subjects): 'content_price': 50.0, 'is_active': True, 'is_late_enrollment_eligible': False, + 'restriction_type': None, } ), ( @@ -440,6 +444,7 @@ def test_get_course_subjects(self, course_metadata, expected_subjects): 'content_price': 50, 'is_active': True, 'is_late_enrollment_eligible': False, + 'restriction_type': None, } ) ) @@ -657,6 +662,21 @@ def test_get_upcoming_course_runs(self, searchable_course, expected_course_runs) 'max_effort': 6, 'weeks_to_complete': 6, }, + { + 'key': 'course-v1:org+course+1T3022', + 'uuid': FUTURE_COURSE_RUN_UUID_1, + 'pacing_type': 'instructor_paced', + 'status': 'unpublished', + 'is_enrollable': True, + 'is_marketable': False, + 'availability': 'Starting Soon', + 'start': "3000-01-04T00:00:00Z", + 'end': "3022-12-31T23:59:00Z", + 'min_effort': 2, + 'max_effort': 6, + 'weeks_to_complete': 6, + COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, + }, ], 'advertised_course_run_uuid': ADVERTISED_COURSE_RUN_UUID }, @@ -678,6 +698,7 @@ def test_get_upcoming_course_runs(self, searchable_course, expected_course_runs) 'content_price': 0.0, 'is_active': False, 'is_late_enrollment_eligible': True, + 'restriction_type': None, }, { 'key': 'course-v1:org+course+1T2027', @@ -696,6 +717,7 @@ def test_get_upcoming_course_runs(self, searchable_course, expected_course_runs) 'content_price': 0.0, 'is_active': False, 'is_late_enrollment_eligible': False, + 'restriction_type': None, }, { 'key': 'course-v1:org+course+1T2028', @@ -714,6 +736,7 @@ def test_get_upcoming_course_runs(self, searchable_course, expected_course_runs) 'content_price': 0.0, 'is_active': False, 'is_late_enrollment_eligible': False, + 'restriction_type': None, }, { 'key': 'course-v1:org+course+1T2021', @@ -732,6 +755,7 @@ def test_get_upcoming_course_runs(self, searchable_course, expected_course_runs) 'content_price': 50, 'is_active': True, 'is_late_enrollment_eligible': False, + 'restriction_type': None, }, { 'key': 'course-v1:org+course+1T3000', @@ -750,6 +774,7 @@ def test_get_upcoming_course_runs(self, searchable_course, expected_course_runs) 'content_price': 0.0, 'is_active': True, 'is_late_enrollment_eligible': False, + 'restriction_type': None, }, { 'key': 'course-v1:org+course+1T3022', @@ -768,7 +793,27 @@ def test_get_upcoming_course_runs(self, searchable_course, expected_course_runs) 'content_price': 0.0, 'is_active': False, 'is_late_enrollment_eligible': False, - } + 'restriction_type': None, + }, + { + 'key': 'course-v1:org+course+1T3022', + 'pacing_type': 'instructor_paced', + 'availability': 'Starting Soon', + 'start': "3000-01-04T00:00:00Z", + 'end': "3022-12-31T23:59:00Z", + 'min_effort': 2, + 'max_effort': 6, + 'weeks_to_complete': 6, + 'upgrade_deadline': 32503680000.0, + 'enroll_by': None, + 'enroll_start': None, + 'has_enroll_by': False, + 'has_enroll_start': False, + 'content_price': 0.0, + 'is_active': False, + 'is_late_enrollment_eligible': False, + 'restriction_type': RESTRICTION_FOR_B2B, + }, ], ), ) diff --git a/enterprise_catalog/apps/catalog/tests/test_models.py b/enterprise_catalog/apps/catalog/tests/test_models.py index 0fabd04f..a9b7e256 100644 --- a/enterprise_catalog/apps/catalog/tests/test_models.py +++ b/enterprise_catalog/apps/catalog/tests/test_models.py @@ -1293,15 +1293,13 @@ def test_store_record_with_query(self): ['published', 'unpublished'], ) self.assertEqual(record.content_key, content_metadata_dict['key']) - self.assertEqual(record.content_uuid, content_metadata_dict['uuid']) + self.assertEqual(str(record.content_uuid), content_metadata_dict['uuid']) 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, + list(record.restricted_run_allowed_for_restricted_course.all().values_list( + 'content_key', flat=True, )), ['course-v1:edX+course+run2'], ) @@ -1345,17 +1343,20 @@ def test_synchronize_restricted_content(self, mock_client): ) content_metadata_dict = { 'key': 'edX+course', + 'aggregation_key': 'course:edX+course', 'uuid': '11111111-1111-1111-1111-111111111111', 'content_type': COURSE, 'course_runs': [ { 'key': 'course-v1:edX+course+run1', + 'aggregation_key': 'courserun:edX+course', 'is_restricted': False, 'status': 'published', 'uuid': str(uuid4()), }, { 'key': 'course-v1:edX+course+run2', + 'aggregation_key': 'courserun:edX+course', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'unpublished', @@ -1363,6 +1364,7 @@ def test_synchronize_restricted_content(self, mock_client): }, { 'key': 'course-v1:edX+course+run3', + 'aggregation_key': 'courserun:edX+course', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'other', @@ -1373,6 +1375,7 @@ def test_synchronize_restricted_content(self, mock_client): course_run_results = [ { 'key': 'course-v1:edX+course+run2', + 'aggregation_key': 'courserun:edX+course', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'unpublished', @@ -1381,6 +1384,7 @@ def test_synchronize_restricted_content(self, mock_client): }, { 'key': 'course-v1:edX+course+run3', + 'aggregation_key': 'courserun:edX+course', 'is_restricted': True, COURSE_RUN_RESTRICTION_TYPE_KEY: RESTRICTION_FOR_B2B, 'status': 'other', @@ -1428,15 +1432,14 @@ def test_synchronize_restricted_content(self, mock_client): 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', - )) + restricted_runs = list( + restricted_course.restricted_run_allowed_for_restricted_course.all().order_by( + 'content_key', + ) + ) self.assertEqual( - [relationship.run.content_key for relationship in restricted_run_relationships], + [run.content_key for run in restricted_runs], ['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') + self.assertEqual(restricted_runs[0].json_metadata['other'], 'stuff') + self.assertEqual(restricted_runs[1].json_metadata['other'], 'things')