Relate restricted runs to CatalogQuery objects (ENT-9405) #947
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pwnage101
changed the title
2 tasks
Comment on lines
+644
to
+647
| catalog_queries_for_restricted_course_run = models.ManyToManyField( | ||
| CatalogQuery, | ||
| related_name='restricted_course_runs', | ||
| ) |
There was a problem hiding this comment.
Can we create an explicit M2M model to represent this and use through?
Comment on lines
+1256
to
+1294
| # At this point, we know the `unrestricted_metadata` partition contains | ||
| # courses and runs that should be related to the CatalogQuery as restricted | ||
| # content, but it's still only a subset of restricted content that needs to | ||
| # be related. What's missing are any restricted runs that didn't match the | ||
| # original content filter, but whose parent (course) key DID match. | ||
| restricted_metadata_content_keys = [get_content_key(entry) for entry in restricted_metadata] | ||
| LOGGER.info( | ||
| 'Retrieved %d restricted content items (%d unique) from course-discovery for catalog query %s', | ||
| len(restricted_metadata_content_keys), | ||
| len(set(restricted_metadata_content_keys)), | ||
| catalog_query, | ||
| ) | ||
| resticted_run_metadata = get_restricted_runs_from_discovery( | ||
| # Intentionally pass the un-partitioned metadata list so that we can | ||
| # discover all restricted runs matching any course that matches the | ||
| # content_filter, restricted-only or not. | ||
| metadata, | ||
| catalog_query, | ||
| dry_run, | ||
| ) | ||
| associated_restricted_run_keys = associate_restricted_runs_with_query( | ||
| # Union the following metadata lists: | ||
| # * `restricted_metadata`: | ||
| # Includes any restricted-only courses, or restricted runs directly | ||
| # matching the content filter, BUT not necessarily all restricted | ||
| # runs beneath matching courses. | ||
| # * `restricted_run_metadata`: | ||
| # Includes all restricted runs part of any course matching the | ||
| # content filter, BUT no restricted-only courses. | ||
| restricted_metadata | resticted_run_metadata, | ||
| catalog_query, | ||
| dry_run | ||
| ) | ||
| LOGGER.info( | ||
| 'Associated %d restricted runs (%d unique) with catalog query %s', | ||
| len(associated_restricted_run_keys), | ||
| len(set(associated_restricted_run_keys)), | ||
| catalog_query, | ||
| ) |
There was a problem hiding this comment.
Maybe throw this whole chunk into a new function?
Comment on lines
+1231
to
+1238
| # Partition metadata dicts by unrestricted vs. restricted. | ||
| restricted_metadata = [] | ||
| unrestricted_metadata = [] | ||
| for metadata_dict in metadata: | ||
| if is_content_restricted(metadata_dict): | ||
| restricted_metadata += metadata_dict | ||
| else: | ||
| unrestricted_metadata += metadata_dict |
There was a problem hiding this comment.
Is it necessary to mix the restricted run fetching in with the unrestricted fetching? Since we're going to make a second set of requests to discovery anyway, can we do as just two completely separate chunks of work?
- Request the content filter without requesting restricted runs.
- Request only the restricted runs defined in the content filter.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two main parts of this:
ENT-9405