Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #26803 from edx/mikix/nudge2

Browse Source 
feat: add 'external course updates' experiment flag
This commit is contained in:
Michael Terry
2021-03-02 15:46:54 -05:00
committed by GitHub
parent f1ff795271 9570c1c72b
commit 877e8f1e83
11 changed files with 266 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
common/djangoapps/student/models.py
View File

@@ -1467,6 +1467,7 @@ class CourseEnrollment(models.Model):
"""
Emits an event to explicitly track course enrollment and unenrollment.
"""
from openedx.core.djangoapps.schedules.config import set_up_external_updates_for_enrollment
try:
context = contexts.course_context_from_course_id(self.course_id)
@@ -1486,11 +1487,14 @@ class CourseEnrollment(models.Model):
}
if event_name == EVENT_NAME_ENROLLMENT_ACTIVATED:
segment_properties['email'] = self.user.email
# This next property is for an experiment, see method's comments for more information
segment_properties['external_course_updates'] = set_up_external_updates_for_enrollment(self.user,
self.course_id)
with tracker.get_tracker().context(event_name, context):
tracker.emit(event_name, data)
segment.track(self.user_id, event_name, segment_properties)
except: # pylint: disable=bare-except
except Exception: # pylint: disable=broad-except
if event_name and self.course_id:
log.exception(
u'Unable to emit event %s for user %s and course %s',

4
common/djangoapps/student/signals/receivers.py
View File

@@ -10,7 +10,7 @@ from django.db import IntegrityError
from django.db.models.signals import post_save, pre_save
from django.dispatch import receiver
from lms.djangoapps.courseware.toggles import courseware_mfe_first_section_celebration_is_active
from lms.djangoapps.courseware.toggles import courseware_mfe_progress_milestones_are_active
from common.djangoapps.student.helpers import EMAIL_EXISTS_MSG_FMT, USERNAME_EXISTS_MSG_FMT, AccountValidationError
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentCelebration, is_email_retired, is_username_retired # lint-amnesty, pylint: disable=line-too-long
@@ -59,7 +59,7 @@ def create_course_enrollment_celebration(sender, instance, created, **kwargs):
# The UI for celebrations is only supported on the MFE right now, so don't turn on
# celebrations unless this enrollment's course is MFE-enabled and has milestones enabled.
if not courseware_mfe_first_section_celebration_is_active(instance.course_id):
if not courseware_mfe_progress_milestones_are_active(instance.course_id):
return
try:

12
common/djangoapps/student/tests/test_enrollment.py
View File

@@ -154,6 +154,18 @@ class EnrollmentTest(UrlResetMixin, SharedModuleStoreTestCase):
# Expect that we're no longer enrolled
assert not CourseEnrollment.is_enrolled(self.user, self.course.id)
@ddt.data(-1, 0, 1)
def test_external_course_updates_signal(self, value):
"""Confirm that we send the external updates experiment bucket with the activation signal"""
with patch('openedx.core.djangoapps.schedules.config.set_up_external_updates_for_enrollment',
return_value=value):
with patch('common.djangoapps.student.models.segment') as mock_segment:
CourseEnrollment.enroll(self.user, self.course.id)
assert mock_segment.track.call_count == 1
assert mock_segment.track.call_args[0][1] == 'edx.course.enrollment.activated'
assert mock_segment.track.call_args[0][2]['external_course_updates'] == value
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_EMAIL_OPT_IN': True})
@patch('openedx.core.djangoapps.user_api.preferences.api.update_email_opt_in')
@ddt.data(

2
common/djangoapps/student/tests/test_receivers.py
View File

@@ -3,7 +3,6 @@
from edx_toggles.toggles.testutils import override_waffle_flag
from lms.djangoapps.courseware.toggles import (
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES,
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_FIRST_SECTION_CELEBRATION,
REDIRECT_TO_COURSEWARE_MICROFRONTEND
)
from common.djangoapps.student.models import CourseEnrollmentCelebration
@@ -17,7 +16,6 @@ class ReceiversTest(SharedModuleStoreTestCase):
"""
@override_waffle_flag(REDIRECT_TO_COURSEWARE_MICROFRONTEND, active=True)
@override_waffle_flag(COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES, active=True)
@override_waffle_flag(COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_FIRST_SECTION_CELEBRATION, active=True)
def test_celebration_created(self):
""" Test that we make celebration objects when enrollments are created """
assert CourseEnrollmentCelebration.objects.count() == 0

21
lms/djangoapps/courseware/toggles.py
View File

@@ -67,22 +67,6 @@ COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES = CourseWaffleFlag(
WAFFLE_FLAG_NAMESPACE, 'mfe_progress_milestones', __name__
)
# .. toggle_name: courseware.mfe_progress_milestones_first_section_celebration
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to display a celebration modal on learner completion of their first section.
# Supports staged rollout to students for a new micro-frontend-based implementation of the
# courseware page.
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2020-10-07
# .. toggle_target_removal_date: None
# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and
# COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES.
# .. toggle_tickets: AA-371
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_FIRST_SECTION_CELEBRATION = CourseWaffleFlag(
WAFFLE_FLAG_NAMESPACE, 'mfe_progress_milestones_first_section_celebration', __name__
)
# .. toggle_name: courseware.mfe_progress_milestones_streak_celebration
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
@@ -152,11 +136,10 @@ def course_exit_page_is_active(course_key):
)
def courseware_mfe_first_section_celebration_is_active(course_key):
def courseware_mfe_progress_milestones_are_active(course_key):
return (
REDIRECT_TO_COURSEWARE_MICROFRONTEND.is_enabled(course_key) and
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES.is_enabled(course_key) and
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_FIRST_SECTION_CELEBRATION.is_enabled(course_key)
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES.is_enabled(course_key)
)

61
openedx/core/djangoapps/schedules/config.py
View File

@@ -2,8 +2,13 @@
Contains configuration for schedules app
"""
from edx_toggles.toggles import LegacyWaffleSwitch, LegacyWaffleSwitchNamespace, WaffleFlag
from crum import get_current_request
from edx_toggles.toggles import LegacyWaffleSwitch, LegacyWaffleFlagNamespace, LegacyWaffleSwitchNamespace, WaffleFlag
from lms.djangoapps.experiments.flags import ExperimentWaffleFlag
from lms.djangoapps.experiments.models import ExperimentData
WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='schedules')
WAFFLE_SWITCH_NAMESPACE = LegacyWaffleSwitchNamespace(name='schedules')
# .. toggle_name: schedules.enable_debugging
@@ -19,3 +24,57 @@ COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH = LegacyWaffleSwitch(
'course_update_show_unsubscribe',
__name__
)
# This experiment waffle is supporting an A/B test we are running on sending course updates from an external service,
# rather than through platform and ACE. See ticket AA-661 for more information.
# Don't use this flag directly, instead use the `set_up_external_updates_for_enrollment` and `query_external_updates`
# methods below. We save this flag decision at enrollment time and don't change it even if the flag changes. So you
# can't just directly look at flag result.
_EXTERNAL_COURSE_UPDATES_EXPERIMENT_ID = 18
_EXTERNAL_COURSE_UPDATES_FLAG = ExperimentWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'external_updates', __name__,
experiment_id=_EXTERNAL_COURSE_UPDATES_EXPERIMENT_ID,
use_course_aware_bucketing=False)
def set_up_external_updates_for_enrollment(user, course_key):
"""
Returns and stores whether a user should be getting the "external course updates" experience.
See the description of this experiment with the waffle flag definition above. But basically, if a user is getting
external course updates for a course, edx-platform just stops sending any updates, trustingn that the user is
receiving them elsewhere.
This is basically just a wrapper around our experiment waffle flag, but only buckets users that directly enrolled
(rather than users enrolled by staff), for technical "waffle-flags-can-only-get-the-user-from-the-request" reasons.
This saves the decision in experiment data tables. It is also idempotent and will not change after the first
call for a given user/course, regardless of how the waffle answer changes.
"""
request = get_current_request()
user_is_valid = request and hasattr(request, 'user') and request.user.id and request.user.id == user.id
experiment_on = _EXTERNAL_COURSE_UPDATES_FLAG.is_experiment_on(course_key)
if user_is_valid and experiment_on:
# Don't send tracking info as it might differ from our saved value, and we already send the bucket in
# enrollment segment events.
bucket = _EXTERNAL_COURSE_UPDATES_FLAG.get_bucket(course_key, track=False)
else:
bucket = -1 # a special value meaning to ignore this enrollment for analytics purposes
data, _created = ExperimentData.objects.get_or_create(experiment_id=_EXTERNAL_COURSE_UPDATES_EXPERIMENT_ID,
user_id=user.id, key=str(course_key),
defaults={'value': str(bucket)})
return int(data.value)
def query_external_updates(user_id, course_id):
"""
Returns a queryset indicating whether the user get the "external course updates" experience for the given course.
This is designed for use as a subquery in a larger queryset, which is why it returns a queryset, rather than a
boolean. But it can also be used to spot-check whether a user is in the external experience for a given course by
casting the returned queryset to a bool.
This looks up the experiment data, saved at enrollment time.
"""
return ExperimentData.objects.filter(experiment_id=_EXTERNAL_COURSE_UPDATES_EXPERIMENT_ID,
user_id=user_id, key=course_id, value='1')

