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

address comments

Browse Source
This commit is contained in:
Matthew Piatetsky
2021-08-02 12:51:00 -04:00
parent 986261a8d0
commit 8183a5ae64
11 changed files with 113 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
lms/djangoapps/course_goals/api.py
View File

@@ -3,7 +3,6 @@ Course Goals Python API
"""
from django.conf import settings
from opaque_keys.edx.keys import CourseKey
from rest_framework.reverse import reverse
@@ -12,9 +11,26 @@ 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=None,
number_of_days_with_visits_per_week_goal=None,
subscribed_to_goal_reminders=None):
def add_course_goal_deprecated(user, course_id, goal_key):
"""
Add a new course goal for the provided user and course. If the goal
already exists, simply update and save the goal.
This method is for the deprecated version of course goals and will be removed as soon
as the newer number of days version of course goals is fully implemented.
Arguments:
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.
"""
course_key = CourseKey.from_string(str(course_id))
CourseGoal.objects.update_or_create(
user=user, course_key=course_key, defaults={'goal_key': goal_key}
)
def add_course_goal(user, course_id, days_per_week,
subscribed_to_reminders):
"""
Add a new course goal for the provided user and course. If the goal
already exists, simply update and save the goal.
@@ -22,22 +38,16 @@ def add_course_goal(user, course_id, goal_key=None,
Arguments:
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
days_per_week (int): number of days learner wants to learn per week
subscribed_to_reminders (bool): whether the learner wants to receive email reminders about their goal
"""
course_key = CourseKey.from_string(str(course_id))
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,
}
)
CourseGoal.objects.update_or_create(
user=user, course_key=course_key, defaults={
'days_per_week': days_per_week,
'subscribed_to_reminders': subscribed_to_reminders,
}
)
def get_course_goal(user, course_key):
@@ -68,8 +78,7 @@ def has_course_goal_permission(request, course_id, user_access):
"""
course_key = CourseKey.from_string(course_id)
has_verified_mode = CourseMode.has_verified_mode(CourseMode.modes_for_course_dict(course_key))
return user_access['is_enrolled'] and has_verified_mode and ENABLE_COURSE_GOALS.is_enabled(course_key) \
and settings.FEATURES.get('ENABLE_COURSE_GOALS')
return user_access['is_enrolled'] and has_verified_mode and ENABLE_COURSE_GOALS.is_enabled(course_key)
def get_course_goal_options():

4
lms/djangoapps/course_goals/handlers.py
View File

@@ -18,8 +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,
'days_per_week': instance.days_per_week,
'subscribed_to_reminders': instance.subscribed_to_reminders,
}
tracker.emit(name, properties)
segment.track(instance.user.id, name, properties)

10
lms/djangoapps/course_goals/migrations/0004_auto_20210726_1338.py → lms/djangoapps/course_goals/migrations/0004_auto_20210806_0137.py
View File

@@ -1,4 +1,4 @@
# Generated by Django 2.2.24 on 2021-07-26 13:38
# Generated by Django 2.2.24 on 2021-08-06 01:37
from django.db import migrations, models
@@ -12,22 +12,22 @@ class Migration(migrations.Migration):
operations = [
migrations.AddField(
model_name='coursegoal',
name='number_of_days_with_visits_per_week_goal',
name='days_per_week',
field=models.PositiveIntegerField(default=0),
),
migrations.AddField(
model_name='coursegoal',
name='subscribed_to_goal_reminders',
name='subscribed_to_reminders',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='historicalcoursegoal',
name='number_of_days_with_visits_per_week_goal',
name='days_per_week',
field=models.PositiveIntegerField(default=0),
),
migrations.AddField(
model_name='historicalcoursegoal',
name='subscribed_to_goal_reminders',
name='subscribed_to_reminders',
field=models.BooleanField(default=False),
),
]

8
lms/djangoapps/course_goals/models.py
View File

@@ -18,8 +18,6 @@ GOAL_KEY_CHOICES = Choices(
('unsure', _('Not sure yet')),
)
NUMBER_OF_DAYS_OPTIONS = [1, 3, 5]
User = get_user_model()
@@ -36,15 +34,15 @@ 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)
days_per_week = 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)
subscribed_to_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} days per week for course {course}'.format(
user=self.user.username,
goal=self.number_of_days_with_visits_per_week_goal,
goal=self.days_per_week,
course=self.course_key,
)

16
lms/djangoapps/course_goals/tests/test_api.py
View File

