diff --git a/lms/djangoapps/course_goals/api.py b/lms/djangoapps/course_goals/api.py index dda86f3513..f132d894a8 100644 --- a/lms/djangoapps/course_goals/api.py +++ b/lms/djangoapps/course_goals/api.py @@ -12,7 +12,9 @@ from lms.djangoapps.course_goals.models import CourseGoal, GOAL_KEY_CHOICES from openedx.features.course_experience import ENABLE_COURSE_GOALS -def add_course_goal(user, course_id, goal_key): +def add_course_goal(user, course_id, goal_key=None, + number_of_days_with_visits_per_week_goal=None, + subscribed_to_goal_reminders=None): """ Add a new course goal for the provided user and course. If the goal already exists, simply update and save the goal. @@ -21,11 +23,21 @@ def add_course_goal(user, course_id, goal_key): user: The user that is setting the goal course_id (string): The id for the course the goal refers to goal_key (string): The goal key for the new goal. + number_of_days_with_visits_per_week_goal (int): number of days learner wants to learn per week + subscribed_to_goal_reminders (bool): whether the learner wants to receive email reminders about their goal """ course_key = CourseKey.from_string(str(course_id)) - CourseGoal.objects.update_or_create( - user=user, course_key=course_key, defaults={'goal_key': goal_key} - ) + if goal_key: + CourseGoal.objects.update_or_create( + user=user, course_key=course_key, defaults={'goal_key': goal_key} + ) + else: + CourseGoal.objects.update_or_create( + user=user, course_key=course_key, defaults={ + 'number_of_days_with_visits_per_week_goal': number_of_days_with_visits_per_week_goal, + 'subscribed_to_goal_reminders': subscribed_to_goal_reminders, + } + ) def get_course_goal(user, course_key): diff --git a/lms/djangoapps/course_goals/handlers.py b/lms/djangoapps/course_goals/handlers.py index 9fa10f1b13..a97072241e 100644 --- a/lms/djangoapps/course_goals/handlers.py +++ b/lms/djangoapps/course_goals/handlers.py @@ -18,6 +18,8 @@ def emit_course_goal_event(sender, instance, **kwargs): # lint-amnesty, pylint: properties = { 'courserun_key': str(instance.course_key), 'goal_key': instance.goal_key, + 'number_of_days_with_visits_per_week_goal': instance.number_of_days_with_visits_per_week_goal, + 'subscribed_to_goal_reminders': instance.subscribed_to_goal_reminders, } tracker.emit(name, properties) segment.track(instance.user.id, name, properties) diff --git a/lms/djangoapps/course_goals/migrations/0004_auto_20210726_1338.py b/lms/djangoapps/course_goals/migrations/0004_auto_20210726_1338.py new file mode 100644 index 0000000000..86ff6f25ec --- /dev/null +++ b/lms/djangoapps/course_goals/migrations/0004_auto_20210726_1338.py @@ -0,0 +1,33 @@ +# Generated by Django 2.2.24 on 2021-07-26 13:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_goals', '0003_historicalcoursegoal'), + ] + + operations = [ + migrations.AddField( + model_name='coursegoal', + name='number_of_days_with_visits_per_week_goal', + field=models.PositiveIntegerField(default=0), + ), + migrations.AddField( + model_name='coursegoal', + name='subscribed_to_goal_reminders', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='historicalcoursegoal', + name='number_of_days_with_visits_per_week_goal', + field=models.PositiveIntegerField(default=0), + ), + migrations.AddField( + model_name='historicalcoursegoal', + name='subscribed_to_goal_reminders', + field=models.BooleanField(default=False), + ), + ] diff --git a/lms/djangoapps/course_goals/models.py b/lms/djangoapps/course_goals/models.py index 076d3dbb5f..13a51a42c4 100644 --- a/lms/djangoapps/course_goals/models.py +++ b/lms/djangoapps/course_goals/models.py @@ -18,6 +18,8 @@ GOAL_KEY_CHOICES = Choices( ('unsure', _('Not sure yet')), ) +NUMBER_OF_DAYS_OPTIONS = [1, 3, 5] + User = get_user_model() @@ -33,12 +35,16 @@ class CourseGoal(models.Model): user = models.ForeignKey(User, blank=False, on_delete=models.CASCADE) course_key = CourseKeyField(max_length=255, db_index=True) + # The goal a user has set for the number of days they want to learn per week + number_of_days_with_visits_per_week_goal = models.PositiveIntegerField(default=0) + # Controls whether a user will receive emails reminding them to stay on track with their learning goal + subscribed_to_goal_reminders = models.BooleanField(default=False) goal_key = models.CharField(max_length=100, choices=GOAL_KEY_CHOICES, default=GOAL_KEY_CHOICES.unsure) history = HistoricalRecords() def __str__(self): - return 'CourseGoal: {user} set goal to {goal} for course {course}'.format( + return 'CourseGoal: {user} set goal to {goal} days per week for course {course}'.format( user=self.user.username, - goal=self.goal_key, + goal=self.number_of_days_with_visits_per_week_goal, course=self.course_key, ) diff --git a/lms/djangoapps/course_goals/tests/test_api.py b/lms/djangoapps/course_goals/tests/test_api.py index 1f7623411a..99983f6027 100644 --- a/lms/djangoapps/course_goals/tests/test_api.py +++ b/lms/djangoapps/course_goals/tests/test_api.py @@ -46,7 +46,10 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase): """ Ensures a correctly formatted post succeeds.""" response = self.post_course_goal(valid=True, goal_key='certify') segment_call.assert_called_once_with(self.user.id, EVENT_NAME_ADDED, { - 'courserun_key': str(self.course.id), 'goal_key': 'certify' + 'courserun_key': str(self.course.id), + 'goal_key': 'certify', + 'number_of_days_with_visits_per_week_goal': 0, + 'subscribed_to_goal_reminders': False, }) assert response.status_code == 201 @@ -79,13 +82,19 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase): self.post_course_goal(valid=True, goal_key='unsure') segment_call.assert_any_call(self.user.id, EVENT_NAME_ADDED, { - 'courserun_key': str(self.course.id), 'goal_key': 'explore' + 'courserun_key': str(self.course.id), 'goal_key': 'explore', + 'number_of_days_with_visits_per_week_goal': 0, + 'subscribed_to_goal_reminders': False, }) segment_call.assert_any_call(self.user.id, EVENT_NAME_UPDATED, { - 'courserun_key': str(self.course.id), 'goal_key': 'certify' + 'courserun_key': str(self.course.id), 'goal_key': 'certify', + 'number_of_days_with_visits_per_week_goal': 0, + 'subscribed_to_goal_reminders': False, }) segment_call.assert_any_call(self.user.id, EVENT_NAME_UPDATED, { - 'courserun_key': str(self.course.id), 'goal_key': 'unsure' + 'courserun_key': str(self.course.id), 'goal_key': 'unsure', + 'number_of_days_with_visits_per_week_goal': 0, + 'subscribed_to_goal_reminders': False, }) current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id) assert len(current_goals) == 1 diff --git a/lms/djangoapps/course_goals/tests/test_goals.py b/lms/djangoapps/course_goals/tests/test_goals.py new file mode 100644 index 0000000000..e7c49502d5 --- /dev/null +++ b/lms/djangoapps/course_goals/tests/test_goals.py @@ -0,0 +1,127 @@ +""" +Unit tests for course_goals djangoapp +""" + +from unittest import mock + +from django.contrib.auth import get_user_model +from django.test.utils import override_settings +from django.urls import reverse +from rest_framework.test import APIClient + +from common.djangoapps.student.models import CourseEnrollment +from edx_toggles.toggles.testutils import override_waffle_flag +from lms.djangoapps.course_goals.models import CourseGoal +from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from openedx.features.course_experience import ENABLE_COURSE_GOALS + +EVENT_NAME_ADDED = 'edx.course.goal.added' +EVENT_NAME_UPDATED = 'edx.course.goal.updated' + +User = get_user_model() + + +@override_waffle_flag(ENABLE_COURSE_GOALS, active=True) +@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) +class TestCourseGoalsAPI(SharedModuleStoreTestCase): + """ + Testing the Course Goals API. + """ + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(emit_signals=True) + + self.user = User.objects.create_user('john', 'lennon@thebeatles.com', 'password') + CourseEnrollment.enroll(self.user, self.course.id) + + self.client = APIClient(enforce_csrf_checks=True) + self.client.login(username=self.user.username, password=self.user.password) + self.client.force_authenticate(user=self.user) + + self.apiUrl = reverse('course-home-save-course-goal') + + def save_course_goal(self, number, subscribed): + """ + Sends a post request to set a course goal and returns the response. + """ + post_data = { + 'course_id': self.course.id, + 'user': self.user.username, + } + if number is not None: + post_data['number_of_days_with_visits_per_week_goal'] = number + if subscribed is not None: + post_data['subscribed_to_goal_reminders'] = subscribed + + response = self.client.post(self.apiUrl, post_data) + return response + + @mock.patch('lms.djangoapps.course_goals.handlers.segment.track') + @override_settings(LMS_SEGMENT_KEY="foobar") + def test_add_goal(self, segment_call): + """ Ensures a correctly formatted post succeeds.""" + self.save_course_goal(1, True) + segment_call.assert_called_once_with(self.user.id, EVENT_NAME_ADDED, { + 'courserun_key': str(self.course.id), + 'number_of_days_with_visits_per_week_goal': 1, + 'subscribed_to_goal_reminders': True, + 'goal_key': 'unsure', + }) + + current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id) + assert len(current_goals) == 1 + assert current_goals[0].number_of_days_with_visits_per_week_goal == 1 + assert current_goals[0].subscribed_to_goal_reminders is True + + @mock.patch('lms.djangoapps.course_goals.handlers.segment.track') + @override_settings(LMS_SEGMENT_KEY="foobar") + def test_update_goal(self, segment_call): + """ Ensures that repeatedly saving a course goal does not create new instances of the goal. """ + self.save_course_goal(1, True) + segment_call.assert_called_with(self.user.id, EVENT_NAME_ADDED, { + 'courserun_key': str(self.course.id), + 'number_of_days_with_visits_per_week_goal': 1, + 'subscribed_to_goal_reminders': True, + 'goal_key': 'unsure', + }) + + self.save_course_goal(3, True) + segment_call.assert_called_with(self.user.id, EVENT_NAME_UPDATED, { + 'courserun_key': str(self.course.id), + 'number_of_days_with_visits_per_week_goal': 3, + 'subscribed_to_goal_reminders': True, + 'goal_key': 'unsure', + }) + + self.save_course_goal(5, False) + segment_call.assert_called_with(self.user.id, EVENT_NAME_UPDATED, { + 'courserun_key': str(self.course.id), + 'number_of_days_with_visits_per_week_goal': 5, + 'subscribed_to_goal_reminders': False, + 'goal_key': 'unsure', + }) + + current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id) + assert len(current_goals) == 1 + assert current_goals[0].number_of_days_with_visits_per_week_goal == 5 + assert current_goals[0].subscribed_to_goal_reminders is False + + def test_add_without_required_arguments(self): + """ Ensures if required arguments are not provided, post does not succeed. """ + response = self.save_course_goal(None, None) + assert len(CourseGoal.objects.filter(user=self.user, course_key=self.course.id)) == 0 + self.assertContains( + response=response, + text="'number_of_days_with_visits_per_week_goal' and 'subscribed_to_goal_reminders' are required.", + status_code=400 + ) + + def test_add_invalid_goal(self): + """ Ensures an incorrectly formatted post does not succeed. """ + response = self.save_course_goal('notnumber', False) + assert response.status_code == 400 + assert len(CourseGoal.objects.filter(user=self.user, course_key=self.course.id)) == 0 diff --git a/lms/djangoapps/course_goals/toggles.py b/lms/djangoapps/course_goals/toggles.py new file mode 100644 index 0000000000..c20bfcc0e9 --- /dev/null +++ b/lms/djangoapps/course_goals/toggles.py @@ -0,0 +1,21 @@ +""" +Toggles for course goals +""" + +from edx_toggles.toggles import LegacyWaffleFlagNamespace + +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag + +WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='course_goals') + +# .. toggle_name: course_goals.number_of_days_goals +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: This flag enables the use of the new version of course goals where users +# .. set a goal for the number of days they want to learn +# .. toggle_warnings: None +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2021-07-27 +# .. toggle_target_removal_date: 2021-09-01 +# .. toggle_tickets: https://openedx.atlassian.net/browse/AA-859 +COURSE_GOALS_NUMBER_OF_DAYS_GOALS = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'number_of_days_goals', __name__) diff --git a/lms/djangoapps/course_goals/views.py b/lms/djangoapps/course_goals/views.py index e99e633a2a..fceffc8fe8 100644 --- a/lms/djangoapps/course_goals/views.py +++ b/lms/djangoapps/course_goals/views.py @@ -53,6 +53,7 @@ class CourseGoalViewSet(viewsets.ModelViewSet): serializer_class = CourseGoalSerializer # Another version of this endpoint exists in ../course_home_api/outline/v1/views.py + # This version is used by the legacy frontend and is deprecated def create(self, post_data): # lint-amnesty, pylint: disable=arguments-differ """ Create a new goal if one does not exist, otherwise update the existing goal. """ # Ensure goal_key is valid diff --git a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py index ec164d28b0..1edfafa962 100644 --- a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py @@ -18,6 +18,7 @@ from common.djangoapps.student.roles import CourseInstructorRole from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests from lms.djangoapps.course_home_api.toggles import COURSE_HOME_USE_LEGACY_FRONTEND +from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from openedx.core.djangoapps.content.learning_sequences.api import replace_course_outline from openedx.core.djangoapps.content.learning_sequences.data import CourseOutlineData, CourseVisibility from openedx.core.djangoapps.content.learning_sequences.toggles import USE_FOR_OUTLINES @@ -219,7 +220,7 @@ class OutlineTabTestViews(BaseCourseHomeTests): assert response.data['access_expiration']['expiration_date'] == deadline @override_waffle_flag(ENABLE_COURSE_GOALS, active=True) - def test_post_course_goal(self): + def test_post_course_goal_deprecated(self): CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) post_data = { @@ -237,6 +238,33 @@ class OutlineTabTestViews(BaseCourseHomeTests): assert selected_goal is not None assert selected_goal['key'] == 'certify' + @override_waffle_flag(ENABLE_COURSE_GOALS, active=True) + @override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) + def test_post_course_goal(self): + """ Test that the api returns the correct response when saving a goal """ + CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) + + post_data = { + 'course_id': self.course.id, + 'number_of_days_with_visits_per_week_goal': 1, + 'subscribed_to_goal_reminders': True, + } + post_course_goal_response = self.client.post(reverse('course-home-save-course-goal'), post_data) + assert post_course_goal_response.status_code == 200 + + response = self.client.get(self.url) + assert response.status_code == 200 + + course_goals = response.json()['course_goals'] + expected_course_goals = { + 'goal_options': [1, 3, 5], + 'selected_goal': { + 'number_of_days_with_visits_per_week_goal': 1, + 'subscribed_to_goal_reminders': True + } + } + assert course_goals == expected_course_goals + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_SPECIAL_EXAMS': True}) @patch('lms.djangoapps.course_api.blocks.transformers.milestones.get_attempt_status_summary') def test_proctored_exam(self, mock_summary): diff --git a/lms/djangoapps/course_home_api/outline/v1/views.py b/lms/djangoapps/course_home_api/outline/v1/views.py index 697461a001..acc5c64e52 100644 --- a/lms/djangoapps/course_home_api/outline/v1/views.py +++ b/lms/djangoapps/course_home_api/outline/v1/views.py @@ -6,6 +6,7 @@ from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block +from django.conf import settings from django.http.response import Http404 from django.urls import reverse from django.utils.translation import gettext as _ @@ -29,6 +30,8 @@ from lms.djangoapps.course_goals.api import ( has_course_goal_permission, valid_course_goals_ordered ) +from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS +from lms.djangoapps.course_goals.models import NUMBER_OF_DAYS_OPTIONS from lms.djangoapps.course_home_api.outline.v1.serializers import OutlineTabSerializer from lms.djangoapps.course_home_api.toggles import ( course_home_legacy_is_active, @@ -46,7 +49,7 @@ from openedx.core.djangoapps.content.learning_sequences.api import ( from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.features.course_duration_limits.access import get_access_expiration_data -from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG +from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, ENABLE_COURSE_GOALS from openedx.features.course_experience.course_tools import CourseToolsPluginManager from openedx.features.course_experience.course_updates import ( dismiss_current_update_for_user, @@ -105,10 +108,10 @@ class OutlineTabView(RetrieveAPIView): resume_block: (bool) Whether the block is the resume block has_scheduled_content: (bool) Whether the block has more content scheduled for the future course_goals: - goal_options: (list) A list of goals where each goal is represented as a tuple (goal_key, goal_string) + goal_options: (list) A list of number of days options for the number of days goal selected_goal: - key: (str) The unique id given to the user's selected goal. - text: (str) The display text for the user's selected goal. + number_of_days_with_visits_per_week_goal: (int) The number of days the learner wants to learn per week + subscribed_to_goal_reminders: (bool) Whether the learner wants email reminders about their goal course_tools: List of serialized Course Tool objects. Each serialization has the following fields: analytics_id: (str) The unique id given to the tool. title: (str) The display title of the tool. @@ -246,23 +249,39 @@ class OutlineTabView(RetrieveAPIView): access_expiration = get_access_expiration_data(request.user, course_overview) cert_data = get_cert_data(request.user, course, enrollment.mode) if is_enrolled else None - # Only show the set course goal message for enrolled, unverified - # users in a course that allows for verified statuses. - is_already_verified = CourseEnrollment.is_enrolled_as_verified(request.user, course_key) - if not is_already_verified and has_course_goal_permission(request, course_key_string, - {'is_enrolled': is_enrolled}): - course_goals = { - 'goal_options': valid_course_goals_ordered(include_unsure=True), - 'selected_goal': None - } + if COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(): + if (is_enrolled and ENABLE_COURSE_GOALS.is_enabled(course_key) + and settings.FEATURES.get('ENABLE_COURSE_GOALS')): - selected_goal = get_course_goal(request.user, course_key) - if selected_goal: - course_goals['selected_goal'] = { - 'key': selected_goal.goal_key, - 'text': get_course_goal_text(selected_goal.goal_key), + course_goals = { + 'goal_options': NUMBER_OF_DAYS_OPTIONS, + 'selected_goal': None } + selected_goal = get_course_goal(request.user, course_key) + if selected_goal: + course_goals['selected_goal'] = { + 'number_of_days_with_visits_per_week_goal': selected_goal.number_of_days_with_visits_per_week_goal, + 'subscribed_to_goal_reminders': selected_goal.subscribed_to_goal_reminders, + } + else: + # Only show the set course goal message for enrolled, unverified + # users in a course that allows for verified statuses. + is_already_verified = CourseEnrollment.is_enrolled_as_verified(request.user, course_key) + if not is_already_verified and has_course_goal_permission(request, course_key_string, + {'is_enrolled': is_enrolled}): + course_goals = { + 'goal_options': valid_course_goals_ordered(include_unsure=True), + 'selected_goal': None + } + + selected_goal = get_course_goal(request.user, course_key) + if selected_goal: + course_goals['selected_goal'] = { + 'key': selected_goal.goal_key, + 'text': get_course_goal_text(selected_goal.goal_key), + } + try: resume_block = get_key_to_last_completed_block(request.user, course.id) resume_course['has_visited_course'] = True @@ -392,17 +411,40 @@ def dismiss_welcome_message(request): def save_course_goal(request): course_id = request.data.get('course_id', None) goal_key = request.data.get('goal_key', None) + number_of_days_with_visits_per_week_goal = request.data.get('number_of_days_with_visits_per_week_goal', None) + subscribed_to_goal_reminders = request.data.get('subscribed_to_goal_reminders', None) # If body doesn't contain 'course_id', return 400 to client. if not course_id: raise ParseError(_("'course_id' is required.")) - # If body doesn't contain 'goal', return 400 to client. - if not goal_key: - raise ParseError(_("'goal_key' is required.")) + if COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(): + # If body doesn't contain the required goals fields, return 400 to client. + if number_of_days_with_visits_per_week_goal is None or subscribed_to_goal_reminders is None: + raise ParseError(_("'number_of_days_with_visits_per_week_goal' and 'subscribed_to_goal_reminders' are required.")) + + try: + number_of_days_with_visits_per_week_goal = int(number_of_days_with_visits_per_week_goal) + except ValueError: + raise ParseError(_("'number_of_days_with_visits_per_week_goal' needs to be an integer")) + subscribed_to_goal_reminders = subscribed_to_goal_reminders == 'True' + + kwargs = { + "number_of_days_with_visits_per_week_goal": number_of_days_with_visits_per_week_goal, + "subscribed_to_goal_reminders": subscribed_to_goal_reminders, + } + + else: + # If body doesn't contain 'goal', return 400 to client. + if not goal_key: + raise ParseError(_("'goal_key' is required.")) + + kwargs = { + "goal_key": goal_key, + } try: - add_course_goal(request.user, course_id, goal_key) + add_course_goal(request.user, course_id, **kwargs) return Response({ 'header': _('Your course goal has been successfully set.'), 'message': _('Course goal updated successfully.'),