Merge pull request #29219 from edx/jhynes/microba-1569_filter_bulk_emails_last_login

feat: filter bulk course email recipients based on last_login date

lms/djangoapps/bulk_email/models.py

@@ -4,11 +4,14 @@ Models for bulk email
 
 
 import logging
+from datetime import datetime
+from dateutil.relativedelta import relativedelta
 
 import markupsafe
 from config_models.models import ConfigurationModel
-from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
+from django.contrib.auth import get_user_model
 from django.db import models
+from django.conf import settings
 
 from opaque_keys.edx.django.models import CourseKeyField
 
@@ -23,6 +26,7 @@ from openedx.core.djangoapps.enrollments.errors import CourseModeNotFoundError
 from openedx.core.lib.html_to_text import html_to_text
 from openedx.core.lib.mail_utils import wrap_message
 
+User = get_user_model()
 log = logging.getLogger(__name__)
 
 
@@ -117,6 +121,13 @@ class Target(models.Model):
             courseenrollment__is_active=True
         )
         enrollment_qset = User.objects.filter(enrollment_query)
+
+        # filter out learners from the message who are no longer active in the course-run based on last login
+        last_login_eligibility_period = settings.BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD
+        if last_login_eligibility_period and isinstance(last_login_eligibility_period, int):
+            cutoff = datetime.now() - relativedelta(months=last_login_eligibility_period)
+            enrollment_qset = enrollment_qset.exclude(last_login__lte=cutoff)
+
         if self.target_type == SEND_TO_MYSELF:
             if user_id is None:
                 raise ValueError("Must define self user to send email to self.")

lms/djangoapps/bulk_email/tests/factories.py

@@ -0,0 +1,14 @@
+"""
+Provides factories for BulkEmail models.
+"""
+
+from factory.django import DjangoModelFactory
+
+from lms.djangoapps.bulk_email.models import SEND_TO_LEARNERS, Target
+
+
+class TargetFactory(DjangoModelFactory):
+    class Meta:
+        model = Target
+
+    target_type = SEND_TO_LEARNERS

lms/djangoapps/bulk_email/tests/test_models.py

@@ -2,19 +2,20 @@
 Unit tests for bulk-email-related models.
 """
 
-
 import datetime
+from dateutil.relativedelta import relativedelta
 from unittest.mock import Mock, patch
 
 import pytest
 import ddt
 from django.core.management import call_command
 from django.test import TestCase
+from django.test.utils import override_settings
 from opaque_keys.edx.keys import CourseKey
 from pytz import UTC
 
 from common.djangoapps.course_modes.models import CourseMode
-from common.djangoapps.student.tests.factories import UserFactory
+from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
 from lms.djangoapps.bulk_email.api import is_bulk_email_feature_enabled
 from lms.djangoapps.bulk_email.models import (
     SEND_TO_COHORT,
@@ -26,6 +27,7 @@ from lms.djangoapps.bulk_email.models import (
     CourseEmailTemplate,
     Optout
 )
+from lms.djangoapps.bulk_email.tests.factories import TargetFactory
 from openedx.core.djangoapps.course_groups.models import CourseCohort
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -310,3 +312,59 @@ class CourseAuthorizationTest(TestCase):
 
         # Now, course should STILL be authorized!
         assert is_bulk_email_feature_enabled(course_id)
+
+
+class TargetFilterTest(ModuleStoreTestCase):
+    """
+    Tests for the optional filtering of recipients from the results of the `get_users` method of the Target model.
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.user1 = UserFactory(last_login=datetime.datetime.now())
+        self.user2 = UserFactory(last_login=datetime.datetime.now() - relativedelta(months=2))
+        self.user3 = UserFactory()
+        self.course = CourseFactory()
+        CourseEnrollmentFactory(
+            is_active=True,
+            mode='verified',
+            course_id=self.course.id,
+            user=self.user1
+        )
+        CourseEnrollmentFactory(
+            is_active=True,
+            mode='audit',
+            course_id=self.course.id,
+            user=self.user2
+        )
+        CourseEnrollmentFactory(
+            is_active=False,
+            mode='verified',
+            course_id=self.course.id,
+            user=self.user3
+        )
+        self.target = TargetFactory()
+
+    @override_settings(BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD=None)
+    def test_target_no_last_login_eligibility(self):
+        """
+        Verifies the default behavior stays the same if the `BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD` is not
+        set.
+        """
+        result = self.target.get_users(self.course.id)
+
+        assert result.count() == 2
+        assert result.filter(id=self.user1.id).exists()
+        assert result.filter(id=self.user2.id).exists()
+
+    @override_settings(BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD=1)
+    def test_target_last_login_eligibility_set(self):
+        """
+        Verifies that users with a `login_date` beyond the treshold set according to the
+        `BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD` are excluded from the final results of the queryset returned
+        callers of the `get_users` method.
+        """
+        result = self.target.get_users(self.course.id)
+
+        assert result.count() == 1
+        assert result.filter(id=self.user1.id).exists()

