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: Course team members are included in "Learners in the track" emails recipients #34609

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lms/djangoapps/bulk_email/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def get_users(self, course_id, user_id=None):
User.objects.filter(
models.Q(courseenrollment__mode=self.coursemodetarget.track.mode_slug)
& enrollment_query
)
).exclude(id__in=staff_instructor_qset)
)
else:
raise ValueError(f"Unrecognized target type {self.target_type}")
Expand Down
44 changes: 42 additions & 2 deletions lms/djangoapps/bulk_email/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from pytz import UTC

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory, StaffFactory
from lms.djangoapps.bulk_email.api import is_bulk_email_feature_enabled
from lms.djangoapps.bulk_email.models import (
SEND_TO_COHORT,
Expand All @@ -25,8 +25,10 @@
CourseAuthorization,
CourseEmail,
CourseEmailTemplate,
CourseModeTarget,
DisabledCourse,
Optout
Optout,
Target,
)
from lms.djangoapps.bulk_email.models_api import is_bulk_email_disabled_for_course
from lms.djangoapps.bulk_email.tests.factories import TargetFactory
Expand Down Expand Up @@ -366,6 +368,7 @@ def setUp(self):
course_id=self.course.id,
user=self.user3
)
self.staff_user = StaffFactory.create(course_key=self.course.id)
self.target = TargetFactory()

@override_settings(BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD=None)
Expand All @@ -391,3 +394,40 @@ def test_target_last_login_eligibility_set(self):

assert result.count() == 1
assert result.filter(id=self.user1.id).exists()

def test_filtering_of_recipients_target_for_audit_track(self):
farhaanbukhsh marked this conversation as resolved.
Show resolved Hide resolved
"""
Verifies the default behavior.

This test ensures that when the `BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD`
setting is not defined, all users enrolled in the course are included in the results.
"""
target = Target.objects.create(target_type=SEND_TO_TRACK)
course_mode = CourseMode.objects.create(
mode_slug=CourseMode.AUDIT,
mode_display_name=CourseMode.AUDIT.capitalize(),
course_id=self.course.id,
)
course_mode_target = CourseModeTarget.objects.create(track=course_mode)
target.coursemodetarget = course_mode_target
result = target.get_users(self.course.id)

assert result.count() == 1
assert result.filter(id=self.user2.id).exists()
Copy link
Member

Choose a reason for hiding this comment

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

can we also check if staff_user does not exist here?


# Ensure staff user is not included
assert not result.filter(id=self.staff_user.id).exists()

def test_filtering_of_recipients_target_for_staff(self):
"""
Test filtering of recipients for a target of type SEND_TO_STAFF.

This test verifies that only staff users are returned for the given target.
It creates a target of type SEND_TO_STAFF and ensures that the correct users
are retrieved.
"""
self.target = TargetFactory(target_type=SEND_TO_STAFF)
result = self.target.get_users(self.course.id)

assert result.count() == 1
assert result.filter(id=self.staff_user.id).exists()
Loading