@@ -48,8 +48,8 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase):
segment_call.assert_called_once_with(self.user.id, EVENT_NAME_ADDED, {
'courserun_key': str(self.course.id),
'goal_key': 'certify',
'number_of_days_with_visits_per_week_goal': 0,
'subscribed_to_goal_reminders': False,
'days_per_week': 0,
'subscribed_to_reminders': False,
})
assert response.status_code == 201
@@ -83,18 +83,18 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase):
segment_call.assert_any_call(self.user.id, EVENT_NAME_ADDED, {
'courserun_key': str(self.course.id), 'goal_key': 'explore',
'number_of_days_with_visits_per_week_goal': 0,
'subscribed_to_goal_reminders': False,
'days_per_week': 0,
'subscribed_to_reminders': False,
})
segment_call.assert_any_call(self.user.id, EVENT_NAME_UPDATED, {
'courserun_key': str(self.course.id), 'goal_key': 'certify',
'number_of_days_with_visits_per_week_goal': 0,
'subscribed_to_goal_reminders': False,
'days_per_week': 0,
'subscribed_to_reminders': False,
})
segment_call.assert_any_call(self.user.id, EVENT_NAME_UPDATED, {
'courserun_key': str(self.course.id), 'goal_key': 'unsure',
'number_of_days_with_visits_per_week_goal': 0,
'subscribed_to_goal_reminders': False,
'days_per_week': 0,
'subscribed_to_reminders': False,
})
current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id)
assert len(current_goals) == 1

35
lms/djangoapps/course_goals/tests/test_goals.py
View File

@@ -2,6 +2,7 @@
Unit tests for course_goals djangoapp
"""
import json
from unittest import mock
from django.contrib.auth import get_user_model
@@ -49,15 +50,15 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase):
Sends a post request to set a course goal and returns the response.
"""
post_data = {
'course_id': self.course.id,
'course_id': str(self.course.id),
'user': self.user.username,
}
if number is not None:
post_data['number_of_days_with_visits_per_week_goal'] = number
post_data['days_per_week'] = number
if subscribed is not None:
post_data['subscribed_to_goal_reminders'] = subscribed
post_data['subscribed_to_reminders'] = subscribed
response = self.client.post(self.apiUrl, post_data)
response = self.client.post(self.apiUrl, json.dumps(post_data), content_type='application/json')
return response
@mock.patch('lms.djangoapps.course_goals.handlers.segment.track')
@@ -67,15 +68,15 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase):
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,
'days_per_week': 1,
'subscribed_to_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
assert current_goals[0].days_per_week == 1
assert current_goals[0].subscribed_to_reminders is True
@mock.patch('lms.djangoapps.course_goals.handlers.segment.track')
@override_settings(LMS_SEGMENT_KEY="foobar")
@@ -84,31 +85,31 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase):
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,
'days_per_week': 1,
'subscribed_to_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,
'days_per_week': 3,
'subscribed_to_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,
'days_per_week': 5,
'subscribed_to_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
assert current_goals[0].days_per_week == 5
assert current_goals[0].subscribed_to_reminders is False
def test_add_without_required_arguments(self):
""" Ensures if required arguments are not provided, post does not succeed. """
@@ -116,7 +117,7 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase):
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.",
text="'days_per_week' and 'subscribed_to_reminders' are required.",
status_code=400
)

2
lms/djangoapps/course_home_api/outline/v1/serializers.py
View File

@@ -66,7 +66,7 @@ class CourseGoalsSerializer(serializers.Serializer):
"""
Serializer for Course Goal data
"""
goal_options = serializers.ListField()
goal_options = serializers.ListField(default=[])
selected_goal = serializers.DictField()

23
lms/djangoapps/course_home_api/outline/v1/tests/test_views.py
View File

