diff --git a/lms/djangoapps/course_goals/admin.py b/lms/djangoapps/course_goals/admin.py new file mode 100644 index 0000000000..b7c1384b1a --- /dev/null +++ b/lms/djangoapps/course_goals/admin.py @@ -0,0 +1,29 @@ +"""Django admin for course_goals""" + +from django.contrib import admin + +from lms.djangoapps.course_goals.models import CourseGoal, UserActivity + + +@admin.register(CourseGoal) +class CourseGoalAdmin(admin.ModelAdmin): + """Admin for CourseGoal""" + list_display = ('id', + 'user', + 'course_key', + 'days_per_week', + 'subscribed_to_reminders') + raw_id_fields = ('user',) + search_fields = ('user__username', 'course_key') + + +@admin.register(UserActivity) +class UserActivityAdmin(admin.ModelAdmin): + """Admin for UserActivity""" + + list_display = ('id', + 'user', + 'course_key', + 'date') + raw_id_fields = ('user',) + search_fields = ('user__username', 'course_key') diff --git a/lms/djangoapps/course_goals/apps.py b/lms/djangoapps/course_goals/apps.py index fb41462d11..fd5b83becf 100644 --- a/lms/djangoapps/course_goals/apps.py +++ b/lms/djangoapps/course_goals/apps.py @@ -13,6 +13,7 @@ class CourseGoalsConfig(AppConfig): Application Configuration for Course Goals. """ name = 'lms.djangoapps.course_goals' + verbose_name = 'Course Goals' def ready(self): """ diff --git a/lms/djangoapps/course_goals/migrations/0006_add_unsubscribe_token.py b/lms/djangoapps/course_goals/migrations/0006_add_unsubscribe_token.py new file mode 100644 index 0000000000..78fe4c4ad9 --- /dev/null +++ b/lms/djangoapps/course_goals/migrations/0006_add_unsubscribe_token.py @@ -0,0 +1,27 @@ +# Generated by Django 2.2.24 on 2021-08-20 19:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_goals', '0005_useractivity'), + ] + + operations = [ + migrations.AlterModelOptions( + name='useractivity', + options={'verbose_name_plural': 'User activities'}, + ), + migrations.AddField( + model_name='coursegoal', + name='unsubscribe_token', + field=models.UUIDField(blank=True, help_text='Used to validate unsubscribe requests without requiring a login', null=True, unique=True, editable=False), + ), + migrations.AddField( + model_name='historicalcoursegoal', + name='unsubscribe_token', + field=models.UUIDField(blank=True, db_index=True, help_text='Used to validate unsubscribe requests without requiring a login', null=True, editable=False), + ), + ] diff --git a/lms/djangoapps/course_goals/migrations/0007_set_unsubscribe_token_default.py b/lms/djangoapps/course_goals/migrations/0007_set_unsubscribe_token_default.py new file mode 100644 index 0000000000..6704d96e7f --- /dev/null +++ b/lms/djangoapps/course_goals/migrations/0007_set_unsubscribe_token_default.py @@ -0,0 +1,24 @@ +# Generated by Django 2.2.24 on 2021-08-20 19:18 + +from django.db import migrations, models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_goals', '0006_add_unsubscribe_token'), + ] + + operations = [ + migrations.AlterField( + model_name='coursegoal', + name='unsubscribe_token', + field=models.UUIDField(blank=True, default=uuid.uuid4, editable=False, help_text='Used to validate unsubscribe requests without requiring a login', null=True, unique=True), + ), + migrations.AlterField( + model_name='historicalcoursegoal', + name='unsubscribe_token', + field=models.UUIDField(blank=True, db_index=True, default=uuid.uuid4, editable=False, help_text='Used to validate unsubscribe requests without requiring a login', null=True), + ), + ] diff --git a/lms/djangoapps/course_goals/models.py b/lms/djangoapps/course_goals/models.py index c2347f7d8c..e44eec2cd8 100644 --- a/lms/djangoapps/course_goals/models.py +++ b/lms/djangoapps/course_goals/models.py @@ -2,6 +2,7 @@ Course Goals Models """ +import uuid from django.contrib.auth import get_user_model from django.db import models @@ -35,8 +36,17 @@ class CourseGoal(models.Model): 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 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_reminders = models.BooleanField(default=False) + + # With this token, anyone can unsubscribe this user from reminders. That's a mild enough action that we don't stress + # about the risk of keeping this key around long term in the database or bother using a higher-security generator + # than uuid4. The worst someone can do with this is unsubscribe us. And we want old tokens sitting in folks' email + # inboxes to still be valid as long as possible. + unsubscribe_token = models.UUIDField(null=True, blank=True, unique=True, editable=False, default=uuid.uuid4, + help_text='Used to validate unsubscribe requests without requiring a login') + goal_key = models.CharField(max_length=100, choices=GOAL_KEY_CHOICES, default=GOAL_KEY_CHOICES.unsure) history = HistoricalRecords() @@ -47,6 +57,12 @@ class CourseGoal(models.Model): course=self.course_key, ) + def save(self, **kwargs): # pylint: disable=arguments-differ + # Ensure we have an unsubscribe token (lazy migration from old goals, before this field was added) + if self.unsubscribe_token is None: + self.unsubscribe_token = uuid.uuid4() + super().save(**kwargs) + class UserActivity(models.Model): """ @@ -62,6 +78,7 @@ class UserActivity(models.Model): class Meta: constraints = [models.UniqueConstraint(fields=['user', 'course_key', 'date'], name='unique_user_course_date')] indexes = [models.Index(fields=['user', 'course_key'], name='user_course_index')] + verbose_name_plural = 'User activities' id = models.BigAutoField(primary_key=True) user = models.ForeignKey(User, on_delete=models.CASCADE) diff --git a/lms/djangoapps/course_goals/views.py b/lms/djangoapps/course_goals/views.py index fceffc8fe8..a2992a64c5 100644 --- a/lms/djangoapps/course_goals/views.py +++ b/lms/djangoapps/course_goals/views.py @@ -52,7 +52,7 @@ class CourseGoalViewSet(viewsets.ModelViewSet): queryset = CourseGoal.objects.all() serializer_class = CourseGoalSerializer - # Another version of this endpoint exists in ../course_home_api/outline/v1/views.py + # Another version of this endpoint exists in ../course_home_api/outline/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. """ diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/serializers.py b/lms/djangoapps/course_home_api/course_metadata/serializers.py similarity index 90% rename from lms/djangoapps/course_home_api/course_metadata/v1/serializers.py rename to lms/djangoapps/course_home_api/course_metadata/serializers.py index de793b34f0..e000f0579c 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/serializers.py +++ b/lms/djangoapps/course_home_api/course_metadata/serializers.py @@ -9,7 +9,7 @@ from django.urls import reverse from django.utils.translation import ugettext as _ from rest_framework import serializers -from lms.djangoapps.course_home_api.mixins import VerifiedModeSerializerMixin +from lms.djangoapps.course_home_api.serializers import VerifiedModeSerializer class CourseTabSerializer(serializers.Serializer): @@ -29,7 +29,7 @@ class CourseTabSerializer(serializers.Serializer): return request.build_absolute_uri(tab.link_func(self.context.get('course'), reverse)) -class CourseHomeMetadataSerializer(VerifiedModeSerializerMixin, serializers.Serializer): +class CourseHomeMetadataSerializer(VerifiedModeSerializer): """ Serializer for the Course Home Course Metadata """ diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/__init__.py b/lms/djangoapps/course_home_api/course_metadata/tests/__init__.py similarity index 100% rename from lms/djangoapps/course_home_api/course_metadata/v1/__init__.py rename to lms/djangoapps/course_home_api/course_metadata/tests/__init__.py diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py similarity index 97% rename from lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py rename to lms/djangoapps/course_home_api/course_metadata/tests/test_views.py index 06f4455f07..6ffa158529 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py @@ -29,7 +29,7 @@ class CourseHomeMetadataTests(BaseCourseHomeTests): """ def setUp(self): super().setUp() - self.url = reverse('course-home-course-metadata', args=[self.course.id]) + self.url = reverse('course-home:course-metadata', args=[self.course.id]) self.staff_user = UserFactory( username='staff', email='staff@example.com', @@ -79,7 +79,7 @@ class CourseHomeMetadataTests(BaseCourseHomeTests): assert self.client.get(self.url).data['username'] == self.user.username def test_get_unknown_course(self): - url = reverse('course-home-course-metadata', args=['course-v1:unknown+course+2T2020']) + url = reverse('course-home:course-metadata', args=['course-v1:unknown+course+2T2020']) response = self.client.get(url) assert response.status_code == 404 diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/views.py b/lms/djangoapps/course_home_api/course_metadata/views.py similarity index 98% rename from lms/djangoapps/course_home_api/course_metadata/v1/views.py rename to lms/djangoapps/course_home_api/course_metadata/views.py index b069f73b63..07b121f830 100644 --- a/lms/djangoapps/course_home_api/course_metadata/v1/views.py +++ b/lms/djangoapps/course_home_api/course_metadata/views.py @@ -13,7 +13,7 @@ from openedx.core.djangoapps.courseware_api.utils import get_celebrations_dict from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_api.api import course_detail -from lms.djangoapps.course_home_api.course_metadata.v1.serializers import CourseHomeMetadataSerializer +from lms.djangoapps.course_home_api.course_metadata.serializers import CourseHomeMetadataSerializer from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.courses import check_course_access from lms.djangoapps.courseware.masquerade import setup_masquerade diff --git a/lms/djangoapps/course_home_api/dates/v1/serializers.py b/lms/djangoapps/course_home_api/dates/serializers.py similarity index 92% rename from lms/djangoapps/course_home_api/dates/v1/serializers.py rename to lms/djangoapps/course_home_api/dates/serializers.py index 0229c54009..b4cd85e887 100644 --- a/lms/djangoapps/course_home_api/dates/v1/serializers.py +++ b/lms/djangoapps/course_home_api/dates/serializers.py @@ -6,7 +6,7 @@ Dates Tab Serializers. Represents the relevant dates for a Course. from rest_framework import serializers -from lms.djangoapps.course_home_api.mixins import DatesBannerSerializerMixin +from lms.djangoapps.course_home_api.serializers import DatesBannerSerializer from lms.djangoapps.courseware.date_summary import VerificationDeadlineDate @@ -44,7 +44,7 @@ class DateSummarySerializer(serializers.Serializer): return getattr(block, 'first_component_block_id', '') -class DatesTabSerializer(DatesBannerSerializerMixin, serializers.Serializer): +class DatesTabSerializer(DatesBannerSerializer): """ Serializer for the Dates Tab """ diff --git a/lms/djangoapps/course_home_api/course_metadata/v1/tests/__init__.py b/lms/djangoapps/course_home_api/dates/tests/__init__.py similarity index 100% rename from lms/djangoapps/course_home_api/course_metadata/v1/tests/__init__.py rename to lms/djangoapps/course_home_api/dates/tests/__init__.py diff --git a/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py b/lms/djangoapps/course_home_api/dates/tests/test_views.py similarity index 95% rename from lms/djangoapps/course_home_api/dates/v1/tests/test_views.py rename to lms/djangoapps/course_home_api/dates/tests/test_views.py index 780859a6d5..0e37fbcaaf 100644 --- a/lms/djangoapps/course_home_api/dates/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/dates/tests/test_views.py @@ -20,7 +20,7 @@ class DatesTabTestViews(BaseCourseHomeTests): """ def setUp(self): super().setUp() - self.url = reverse('course-home-dates-tab', args=[self.course.id]) + self.url = reverse('course-home:dates-tab', args=[self.course.id]) ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2017, 1, 1)) @ddt.data(CourseMode.AUDIT, CourseMode.VERIFIED) @@ -49,7 +49,7 @@ class DatesTabTestViews(BaseCourseHomeTests): assert response.status_code == 401 def test_get_unknown_course(self): - url = reverse('course-home-dates-tab', args=['course-v1:unknown+course+2T2020']) + url = reverse('course-home:dates-tab', args=['course-v1:unknown+course+2T2020']) response = self.client.get(url) assert response.status_code == 404 diff --git a/lms/djangoapps/course_home_api/dates/v1/views.py b/lms/djangoapps/course_home_api/dates/views.py similarity index 98% rename from lms/djangoapps/course_home_api/dates/v1/views.py rename to lms/djangoapps/course_home_api/dates/views.py index 4cb42db73a..994ef4cb6b 100644 --- a/lms/djangoapps/course_home_api/dates/v1/views.py +++ b/lms/djangoapps/course_home_api/dates/views.py @@ -12,7 +12,7 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from common.djangoapps.student.models import CourseEnrollment -from lms.djangoapps.course_home_api.dates.v1.serializers import DatesTabSerializer +from lms.djangoapps.course_home_api.dates.serializers import DatesTabSerializer from lms.djangoapps.course_home_api.toggles import course_home_legacy_is_active from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs diff --git a/lms/djangoapps/course_home_api/outline/v1/serializers.py b/lms/djangoapps/course_home_api/outline/serializers.py similarity index 91% rename from lms/djangoapps/course_home_api/outline/v1/serializers.py rename to lms/djangoapps/course_home_api/outline/serializers.py index 1cab4cd27f..43ea7ef7ea 100644 --- a/lms/djangoapps/course_home_api/outline/v1/serializers.py +++ b/lms/djangoapps/course_home_api/outline/serializers.py @@ -5,9 +5,9 @@ Outline Tab Serializers. from django.utils.translation import ngettext from rest_framework import serializers -from lms.djangoapps.course_home_api.dates.v1.serializers import DateSummarySerializer -from lms.djangoapps.course_home_api.progress.v1.serializers import CertificateDataSerializer -from lms.djangoapps.course_home_api.mixins import DatesBannerSerializerMixin, VerifiedModeSerializerMixin +from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer +from lms.djangoapps.course_home_api.progress.serializers import CertificateDataSerializer +from lms.djangoapps.course_home_api.serializers import DatesBannerSerializer, VerifiedModeSerializer class CourseBlockSerializer(serializers.Serializer): @@ -110,7 +110,7 @@ class ResumeCourseSerializer(serializers.Serializer): url = serializers.URLField() -class OutlineTabSerializer(DatesBannerSerializerMixin, VerifiedModeSerializerMixin, serializers.Serializer): +class OutlineTabSerializer(DatesBannerSerializer, VerifiedModeSerializer): """ Serializer for the Outline Tab """ diff --git a/lms/djangoapps/course_home_api/dates/v1/__init__.py b/lms/djangoapps/course_home_api/outline/tests/__init__.py similarity index 100% rename from lms/djangoapps/course_home_api/dates/v1/__init__.py rename to lms/djangoapps/course_home_api/outline/tests/__init__.py diff --git a/lms/djangoapps/course_goals/tests/test_goals.py b/lms/djangoapps/course_home_api/outline/tests/test_goals.py similarity index 73% rename from lms/djangoapps/course_goals/tests/test_goals.py rename to lms/djangoapps/course_home_api/outline/tests/test_goals.py index e6b88c0b17..72d3be5fb2 100644 --- a/lms/djangoapps/course_goals/tests/test_goals.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_goals.py @@ -3,6 +3,7 @@ Unit tests for course_goals djangoapp """ import json +import uuid from unittest import mock from django.contrib.auth import get_user_model @@ -11,14 +12,16 @@ from django.urls import reverse from rest_framework.test import APIClient from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.tests.factories import UserFactory 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 lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from openedx.features.course_experience import ENABLE_COURSE_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' @@ -43,7 +46,7 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase): 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') + self.apiUrl = reverse('course-home:save-course-goal') def save_course_goal(self, number, subscribed): """ @@ -126,3 +129,42 @@ class TestCourseGoalsAPI(SharedModuleStoreTestCase): 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 + + +class TestUnsubscribeAPI(BaseCourseHomeTests): + """ + Testing the unsubscribe API. + """ + def unsubscribe(self, token): + url = reverse('course-home:unsubscribe-from-course-goal', kwargs={'token': token}) + return self.client.post(url) + + def make_goal(self, course_key, **kwargs) -> CourseGoal: + return CourseGoal.objects.create(user=self.user, course_key=course_key, **kwargs) + + def test_happy_path(self): + goal = self.make_goal(self.course.id, subscribed_to_reminders=True) + goal2 = self.make_goal('course-v1:foo+bar+2T2020', subscribed_to_reminders=True) # a control group + + def unsubscribe_and_check(): + response = self.unsubscribe(goal.unsubscribe_token) + goal.refresh_from_db() + goal2.refresh_from_db() + assert response.status_code == 200 + assert not goal.subscribed_to_reminders + assert goal2.subscribed_to_reminders + assert response.json() == {'course_title': self.course.display_name} + + unsubscribe_and_check() + + # Unsubscribe again to confirm that we're not like, toggling the subscription status or anything + unsubscribe_and_check() + + def test_bad_token(self): + response = self.unsubscribe(uuid.uuid4()) + assert response.status_code == 404 + + def test_bad_course(self): + goal = self.make_goal('course-v1:foo+bar+2T2020') + response = self.unsubscribe(goal.unsubscribe_token) + assert response.status_code == 404 diff --git a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py similarity index 96% rename from lms/djangoapps/course_home_api/outline/v1/tests/test_views.py rename to lms/djangoapps/course_home_api/outline/tests/test_view.py index a52609f3c7..6c787ffa08 100644 --- a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -45,7 +45,7 @@ class OutlineTabTestViews(BaseCourseHomeTests): def setUp(self): super().setUp() - self.url = reverse('course-home-outline-tab', args=[self.course.id]) + self.url = reverse('course-home:outline-tab', args=[self.course.id]) @override_waffle_flag(ENABLE_COURSE_GOALS, active=True) @ddt.data(CourseMode.AUDIT, CourseMode.VERIFIED) @@ -158,7 +158,7 @@ class OutlineTabTestViews(BaseCourseHomeTests): assert self.client.get(self.url).data['handouts_html'] == '

