Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: more restricted runs and contains_content_items logic #916

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

iloveagent57
Copy link
Contributor

@iloveagent57 iloveagent57 commented Aug 21, 2024

* contains_content_items endpoint now both utilize the same shared logic around restricted runs
* contains_content_items top-level key returns true based on explicit content membership *or*
membership in configured restricted runs mapping
* The previous point implies that requests about restricted course runs
will response with contains_content_items=true, even if the run is
not tied directly to the catalog via ContentMetadata records.
ENT-9386

Post-review

  • Squash commits into discrete sets of changes
  • Ensure that once the changes have been deployed to stage, prod is manually deployed

@iloveagent57 iloveagent57 force-pushed the aed/more-restricted-stuff branch from a18224c to df2fec4 Compare August 21, 2024 15:30
Copy link
Contributor

@pwnage101 pwnage101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest LGTM!

@iloveagent57 iloveagent57 force-pushed the aed/more-restricted-stuff branch from df2fec4 to 0fa6ed3 Compare August 22, 2024 13:58
Copy link
Member

@adamstankiewicz adamstankiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending any test updates (happy to re-review once tests updated).


any_catalog_contains_content_items = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[kudos] Nice cleanup of any_catalog_contains_content_items, and only relying on the empty vs. non-empty catalogs_that_contain_course.

@iloveagent57 iloveagent57 force-pushed the aed/more-restricted-stuff branch 2 times, most recently from 6666727 to 5d81b06 Compare August 22, 2024 15:32
@iloveagent57 iloveagent57 force-pushed the aed/more-restricted-stuff branch from 5d81b06 to 0bdece2 Compare August 22, 2024 18:24
* contains_content_items endpoint now both utilize the same shared logic around restricted runs
* contains_content_items top-level key returns true based on explicit content membership *or*
membership in configured restricted runs mapping
* The previous point implies that requests about restricted course runs
will response with contains_content_items=true, even if the run is
not tied directly to the catalog via ContentMetadata records.
ENT-9386
@iloveagent57 iloveagent57 force-pushed the aed/more-restricted-stuff branch from 0bdece2 to 83bcb90 Compare August 22, 2024 18:31
@iloveagent57 iloveagent57 merged commit 97985a1 into master Aug 22, 2024
4 checks passed
@iloveagent57 iloveagent57 deleted the aed/more-restricted-stuff branch August 22, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants