From 7b1fead04c2add52d05e99ced5b0fb9706221880 Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Tue, 11 Jan 2022 16:22:54 -0500 Subject: [PATCH] feat: AA-1138: Adds ability to have Weekly Goal Celebration Modal in MFE Adds celebrate_weekly_goal to the CourseEnrollmentCelebration and includes logic for only returning True if the db field is true and the learner has hit their goal this week. Adds ability to set to false via the API already used by the frontend. Default db value is False, but all new enrollments after merge will be set to True. --- common/djangoapps/student/admin.py | 2 +- ...llmentcelebration_celebrate_weekly_goal.py | 18 +++++++ common/djangoapps/student/models.py | 53 ++++++++++++++++--- .../djangoapps/student/signals/receivers.py | 1 + .../student/tests/test_parental_controls.py | 8 ++- .../student/tests/test_receivers.py | 8 ++- .../courseware_api/tests/test_views.py | 18 +++++-- .../core/djangoapps/courseware_api/utils.py | 2 + .../core/djangoapps/courseware_api/views.py | 10 ++++ 9 files changed, 99 insertions(+), 21 deletions(-) create mode 100644 common/djangoapps/student/migrations/0044_courseenrollmentcelebration_celebrate_weekly_goal.py diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index d8176e3d2d..b6546fc672 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -524,7 +524,7 @@ class AllowedAuthUserAdmin(admin.ModelAdmin): class CourseEnrollmentCelebrationAdmin(DisableEnrollmentAdminMixin, admin.ModelAdmin): """Admin interface for the CourseEnrollmentCelebration model. """ raw_id_fields = ('enrollment',) - list_display = ('id', 'course', 'user', 'celebrate_first_section') + list_display = ('id', 'course', 'user', 'celebrate_first_section', 'celebrate_weekly_goal',) search_fields = ('enrollment__course__id', 'enrollment__user__username') class Meta: diff --git a/common/djangoapps/student/migrations/0044_courseenrollmentcelebration_celebrate_weekly_goal.py b/common/djangoapps/student/migrations/0044_courseenrollmentcelebration_celebrate_weekly_goal.py new file mode 100644 index 0000000000..59ca6827a5 --- /dev/null +++ b/common/djangoapps/student/migrations/0044_courseenrollmentcelebration_celebrate_weekly_goal.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.11 on 2022-01-11 14:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0043_remove_userprofile_allow_certificate'), + ] + + operations = [ + migrations.AddField( + model_name='courseenrollmentcelebration', + name='celebrate_weekly_goal', + field=models.BooleanField(default=False), + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index f976f493d4..35dffec2e7 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -18,7 +18,7 @@ import json # lint-amnesty, pylint: disable=wrong-import-order import logging # lint-amnesty, pylint: disable=wrong-import-order import uuid # lint-amnesty, pylint: disable=wrong-import-order from collections import defaultdict, namedtuple # lint-amnesty, pylint: disable=wrong-import-order -from datetime import datetime, timedelta # lint-amnesty, pylint: disable=wrong-import-order +from datetime import date, datetime, timedelta # lint-amnesty, pylint: disable=wrong-import-order from functools import total_ordering # lint-amnesty, pylint: disable=wrong-import-order from importlib import import_module # lint-amnesty, pylint: disable=wrong-import-order from urllib.parse import urlencode # lint-amnesty, pylint: disable=wrong-import-order @@ -683,11 +683,11 @@ class UserProfile(models.Model): self.set_meta(meta) self.save() - def requires_parental_consent(self, date=None, age_limit=None, default_requires_consent=True): + def requires_parental_consent(self, year=None, age_limit=None, default_requires_consent=True): """Returns true if this user requires parental consent. Args: - date (Date): The date for which consent needs to be tested (defaults to now). + year (int): The year for which consent needs to be tested (defaults to now). age_limit (int): The age limit at which parental consent is no longer required. This defaults to the value of the setting 'PARENTAL_CONTROL_AGE_LIMIT'. default_requires_consent (bool): True if users require parental consent if they @@ -712,10 +712,10 @@ class UserProfile(models.Model): if year_of_birth is None: return default_requires_consent - if date is None: + if year is None: age = self.age else: - age = self._calculate_age(date.year, year_of_birth) + age = self._calculate_age(year, year_of_birth) return age < age_limit @@ -3445,15 +3445,22 @@ class CourseEnrollmentCelebration(TimeStampedModel): """ enrollment = models.OneToOneField(CourseEnrollment, models.CASCADE, related_name='celebration') celebrate_first_section = models.BooleanField(default=False) + celebrate_weekly_goal = models.BooleanField(default=False) def __str__(self): return ( - '[CourseEnrollmentCelebration] course: {}; user: {}; first_section: {};' - ).format(self.enrollment.course.id, self.enrollment.user.username, self.celebrate_first_section) + '[CourseEnrollmentCelebration] course: {}; user: {}' + ).format(self.enrollment.course.id, self.enrollment.user.username) @staticmethod def should_celebrate_first_section(enrollment): - """ Returns the celebration value for first_section with appropriate fallback if it doesn't exist """ + """ + Returns the celebration value for first_section with appropriate fallback if it doesn't exist. + + The frontend will use this result and additional information calculated to actually determine + if the first section celebration will render. In other words, the value returned here is + NOT the final value used. + """ if not enrollment: return False try: @@ -3461,6 +3468,36 @@ class CourseEnrollmentCelebration(TimeStampedModel): except CourseEnrollmentCelebration.DoesNotExist: return False + @staticmethod + def should_celebrate_weekly_goal(enrollment): + """ + Returns the celebration value for weekly_goal with appropriate fallback if it doesn't exist. + + The frontend will use this result directly to determine if the weekly goal celebration + should be rendered. The value returned here IS the final value used. + """ + # Avoiding circular import + from lms.djangoapps.course_goals.models import CourseGoal, UserActivity + try: + if not enrollment or not enrollment.celebration.celebrate_weekly_goal: + return False + except CourseEnrollmentCelebration.DoesNotExist: + return False + + try: + goal = CourseGoal.objects.get(user=enrollment.user, course_key=enrollment.course.id) + if not goal.days_per_week: + return False + + today = date.today() + monday_date = today - timedelta(days=today.weekday()) + week_activity_count = UserActivity.objects.filter( + user=enrollment.user, course_key=enrollment.course.id, date__gte=monday_date, + ).count() + return week_activity_count == goal.days_per_week + except CourseGoal.DoesNotExist: + return False + class UserPasswordToggleHistory(TimeStampedModel): """ diff --git a/common/djangoapps/student/signals/receivers.py b/common/djangoapps/student/signals/receivers.py index 6af93ee106..5c4e754f7f 100644 --- a/common/djangoapps/student/signals/receivers.py +++ b/common/djangoapps/student/signals/receivers.py @@ -74,6 +74,7 @@ def create_course_enrollment_celebration(sender, instance, created, **kwargs): CourseEnrollmentCelebration.objects.create( enrollment=instance, celebrate_first_section=True, + celebrate_weekly_goal=True, ) except IntegrityError: # A celebration object was already created. Shouldn't happen, but ignore it if it does. diff --git a/common/djangoapps/student/tests/test_parental_controls.py b/common/djangoapps/student/tests/test_parental_controls.py index 058e26a7cb..4e49f88af9 100644 --- a/common/djangoapps/student/tests/test_parental_controls.py +++ b/common/djangoapps/student/tests/test_parental_controls.py @@ -1,8 +1,6 @@ """Unit tests for parental controls.""" -import datetime - from django.test import TestCase from django.test.utils import override_settings from django.utils.timezone import now @@ -60,13 +58,13 @@ class ProfileParentalControlsTest(TestCase): # Verify for a child born 13 years agp self.set_year_of_birth(current_year - 13) assert self.profile.requires_parental_consent() - assert self.profile.requires_parental_consent(date=datetime.date(current_year, 12, 31)) - assert not self.profile.requires_parental_consent(date=datetime.date((current_year + 1), 1, 1)) + assert self.profile.requires_parental_consent(year=current_year) + assert not self.profile.requires_parental_consent(year=(current_year + 1)) # Verify for a child born 14 years ago self.set_year_of_birth(current_year - 14) assert not self.profile.requires_parental_consent() - assert not self.profile.requires_parental_consent(date=datetime.date(current_year, 1, 1)) + assert not self.profile.requires_parental_consent(year=current_year) def test_profile_image(self): """Verify that a profile's image obeys parental controls.""" diff --git a/common/djangoapps/student/tests/test_receivers.py b/common/djangoapps/student/tests/test_receivers.py index 7327147add..85a45f3cf6 100644 --- a/common/djangoapps/student/tests/test_receivers.py +++ b/common/djangoapps/student/tests/test_receivers.py @@ -28,7 +28,9 @@ class ReceiversTest(SharedModuleStoreTestCase): # Test initial creation upon an enrollment being made enrollment = CourseEnrollmentFactory() assert CourseEnrollmentCelebration.objects.count() == 1 - celebration = CourseEnrollmentCelebration.objects.get(enrollment=enrollment, celebrate_first_section=True) + celebration = CourseEnrollmentCelebration.objects.get( + enrollment=enrollment, celebrate_first_section=True, celebrate_weekly_goal=True + ) # Test nothing changes if we update that enrollment celebration.celebrate_first_section = False @@ -36,7 +38,9 @@ class ReceiversTest(SharedModuleStoreTestCase): enrollment.mode = 'test-mode' enrollment.save() assert CourseEnrollmentCelebration.objects.count() == 1 - CourseEnrollmentCelebration.objects.get(enrollment=enrollment, celebrate_first_section=False) + CourseEnrollmentCelebration.objects.get( + enrollment=enrollment, celebrate_first_section=False, celebrate_weekly_goal=True + ) def test_celebration_gated_by_waffle(self): """ Test we don't make a celebration if the MFE redirect waffle flag is off """ diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index 0976e89a3a..a1f0a75c72 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -166,6 +166,8 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): assert found, 'external link not in course tabs' assert not response.data['user_has_passing_grade'] + assert response.data['celebrations']['first_section'] + assert not response.data['celebrations']['weekly_goal'] # This import errors in cms if it is imported at the top level from lms.djangoapps.course_goals.api import get_course_goal @@ -532,13 +534,15 @@ class CelebrationApiTestViews(BaseCoursewareTests, MasqueradeMixin): @ddt.data(True, False) def test_happy_path(self, update): if update: - CourseEnrollmentCelebrationFactory(enrollment=self.enrollment, celebrate_first_section=False) + CourseEnrollmentCelebrationFactory(enrollment=self.enrollment) - response = self.client.post(self.url, {'first_section': True}, content_type='application/json') + data = {'first_section': True, 'weekly_goal': True} + response = self.client.post(self.url, data, content_type='application/json') assert response.status_code == (200 if update else 201) celebration = CourseEnrollmentCelebration.objects.first() assert celebration.celebrate_first_section + assert celebration.celebrate_weekly_goal assert celebration.enrollment.id == self.enrollment.id def test_extra_data(self): @@ -572,12 +576,16 @@ class CelebrationApiTestViews(BaseCoursewareTests, MasqueradeMixin): user = UserFactory() CourseEnrollment.enroll(user, self.course.id, 'verified') - response = self.client.post(self.url, {'first_section': True}, content_type='application/json') + data = {'first_section': True, 'weekly_goal': False} + response = self.client.post(self.url, data, content_type='application/json') assert response.status_code == 201 self.update_masquerade(username=user.username) - response = self.client.post(self.url, {'first_section': False}, content_type='application/json') + data = {'first_section': False, 'weekly_goal': True} + response = self.client.post(self.url, data, content_type='application/json') assert response.status_code == 202 celebration = CourseEnrollmentCelebration.objects.first() - assert celebration.celebrate_first_section # make sure it didn't change during masquerade attempt + # make sure they didn't change during masquerade attempt + assert celebration.celebrate_first_section + assert not celebration.celebrate_weekly_goal diff --git a/openedx/core/djangoapps/courseware_api/utils.py b/openedx/core/djangoapps/courseware_api/utils.py index d2a1947973..c7f28937d5 100644 --- a/openedx/core/djangoapps/courseware_api/utils.py +++ b/openedx/core/djangoapps/courseware_api/utils.py @@ -20,6 +20,7 @@ def get_celebrations_dict(user, enrollment, course, browser_timezone): 'first_section': False, 'streak_length_to_celebrate': None, 'streak_discount_enabled': False, + 'weekly_goal': False, } streak_length_to_celebrate = UserCelebration.perform_streak_updates( @@ -29,6 +30,7 @@ def get_celebrations_dict(user, enrollment, course, browser_timezone): 'first_section': CourseEnrollmentCelebration.should_celebrate_first_section(enrollment), 'streak_length_to_celebrate': streak_length_to_celebrate, 'streak_discount_enabled': False, + 'weekly_goal': CourseEnrollmentCelebration.should_celebrate_weekly_goal(enrollment), } if streak_length_to_celebrate: diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index aa5831f542..72c44092e7 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -407,6 +407,12 @@ class CoursewareInformation(RetrieveAPIView): * masquerading_expired_course: (bool) Whether this course is expired for the masqueraded user * upgrade_deadline: (str) Last chance to upgrade, in ISO 8601 notation (or None if can't upgrade anymore) * upgrade_url: (str) Upgrade linke (or None if can't upgrade anymore) + * celebrations: An object detailing which celebrations to render + * first_section: (bool) If the first section celebration should render + Note: Also uses information from frontend so this value is not final + * streak_length_to_celebrate: (int) The streak length to celebrate for the learner + * streak_discount_enabled: (bool) If the frontend should render an upgrade discount for hitting the streak + * weekly_goal: (bool) If the weekly goal celebration should render * course_goals: * selected_goal: * days_per_week: (int) The number of days the learner wants to learn per week @@ -697,6 +703,7 @@ class Celebration(DeveloperErrorViewMixin, APIView): Body consists of the following fields: * first_section (bool): whether we should celebrate when a user finishes their first section of a course + * weekly_goal (bool): whether we should celebrate when a user hits their weekly learning goal in a course **Returns** @@ -731,6 +738,7 @@ class Celebration(DeveloperErrorViewMixin, APIView): data = dict(request.data) first_section = data.pop('first_section', None) + weekly_goal = data.pop('weekly_goal', None) if data: return Response(status=400) # there were parameters we didn't recognize @@ -741,6 +749,8 @@ class Celebration(DeveloperErrorViewMixin, APIView): defaults = {} if first_section is not None: defaults['celebrate_first_section'] = first_section + if weekly_goal is not None: + defaults['celebrate_weekly_goal'] = weekly_goal if defaults: _, created = CourseEnrollmentCelebration.objects.update_or_create(enrollment=enrollment, defaults=defaults)