21
openedx/core/djangoapps/schedules/migrations/0020_remove_config_rollout_fields.py Normal file
View File

@@ -0,0 +1,21 @@
# Generated by Django 2.2.19 on 2021-03-02 14:39
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('schedules', '0019_auto_20200316_1935'),
]
operations = [
migrations.RemoveField(
model_name='scheduleconfig',
name='create_schedules',
),
migrations.RemoveField(
model_name='scheduleconfig',
name='hold_back_ratio',
),
]

2
openedx/core/djangoapps/schedules/models.py
View File

@@ -54,14 +54,12 @@ class ScheduleConfig(ConfigurationModel):
KEY_FIELDS = ('site',)
site = models.ForeignKey(Site, on_delete=models.CASCADE)
create_schedules = models.BooleanField(default=False) # deprecated, do not use
enqueue_recurring_nudge = models.BooleanField(default=False)
deliver_recurring_nudge = models.BooleanField(default=False)
enqueue_upgrade_reminder = models.BooleanField(default=False)
deliver_upgrade_reminder = models.BooleanField(default=False)
enqueue_course_update = models.BooleanField(default=False)
deliver_course_update = models.BooleanField(default=False)
hold_back_ratio = models.FloatField(default=0) # deprecated, do not use
class ScheduleExperience(models.Model):