@@ -8,6 +8,7 @@ from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
from unittest.mock import Mock, patch
import ddt
import json
from django.conf import settings
from django.urls import reverse
from edx_toggles.toggles.testutils import override_waffle_flag
@@ -244,12 +245,16 @@ class OutlineTabTestViews(BaseCourseHomeTests):
""" 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)
post_data = json.dumps({
'course_id': str(self.course.id),
'days_per_week': 1,
'subscribed_to_reminders': True,
})
post_course_goal_response = self.client.post(
reverse('course-home-save-course-goal'),
post_data,
content_type='application/json',
)
assert post_course_goal_response.status_code == 200
response = self.client.get(self.url)
@@ -257,10 +262,10 @@ class OutlineTabTestViews(BaseCourseHomeTests):
course_goals = response.json()['course_goals']
expected_course_goals = {
'goal_options': [1, 3, 5],
'goal_options': [],
'selected_goal': {
'number_of_days_with_visits_per_week_goal': 1,
'subscribed_to_goal_reminders': True
'days_per_week': 1,
'subscribed_to_reminders': True
}
}
assert course_goals == expected_course_goals

68
lms/djangoapps/course_home_api/outline/v1/views.py
View File

@@ -22,16 +22,17 @@ from rest_framework.response import Response
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.util.json_request import expect_json
from common.djangoapps.util.views import expose_header
from lms.djangoapps.course_goals.api import (
add_course_goal,
add_course_goal_deprecated,
get_course_goal,
get_course_goal_text,
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,
@@ -108,10 +109,9 @@ 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 number of days options for the number of days goal
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
days_per_week: (int) The number of days the learner wants to learn per week
subscribed_to_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.
@@ -250,19 +250,17 @@ class OutlineTabView(RetrieveAPIView):
cert_data = get_cert_data(request.user, course, enrollment.mode) if is_enrolled else 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')):
if (is_enrolled and ENABLE_COURSE_GOALS.is_enabled(course_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,
'days_per_week': selected_goal.days_per_week,
'subscribed_to_reminders': selected_goal.subscribed_to_reminders,
}
else:
# Only show the set course goal message for enrolled, unverified
@@ -409,45 +407,39 @@ def dismiss_welcome_message(request):
@authentication_classes((JwtAuthentication, SessionAuthenticationAllowInactiveUser,))
@permission_classes((IsAuthenticated,))
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)
course_id = request.data.get('course_id')
goal_key = request.data.get('goal_key')
days_per_week = request.data.get('days_per_week')
subscribed_to_reminders = request.data.get('subscribed_to_reminders')
# If body doesn't contain 'course_id', return 400 to client.
if not course_id:
raise ParseError(_("'course_id' is required."))
raise ParseError("'course_id' 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."))
if days_per_week is None or subscribed_to_reminders is None:
raise ParseError("'days_per_week' and 'subscribed_to_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,
}
add_course_goal(request.user, course_id, days_per_week, subscribed_to_reminders)
return Response({
'header': _('Your course goal has been successfully set.'),
'message': _('Course goal updated successfully.'),
})
except Exception:
raise UnableToSaveCourseGoal
else:
# If body doesn't contain 'goal', return 400 to client.
if not goal_key:
raise ParseError(_("'goal_key' is required."))
raise ParseError("'goal_key' is required.")
kwargs = {
"goal_key": goal_key,
}
try:
add_course_goal(request.user, course_id, **kwargs)
return Response({
'header': _('Your course goal has been successfully set.'),
'message': _('Course goal updated successfully.'),
})
except Exception:
raise UnableToSaveCourseGoal
try:
add_course_goal_deprecated(request.user, course_id, goal_key)
return Response({
'header': _('Your course goal has been successfully set.'),
'message': _('Course goal updated successfully.'),
})
except Exception:
raise UnableToSaveCourseGoal

3
lms/envs/common.py
View File

@@ -801,9 +801,6 @@ FEATURES = {
# .. toggle_tickets: https://github.com/edx/edx-platform/pull/15006
'ENABLE_BULK_ENROLLMENT_VIEW': False,
# Whether course goals is enabled.
'ENABLE_COURSE_GOALS': True,
# Set to enable Enterprise integration
'ENABLE_ENTERPRISE_INTEGRATION': False,

6
openedx/features/course_experience/tests/views/test_course_home.py
View File

@@ -27,7 +27,7 @@ from common.djangoapps.student.tests.factories import OrgStaffFactory
from common.djangoapps.student.tests.factories import StaffFactory
from lms.djangoapps.commerce.models import CommerceConfiguration
from lms.djangoapps.commerce.utils import EcommerceService
from lms.djangoapps.course_goals.api import add_course_goal, get_course_goal
from lms.djangoapps.course_goals.api import add_course_goal_deprecated, get_course_goal
from lms.djangoapps.course_home_api.toggles import COURSE_HOME_USE_LEGACY_FRONTEND
from lms.djangoapps.courseware.tests.helpers import get_expiration_banner_text
from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
@@ -755,7 +755,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
self.assertContains(response, TEST_COURSE_GOAL_OPTIONS)
# Verify that enrolled users that have set a course goal are not shown the set course goal message.
add_course_goal(user, verifiable_course.id, COURSE_GOAL_DISMISS_OPTION)
add_course_goal_deprecated(user, verifiable_course.id, COURSE_GOAL_DISMISS_OPTION)
response = self.client.get(course_home_url(verifiable_course))
self.assertNotContains(response, TEST_COURSE_GOAL_OPTIONS)
@@ -797,7 +797,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
self.assertContains(response, TEST_COURSE_GOAL_UPDATE_FIELD_HIDDEN)
# Verify that enrolled users that have set a course goal are shown a visible update goal selection field.
add_course_goal(user, verifiable_course.id, COURSE_GOAL_DISMISS_OPTION)
add_course_goal_deprecated(user, verifiable_course.id, COURSE_GOAL_DISMISS_OPTION)
response = self.client.get(course_home_url(verifiable_course))
self.assertContains(response, TEST_COURSE_GOAL_UPDATE_FIELD)
self.assertNotContains(response, TEST_COURSE_GOAL_UPDATE_FIELD_HIDDEN)