-
Notifications
You must be signed in to change notification settings - Fork 176
fix: editors fetching query for self paced courses in send_course_deadline_emails command #4640
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
Conversation
@@ -97,7 +97,7 @@ def handle_send_email_to_pcs_and_editors(self, course, course_run, email_variant | |||
This method retrieves the email addresses of course editors and project coordinators associated with the course | |||
and schedules the email to be sent using the `process_send_course_deadline_email` task. | |||
""" | |||
course_editors = list(course.editors.values_list('user__email', flat=True).distinct()) | |||
course_editors = list(course.draft_version.editors.values_list('user__email', flat=True).distinct()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use Course.objects.filter_drafts()
, as it has been used in editor API, to ensure that if a draft exists, return that. Otherwise, return non-draft.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Course.objects.filter_drafts() basically returns a queryset of all draft courses. Are you referring to adding the uuid here to this queryset to fetch only specific course?
Or should we use it in the command’s handle
method in filtering instead?
Line 44 in 31e5143
courses_with_self_paced_runs = Course.objects.filter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or should we use it in the command’s handle method in filtering instead?
this. This will do the required filtering at the very top and we would not need to add this draft_version handling here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we update them here, we still need to access the official version below because the draft version of a course only has an associated draft run, which is not marketable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. Makes sense.
db26df8
to
330b6f2
Compare
…dline_emails command (openedx#4640) * fix: editors fetching query for self paced courses in send_course_deadline_emails command * test: update unittests accordingly * fix: a test
This PR fixes the editors query to fetch them from the draft version of the course instead of the official version. While testing the send_course_deadline_emails command, I found that course editors are linked to the draft entry of the course, not the non-draft version.
I've verified email sending functionality by assigning manually course_editor to draft entry of course.