11
openedx/core/djangoapps/schedules/resolvers.py
View File

@@ -8,7 +8,7 @@ import attr
from django.conf import settings
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.contrib.staticfiles.templatetags.staticfiles import static
from django.db.models import F, Q
from django.db.models import Exists, F, OuterRef, Q
from django.urls import reverse
from edx_ace.recipient import Recipient
from edx_ace.recipient_resolver import RecipientResolver
@@ -18,7 +18,9 @@ from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link, can_
from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.course_date_signals.utils import get_expected_duration
from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH
from openedx.core.djangoapps.schedules.config import (
COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH, query_external_updates
)
from openedx.core.djangoapps.schedules.content_highlights import get_week_highlights, get_next_section_highlights
from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist
from openedx.core.djangoapps.schedules.message_types import CourseUpdate, InstructorLedCourseUpdate
@@ -142,6 +144,11 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver):
enrollment__user__in=users,
enrollment__is_active=True,
**schedule_day_equals_target_day_filter
).annotate(
external_updates_enabled=Exists(query_external_updates(OuterRef('enrollment__user_id'),
OuterRef('enrollment__course_id'))),
).exclude(
external_updates_enabled=True,
).order_by(order_by)
schedules = self.filter_by_org(schedules)

115
openedx/core/djangoapps/schedules/tests/test_config.py Normal file
View File

@@ -0,0 +1,115 @@
"""
Tests for schedules config flag code
"""
import crum
import ddt
from django.test import TestCase
from django.test.client import RequestFactory
from opaque_keys.edx.keys import CourseKey
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.experiments.models import ExperimentData
from lms.djangoapps.experiments.testutils import override_experiment_waffle_flag
from openedx.core.djangoapps.schedules.config import (
_EXTERNAL_COURSE_UPDATES_FLAG, query_external_updates, set_up_external_updates_for_enrollment
)
from openedx.core.djangolib.testing.utils import skip_unless_lms
@ddt.ddt
@skip_unless_lms
class ScheduleConfigExternalUpdatesTests(TestCase):
"""Tests for the 'external course updates' experiment code"""
@classmethod
def setUpClass(cls):
super().setUpClass()
cls.user = UserFactory()
cls.course_key = CourseKey.from_string('A/B/C')
def set_request(self, with_user=True, user=None):
"""Configures a current request, as required by the experiment code"""
request = RequestFactory()
if with_user:
request.user = user or self.user
self.addCleanup(crum.set_current_request, None)
crum.set_current_request(request)
def set_up_updates(self, active=True, bucket=1):
"""Sets up the external updates experiment data, with the given bucket"""
with override_experiment_waffle_flag(_EXTERNAL_COURSE_UPDATES_FLAG, active=active, bucket=bucket):
return set_up_external_updates_for_enrollment(self.user, self.course_key)
def test_set_up_fails_with_no_request(self):
"""No request fails"""
assert self.set_up_updates() == -1
def test_set_up_fails_with_no_user(self):
"""No user fails"""
self.set_request(with_user=False)
assert self.set_up_updates() == -1
def test_set_up_fails_with_anon_user(self):
"""Anon user fails"""
self.set_request(user=UserFactory(id=0))
assert self.set_up_updates() == -1
def test_set_up_fails_with_different_user(self):
"""Different user fails"""
self.set_request(user=UserFactory())
assert self.set_up_updates() == -1
def test_set_up_happy_path(self):
"""Sanity check above tests by just needing a request and confirming we're good"""
self.set_request()
assert self.set_up_updates() == 1 # bucket 1 is default for set_up_updates
@ddt.data(
(True, 0, 0), # bucket zero
(True, 1, 1), # bucket one
(False, 0, -1), # experiment off
)
@ddt.unpack
def test_set_up_returns_and_saves_result(self, active, bucket, expected):
"""Confirm that the setup call works and saves the result in the database"""
self.set_request()
assert self.set_up_updates(active=active, bucket=bucket) == expected
stored = ExperimentData.objects.get(experiment_id=18, user_id=self.user.id, key=str(self.course_key))
assert stored.value == str(expected)
def test_set_up_does_not_change_results(self):
"""Confirm that the setup call will not change its answer as flag changes"""
self.set_request()
assert self.set_up_updates() == 1
assert self.set_up_updates(active=False) == 1
# Sanity check that if we wipe saved data, we do get -1 for that last call again
ExperimentData.objects.all().delete()
assert self.set_up_updates(active=False) == -1
def test_query_external_updates(self):
"""Check that the query method hits ExperimentData correctly (and not any waffle code)"""
user2 = UserFactory()
user3 = UserFactory()
user4 = UserFactory()
ExperimentData.objects.create(experiment_id=18, user_id=self.user.id, key='A/B/C', value='1')
ExperimentData.objects.create(experiment_id=18, user_id=user2.id, key='A/B/C', value='0')
ExperimentData.objects.create(experiment_id=18, user_id=user3.id, key='A/B/C', value='-1')
ExperimentData.objects.create(experiment_id=18, user_id=user4.id, key='A/B/C', value='1')
ExperimentData.objects.create(experiment_id=18, user_id=self.user.id, key='A/B/D', value='1')
ExperimentData.objects.create(experiment_id=18, user_id=self.user.id, key='A/B/E', value='0')
ExperimentData.objects.create(experiment_id=18, user_id=self.user.id, key='A/B/F', value='-1')
assert query_external_updates(self.user.id, 'A/B/C')
assert query_external_updates(self.user.id, 'A/B/D')
assert not query_external_updates(self.user.id, 'A/B/E')
assert not query_external_updates(self.user.id, 'A/B/F')
assert not query_external_updates(user2.id, 'A/B/C')
assert not query_external_updates(user3.id, 'A/B/C')
assert query_external_updates(user4.id, 'A/B/C')