Hi

' def test_get_unknown_course(self): - url = reverse('course-home-outline-tab', args=['course-v1:unknown+course+2T2020']) + url = reverse('course-home:outline-tab', args=['course-v1:unknown+course+2T2020']) response = self.client.get(url) assert response.status_code == 404 @@ -228,7 +228,7 @@ class OutlineTabTestViews(BaseCourseHomeTests): 'course_id': self.course.id, 'goal_key': 'certify' } - post_course_goal_response = self.client.post(reverse('course-home-save-course-goal'), post_data) + 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) @@ -251,7 +251,7 @@ class OutlineTabTestViews(BaseCourseHomeTests): 'subscribed_to_reminders': True, }) post_course_goal_response = self.client.post( - reverse('course-home-save-course-goal'), + reverse('course-home:save-course-goal'), post_data, content_type='application/json', ) @@ -299,7 +299,7 @@ class OutlineTabTestViews(BaseCourseHomeTests): 'short_description': 'My Exam', 'suggested_icon': 'fa-foo-bar', } - url = reverse('course-home-outline-tab', args=[course.id]) + url = reverse('course-home:outline-tab', args=[course.id]) CourseEnrollment.enroll(self.user, course.id) response = self.client.get(url) @@ -323,7 +323,7 @@ class OutlineTabTestViews(BaseCourseHomeTests): sequential2 = ItemFactory.create(display_name='Ungraded', category='sequential', parent_location=chapter.location) ItemFactory.create(category='problem', parent_location=sequential2.location) - url = reverse('course-home-outline-tab', args=[course.id]) + url = reverse('course-home:outline-tab', args=[course.id]) CourseEnrollment.enroll(self.user, course.id) response = self.client.get(url) @@ -338,8 +338,8 @@ class OutlineTabTestViews(BaseCourseHomeTests): assert ungraded_data['icon'] is None @override_waffle_flag(COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, active=True) - @patch('lms.djangoapps.course_home_api.outline.v1.views.generate_offer_data', new=Mock(return_value={'a': 1})) - @patch('lms.djangoapps.course_home_api.outline.v1.views.get_access_expiration_data', new=Mock(return_value={'b': 1})) + @patch('lms.djangoapps.course_home_api.outline.views.generate_offer_data', new=Mock(return_value={'a': 1})) + @patch('lms.djangoapps.course_home_api.outline.views.get_access_expiration_data', new=Mock(return_value={'b': 1})) @ddt.data(*itertools.product([True, False], [True, False], [None, COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE])) @ddt.unpack diff --git a/lms/djangoapps/course_home_api/outline/v1/views.py b/lms/djangoapps/course_home_api/outline/views.py similarity index 93% rename from lms/djangoapps/course_home_api/outline/v1/views.py rename to lms/djangoapps/course_home_api/outline/views.py index f842275a37..d37ca1720f 100644 --- a/lms/djangoapps/course_home_api/outline/v1/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -6,8 +6,8 @@ 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.shortcuts import get_object_or_404 from django.urls import reverse from django.utils.translation import gettext as _ from edx_django_utils import monitoring as monitoring_utils @@ -22,7 +22,6 @@ 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, @@ -32,8 +31,9 @@ from lms.djangoapps.course_goals.api import ( has_course_goal_permission, valid_course_goals_ordered ) +from lms.djangoapps.course_goals.models import CourseGoal from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS -from lms.djangoapps.course_home_api.outline.v1.serializers import OutlineTabSerializer +from lms.djangoapps.course_home_api.outline.serializers import OutlineTabSerializer from lms.djangoapps.course_home_api.toggles import ( course_home_legacy_is_active, ) @@ -47,7 +47,7 @@ from openedx.core.djangoapps.content.learning_sequences.api import ( get_user_course_outline, public_api_available as learning_sequences_api_available, ) -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_404 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, ENABLE_COURSE_GOALS @@ -192,7 +192,7 @@ class OutlineTabView(RetrieveAPIView): user_is_masquerading = is_masquerading(request.user, course_key, course_masquerade=masquerade_object) - course_overview = CourseOverview.get_from_id(course_key) + course_overview = get_course_overview_or_404(course_key) enrollment = CourseEnrollment.get_enrollment(request.user, course_key) enrollment_mode = getattr(enrollment, 'mode', None) allow_anonymous = COURSE_ENABLE_UNENROLLED_ACCESS_FLAG.is_enabled(course_key) @@ -443,3 +443,34 @@ def save_course_goal(request): }) except Exception: raise UnableToSaveCourseGoal + + +@api_view(['POST']) +def unsubscribe_from_course_goal_by_token(request, token): + """ + API calls to unsubscribe from course goal reminders. + + Note that this does not require authentication - this view may be hit from an email on a different device than + normal or whatever. We should still be able to unsubscribe the user. Instead, we use a token in the email to + validate that they have permission to unsubscribe. + + This endpoint is very tightly scoped (only unsubscribe: no subscribing, no PII) because it is unauthenticated. + + **Example Requests** + POST api/course_home/v1/unsubscribe_from_course_goal/{token} + + **Example Response Data** + {'course_title': 'Cats & Dogs In Canadian Media'} + + Returns a 404 response if the token was not found. Otherwise, returns some basic course info. But no PII. + """ + # First update the goal + goal = get_object_or_404(CourseGoal, unsubscribe_token=token) + goal.subscribed_to_reminders = False + goal.save() + + # Now generate a response + course_overview = get_course_overview_or_404(goal.course_key) + return Response({ + 'course_title': course_overview.display_name, + }) diff --git a/lms/djangoapps/course_home_api/dates/v1/tests/__init__.py b/lms/djangoapps/course_home_api/progress/__init__.py similarity index 100% rename from lms/djangoapps/course_home_api/dates/v1/tests/__init__.py rename to lms/djangoapps/course_home_api/progress/__init__.py diff --git a/lms/djangoapps/course_home_api/progress/v1/serializers.py b/lms/djangoapps/course_home_api/progress/serializers.py similarity index 87% rename from lms/djangoapps/course_home_api/progress/v1/serializers.py rename to lms/djangoapps/course_home_api/progress/serializers.py index 718dd5386f..fd9f97682a 100644 --- a/lms/djangoapps/course_home_api/progress/v1/serializers.py +++ b/lms/djangoapps/course_home_api/progress/serializers.py @@ -7,10 +7,10 @@ from rest_framework import serializers from rest_framework.reverse import reverse from pytz import UTC -from lms.djangoapps.course_home_api.mixins import VerifiedModeSerializerMixin +from lms.djangoapps.course_home_api.serializers import ReadOnlySerializer, VerifiedModeSerializer -class CourseGradeSerializer(serializers.Serializer): +class CourseGradeSerializer(ReadOnlySerializer): """ Serializer for course grade """ @@ -19,7 +19,7 @@ class CourseGradeSerializer(serializers.Serializer): is_passing = serializers.BooleanField(source='passed') -class SubsectionScoresSerializer(serializers.Serializer): +class SubsectionScoresSerializer(ReadOnlySerializer): """ Serializer for subsections in section_scores """ @@ -40,6 +40,7 @@ class SubsectionScoresSerializer(serializers.Serializer): return str(subsection.location) def get_problem_scores(self, subsection): + """Problem scores for this subsection""" problem_scores = [ { 'earned': score.earned, @@ -54,7 +55,7 @@ class SubsectionScoresSerializer(serializers.Serializer): Returns the URL for the subsection while taking into account if the course team has marked the subsection's visibility as hide after due. """ - hide_url_date = (subsection.self_paced and subsection.end) or subsection.due + hide_url_date = subsection.end if subsection.self_paced else subsection.due if (not self.context['staff_access'] and subsection.hide_after_due and hide_url_date and datetime.now(UTC) > hide_url_date): return None @@ -71,7 +72,7 @@ class SubsectionScoresSerializer(serializers.Serializer): return not course_blocks.get_xblock_field(subsection.location, 'contains_gated_content', False) -class SectionScoresSerializer(serializers.Serializer): +class SectionScoresSerializer(ReadOnlySerializer): """ Serializer for sections in section_scores """ @@ -79,7 +80,7 @@ class SectionScoresSerializer(serializers.Serializer): subsections = SubsectionScoresSerializer(source='sections', many=True) -class GradingPolicySerializer(serializers.Serializer): +class GradingPolicySerializer(ReadOnlySerializer): """ Serializer for grading policy """ @@ -96,7 +97,7 @@ class GradingPolicySerializer(serializers.Serializer): } for assignment_policy in grading_policy['GRADER']] -class CertificateDataSerializer(serializers.Serializer): +class CertificateDataSerializer(ReadOnlySerializer): """ Serializer for certificate data """ @@ -106,7 +107,7 @@ class CertificateDataSerializer(serializers.Serializer): certificate_available_date = serializers.DateTimeField() -class VerificationDataSerializer(serializers.Serializer): +class VerificationDataSerializer(ReadOnlySerializer): """ Serializer for verification data object """ @@ -115,7 +116,7 @@ class VerificationDataSerializer(serializers.Serializer): status_date = serializers.DateTimeField() -class ProgressTabSerializer(VerifiedModeSerializerMixin): +class ProgressTabSerializer(VerifiedModeSerializer): """ Serializer for progress tab """ diff --git a/lms/djangoapps/course_home_api/outline/v1/tests/__init__.py b/lms/djangoapps/course_home_api/progress/tests/__init__.py similarity index 100% rename from lms/djangoapps/course_home_api/outline/v1/tests/__init__.py rename to lms/djangoapps/course_home_api/progress/tests/__init__.py diff --git a/lms/djangoapps/course_home_api/progress/v1/tests/test_views.py b/lms/djangoapps/course_home_api/progress/tests/test_views.py similarity index 95% rename from lms/djangoapps/course_home_api/progress/v1/tests/test_views.py rename to lms/djangoapps/course_home_api/progress/tests/test_views.py index 9100299826..53df6d04f4 100644 --- a/lms/djangoapps/course_home_api/progress/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/progress/tests/test_views.py @@ -33,7 +33,7 @@ class ProgressTabTestViews(BaseCourseHomeTests): """ def setUp(self): super().setUp() - self.url = reverse('course-home-progress-tab', args=[self.course.id]) + self.url = reverse('course-home:progress-tab', args=[self.course.id]) @override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True) @ddt.data(CourseMode.AUDIT, CourseMode.VERIFIED) @@ -45,7 +45,7 @@ class ProgressTabTestViews(BaseCourseHomeTests): assert response.data['section_scores'] is not None for chapter in response.data['section_scores']: assert chapter is not None - assert ('settings/grading/' + str(self.course.id)) in response.data['studio_url'] + assert 'settings/grading/' + str(self.course.id) in response.data['studio_url'] assert response.data['verification_data'] is not None assert response.data['verification_data']['status'] == 'none' if enrollment_mode == CourseMode.VERIFIED: @@ -74,7 +74,7 @@ class ProgressTabTestViews(BaseCourseHomeTests): @override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True) def test_get_unknown_course(self): - url = reverse('course-home-progress-tab', args=['course-v1:unknown+course+2T2020']) + url = reverse('course-home:progress-tab', args=['course-v1:unknown+course+2T2020']) response = self.client.get(url) assert response.status_code == 404 @@ -108,7 +108,7 @@ class ProgressTabTestViews(BaseCourseHomeTests): def test_has_scheduled_content_data(self): CourseEnrollment.enroll(self.user, self.course.id) future = now() + timedelta(days=30) - chapter = ItemFactory(parent=self.course, category='chapter', start=future) + ItemFactory(parent=self.course, category='chapter', start=future) response = self.client.get(self.url) assert response.status_code == 200 assert response.json()['has_scheduled_content'] @@ -127,7 +127,7 @@ class ProgressTabTestViews(BaseCourseHomeTests): @override_waffle_flag(COURSE_HOME_MICROFRONTEND_PROGRESS_TAB, active=True) def test_user_has_passing_grade(self): CourseEnrollment.enroll(self.user, self.course.id) - self.course._grading_policy['GRADE_CUTOFFS']['Pass'] = 0 + self.course.grade_cutoffs = {'Pass': 0} self.update_course(self.course, self.user.id) response = self.client.get(self.url) assert response.status_code == 200 @@ -192,12 +192,12 @@ class ProgressTabTestViews(BaseCourseHomeTests): assert response.data['username'] == self.user.username other_user = UserFactory() - self.url = reverse('course-home-progress-tab-other-student', args=[self.course.id, other_user.id]) + self.url = reverse('course-home:progress-tab-other-student', args=[self.course.id, other_user.id]) CourseEnrollment.enroll(other_user, self.course.id) # users with the ccx coach role can view other students' progress pages with patch( - 'lms.djangoapps.course_home_api.progress.v1.views.has_ccx_coach_role', + 'lms.djangoapps.course_home_api.progress.views.has_ccx_coach_role', return_value=True ): response = self.client.get(self.url) diff --git a/lms/djangoapps/course_home_api/progress/v1/__init__.py b/lms/djangoapps/course_home_api/progress/v1/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/lms/djangoapps/course_home_api/progress/v1/tests/__init__.py b/lms/djangoapps/course_home_api/progress/v1/tests/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/lms/djangoapps/course_home_api/progress/v1/views.py b/lms/djangoapps/course_home_api/progress/views.py similarity index 86% rename from lms/djangoapps/course_home_api/progress/v1/views.py rename to lms/djangoapps/course_home_api/progress/views.py index e8e2e31c19..77dabe1956 100644 --- a/lms/djangoapps/course_home_api/progress/v1/views.py +++ b/lms/djangoapps/course_home_api/progress/views.py @@ -2,8 +2,8 @@ Progress Tab Views """ +from django.contrib.auth import get_user_model from django.http.response import Http404 -from django.contrib.auth.models import User from edx_django_utils import monitoring as monitoring_utils from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser @@ -14,14 +14,16 @@ from rest_framework.response import Response from xmodule.modulestore.django import modulestore from common.djangoapps.student.models import CourseEnrollment -from lms.djangoapps.course_home_api.progress.v1.serializers import ProgressTabSerializer +from lms.djangoapps.course_home_api.progress.serializers import ProgressTabSerializer from lms.djangoapps.course_home_api.toggles import course_home_mfe_progress_tab_is_active from lms.djangoapps.courseware.access import has_access, has_ccx_coach_role from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.course_blocks.transformers import start_date from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException -from lms.djangoapps.courseware.courses import get_course_blocks_completion_summary, get_course_with_access, get_studio_url +from lms.djangoapps.courseware.courses import ( + get_course_blocks_completion_summary, get_course_with_access, get_studio_url, +) from lms.djangoapps.courseware.masquerade import setup_masquerade from lms.djangoapps.courseware.views.views import get_cert_data @@ -34,6 +36,8 @@ from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiv from openedx.features.content_type_gating.block_transformers import ContentTypeGateTransformer from openedx.features.enterprise_support.utils import get_enterprise_learner_generic_name +User = get_user_model() + class ProgressTabView(RetrieveAPIView): """ @@ -78,7 +82,8 @@ class ProgressTabView(RetrieveAPIView): learner_has_access: (bool) whether the learner has access to the subsection (could be FBE gated) num_points_earned: (int) the amount of points the user has earned for the given subsection num_points_possible: (int) the total amount of points possible for the given subsection - percent_graded: (float) the percentage of total points the user has received a grade for in a given subsection + percent_graded: (float) the percentage of total points the user has received a grade for in a given + subsection problem_scores: List of objects that represent individual problem scores with the following fields: earned: (float) number of earned points possible: (float) number of possible points @@ -86,18 +91,19 @@ class ProgressTabView(RetrieveAPIView): ('always', 'never', 'past_due', values defined in common/lib/xmodule/xmodule/modulestore/inheritance.py) show_grades: (bool) a bool for whether to show grades based on the access the user has - url: (str or None) the absolute path url to the Subsection or None if the Subsection is no longer accessible - to the learner due to a hide_after_due course team setting + url: (str or None) the absolute path url to the Subsection or None if the Subsection is no longer + accessible to the learner due to a hide_after_due course team setting enrollment_mode: (str) a str representing the enrollment the user has ('audit', 'verified', ...) grading_policy: assignment_policies: List of serialized assignment grading policy objects, each has the following fields: - num_droppable: (int) the number of lowest scored assignments that will not be counted towards the final grade + num_droppable: (int) the number of lowest scored assignments that will not be counted towards the final + grade short_label: (str) the abbreviated name given to the assignment type type: (str) the assignment type weight: (float) the percent weight the given assigment type has on the overall grade grade_range: an object containing the grade range cutoffs. The exact keys in the object can vary, but they - range from just 'Pass', to a combination of 'A', 'B', 'C', and 'D'. If a letter grade is present, - 'Pass' is not included. + range from just 'Pass', to a combination of 'A', 'B', 'C', and 'D'. If a letter grade is + present, 'Pass' is not included. studio_url: (str) a str of the link to the grading in studio for the course verification_data: an object containing link: (str) the link to either start or retry ID verification @@ -119,15 +125,44 @@ class ProgressTabView(RetrieveAPIView): permission_classes = (IsAuthenticated,) serializer_class = ProgressTabSerializer + def _get_student_user(self, request, course_key, student_id, is_staff): + """Gets the student User object, either from coaching, masquerading, or normal actual request""" + if student_id: + try: + student_id = int(student_id) + except ValueError as e: + raise Http404 from e + + if student_id is None or student_id == request.user.id: + _, student = setup_masquerade( + request, + course_key, + staff_access=is_staff, + reset_masquerade_data=True + ) + return student + + # When a student_id is passed in, we display the progress page for the user + # with the provided user id, rather than the requesting user + try: + coach_access = has_ccx_coach_role(request.user, course_key) + except CCXLocatorValidationException: + coach_access = False + + has_access_on_students_profiles = is_staff or coach_access + # Requesting access to a different student's profile + if not has_access_on_students_profiles: + raise Http404 + + try: + return User.objects.get(id=student_id) + except User.DoesNotExist as exc: + raise Http404 from exc + def get(self, request, *args, **kwargs): course_key_string = kwargs.get('course_key_string') course_key = CourseKey.from_string(course_key_string) student_id = kwargs.get('student_id') - if student_id: - try: - student_id = int(student_id) - except ValueError: - raise Http404 if not course_home_mfe_progress_tab_is_active(course_key): raise Http404 @@ -138,30 +173,7 @@ class ProgressTabView(RetrieveAPIView): monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) is_staff = bool(has_access(request.user, 'staff', course_key)) - if student_id is None or student_id == request.user.id: - _, student = setup_masquerade( - request, - course_key, - staff_access=is_staff, - reset_masquerade_data=True - ) - else: - # When a student_id is passed in, we display the progress page for the user - # with the provided user id, rather than the requesting user - try: - coach_access = has_ccx_coach_role(request.user, course_key) - except CCXLocatorValidationException: - coach_access = False - - has_access_on_students_profiles = is_staff or coach_access - # Requesting access to a different student's profile - if not has_access_on_students_profiles: - raise Http404 - try: - student = User.objects.get(id=student_id) - except User.DoesNotExist as exc: - raise Http404 from exc - + student = self._get_student_user(request, course_key, student_id, is_staff) username = get_enterprise_learner_generic_name(request) or student.username course = get_course_with_access(student, 'load', course_key, check_if_enrolled=False) @@ -219,7 +231,7 @@ class ProgressTabView(RetrieveAPIView): 'completion_summary': get_course_blocks_completion_summary(course_key, student), 'course_grade': course_grade, 'has_scheduled_content': has_scheduled_content, - 'section_scores': course_grade.chapter_grades.values(), + 'section_scores': list(course_grade.chapter_grades.values()), 'enrollment_mode': enrollment_mode, 'grading_policy': grading_policy, 'studio_url': get_studio_url(course, 'settings/grading'), @@ -229,7 +241,7 @@ class ProgressTabView(RetrieveAPIView): context['staff_access'] = is_staff context['course_blocks'] = course_blocks context['course_key'] = course_key - # course_overview and enrollment will be used by VerifiedModeSerializerMixin + # course_overview and enrollment will be used by VerifiedModeSerializer context['course_overview'] = course_overview context['enrollment'] = enrollment serializer = self.get_serializer_class()(data, context=context) diff --git a/lms/djangoapps/course_home_api/mixins.py b/lms/djangoapps/course_home_api/serializers.py similarity index 86% rename from lms/djangoapps/course_home_api/mixins.py rename to lms/djangoapps/course_home_api/serializers.py index eb540c9930..d3aa561733 100644 --- a/lms/djangoapps/course_home_api/mixins.py +++ b/lms/djangoapps/course_home_api/serializers.py @@ -1,6 +1,6 @@ # pylint: disable=abstract-method """ -Course Home Mixins. +Course Home Serializers. """ from opaque_keys.edx.keys import CourseKey @@ -13,7 +13,16 @@ from openedx.features.course_experience import DISPLAY_COURSE_SOCK_FLAG from openedx.features.course_experience.utils import dates_banner_should_display -class DatesBannerSerializerMixin(serializers.Serializer): +class ReadOnlySerializer(serializers.Serializer): + """Serializers have an abstract create & update, but we often don't need them. So this silences the linter.""" + def create(self, validated_data): + pass + + def update(self, instance, validated_data): + pass + + +class DatesBannerSerializer(ReadOnlySerializer): """ Serializer Mixin for displaying the dates banner. Can be added to any serializer who's tab wants to display it. @@ -44,7 +53,7 @@ class DatesBannerSerializerMixin(serializers.Serializer): return info -class VerifiedModeSerializerMixin(serializers.Serializer): +class VerifiedModeSerializer(ReadOnlySerializer): """ Serializer Mixin for displaying verified mode upgrade information. diff --git a/lms/djangoapps/course_home_api/urls.py b/lms/djangoapps/course_home_api/urls.py index 671a90622f..b5ffc08481 100644 --- a/lms/djangoapps/course_home_api/urls.py +++ b/lms/djangoapps/course_home_api/urls.py @@ -6,68 +6,71 @@ Contains all the URLs for the Course Home from django.conf import settings from django.urls import re_path -from lms.djangoapps.course_home_api.course_metadata.v1.views import CourseHomeMetadataView -from lms.djangoapps.course_home_api.dates.v1.views import DatesTabView -from lms.djangoapps.course_home_api.outline.v1.views import OutlineTabView, dismiss_welcome_message, save_course_goal -from lms.djangoapps.course_home_api.progress.v1.views import ProgressTabView +from lms.djangoapps.course_home_api.course_metadata.views import CourseHomeMetadataView +from lms.djangoapps.course_home_api.dates.views import DatesTabView +from lms.djangoapps.course_home_api.outline.views import ( + OutlineTabView, dismiss_welcome_message, save_course_goal, unsubscribe_from_course_goal_by_token, +) +from lms.djangoapps.course_home_api.progress.views import ProgressTabView + +# This API is a BFF ("backend for frontend") designed for the learning MFE. It's not versioned because there is no +# guarantee of stability over time. It may change from one open edx release to another. Don't write any scripts +# that depend on it. urlpatterns = [] # URL for Course metadata content urlpatterns += [ re_path( - fr'v1/course_metadata/{settings.COURSE_KEY_PATTERN}', + fr'course_metadata/{settings.COURSE_KEY_PATTERN}', CourseHomeMetadataView.as_view(), - name='course-home-course-metadata' + name='course-metadata' ), ] # Dates Tab URLs urlpatterns += [ re_path( - fr'v1/dates/{settings.COURSE_KEY_PATTERN}', + fr'dates/{settings.COURSE_KEY_PATTERN}', DatesTabView.as_view(), - name='course-home-dates-tab' + name='dates-tab' ), ] # Outline Tab URLs urlpatterns += [ re_path( - fr'v1/outline/{settings.COURSE_KEY_PATTERN}', + fr'outline/{settings.COURSE_KEY_PATTERN}', OutlineTabView.as_view(), - name='course-home-outline-tab' + name='outline-tab' ), -] - -urlpatterns += [ re_path( - r'v1/dismiss_welcome_message', + r'dismiss_welcome_message', dismiss_welcome_message, - name='course-experience-dismiss-welcome-message' + name='dismiss-welcome-message' ), -] - -urlpatterns += [ re_path( - r'v1/save_course_goal', + r'save_course_goal', save_course_goal, - name='course-home-save-course-goal' + name='save-course-goal' + ), + re_path( + r'unsubscribe_from_course_goal/(?P[^/]*)$', + unsubscribe_from_course_goal_by_token, + name='unsubscribe-from-course-goal' ), ] # Progress Tab URLs urlpatterns += [ re_path( - fr'v1/progress/{settings.COURSE_KEY_PATTERN}/(?P[^/]+)', + fr'progress/{settings.COURSE_KEY_PATTERN}/(?P[^/]+)', ProgressTabView.as_view(), - name='course-home-progress-tab-other-student' + name='progress-tab-other-student' ), -] -urlpatterns += [ re_path( - fr'v1/progress/{settings.COURSE_KEY_PATTERN}', + fr'progress/{settings.COURSE_KEY_PATTERN}', ProgressTabView.as_view(), - name='course-home-progress-tab' + name='progress-tab' ), ] diff --git a/lms/urls.py b/lms/urls.py index c7e03d45a3..13e8e7720a 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -986,7 +986,13 @@ urlpatterns.extend(get_plugin_url_patterns(ProjectType.LMS)) # Course Home API urls urlpatterns += [ - url(r'^api/course_home/', include('lms.djangoapps.course_home_api.urls')), + # This is a BFF ("backend for frontend") djangoapp for the Learning MFE (like courseware_api). + # It will change and morph as needed for the frontend, and is not a stable API on which other code can rely. + url(r'^api/course_home/', include(('lms.djangoapps.course_home_api.urls', 'course-home'))), + + # This v1 version is just kept for transitional reasons and is going away as soon as the MFE stops referencing it. + # We don't promise any sort of versioning stability. + url(r'^api/course_home/v1/', include(('lms.djangoapps.course_home_api.urls', 'course-home-v1'))), ] # Course Experience API urls diff --git a/openedx/core/djangoapps/content/course_overviews/api.py b/openedx/core/djangoapps/content/course_overviews/api.py index 17400671e8..fcad69c7e0 100644 --- a/openedx/core/djangoapps/content/course_overviews/api.py +++ b/openedx/core/djangoapps/content/course_overviews/api.py @@ -3,6 +3,8 @@ CourseOverview api """ import logging +from django.http.response import Http404 + from openedx.core.djangoapps.catalog.api import get_course_run_details from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.serializers import ( @@ -25,6 +27,18 @@ def get_course_overview_or_none(course_id): return None +def get_course_overview_or_404(course_id): + """ + Retrieve and return course overview data for the provided course id. + + If the course overview does not exist, raises Http404. + """ + try: + return CourseOverview.get_from_id(course_id) + except CourseOverview.DoesNotExist as e: + raise Http404(f"Course overview does not exist for {course_id}") from e + + def get_pseudo_course_overview(course_id): """ Returns a pseudo course overview object for a deleted course. diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_api.py b/openedx/core/djangoapps/content/course_overviews/tests/test_api.py index c69fed072c..9277750295 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_api.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_api.py @@ -3,11 +3,13 @@ course_overview api tests """ from mock import patch +from django.http.response import Http404 from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.api import ( + get_course_overview_or_404, get_course_overview_or_none, get_course_overviews, get_course_overviews_from_ids, @@ -27,7 +29,7 @@ class TestCourseOverviewsApi(ModuleStoreTestCase): for _ in range(3): CourseOverviewFactory.create() - def test_get_course_overview_or_none(self): + def test_get_course_overview_or_none_success(self): """ Test for `test_get_course_overview_or_none` function when the overview exists. """ @@ -43,6 +45,22 @@ class TestCourseOverviewsApi(ModuleStoreTestCase): retrieved_course_overview = get_course_overview_or_none(course_run_key) assert retrieved_course_overview is None + def test_get_course_overview_or_404_success(self): + """ + Test for `test_get_course_overview_or_404` function when the overview exists. + """ + course_overview = CourseOverviewFactory.create() + retrieved_course_overview = get_course_overview_or_404(course_overview.id) + assert course_overview.id == retrieved_course_overview.id + + def test_get_course_overview_or_404_missing(self): + """ + Test for `test_get_course_overview_or_404` function when the overview does not exist. + """ + course_run_key = CourseKey.from_string('course-v1:coping+with+deletions') + with self.assertRaises(Http404): + get_course_overview_or_404(course_run_key) + def test_get_course_overview_from_ids(self): """ Test for `get_course_overviews_from_ids` function. diff --git a/openedx/core/djangoapps/courseware_api/serializers.py b/openedx/core/djangoapps/courseware_api/serializers.py index 2395cc1761..0ab3ee36ff 100644 --- a/openedx/core/djangoapps/courseware_api/serializers.py +++ b/openedx/core/djangoapps/courseware_api/serializers.py @@ -4,7 +4,7 @@ Course API Serializers. Representing course catalog data from rest_framework import serializers -from lms.djangoapps.course_home_api.progress.v1.serializers import CertificateDataSerializer +from lms.djangoapps.course_home_api.progress.serializers import CertificateDataSerializer from openedx.core.lib.api.fields import AbsoluteURLField diff --git a/openedx/features/course_experience/api/v1/serializers.py b/openedx/features/course_experience/api/v1/serializers.py index ded6b05349..10f2a785d1 100644 --- a/openedx/features/course_experience/api/v1/serializers.py +++ b/openedx/features/course_experience/api/v1/serializers.py @@ -5,11 +5,12 @@ from rest_framework import serializers from opaque_keys.edx.keys import CourseKey -from lms.djangoapps.course_home_api.mixins import DatesBannerSerializerMixin +from lms.djangoapps.course_home_api.serializers import DatesBannerSerializer from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -class CourseDeadlinesMobileSerializer(DatesBannerSerializerMixin): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring +class CourseDeadlinesMobileSerializer(DatesBannerSerializer): + """Serializer for course deadlines""" has_ended = serializers.SerializerMethodField() def get_has_ended(self, _): # lint-amnesty, pylint: disable=missing-function-docstring