lms/djangoapps/bulk_email/tests/test_tasks.py

@@ -7,6 +7,8 @@ paths actually work.
 """
 
 
+from datetime import datetime
+from dateutil.relativedelta import relativedelta
 import json
 from itertools import chain, cycle, repeat
 from smtplib import SMTPAuthenticationError, SMTPConnectError, SMTPDataError, SMTPServerDisconnected
@@ -28,6 +30,7 @@ from boto.ses.exceptions import (
 from celery.states import FAILURE, SUCCESS
 from django.conf import settings
 from django.core.management import call_command
+from django.test.utils import override_settings
 from opaque_keys.edx.locator import CourseLocator
 
 from lms.djangoapps.bulk_email.tasks import _get_course_email_context
@@ -472,3 +475,20 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
         assert 'account_settings_url' in result
         assert 'email_settings_url' in result
         assert 'platform_name' in result
+
+    @override_settings(BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD=1)
+    def test_ineligible_recipients_filtered_by_last_login(self):
+        """
+        Test that verifies active and enrolled students with last_login dates beyond the set threshold are not sent bulk
+        course emails.
+        """
+        # create students and enrollments for test, then update the last_login dates to fit the scenario under test
+        students = self._create_students(2)
+        students[0].last_login = datetime.now()
+        students[1].last_login = datetime.now() - relativedelta(months=2)
+
+        with patch('lms.djangoapps.bulk_email.tasks.get_connection', autospec=True) as get_conn:
+            get_conn.return_value.send_messages.side_effect = cycle([None])
+            # we should expect only one email to be sent as the other learner is not eligible to receive the message
+            # based on their last_login date
+            self._test_run_with_task(send_bulk_course_email, 'emailed', 1, 1)

lms/envs/common.py

@@ -4841,3 +4841,9 @@ TEAMS_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-runn
 TEXTBOOKS_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/textbooks.html"
 WIKI_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/course_wiki.html"
 CUSTOM_PAGES_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/pages.html#adding-custom-pages"
+
+################# Bulk Course Email Settings #################
+# If set, recipients of bulk course email messages will be filtered based on the last_login date of their User account.
+# The expected value is an Integer representing the cutoff point (in months) for inclusion to the message. Example:
+# a value of `3` would include learners who have logged in within the past 3 months.
+BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD = None

lms/envs/devstack.py

@@ -401,11 +401,6 @@ if FEATURES.get('ENABLE_ENTERPRISE_INTEGRATION'):
 DCS_SESSION_COOKIE_SAMESITE = 'Lax'
 DCS_SESSION_COOKIE_SAMESITE_FORCE_ALL = True
 
-#####################################################################
-# See if the developer has any local overrides.
-if os.path.isfile(join(dirname(abspath(__file__)), 'private.py')):
-    from .private import *  # pylint: disable=import-error,wildcard-import
-
 ########################## THEMING  #######################
 # If you want to enable theming in devstack, uncomment this section and add any relevant
 # theme directories to COMPREHENSIVE_THEME_DIRS
@@ -439,3 +434,9 @@ PROCTORING_USER_OBFUSCATION_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
 
 #################### Webpack Configuration Settings ##############################
 WEBPACK_LOADER['DEFAULT']['TIMEOUT'] = 5
+
+################# New settings must go ABOVE this line #################
+########################################################################
+# See if the developer has any local overrides.
+if os.path.isfile(join(dirname(abspath(__file__)), 'private.py')):
+    from .private import *  # pylint: disable=import-error,wildcard-import