41
openedx/core/djangoapps/schedules/tests/test_resolvers.py
View File

@@ -6,12 +6,20 @@ Tests for schedules resolvers
import datetime
from unittest.mock import Mock
import crum
import ddt
import pytz
from django.test import TestCase
from django.test.client import RequestFactory
from django.test.utils import override_settings
from testfixtures import LogCapture
from waffle.testutils import override_switch
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from lms.djangoapps.experiments.testutils import override_experiment_waffle_flag
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.schedules.config import _EXTERNAL_COURSE_UPDATES_FLAG
from openedx.core.djangoapps.schedules.models import Schedule
from openedx.core.djangoapps.schedules.resolvers import (
LOG,
@@ -22,7 +30,6 @@ from openedx.core.djangoapps.schedules.resolvers import (
from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
from openedx.core.djangolib.testing.utils import CacheIsolationMixin, skip_unless_lms
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -93,6 +100,38 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase):
mock_query.exclude.assert_called_once_with(enrollment__course__org__in=expected_org_list)
assert result == mock_query.exclude.return_value
@ddt.data(0, 1)
def test_external_course_updates(self, bucket):
"""Confirm that we exclude enrollments in the external course updates experiment"""
user = UserFactory()
overview1 = CourseOverviewFactory(has_highlights=False) # set has_highlights just to avoid a modulestore lookup
overview2 = CourseOverviewFactory(has_highlights=False)
# We need to enroll with a request, because our specific experiment code expects it
self.addCleanup(crum.set_current_request, None)
request = RequestFactory()
request.user = user
crum.set_current_request(request)
enrollment1 = CourseEnrollment.enroll(user, overview1.id)
with override_experiment_waffle_flag(_EXTERNAL_COURSE_UPDATES_FLAG, bucket=bucket):
enrollment2 = CourseEnrollment.enroll(user, overview2.id)
# OK, at this point, we'd expect course1 to be returned, but course2's enrollment to be excluded by the
# experiment. Note that the experiment waffle is currently inactive, but they should still be excluded because
# they were bucketed at enrollment time.
bin_num = BinnedSchedulesBaseResolver.bin_num_for_user_id(user.id)
resolver = BinnedSchedulesBaseResolver(None, self.site, datetime.datetime.now(pytz.UTC), 0, bin_num)
resolver.schedule_date_field = 'created'
schedules = resolver.get_schedules_with_target_date_by_bin_and_orgs()
if bucket == 1:
assert len(schedules) == 1
assert schedules[0].enrollment == enrollment1
else:
assert len(schedules) == 2
assert {s.enrollment for s in schedules} == {enrollment1, enrollment2}
@skip_unless_lms
class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase):