feat: support course run based assignments in credits_available, expiration, emails, etc. #561
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
adamstankiewicz
force-pushed
the
ags/ent-8876-pt2
branch
3 times, most recently
from
bd10ac0
to
628233f
Compare
…ration, emails, etc.
adamstankiewicz
force-pushed
the
ags/ent-8876-pt2
branch
from
628233f
to
a0d5246
Compare
enterprise_access/apps/content_assignments/content_metadata_api.py
Outdated
Show resolved
Hide resolved
pwnage101
reviewed
Sep 11, 2024
enterprise_access/apps/content_assignments/content_metadata_api.py
Outdated
Show resolved
Hide resolved
adamstankiewicz
force-pushed
the
ags/ent-8876-pt2
branch
from
e11c90b
to
263dd2f
Compare
adamstankiewicz
force-pushed
the
ags/ent-8876-pt2
branch
from
4068f6e
to
69b0ff2
Compare
pwnage101
reviewed
Sep 11, 2024
Comment on lines
30
to
34
| if is_assigned_course_run: | ||
| metadata_by_key[assignment_content_key] = course_metadata | ||
| else: | ||
| # if not is_assigned_course_run, we can assume it's a (legacy) course-based assignment | ||
| metadata_by_key[course_key] = course_metadata |
There was a problem hiding this comment.
Aren't these still the same logic in each block? If the assignment is a legacy assignment, then course_key is equivalent to assignment_content_key, is it not?
Suggested change
| if is_assigned_course_run: | |
| metadata_by_key[assignment_content_key] = course_metadata | |
| else: | |
| # if not is_assigned_course_run, we can assume it's a (legacy) course-based assignment | |
| metadata_by_key[course_key] = course_metadata | |
| metadata_by_key[assignment_content_key] = course_metadata |
There was a problem hiding this comment.
@pwnage101 Ahhh, I understand now; you're absolutely correct. I pushed another commit 91ac3ab with a refactor. Let me know what you think!
There was a problem hiding this comment.
[inform] @pwnage101 I updated once more in cb9d27b.
pwnage101
reviewed
Sep 11, 2024
enterprise_access/apps/content_assignments/content_metadata_api.py
Outdated
Show resolved
Hide resolved
...prise_access/apps/content_assignments/management/commands/automatically_nudge_assignments.py
Outdated
Show resolved
Hide resolved
adamstankiewicz
commented
Sep 11, 2024
enterprise_access/apps/content_assignments/content_metadata_api.py
Outdated
Show resolved
Hide resolved
iloveagent57
approved these changes
Sep 12, 2024
enterprise_access/apps/content_assignments/content_metadata_api.py
Outdated
Show resolved
Hide resolved
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.
Description:
Relying on the update to enterprise-catalog's
get_content_metadataREST API to return the top-level course object when passing course run key(s) to the?content_keysquery parameter and the addition ofnormalized_metadata_by_run, this PR updates theget_content_metadata_for_assignmentsmethod and where it's consumed to supportcredits_available, expiration management commands, and email notifications related to assignments.Jira:
ENT-8876
Merge checklist:
./manage.py makemigrationshas been runPost merge: