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

fix: enrollment intention saves should not be blocked on catalog inclusion #2314

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pwnage101
Copy link
Contributor

Before this commit, normal replication delays from discovery->catalog can easily result in enrollment intentions being un-saveable when simultaneously modifying catalog definitions to include the content. Conversely, normal replication delays can also allow saving an enrollment intention that would eventually become invalid due to content being removed from the catalog, creating a false sense of correctness.

After this commit, we no longer assert during intention save() that the customer's catalogs actually contain the content. Instead, we assert that the content exists at all in the enterprise-catalog database by querying the customer- and catalog-agnostic
/api/v1/content-metadata/?content_identifiers=<content_key> endpoint.

Subsequent commits/PRs may introduce dynamic warning messaging (non-blocking) on django admin pages for the customer and intention when the intention's content key is not contained in the customer's catalogs.

ENT-9840

…usion

Before this commit, normal replication delays from discovery->catalog
can easily result in enrollment intentions being un-saveable when
simultaneously modifying catalog definitions to include the content.
Conversely, normal replication delays can also allow saving an
enrollment intention that would eventually become invalid due to content
being removed from the catalog, creating a false sense of correctness.

After this commit, we no longer assert during intention save() that the
customer's catalogs actually contain the content. Instead, we assert
that the content exists at all in the enterprise-catalog database by
querying the customer- and catalog-agnostic
`/api/v1/content-metadata/?content_identifiers=<content_key>` endpoint.

Subsequent commits/PRs may introduce dynamic warning messaging
(non-blocking) on django admin pages for the customer and intention when
the intention's content key is not contained in the customer's catalogs.

ENT-9840
@pwnage101
Copy link
Contributor Author

Unfortunately, the API that I switched to (/api/v1/content-metadata/?content_identifiers=<>) doesn't automatically return a top-level course when given a course run key. This was not caught within existing unit tests because the assumption is baked into the mocks. I'll need to make a few more tweaks and put it back up for review.

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.

1 participant