diff --git a/common/djangoapps/util/query.py b/common/djangoapps/util/query.py index 0a65f20488..865f0aee9f 100644 --- a/common/djangoapps/util/query.py +++ b/common/djangoapps/util/query.py @@ -6,6 +6,7 @@ from django.conf import settings def use_read_replica_if_available(queryset): """ - If there is a database called 'read_replica', use that database for the queryset. + If there is a database called 'read_replica', + use that database for the queryset / manager. """ return queryset.using("read_replica") if "read_replica" in settings.DATABASES else queryset diff --git a/lms/djangoapps/program_enrollments/api/v1/constants.py b/lms/djangoapps/program_enrollments/api/v1/constants.py deleted file mode 100644 index 7feed8dbe1..0000000000 --- a/lms/djangoapps/program_enrollments/api/v1/constants.py +++ /dev/null @@ -1,65 +0,0 @@ -""" - Constants and strings for the course-enrollment app -""" - -# Captures strings composed of alphanumeric characters a-f and dashes. -PROGRAM_UUID_PATTERN = r'(?P[A-Fa-f0-9-]+)' -MAX_ENROLLMENT_RECORDS = 25 - -# The name of the key that identifies students for POST/PATCH requests -REQUEST_STUDENT_KEY = 'student_key' - -ENABLE_ENROLLMENT_RESET_FLAG = 'ENABLE_ENROLLMENT_RESET' - - -class BaseEnrollmentResponseStatuses(object): - """ - Class to group common response statuses - """ - DUPLICATED = 'duplicated' - INVALID_STATUS = "invalid-status" - CONFLICT = "conflict" - ILLEGAL_OPERATION = "illegal-operation" - NOT_IN_PROGRAM = "not-in-program" - INTERNAL_ERROR = "internal-error" - - ERROR_STATUSES = { - DUPLICATED, - INVALID_STATUS, - CONFLICT, - ILLEGAL_OPERATION, - NOT_IN_PROGRAM, - INTERNAL_ERROR, - } - - -class CourseEnrollmentResponseStatuses(BaseEnrollmentResponseStatuses): - """ - Class to group response statuses returned by the course enrollment endpoint - """ - ACTIVE = "active" - INACTIVE = "inactive" - NOT_FOUND = "not-found" - - ERROR_STATUSES = BaseEnrollmentResponseStatuses.ERROR_STATUSES | {NOT_FOUND} - - -class ProgramEnrollmentResponseStatuses(BaseEnrollmentResponseStatuses): - """ - Class to group response statuses returned by the program enrollment endpoint - """ - ENROLLED = 'enrolled' - PENDING = 'pending' - SUSPENDED = 'suspended' - CANCELED = 'canceled' - - VALID_STATUSES = [ENROLLED, PENDING, SUSPENDED, CANCELED] - - -class CourseRunProgressStatuses(object): - """ - Class to group statuses that a course run can be in with respect to user progress. - """ - IN_PROGRESS = 'in_progress' - UPCOMING = 'upcoming' - COMPLETED = 'completed' diff --git a/lms/djangoapps/program_enrollments/api/v1/tests/test_serializers.py b/lms/djangoapps/program_enrollments/api/v1/tests/test_serializers.py deleted file mode 100644 index 87b53ecf3e..0000000000 --- a/lms/djangoapps/program_enrollments/api/v1/tests/test_serializers.py +++ /dev/null @@ -1,46 +0,0 @@ -""" -Unit tests for ProgramEnrollment serializers. -""" -from __future__ import absolute_import, unicode_literals - -from uuid import uuid4 - -from django.test import TestCase - -from lms.djangoapps.program_enrollments.api.v1.serializers import ProgramEnrollmentSerializer -from lms.djangoapps.program_enrollments.models import ProgramEnrollment -from student.tests.factories import UserFactory - - -class ProgramEnrollmentSerializerTests(TestCase): - """ - Tests for the ProgramEnrollment serializer. - """ - def setUp(self): - """ - Set up the test data used in the specific tests - """ - super(ProgramEnrollmentSerializerTests, self).setUp() - self.user = UserFactory.create() - self.enrollment = ProgramEnrollment.objects.create( - user=self.user, - external_user_key='abc', - program_uuid=uuid4(), - curriculum_uuid=uuid4(), - status='enrolled' - ) - self.serializer = ProgramEnrollmentSerializer(instance=self.enrollment) - - def test_serializer_contains_expected_fields(self): - data = self.serializer.data - - self.assertEqual( - set(data.keys()), - set([ - 'user', - 'external_user_key', - 'program_uuid', - 'curriculum_uuid', - 'status' - ]) - ) diff --git a/lms/djangoapps/program_enrollments/apps.py b/lms/djangoapps/program_enrollments/apps.py index 67d4eb3ee6..55d1465691 100644 --- a/lms/djangoapps/program_enrollments/apps.py +++ b/lms/djangoapps/program_enrollments/apps.py @@ -20,7 +20,7 @@ class ProgramEnrollmentsConfig(AppConfig): ProjectType.LMS: { PluginURLs.NAMESPACE: 'programs_api', PluginURLs.REGEX: 'api/program_enrollments/', - PluginURLs.RELATIVE_PATH: 'api.urls', + PluginURLs.RELATIVE_PATH: 'rest_api.urls', } }, } diff --git a/lms/djangoapps/program_enrollments/constants.py b/lms/djangoapps/program_enrollments/constants.py new file mode 100644 index 0000000000..f6edee1561 --- /dev/null +++ b/lms/djangoapps/program_enrollments/constants.py @@ -0,0 +1,41 @@ +""" +Constants used throughout the program_enrollments app and exposed to other +in-process apps through api.py. +""" + + +class ProgramEnrollmentStatuses(object): + """ + Status that a user may have enrolled in a program. + + TODO: Define the semantics of each of these (EDUCATOR-4958) + """ + ENROLLED = 'enrolled' + PENDING = 'pending' + SUSPENDED = 'suspended' + CANCELED = 'canceled' + __ACTIVE__ = (ENROLLED, PENDING) + __ALL__ = (ENROLLED, PENDING, SUSPENDED, CANCELED) + + # Note: Any changes to this value will trigger a migration on + # ProgramEnrollment! + __MODEL_CHOICES__ = ( + (status, status) for status in __ALL__ + ) + + +class ProgramCourseEnrollmentStatuses(object): + """ + Status that a user may have enrolled in a course. + + TODO: Consider whether we need these (EDUCATOR-4958) + """ + ACTIVE = 'active' + INACTIVE = 'inactive' + __ALL__ = (ACTIVE, INACTIVE) + + # Note: Any changes to this value will trigger a migration on + # ProgramCourseEnrollment! + __MODEL_CHOICES__ = ( + (status, status) for status in __ALL__ + ) diff --git a/lms/djangoapps/program_enrollments/models.py b/lms/djangoapps/program_enrollments/models.py index 321bd01265..7a7e9827f3 100644 --- a/lms/djangoapps/program_enrollments/models.py +++ b/lms/djangoapps/program_enrollments/models.py @@ -16,11 +16,11 @@ from simple_history.models import HistoricalRecords from six import text_type from course_modes.models import CourseMode -from lms.djangoapps.program_enrollments.api.v1.constants import \ - CourseEnrollmentResponseStatuses as ProgramCourseEnrollmentResponseStatuses from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.models import CourseEnrollment, NonExistentCourseError +from .constants import ProgramCourseEnrollmentStatuses, ProgramEnrollmentStatuses + logger = logging.getLogger(__name__) # pylint: disable=invalid-name @@ -32,12 +32,7 @@ class ProgramEnrollment(TimeStampedModel): # pylint: disable=model-missing-unic .. pii_types: other .. pii_retirement: local_api """ - STATUSES = ( - ('enrolled', 'enrolled'), - ('pending', 'pending'), - ('suspended', 'suspended'), - ('canceled', 'canceled'), - ) + STATUS_CHOICES = ProgramEnrollmentStatuses.__MODEL_CHOICES__ class Meta(object): app_label = "program_enrollments" @@ -61,7 +56,7 @@ class ProgramEnrollment(TimeStampedModel): # pylint: disable=model-missing-unic ) program_uuid = models.UUIDField(db_index=True, null=False) curriculum_uuid = models.UUIDField(db_index=True, null=False) - status = models.CharField(max_length=9, choices=STATUSES) + status = models.CharField(max_length=9, choices=STATUS_CHOICES) historical_records = HistoricalRecords() def clean(self): @@ -121,10 +116,7 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin .. no_pii: """ - STATUSES = ( - ('active', 'active'), - ('inactive', 'inactive'), - ) + STATUS_CHOICES = ProgramCourseEnrollmentStatuses.__MODEL_CHOICES__ class Meta(object): app_label = "program_enrollments" @@ -151,7 +143,7 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin blank=True, ) course_key = CourseKeyField(max_length=255) - status = models.CharField(max_length=9, choices=STATUSES) + status = models.CharField(max_length=9, choices=STATUS_CHOICES) historical_records = HistoricalRecords() def __str__(self): @@ -182,9 +174,9 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin self.status = status if self.course_enrollment: - if status == ProgramCourseEnrollmentResponseStatuses.ACTIVE: + if status == ProgramCourseEnrollmentStatuses.ACTIVE: self.course_enrollment.activate() - elif status == ProgramCourseEnrollmentResponseStatuses.INACTIVE: + elif status == ProgramCourseEnrollmentStatuses.INACTIVE: self.course_enrollment.deactivate() else: message = ("Changed {enrollment} status to {status}, not changing course_enrollment" @@ -192,8 +184,8 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin logger.warn(message.format( enrollment=self, status=status, - active=ProgramCourseEnrollmentResponseStatuses.ACTIVE, - inactive=ProgramCourseEnrollmentResponseStatuses.INACTIVE + active=ProgramCourseEnrollmentStatuses.ACTIVE, + inactive=ProgramCourseEnrollmentStatuses.INACTIVE )) elif self.program_enrollment.user: logger.warn("User {user} {program_enrollment} {course_key} has no course_enrollment".format( @@ -211,7 +203,10 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin try: CourseOverview.get_from_id(self.course_key) except CourseOverview.DoesNotExist: - logger.warning(u"User %s failed to enroll in non-existent course %s", user.id, text_type(self.course_key)) + logger.warning( + u"User %s failed to enroll in non-existent course %s", user.id, + text_type(self.course_key), + ) raise NonExistentCourseError if CourseEnrollment.is_enrolled(user, self.course_key): @@ -219,13 +214,16 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin user=user, course_id=self.course_key, ) - if course_enrollment.mode == CourseMode.AUDIT or course_enrollment.mode == CourseMode.HONOR: + if course_enrollment.mode in {CourseMode.AUDIT, CourseMode.HONOR}: course_enrollment.mode = CourseMode.MASTERS course_enrollment.save() self.course_enrollment = course_enrollment - message = ("Attempted to create course enrollment for user={user} and course={course}" - " but an enrollment already exists. Existing enrollment will be used instead") - logger.info(message.format(user=user.id, course=self.course_key)) + message_template = ( + "Attempted to create course enrollment for user={user} " + "and course={course} but an enrollment already exists. " + "Existing enrollment will be used instead." + ) + logger.info(message_template.format(user=user.id, course=self.course_key)) else: self.course_enrollment = CourseEnrollment.enroll( user, @@ -233,6 +231,6 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin mode=CourseMode.MASTERS, check_access=False, ) - if self.status == ProgramCourseEnrollmentResponseStatuses.INACTIVE: + if self.status == ProgramCourseEnrollmentStatuses.INACTIVE: self.course_enrollment.deactivate() self.save() diff --git a/lms/djangoapps/program_enrollments/api/__init__.py b/lms/djangoapps/program_enrollments/rest_api/__init__.py similarity index 100% rename from lms/djangoapps/program_enrollments/api/__init__.py rename to lms/djangoapps/program_enrollments/rest_api/__init__.py diff --git a/lms/djangoapps/program_enrollments/api/urls.py b/lms/djangoapps/program_enrollments/rest_api/urls.py similarity index 70% rename from lms/djangoapps/program_enrollments/api/urls.py rename to lms/djangoapps/program_enrollments/rest_api/urls.py index 973bb62025..6c95b7d39d 100644 --- a/lms/djangoapps/program_enrollments/api/urls.py +++ b/lms/djangoapps/program_enrollments/rest_api/urls.py @@ -6,8 +6,10 @@ from __future__ import absolute_import from django.conf.urls import include, url +from .v1 import urls as v1_urls + app_name = 'lms.djangoapps.program_enrollments' urlpatterns = [ - url(r'^v1/', include('program_enrollments.api.v1.urls', namespace='v1')) + url(r'^v1/', include(v1_urls)) ] diff --git a/lms/djangoapps/program_enrollments/api/v1/__init__.py b/lms/djangoapps/program_enrollments/rest_api/v1/__init__.py similarity index 100% rename from lms/djangoapps/program_enrollments/api/v1/__init__.py rename to lms/djangoapps/program_enrollments/rest_api/v1/__init__.py diff --git a/lms/djangoapps/program_enrollments/rest_api/v1/constants.py b/lms/djangoapps/program_enrollments/rest_api/v1/constants.py new file mode 100644 index 0000000000..5136c5ff6b --- /dev/null +++ b/lms/djangoapps/program_enrollments/rest_api/v1/constants.py @@ -0,0 +1,106 @@ +""" +Constants used throughout the program_enrollments V1 API. +""" + +from lms.djangoapps.program_enrollments.constants import ProgramCourseEnrollmentStatuses, ProgramEnrollmentStatuses + +# Captures strings composed of alphanumeric characters a-f and dashes. +PROGRAM_UUID_PATTERN = r'(?P[A-Fa-f0-9-]+)' + +# Maximum number of students that may be enrolled at once. +MAX_ENROLLMENT_RECORDS = 25 + +# The name of the key that identifies students for POST/PATCH requests +REQUEST_STUDENT_KEY = 'student_key' + +# This flag should only be enabled on sandboxes. +# It enables the endpoint that wipes all program enrollments. +ENABLE_ENROLLMENT_RESET_FLAG = 'ENABLE_ENROLLMENT_RESET' + + +class _EnrollmentErrorStatuses(object): + """ + Error statuses common to program and program-course enrollments responses. + """ + + # Same student key supplied more than once. + DUPLICATED = 'duplicated' + + # Requested target status is invalid + INVALID_STATUS = "invalid-status" + + # In the case of a POST request, the enrollment already exists. + CONFLICT = "conflict" + + # Although the request is syntactically valid, + # the change being made is not supported. + # For example, it may be illegal to change a user's status back to A + # after changing it to B, where A and B are two hypothetical enrollment + # statuses. + ILLEGAL_OPERATION = "illegal-operation" + + # Could not modify program enrollment or create program-course + # enrollment because the student is not enrolled in the program in the + # first place. + NOT_IN_PROGRAM = "not-in-program" + + # Something unexpected went wrong. + # If API users are seeing this, we need to investigate. + INTERNAL_ERROR = "internal-error" + + __ALL__ = ( + DUPLICATED, + INVALID_STATUS, + CONFLICT, + ILLEGAL_OPERATION, + NOT_IN_PROGRAM, + INTERNAL_ERROR, + ) + + +class ProgramResponseStatuses( + ProgramEnrollmentStatuses, + _EnrollmentErrorStatuses, +): + """ + Valid program enrollment response statuses. + + Combines error statuses and OK statuses. + """ + __OK__ = ProgramEnrollmentStatuses.__ALL__ + __ERRORS__ = _EnrollmentErrorStatuses.__ALL__ + __ALL__ = __OK__ + __ERRORS__ + + +class ProgramCourseResponseStatuses( + ProgramCourseEnrollmentStatuses, + _EnrollmentErrorStatuses, +): + """ + Valid program-course enrollment response statuses. + + Combines error statuses and OK statuses. + """ + + # Could not modify program-course enrollment because the user + # is not enrolled in the course in the first place. + NOT_FOUND = "not-found" + + __OK__ = ProgramCourseEnrollmentStatuses.__ALL__ + __ERRORS__ = (NOT_FOUND,) + _EnrollmentErrorStatuses.__ALL__ + __ALL__ = __OK__ + __ERRORS__ + + +class CourseRunProgressStatuses(object): + """ + Statuses that a course run can be in with respect to user progress. + """ + IN_PROGRESS = 'in_progress' + UPCOMING = 'upcoming' + COMPLETED = 'completed' + + __ALL__ = ( + IN_PROGRESS, + UPCOMING, + COMPLETED, + ) diff --git a/lms/djangoapps/program_enrollments/api/v1/serializers.py b/lms/djangoapps/program_enrollments/rest_api/v1/serializers.py similarity index 60% rename from lms/djangoapps/program_enrollments/api/v1/serializers.py rename to lms/djangoapps/program_enrollments/rest_api/v1/serializers.py index 942daf6ece..3e5368c922 100644 --- a/lms/djangoapps/program_enrollments/api/v1/serializers.py +++ b/lms/djangoapps/program_enrollments/rest_api/v1/serializers.py @@ -6,11 +6,12 @@ from __future__ import absolute_import from rest_framework import serializers from six import text_type +from lms.djangoapps.program_enrollments.constants import ProgramCourseEnrollmentStatuses, ProgramEnrollmentStatuses from lms.djangoapps.program_enrollments.models import ProgramCourseEnrollment, ProgramEnrollment -from lms.djangoapps.program_enrollments.api.v1.constants import ( - CourseRunProgressStatuses, - ProgramEnrollmentResponseStatuses -) + +from .constants import CourseRunProgressStatuses + +# pylint: disable=abstract-method class InvalidStatusMixin(object): @@ -19,72 +20,18 @@ class InvalidStatusMixin(object): """ def has_invalid_status(self): """ - Returns whether or not this serializer has an invalid error choice on the "status" field + Returns whether or not this serializer has an invalid error choice on + the "status" field. """ - try: - for status_error in self.errors['status']: - if status_error.code == 'invalid_choice': - return True - except KeyError: - pass + for status_error in self.errors.get('status', []): + if status_error.code == 'invalid_choice': + return True return False -# pylint: disable=abstract-method -class ProgramEnrollmentSerializer(serializers.ModelSerializer, InvalidStatusMixin): +class ProgramEnrollmentSerializer(serializers.Serializer): """ - Serializer for Program Enrollments - """ - - class Meta(object): - model = ProgramEnrollment - fields = ('user', 'external_user_key', 'program_uuid', 'curriculum_uuid', 'status') - validators = [] - - def validate(self, attrs): - """ This modifies self.instance in the case of updates """ - if not self.instance: - enrollment = ProgramEnrollment(**attrs) - enrollment.full_clean() - else: - for key, value in attrs.items(): - setattr(self.instance, key, value) - self.instance.full_clean() - - return attrs - - def create(self, validated_data): - return ProgramEnrollment.objects.create(**validated_data) - - -class BaseProgramEnrollmentRequestMixin(serializers.Serializer, InvalidStatusMixin): - """ - Base fields for all program enrollment related serializers - """ - student_key = serializers.CharField() - status = serializers.ChoiceField( - allow_blank=False, - choices=ProgramEnrollmentResponseStatuses.VALID_STATUSES - ) - - -class ProgramEnrollmentCreateRequestSerializer(BaseProgramEnrollmentRequestMixin): - """ - Serializer for program enrollment creation requests - """ - curriculum_uuid = serializers.UUIDField() - - -class ProgramEnrollmentModifyRequestSerializer(BaseProgramEnrollmentRequestMixin): - """ - Serializer for program enrollment modification requests - """ - pass - - -class ProgramEnrollmentListSerializer(serializers.Serializer): - """ - Serializer for listing enrollments in a program. + Serializer for displaying enrollments in a program. """ student_key = serializers.CharField(source='external_user_key') status = serializers.CharField() @@ -98,20 +45,9 @@ class ProgramEnrollmentListSerializer(serializers.Serializer): return bool(obj.user) -# pylint: disable=abstract-method -class ProgramCourseEnrollmentRequestSerializer(serializers.Serializer, InvalidStatusMixin): +class ProgramCourseEnrollmentSerializer(serializers.Serializer): """ - Serializer for request to create a ProgramCourseEnrollment - """ - STATUS_CHOICES = ['active', 'inactive'] - - student_key = serializers.CharField(allow_blank=False) - status = serializers.ChoiceField(allow_blank=False, choices=STATUS_CHOICES) - - -class ProgramCourseEnrollmentListSerializer(serializers.Serializer): - """ - Serializer for listing course enrollments in a program. + Serializer for displaying program-course enrollments. """ student_key = serializers.SerializerMethodField() status = serializers.CharField() @@ -131,60 +67,57 @@ class ProgramCourseEnrollmentListSerializer(serializers.Serializer): return text_type(obj.program_enrollment.curriculum_uuid) -class ProgramCourseGradeResult(object): +class ProgramEnrollmentRequestMixin(InvalidStatusMixin, serializers.Serializer): """ - Represents a courserun grade for a user enrolled through a program. - - Can be passed to ProgramCourseGradeResultSerializer. + Base fields for all program enrollment related serializers. """ - is_error = False - - def __init__(self, program_course_enrollment, course_grade): - """ - Creates a new grade result given a ProgramCourseEnrollment object - and a course grade object. - """ - self.student_key = program_course_enrollment.program_enrollment.external_user_key - self.passed = course_grade.passed - self.percent = course_grade.percent - self.letter_grade = course_grade.letter_grade + student_key = serializers.CharField() + status = serializers.ChoiceField( + allow_blank=False, + choices=ProgramEnrollmentStatuses.__ALL__, + ) -class ProgramCourseGradeErrorResult(object): +class ProgramEnrollmentCreateRequestSerializer(ProgramEnrollmentRequestMixin): """ - Represents a failure to load a courserun grade for a user enrolled through - a program. - - Can be passed to ProgramCourseGradeResultSerializer. + Serializer for program enrollment creation requests. """ - is_error = True - - def __init__(self, program_course_enrollment, exception=None): - """ - Creates a new course grade error object given a - ProgramCourseEnrollment and an exception. - """ - self.student_key = program_course_enrollment.program_enrollment.external_user_key - self.error = text_type(exception) if exception else u"Unknown error" + curriculum_uuid = serializers.UUIDField() -class ProgramCourseGradeResultSerializer(serializers.Serializer): +class ProgramEnrollmentModifyRequestSerializer(ProgramEnrollmentRequestMixin): + """ + Serializer for program enrollment modification requests + """ + pass + + +class ProgramCourseEnrollmentRequestSerializer(serializers.Serializer, InvalidStatusMixin): + """ + Serializer for request to create a ProgramCourseEnrollment + """ + student_key = serializers.CharField(allow_blank=False) + status = serializers.ChoiceField( + allow_blank=False, + choices=ProgramCourseEnrollmentStatuses.__ALL__, + ) + + +class ProgramCourseGradeSerializer(serializers.Serializer): """ Serializer for a user's grade in a program courserun. - Meant to be used with ProgramCourseGradeResult - or ProgramCourseGradeErrorResult as input. - Absence of fields other than `student_key` will be ignored. + Meant to be used with BaseProgramCourseGrade. """ # Required student_key = serializers.CharField() - # From ProgramCourseGradeResult only + # From ProgramCourseGradeOk only passed = serializers.BooleanField(required=False) percent = serializers.FloatField(required=False) letter_grade = serializers.CharField(required=False) - # From ProgramCourseGradeErrorResult only + # From ProgramCourseGradeError only error = serializers.CharField(required=False) @@ -225,3 +158,62 @@ class CourseRunOverviewListSerializer(serializers.Serializer): Serializer for a list of course run overviews. """ course_runs = serializers.ListField(child=CourseRunOverviewSerializer()) + + +# TODO: The following classes are not serializers, and should probably +# be moved to api.py as part of EDUCATOR-4321. + + +class BaseProgramCourseGrade(object): + """ + Base for either a courserun grade or grade-loading failure. + + Can be passed to ProgramCourseGradeResultSerializer. + """ + is_error = None # Override in subclass + + def __init__(self, program_course_enrollment): + """ + Given a ProgramCourseEnrollment, + create a BaseProgramCourseGradeResult instance. + """ + self.student_key = ( + program_course_enrollment.program_enrollment.external_user_key + ) + + +class ProgramCourseGradeOk(BaseProgramCourseGrade): + """ + Represents a courserun grade for a user enrolled through a program. + """ + is_error = False + + def __init__(self, program_course_enrollment, course_grade): + """ + Given a ProgramCourseEnrollment and course grade object, + create a ProgramCourseGradeOk. + """ + super(ProgramCourseGradeOk, self).__init__( + program_course_enrollment + ) + self.passed = course_grade.passed + self.percent = course_grade.percent + self.letter_grade = course_grade.letter_grade + + +class ProgramCourseGradeError(BaseProgramCourseGrade): + """ + Represents a failure to load a courserun grade for a user enrolled through + a program. + """ + is_error = True + + def __init__(self, program_course_enrollment, exception=None): + """ + Given a ProgramCourseEnrollment and an Exception, + create a ProgramCourseGradeError. + """ + super(ProgramCourseGradeError, self).__init__( + program_course_enrollment + ) + self.error = text_type(exception) if exception else u"Unknown error" diff --git a/lms/djangoapps/program_enrollments/api/v1/tests/__init__.py b/lms/djangoapps/program_enrollments/rest_api/v1/tests/__init__.py similarity index 100% rename from lms/djangoapps/program_enrollments/api/v1/tests/__init__.py rename to lms/djangoapps/program_enrollments/rest_api/v1/tests/__init__.py diff --git a/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py b/lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py similarity index 80% rename from lms/djangoapps/program_enrollments/api/v1/tests/test_views.py rename to lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py index 69f7c7509c..d2f74aef55 100644 --- a/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py +++ b/lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py @@ -16,7 +16,7 @@ from django.test import override_settings from django.urls import reverse from freezegun import freeze_time from opaque_keys.edx.keys import CourseKey -from organizations.tests.factories import OrganizationFactory +from organizations.tests.factories import OrganizationFactory as LMSOrganizationFactory from pytz import UTC from rest_framework import status from rest_framework.test import APITestCase @@ -28,21 +28,17 @@ from course_modes.models import CourseMode from lms.djangoapps.certificates.models import CertificateStatuses from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory, InstructorFactory -from lms.djangoapps.program_enrollments.api.v1.constants import ( - ENABLE_ENROLLMENT_RESET_FLAG, - MAX_ENROLLMENT_RECORDS, - REQUEST_STUDENT_KEY -) -from lms.djangoapps.program_enrollments.api.v1.constants import CourseEnrollmentResponseStatuses as CourseStatuses -from lms.djangoapps.program_enrollments.api.v1.constants import CourseRunProgressStatuses -from lms.djangoapps.program_enrollments.api.v1.constants import ProgramEnrollmentResponseStatuses as ProgramStatuses from lms.djangoapps.program_enrollments.models import ProgramCourseEnrollment, ProgramEnrollment from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory from lms.djangoapps.program_enrollments.utils import ProviderDoesNotExistException from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, PROGRAMS_BY_ORGANIZATION_CACHE_KEY_TPL -from openedx.core.djangoapps.catalog.tests.factories import CourseFactory, CourseRunFactory -from openedx.core.djangoapps.catalog.tests.factories import OrganizationFactory as CatalogOrganizationFactory -from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory +from openedx.core.djangoapps.catalog.tests.factories import ( + CourseFactory, + CourseRunFactory, + CurriculumFactory, + OrganizationFactory, + ProgramFactory +) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangolib.testing.utils import CacheIsolationMixin @@ -53,6 +49,18 @@ from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory as ModulestoreCourseFactory from xmodule.modulestore.tests.factories import ItemFactory +from ..constants import ( + ENABLE_ENROLLMENT_RESET_FLAG, + MAX_ENROLLMENT_RECORDS, + REQUEST_STUDENT_KEY, + CourseRunProgressStatuses +) +from ..constants import ProgramCourseResponseStatuses as CourseStatuses +from ..constants import ProgramResponseStatuses as ProgramStatuses + +_REST_API_MOCK_FMT = 'lms.djangoapps.program_enrollments.rest_api.{}' +_VIEW_MOCK_FMT = _REST_API_MOCK_FMT.format('v1.views.{}') + class ProgramCacheTestCaseMixin(CacheIsolationMixin): """ @@ -60,25 +68,10 @@ class ProgramCacheTestCaseMixin(CacheIsolationMixin): """ ENABLED_CACHES = ['default'] - @staticmethod - def setup_catalog_cache(program_uuid, organization_key): - """ - helper function to initialize a cached program with an single authoring_organization - """ - catalog_org = CatalogOrganizationFactory.create(key=organization_key) - program = ProgramFactory.create( - uuid=program_uuid, - authoring_organizations=[catalog_org] - ) - cache.set(PROGRAM_CACHE_KEY_TPL.format(uuid=program_uuid), program, None) - return program - - @staticmethod - def set_program_in_catalog_cache(program_uuid, program): + def set_program_in_catalog_cache(self, program_uuid, program): cache.set(PROGRAM_CACHE_KEY_TPL.format(uuid=program_uuid), program, None) - @staticmethod - def set_org_in_catalog_cache(organization, program_uuids): + def set_org_in_catalog_cache(self, organization, program_uuids): cache.set(PROGRAMS_BY_ORGANIZATION_CACHE_KEY_TPL.format(org_key=organization.short_name), program_uuids) @@ -87,47 +80,52 @@ class ListViewTestMixin(ProgramCacheTestCaseMixin): Mixin to define some shared test data objects for program/course enrollment list view tests. """ - view_name = None + view_name = 'SET-ME-IN-SUBCLASS' @classmethod def setUpClass(cls): super(ListViewTestMixin, cls).setUpClass() cls.start_cache_isolation() - cls.program_uuid = '00000000-1111-2222-3333-444444444444' - cls.program_uuid_tmpl = '00000000-1111-2222-3333-4444444444{0:02d}' - cls.curriculum_uuid = 'aaaaaaaa-1111-2222-3333-444444444444' - cls.other_curriculum_uuid = 'bbbbbbbb-1111-2222-3333-444444444444' cls.organization_key = "orgkey" + catalog_org = OrganizationFactory(key=cls.organization_key) + cls.program_uuid = UUID('00000000-1111-2222-3333-444444444444') + cls.program_uuid_tmpl = '00000000-1111-2222-3333-4444444444{0:02d}' + cls.curriculum_uuid = UUID('aaaaaaaa-1111-2222-3333-444444444444') + cls.other_curriculum_uuid = UUID('bbbbbbbb-1111-2222-3333-444444444444') + inactive_curriculum_uuid = UUID('cccccccc-1111-2222-3333-444444444444') - cls.program = cls.setup_catalog_cache(cls.program_uuid, cls.organization_key) + catalog_course_id_str = 'course-v1:edX+ToyX' + course_run_id_str = '{}+Toy_Course'.format(catalog_course_id_str) + cls.course_id = CourseKey.from_string(course_run_id_str) + CourseOverviewFactory(id=cls.course_id) + course_run = CourseRunFactory(key=course_run_id_str) + course = CourseFactory(key=catalog_course_id_str, course_runs=[course_run]) + inactive_curriculum = CurriculumFactory(uuid=inactive_curriculum_uuid, is_active=False) + cls.curriculum = CurriculumFactory(uuid=cls.curriculum_uuid, courses=[course]) + cls.program = ProgramFactory( + uuid=cls.program_uuid, + authoring_organizations=[catalog_org], + curricula=[inactive_curriculum, cls.curriculum], + ) - cls.course_id = CourseKey.from_string('course-v1:edX+ToyX+Toy_Course') - _ = CourseOverviewFactory.create(id=cls.course_id) + cls.course_not_in_program = CourseFactory() + cls.course_not_in_program_id = CourseKey.from_string( + cls.course_not_in_program["course_runs"][0]["key"] + ) cls.password = 'password' - cls.student = UserFactory.create(username='student', password=cls.password) - cls.global_staff = GlobalStaffFactory.create(username='global-staff', password=cls.password) + cls.student = UserFactory(username='student', password=cls.password) + cls.global_staff = GlobalStaffFactory(username='global-staff', password=cls.password) + + def setUp(self): + super(ListViewTestMixin, self).setUp() + self.set_program_in_catalog_cache(self.program_uuid, self.program) @classmethod def tearDownClass(cls): super(ListViewTestMixin, cls).tearDownClass() cls.end_cache_isolation() - def setUp(self): - super(ListViewTestMixin, self).setUp() - - self.set_program_in_catalog_cache(self.program_uuid, self.program) - self.curriculum = next(c for c in self.program['curricula'] if c['is_active']) - self.course = self.curriculum['courses'][0] - self.course_run = self.course["course_runs"][0] - self.course_key = CourseKey.from_string(self.course_run["key"]) - CourseOverviewFactory(id=self.course_key) - self.course_not_in_program = CourseFactory() - self.course_not_in_program_key = CourseKey.from_string( - self.course_not_in_program["course_runs"][0]["key"] - ) - CourseOverviewFactory(id=self.course_not_in_program_key) - def get_url(self, program_uuid=None, course_id=None): """ Returns the primary URL requested by the test case. """ kwargs = {'program_uuid': program_uuid or self.program_uuid} @@ -182,7 +180,7 @@ class UserProgramReadOnlyAccessViewTest(ListViewTestMixin, APITestCase): mock_return_value = [program for program in self.mock_program_data if program['type'] == program_type] with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_programs_by_type', + _VIEW_MOCK_FMT.format('get_programs_by_type'), autospec=True, return_value=mock_return_value ) as mock_get_programs_by_type: @@ -196,7 +194,7 @@ class UserProgramReadOnlyAccessViewTest(ListViewTestMixin, APITestCase): self.client.login(username=self.course_staff.username, password=self.password) with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_programs', + _VIEW_MOCK_FMT.format('get_programs'), autospec=True, return_value=[self.mock_program_data[0]] ) as mock_get_programs: @@ -215,7 +213,7 @@ class UserProgramReadOnlyAccessViewTest(ListViewTestMixin, APITestCase): self.client.login(username=self.course_staff.username, password=self.password) with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_programs', + _VIEW_MOCK_FMT.format('get_programs'), autospec=True, side_effect=[[self.mock_program_data[0]], [self.mock_program_data[2]]] ) as mock_get_programs: @@ -228,7 +226,7 @@ class UserProgramReadOnlyAccessViewTest(ListViewTestMixin, APITestCase): mock.call(course=other_course_key), ], any_order=True) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True, return_value=None) + @mock.patch(_VIEW_MOCK_FMT.format('get_programs'), autospec=True, return_value=None) def test_learner_200_if_no_programs_enrolled(self, mock_get_programs): self.client.login(username=self.student.username, password=self.password) response = self.client.get(reverse(self.view_name)) @@ -249,7 +247,7 @@ class UserProgramReadOnlyAccessViewTest(ListViewTestMixin, APITestCase): self.client.login(username=self.student.username, password=self.password) with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_programs', + _VIEW_MOCK_FMT.format('get_programs'), autospec=True, return_value=self.mock_program_data ) as mock_get_programs: @@ -294,27 +292,25 @@ class ProgramEnrollmentListTest(ListViewTestMixin, APITestCase): """ ProgramEnrollment.objects.filter(program_uuid=self.program_uuid).delete() - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True, return_value=None) - def test_404_if_no_program_with_key(self, mock_get_programs): + def test_404_if_no_program_with_key(self): self.client.login(username=self.global_staff.username, password=self.password) - response = self.client.get(self.get_url(self.program_uuid)) + fake_program_uuid = UUID(self.program_uuid_tmpl.format(88)) + response = self.client.get(self.get_url(fake_program_uuid)) assert status.HTTP_404_NOT_FOUND == response.status_code - mock_get_programs.assert_called_once_with(uuid=self.program_uuid) def test_403_if_not_staff(self): self.client.login(username=self.student.username, password=self.password) - response = self.client.get(self.get_url(self.program_uuid)) + response = self.client.get(self.get_url()) assert status.HTTP_403_FORBIDDEN == response.status_code def test_401_if_anonymous(self): - response = self.client.get(self.get_url(self.program_uuid)) + response = self.client.get(self.get_url()) assert status.HTTP_401_UNAUTHORIZED == response.status_code def test_200_empty_results(self): self.client.login(username=self.global_staff.username, password=self.password) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.get(self.get_url(self.program_uuid)) + response = self.client.get(self.get_url()) assert status.HTTP_200_OK == response.status_code expected = { @@ -328,8 +324,7 @@ class ProgramEnrollmentListTest(ListViewTestMixin, APITestCase): self.client.login(username=self.global_staff.username, password=self.password) self.create_program_enrollments() - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.get(self.get_url(self.program_uuid)) + response = self.client.get(self.get_url()) assert status.HTTP_200_OK == response.status_code expected = { @@ -360,46 +355,45 @@ class ProgramEnrollmentListTest(ListViewTestMixin, APITestCase): self.client.login(username=self.global_staff.username, password=self.password) self.create_program_enrollments() - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - url = self.get_url(self.program_uuid) + '?page_size=2' - response = self.client.get(url) + url = self.get_url() + '?page_size=2' + response = self.client.get(url) - assert status.HTTP_200_OK == response.status_code - expected_results = [ - { - 'student_key': 'user-0', 'status': 'pending', 'account_exists': False, - 'curriculum_uuid': text_type(self.curriculum_uuid), - }, - { - 'student_key': 'user-1', 'status': 'pending', 'account_exists': False, - 'curriculum_uuid': text_type(self.curriculum_uuid), - }, - ] - assert expected_results == response.data['results'] - # there's going to be a 'cursor' query param, but we have no way of knowing it's value - assert response.data['next'] is not None - assert self.get_url(self.program_uuid) in response.data['next'] - assert '?cursor=' in response.data['next'] - assert response.data['previous'] is None + assert status.HTTP_200_OK == response.status_code + expected_results = [ + { + 'student_key': 'user-0', 'status': 'pending', 'account_exists': False, + 'curriculum_uuid': text_type(self.curriculum_uuid), + }, + { + 'student_key': 'user-1', 'status': 'pending', 'account_exists': False, + 'curriculum_uuid': text_type(self.curriculum_uuid), + }, + ] + assert expected_results == response.data['results'] + # there's going to be a 'cursor' query param, but we have no way of knowing it's value + assert response.data['next'] is not None + assert self.get_url() in response.data['next'] + assert '?cursor=' in response.data['next'] + assert response.data['previous'] is None - next_response = self.client.get(response.data['next']) - assert status.HTTP_200_OK == next_response.status_code - next_expected_results = [ - { - 'student_key': 'user-2', 'status': 'enrolled', 'account_exists': True, - 'curriculum_uuid': text_type(self.curriculum_uuid), - }, - { - 'student_key': 'user-3', 'status': 'enrolled', 'account_exists': True, - 'curriculum_uuid': text_type(self.curriculum_uuid), - }, - ] - assert next_expected_results == next_response.data['results'] - assert next_response.data['next'] is None - # there's going to be a 'cursor' query param, but we have no way of knowing it's value - assert next_response.data['previous'] is not None - assert self.get_url(self.program_uuid) in next_response.data['previous'] - assert '?cursor=' in next_response.data['previous'] + next_response = self.client.get(response.data['next']) + assert status.HTTP_200_OK == next_response.status_code + next_expected_results = [ + { + 'student_key': 'user-2', 'status': 'enrolled', 'account_exists': True, + 'curriculum_uuid': text_type(self.curriculum_uuid), + }, + { + 'student_key': 'user-3', 'status': 'enrolled', 'account_exists': True, + 'curriculum_uuid': text_type(self.curriculum_uuid), + }, + ] + assert next_expected_results == next_response.data['results'] + assert next_response.data['next'] is None + # there's going to be a 'cursor' query param, but we have no way of knowing it's value + assert next_response.data['previous'] is not None + assert self.get_url() in next_response.data['previous'] + assert '?cursor=' in next_response.data['previous'] class ProgramEnrollmentDataMixin(object): @@ -438,7 +432,7 @@ class ProgramEnrollmentDataMixin(object): course_enrollment = None if program_enrollment.user: course_enrollment = CourseEnrollmentFactory.create( - course_id=self.course_key, + course_id=self.course_id, user=program_enrollment.user, mode=CourseMode.MASTERS ) @@ -446,7 +440,7 @@ class ProgramEnrollmentDataMixin(object): course_enrollment.save() return ProgramCourseEnrollmentFactory.create( program_enrollment=program_enrollment, - course_key=self.course_key, + course_key=self.course_id, course_enrollment=course_enrollment, status=course_status, ) @@ -476,7 +470,7 @@ class BaseCourseEnrollmentTestsMixin(ProgramEnrollmentDataMixin, ListViewTestMix def setUp(self): super(BaseCourseEnrollmentTestsMixin, self).setUp() - self.default_url = self.get_url(self.program_uuid, self.course_key) + self.default_url = self.get_url(course_id=self.course_id) self.log_in_staff() def assert_program_course_enrollment(self, external_user_key, expected_status, has_user, mode=CourseMode.MASTERS): @@ -489,12 +483,12 @@ class BaseCourseEnrollmentTestsMixin(ProgramEnrollmentDataMixin, ListViewTestMix program_enrollment__program_uuid=self.program_uuid ) self.assertEqual(expected_status, enrollment.status) - self.assertEqual(self.course_key, enrollment.course_key) + self.assertEqual(self.course_id, enrollment.course_key) course_enrollment = enrollment.course_enrollment if has_user: self.assertIsNotNone(course_enrollment) self.assertEqual(expected_status == "active", course_enrollment.is_active) - self.assertEqual(self.course_key, course_enrollment.course_id) + self.assertEqual(self.course_id, course_enrollment.course_id) self.assertEqual(mode, course_enrollment.mode) else: self.assertIsNone(course_enrollment) @@ -520,9 +514,9 @@ class BaseCourseEnrollmentTestsMixin(ProgramEnrollmentDataMixin, ListViewTestMix def test_404_not_found(self): nonexistant_course_key = CourseKey.from_string("course-v1:fake+fake+fake") paths = [ - self.get_url(uuid4(), self.course_key), # program not found - self.get_url(self.program_uuid, nonexistant_course_key), # course not found - self.get_url(self.program_uuid, self.course_not_in_program_key), # course not in program + self.get_url(uuid4(), self.course_id), # program not found + self.get_url(course_id=nonexistant_course_key), # course not found + self.get_url(course_id=self.course_not_in_program_id), # course not in program ] request_data = [self.learner_enrollment("learner-1")] for path_404 in paths: @@ -590,7 +584,6 @@ class BaseCourseEnrollmentTestsMixin(ProgramEnrollmentDataMixin, ListViewTestMix request_data = [self.learner_enrollment('learner-1')] request_data.extend(bad_records) response = self.request(self.default_url, request_data) - self.assertEqual(response.status_code, 400) self.assertIn('invalid enrollment record', response.data) @@ -661,7 +654,7 @@ class CourseEnrollmentPostTests(BaseCourseEnrollmentTestsMixin, APITestCase): that enrollment should be linked but not overwritten as masters. """ CourseEnrollmentFactory.create( - course_id=self.course_key, + course_id=self.course_id, user=self.student, mode=CourseMode.VERIFIED ) @@ -825,33 +818,31 @@ class ProgramCourseEnrollmentListTest(ListViewTestMixin, APITestCase): course_key=self.course_id ).delete() - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True, return_value=None) - def test_404_if_no_program_with_key(self, mock_get_programs): + def test_404_if_no_program_with_key(self): self.client.login(username=self.global_staff.username, password=self.password) - response = self.client.get(self.get_url(self.program_uuid, self.course_id)) + fake_program_uuid = UUID(self.program_uuid_tmpl.format(88)) + response = self.client.get(self.get_url(fake_program_uuid, self.course_id)) assert status.HTTP_404_NOT_FOUND == response.status_code - mock_get_programs.assert_called_once_with(uuid=self.program_uuid) def test_404_if_course_does_not_exist(self): other_course_key = CourseKey.from_string('course-v1:edX+ToyX+Other_Course') self.client.login(username=self.global_staff.username, password=self.password) - response = self.client.get(self.get_url(self.program_uuid, other_course_key)) + response = self.client.get(self.get_url(course_id=other_course_key)) assert status.HTTP_404_NOT_FOUND == response.status_code def test_403_if_not_staff(self): self.client.login(username=self.student.username, password=self.password) - response = self.client.get(self.get_url(self.program_uuid, self.course_id)) + response = self.client.get(self.get_url(course_id=self.course_id)) assert status.HTTP_403_FORBIDDEN == response.status_code def test_401_if_anonymous(self): - response = self.client.get(self.get_url(self.program_uuid, self.course_id)) + response = self.client.get(self.get_url(course_id=self.course_id)) assert status.HTTP_401_UNAUTHORIZED == response.status_code def test_200_empty_results(self): self.client.login(username=self.global_staff.username, password=self.password) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.get(self.get_url(self.program_uuid, self.course_id)) + response = self.client.get(self.get_url(course_id=self.course_id)) assert status.HTTP_200_OK == response.status_code expected = { @@ -865,8 +856,7 @@ class ProgramCourseEnrollmentListTest(ListViewTestMixin, APITestCase): self.client.login(username=self.global_staff.username, password=self.password) self.create_course_enrollments() - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.get(self.get_url(self.program_uuid, self.course_id)) + response = self.client.get(self.get_url(course_id=self.course_id)) assert status.HTTP_200_OK == response.status_code expected = { @@ -889,47 +879,47 @@ class ProgramCourseEnrollmentListTest(ListViewTestMixin, APITestCase): self.client.login(username=self.global_staff.username, password=self.password) self.create_course_enrollments() - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - url = self.get_url(self.program_uuid, self.course_id) + '?page_size=1' - response = self.client.get(url) + url = self.get_url(course_id=self.course_id) + '?page_size=1' + response = self.client.get(url) - assert status.HTTP_200_OK == response.status_code - expected_results = [ - { - 'student_key': 'user-0', 'status': 'active', 'account_exists': True, - 'curriculum_uuid': text_type(self.curriculum_uuid), - }, - ] - assert expected_results == response.data['results'] - # there's going to be a 'cursor' query param, but we have no way of knowing it's value - assert response.data['next'] is not None - assert self.get_url(self.program_uuid, self.course_id) in response.data['next'] - assert '?cursor=' in response.data['next'] - assert response.data['previous'] is None + assert status.HTTP_200_OK == response.status_code + expected_results = [ + { + 'student_key': 'user-0', 'status': 'active', 'account_exists': True, + 'curriculum_uuid': text_type(self.curriculum_uuid), + }, + ] + assert expected_results == response.data['results'] + # there's going to be a 'cursor' query param, but we have no way of knowing it's value + assert response.data['next'] is not None + assert self.get_url(course_id=self.course_id) in response.data['next'] + assert '?cursor=' in response.data['next'] + assert response.data['previous'] is None - next_response = self.client.get(response.data['next']) - assert status.HTTP_200_OK == next_response.status_code - next_expected_results = [ - { - 'student_key': 'user-0', 'status': 'inactive', 'account_exists': True, - 'curriculum_uuid': text_type(self.other_curriculum_uuid), - }, - ] - assert next_expected_results == next_response.data['results'] - assert next_response.data['next'] is None - # there's going to be a 'cursor' query param, but we have no way of knowing it's value - assert next_response.data['previous'] is not None - assert self.get_url(self.program_uuid, self.course_id) in next_response.data['previous'] - assert '?cursor=' in next_response.data['previous'] + next_response = self.client.get(response.data['next']) + assert status.HTTP_200_OK == next_response.status_code + next_expected_results = [ + { + 'student_key': 'user-0', 'status': 'inactive', 'account_exists': True, + 'curriculum_uuid': text_type(self.other_curriculum_uuid), + }, + ] + assert next_expected_results == next_response.data['results'] + assert next_response.data['next'] is None + # there's going to be a 'cursor' query param, but we have no way of knowing it's value + assert next_response.data['previous'] is not None + assert self.get_url(course_id=self.course_id) in next_response.data['previous'] + assert '?cursor=' in next_response.data['previous'] @ddt.ddt -class BaseProgramEnrollmentWriteTestsMixin(object): +class BaseProgramEnrollmentWriteTestsMixin(ListViewTestMixin): """ Mixin class that defines common tests for program enrollment write endpoints """ add_uuid = False - program_uuid = '00000000-1111-2222-3333-444444444444' success_status = 200 + view_name = 'programs_api:v1:program_enrollments' + def student_enrollment(self, enrollment_status, external_user_key=None, prepare_student=False): """ Convenience method to create a student enrollment record """ enrollment = { @@ -945,11 +935,6 @@ class BaseProgramEnrollmentWriteTestsMixin(object): def prepare_student(self, enrollment): pass - def get_url(self, program_uuid=None): - if program_uuid is None: - program_uuid = uuid4() - return reverse('programs_api:v1:program_enrollments', args=[program_uuid]) - def test_unauthenticated(self): self.client.logout() request_data = [self.student_enrollment('enrolled')] @@ -958,8 +943,7 @@ class BaseProgramEnrollmentWriteTestsMixin(object): def test_enrollment_payload_limit(self): request_data = [self.student_enrollment('enrolled') for _ in range(MAX_ENROLLMENT_RECORDS + 1)] - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.request(self.get_url(), json.dumps(request_data), content_type='application/json') + response = self.request(self.get_url(), json.dumps(request_data), content_type='application/json') self.assertEqual(response.status_code, status.HTTP_413_REQUEST_ENTITY_TOO_LARGE) def test_duplicate_enrollment(self): @@ -968,19 +952,17 @@ class BaseProgramEnrollmentWriteTestsMixin(object): self.student_enrollment('enrolled', '001'), ] - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.request(self.get_url(), json.dumps(request_data), content_type='application/json') + response = self.request(self.get_url(), json.dumps(request_data), content_type='application/json') self.assertEqual(response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY) self.assertEqual(response.data, {'001': 'duplicated'}) def test_unprocessable_enrollment(self): - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.request( - self.get_url(), - json.dumps([{'status': 'enrolled'}]), - content_type='application/json' - ) + response = self.request( + self.get_url(), + json.dumps([{'status': 'enrolled'}]), + content_type='application/json' + ) self.assertEqual(response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY) self.assertEqual(response.data, 'invalid enrollment record') @@ -989,8 +971,7 @@ class BaseProgramEnrollmentWriteTestsMixin(object): self.client.login(username=student.username, password='password') request_data = [self.student_enrollment('enrolled')] - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.request(self.get_url(), json.dumps(request_data), content_type='application/json') + response = self.request(self.get_url(), json.dumps(request_data), content_type='application/json') self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_program_not_found(self): @@ -1009,13 +990,11 @@ class BaseProgramEnrollmentWriteTestsMixin(object): [{'status': 'pending'}, {'status': 'pending'}], ) def test_no_student_key(self, bad_records): - program_uuid = uuid4() - url = reverse('programs_api:v1:program_enrollments', args=[program_uuid]) + url = self.get_url() enrollments = [self.student_enrollment('enrolled', '001', True)] enrollments.extend(bad_records) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.request(url, json.dumps(enrollments), content_type='application/json') + response = self.request(url, json.dumps(enrollments), content_type='application/json') self.assertEqual(422, response.status_code) self.assertEqual('invalid enrollment record', response.data) @@ -1025,14 +1004,13 @@ class BaseProgramEnrollmentWriteTestsMixin(object): enrollment = self.student_enrollment('enrolled', 'learner-01') enrollment['favorite_pokemon'] = 'bulbasaur' enrollments = [enrollment] - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_user_by_program_id', - autospec=True, - return_value=None - ): - url = self.get_url(program_uuid=self.program_uuid) - response = self.request(url, json.dumps(enrollments), content_type='application/json') + with mock.patch( + _VIEW_MOCK_FMT.format('get_user_by_program_id'), + autospec=True, + return_value=None + ): + url = self.get_url() + response = self.request(url, json.dumps(enrollments), content_type='application/json') self.assertEqual(self.success_status, response.status_code) self.assertDictEqual( response.data, @@ -1049,23 +1027,21 @@ class ProgramEnrollmentViewPostTests(BaseProgramEnrollmentWriteTestsMixin, APITe success_status = status.HTTP_201_CREATED success_status = 201 + view_name = 'programs_api:v1:program_enrollments' + def setUp(self): super(ProgramEnrollmentViewPostTests, self).setUp() self.request = self.client.post - global_staff = GlobalStaffFactory.create(username='global-staff', password='password') - self.client.login(username=global_staff.username, password='password') + self.client.login(username=self.global_staff.username, password='password') def tearDown(self): super(ProgramEnrollmentViewPostTests, self).tearDown() ProgramEnrollment.objects.all().delete() def test_successful_program_enrollments_no_existing_user(self): - program_key = uuid4() statuses = ['pending', 'enrolled', 'pending'] external_user_keys = ['abc1', 'efg2', 'hij3'] - - curriculum_uuid = uuid4() - curriculum_uuids = [curriculum_uuid, curriculum_uuid, uuid4()] + curriculum_uuids = [self.curriculum_uuid, self.curriculum_uuid, uuid4()] post_data = [ { REQUEST_STUDENT_KEY: e, @@ -1075,14 +1051,13 @@ class ProgramEnrollmentViewPostTests(BaseProgramEnrollmentWriteTestsMixin, APITe for e, s, c in zip(external_user_keys, statuses, curriculum_uuids) ] - url = reverse('programs_api:v1:program_enrollments', args=[program_key]) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_user_by_program_id', - autospec=True, - return_value=None - ): - response = self.client.post(url, json.dumps(post_data), content_type='application/json') + url = self.get_url(program_uuid=0) + with mock.patch( + _VIEW_MOCK_FMT.format('get_user_by_program_id'), + autospec=True, + return_value=None + ): + response = self.client.post(url, json.dumps(post_data), content_type='application/json') self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -1090,75 +1065,60 @@ class ProgramEnrollmentViewPostTests(BaseProgramEnrollmentWriteTestsMixin, APITe enrollment = ProgramEnrollment.objects.get(external_user_key=external_user_keys[i]) self.assertEqual(enrollment.external_user_key, external_user_keys[i]) - self.assertEqual(enrollment.program_uuid, program_key) + self.assertEqual(enrollment.program_uuid, self.program_uuid) self.assertEqual(enrollment.status, statuses[i]) self.assertEqual(enrollment.curriculum_uuid, curriculum_uuids[i]) - self.assertEqual(enrollment.user, None) + self.assertIsNone(enrollment.user) def test_successful_program_enrollments_existing_user(self): - program_key = uuid4() - curriculum_uuid = uuid4() - post_data = [ { 'status': 'enrolled', REQUEST_STUDENT_KEY: 'abc1', - 'curriculum_uuid': str(curriculum_uuid) + 'curriculum_uuid': str(self.curriculum_uuid) } ] - user = User.objects.create_user('test_user', 'test@example.com', 'password') - - url = reverse('programs_api:v1:program_enrollments', args=[program_key]) - - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_user_by_program_id', - autospec=True, - return_value=user - ): - response = self.client.post(url, json.dumps(post_data), content_type='application/json') - + url = self.get_url() + with mock.patch( + _VIEW_MOCK_FMT.format('get_user_by_program_id'), + autospec=True, + return_value=user + ): + response = self.client.post(url, json.dumps(post_data), content_type='application/json') self.assertEqual(response.status_code, status.HTTP_201_CREATED) - enrollment = ProgramEnrollment.objects.get(external_user_key='abc1') - self.assertEqual(enrollment.external_user_key, 'abc1') - self.assertEqual(enrollment.program_uuid, program_key) + self.assertEqual(enrollment.program_uuid, self.program_uuid) self.assertEqual(enrollment.status, 'enrolled') - self.assertEqual(enrollment.curriculum_uuid, curriculum_uuid) + self.assertEqual(enrollment.curriculum_uuid, self.curriculum_uuid) self.assertEqual(enrollment.user, user) def test_program_enrollments_no_idp(self): - program_key = uuid4() - curriculum_uuid = uuid4() - post_data = [ { 'status': 'enrolled', REQUEST_STUDENT_KEY: 'abc{}'.format(i), - 'curriculum_uuid': str(curriculum_uuid) + 'curriculum_uuid': str(self.curriculum_uuid) } for i in range(3) ] - url = reverse('programs_api:v1:program_enrollments', args=[program_key]) - - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - with mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_user_by_program_id', - autospec=True, - side_effect=ProviderDoesNotExistException() - ): - response = self.client.post(url, json.dumps(post_data), content_type='application/json') + url = self.get_url() + with mock.patch( + _VIEW_MOCK_FMT.format('get_user_by_program_id'), + autospec=True, + side_effect=ProviderDoesNotExistException() + ): + response = self.client.post(url, json.dumps(post_data), content_type='application/json') self.assertEqual(response.status_code, status.HTTP_201_CREATED) for i in range(3): enrollment = ProgramEnrollment.objects.get(external_user_key='abc{}'.format(i)) - self.assertEqual(enrollment.program_uuid, program_key) + self.assertEqual(enrollment.program_uuid, self.program_uuid) self.assertEqual(enrollment.status, 'enrolled') - self.assertEqual(enrollment.curriculum_uuid, curriculum_uuid) + self.assertEqual(enrollment.curriculum_uuid, self.curriculum_uuid) self.assertIsNone(enrollment.user) @@ -1173,17 +1133,6 @@ class ProgramEnrollmentViewPatchTests(BaseProgramEnrollmentWriteTestsMixin, APIT def setUp(self): super(ProgramEnrollmentViewPatchTests, self).setUp() self.request = self.client.patch - - self.curriculum_uuid = 'aaaaaaaa-1111-2222-3333-444444444444' - self.other_curriculum_uuid = 'bbbbbbbb-1111-2222-3333-444444444444' - - self.course_id = CourseKey.from_string('course-v1:edX+ToyX+Toy_Course') - _ = CourseOverviewFactory.create(id=self.course_id) - - self.password = 'password' - self.student = UserFactory.create(username='student', password=self.password) - self.global_staff = GlobalStaffFactory.create(username='global-staff', password=self.password) - self.client.login(username=self.global_staff.username, password=self.password) def prepare_student(self, enrollment): @@ -1214,9 +1163,8 @@ class ProgramEnrollmentViewPatchTests(BaseProgramEnrollmentWriteTestsMixin, APIT {REQUEST_STUDENT_KEY: 'user-3', 'status': 'enrolled'}, ] - url = reverse('programs_api:v1:program_enrollments', args=[self.program_uuid]) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.patch(url, json.dumps(post_data), content_type='application/json') + url = self.get_url() + response = self.client.patch(url, json.dumps(post_data), content_type='application/json') for enrollment in enrollments.values(): enrollment.refresh_from_db() @@ -1257,9 +1205,8 @@ class ProgramEnrollmentViewPatchTests(BaseProgramEnrollmentWriteTestsMixin, APIT self.student_enrollment('enrolled', 'user-1'), ] - url = reverse('programs_api:v1:program_enrollments', args=[self.program_uuid]) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.patch(url, json.dumps(patch_data), content_type='application/json') + url = self.get_url() + response = self.client.patch(url, json.dumps(patch_data), content_type='application/json') for enrollment in enrollments.values(): enrollment.refresh_from_db() @@ -1298,9 +1245,8 @@ class ProgramEnrollmentViewPatchTests(BaseProgramEnrollmentWriteTestsMixin, APIT self.student_enrollment('enrolled', 'user-who-is-not-in-program'), ] - url = reverse('programs_api:v1:program_enrollments', args=[self.program_uuid]) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.patch(url, json.dumps(patch_data), content_type='application/json') + url = self.get_url() + response = self.client.patch(url, json.dumps(patch_data), content_type='application/json') for enrollment in enrollments.values(): enrollment.refresh_from_db() @@ -1333,15 +1279,9 @@ class ProgramEnrollmentViewPutTests(BaseProgramEnrollmentWriteTestsMixin, APITes def setUp(self): super(ProgramEnrollmentViewPutTests, self).setUp() self.request = self.client.put - - self.program_uuid = '00000000-1111-2222-3333-444444444444' - self.curriculum_uuid = 'aaaaaaaa-1111-2222-3333-444444444444' - - self.global_staff = GlobalStaffFactory.create(username='global-staff', password='password') self.client.login(username=self.global_staff.username, password='password') - patch_get_user = mock.patch( - 'lms.djangoapps.program_enrollments.api.v1.views.get_user_by_program_id', + _VIEW_MOCK_FMT.format('get_user_by_program_id'), autospec=True, return_value=None ) @@ -1371,9 +1311,8 @@ class ProgramEnrollmentViewPutTests(BaseProgramEnrollmentWriteTestsMixin, APITes external_user_key=enrollment[REQUEST_STUDENT_KEY], ) - url = self.get_url(program_uuid=self.program_uuid) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.put(url, json.dumps(request_data), content_type='application/json') + url = self.get_url() + response = self.client.put(url, json.dumps(request_data), content_type='application/json') self.assertEqual(self.success_status, response.status_code) self.assertEqual(5, len(response.data)) for response_status in response.data.values(): @@ -1397,9 +1336,8 @@ class ProgramEnrollmentViewPutTests(BaseProgramEnrollmentWriteTestsMixin, APITes external_user_key='learner-04', ) - url = self.get_url(program_uuid=self.program_uuid) - with mock.patch('lms.djangoapps.program_enrollments.api.v1.views.get_programs', autospec=True): - response = self.client.put(url, json.dumps(request_data), content_type='application/json') + url = self.get_url() + response = self.client.put(url, json.dumps(request_data), content_type='application/json') self.assertEqual(self.success_status, response.status_code) self.assertEqual(4, len(response.data)) for response_status in response.data.values(): @@ -1468,7 +1406,11 @@ class ProgramCourseEnrollmentOverviewViewTests(ProgramCacheTestCaseMixin, Shared ) # create program - self.program = self.setup_catalog_cache(self.program_uuid, 'organization_key') + catalog_org = OrganizationFactory(key='organization_key') + self.program = ProgramFactory( + uuid=self.program_uuid, + authoring_organizations=[catalog_org], + ) self.program['curricula'][0]['courses'].append(self.course) self.set_program_in_catalog_cache(self.program_uuid, self.program) @@ -1546,7 +1488,7 @@ class ProgramCourseEnrollmentOverviewViewTests(ProgramCacheTestCaseMixin, Shared expected_course_run_ids.add(text_type(other_course_key)) self.assertEqual(expected_course_run_ids, actual_course_run_ids) - _GET_RESUME_URL = 'lms.djangoapps.program_enrollments.api.v1.views.get_resume_urls_for_enrollments' + _GET_RESUME_URL = _VIEW_MOCK_FMT.format('get_resume_urls_for_enrollments') @mock.patch(_GET_RESUME_URL) def test_blank_resume_url_omitted(self, mock_get_resume_urls): @@ -1649,7 +1591,8 @@ class ProgramCourseEnrollmentOverviewViewTests(ProgramCacheTestCaseMixin, Shared display_name='unit_1' ) - with mock.patch('lms.djangoapps.program_enrollments.api.api.get_dates_for_course') as mock_get_dates: + mock_path = _REST_API_MOCK_FMT.format('v1.utils.get_dates_for_course') + with mock.patch(mock_path) as mock_get_dates: mock_get_dates.return_value = { (section_1.location, 'due'): section_1.due, (section_1.location, 'start'): section_1.start, @@ -1900,34 +1843,34 @@ class ProgramCourseGradeListTest(ProgramEnrollmentDataMixin, ListViewTestMixin, def mock_course_grade(percent=75.0, passed=True, letter_grade='B'): return mock.MagicMock(percent=percent, passed=passed, letter_grade=letter_grade) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.CourseGradeFactory') + @mock.patch(_VIEW_MOCK_FMT.format('CourseGradeFactory')) def test_204_no_grades_to_return(self, mock_course_grade_factory): mock_course_grade_factory.return_value.iter.return_value = [] self.log_in_staff() - url = self.get_url(program_uuid=self.program_uuid, course_id=self.course_key) + url = self.get_url(course_id=self.course_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertEqual(response.data['results'], []) def test_401_if_unauthenticated(self): - url = self.get_url(program_uuid=self.program_uuid, course_id=self.course_key) + url = self.get_url(course_id=self.course_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) def test_403_if_not_staff(self): self.log_in_non_staff() - url = self.get_url(program_uuid=self.program_uuid, course_id=self.course_key) + url = self.get_url(course_id=self.course_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_404_not_found(self): - fake_program_uuid = self.program_uuid_tmpl.format(99) + fake_program_uuid = UUID(self.program_uuid_tmpl.format(99)) self.log_in_staff() - url = self.get_url(program_uuid=fake_program_uuid, course_id=self.course_key) + url = self.get_url(program_uuid=fake_program_uuid, course_id=self.course_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.CourseGradeFactory') + @mock.patch(_VIEW_MOCK_FMT.format('CourseGradeFactory')) def test_200_grades_with_no_exceptions(self, mock_course_grade_factory): other_student = UserFactory.create(username='other_student') self.create_program_and_course_enrollments('student-key', user=self.student) @@ -1939,7 +1882,7 @@ class ProgramCourseGradeListTest(ProgramEnrollmentDataMixin, ListViewTestMixin, mock_course_grade_factory.return_value.iter.return_value = mock_course_grades self.log_in_staff() - url = self.get_url(program_uuid=self.program_uuid, course_id=self.course_key) + url = self.get_url(course_id=self.course_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) expected_results = [ @@ -1958,7 +1901,7 @@ class ProgramCourseGradeListTest(ProgramEnrollmentDataMixin, ListViewTestMixin, ] self.assertEqual(response.data['results'], expected_results) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.CourseGradeFactory') + @mock.patch(_VIEW_MOCK_FMT.format('CourseGradeFactory')) def test_207_grades_with_some_exceptions(self, mock_course_grade_factory): other_student = UserFactory.create(username='other_student') self.create_program_and_course_enrollments('student-key', user=self.student) @@ -1970,7 +1913,7 @@ class ProgramCourseGradeListTest(ProgramEnrollmentDataMixin, ListViewTestMixin, mock_course_grade_factory.return_value.iter.return_value = mock_course_grades self.log_in_staff() - url = self.get_url(program_uuid=self.program_uuid, course_id=self.course_key) + url = self.get_url(course_id=self.course_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS) expected_results = [ @@ -1987,7 +1930,7 @@ class ProgramCourseGradeListTest(ProgramEnrollmentDataMixin, ListViewTestMixin, ] self.assertEqual(response.data['results'], expected_results) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.CourseGradeFactory') + @mock.patch(_VIEW_MOCK_FMT.format('CourseGradeFactory')) def test_422_grades_with_only_exceptions(self, mock_course_grade_factory): other_student = UserFactory.create(username='other_student') self.create_program_and_course_enrollments('student-key', user=self.student) @@ -1999,7 +1942,7 @@ class ProgramCourseGradeListTest(ProgramEnrollmentDataMixin, ListViewTestMixin, mock_course_grade_factory.return_value.iter.return_value = mock_course_grades self.log_in_staff() - url = self.get_url(program_uuid=self.program_uuid, course_id=self.course_key) + url = self.get_url(course_id=self.course_id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY) expected_results = [ @@ -2028,10 +1971,10 @@ class EnrollmentDataResetViewTests(ProgramCacheTestCaseMixin, APITestCase): super(EnrollmentDataResetViewTests, self).setUp() self.start_cache_isolation() - self.organization = OrganizationFactory(short_name='uox') + self.organization = LMSOrganizationFactory(short_name='uox') self.provider = SAMLProviderConfigFactory(organization=self.organization) - self.global_staff = GlobalStaffFactory.create(username='global-staff', password='password') + self.global_staff = GlobalStaffFactory(username='global-staff', password='password') self.client.login(username=self.global_staff.username, password='password') def request(self, organization): @@ -2045,14 +1988,14 @@ class EnrollmentDataResetViewTests(ProgramCacheTestCaseMixin, APITestCase): self.end_cache_isolation() super(EnrollmentDataResetViewTests, self).tearDown() - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.call_command', autospec=True) + @mock.patch(_VIEW_MOCK_FMT.format('call_command'), autospec=True) def test_feature_disabled_by_default(self, mock_call_command): response = self.request(self.organization.short_name) self.assertEqual(response.status_code, status.HTTP_501_NOT_IMPLEMENTED) mock_call_command.assert_has_calls([]) @override_settings(FEATURES=FEATURES_WITH_ENABLED) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.call_command', autospec=True) + @mock.patch(_VIEW_MOCK_FMT.format('call_command'), autospec=True) def test_403_for_non_staff(self, mock_call_command): student = UserFactory.create(username='student', password='password') self.client.login(username=student.username, password='password') @@ -2061,7 +2004,7 @@ class EnrollmentDataResetViewTests(ProgramCacheTestCaseMixin, APITestCase): mock_call_command.assert_has_calls([]) @override_settings(FEATURES=FEATURES_WITH_ENABLED) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.call_command', autospec=True) + @mock.patch(_VIEW_MOCK_FMT.format('call_command'), autospec=True) def test_reset(self, mock_call_command): programs = [str(uuid4()), str(uuid4())] self.set_org_in_catalog_cache(self.organization, programs) @@ -2074,9 +2017,9 @@ class EnrollmentDataResetViewTests(ProgramCacheTestCaseMixin, APITestCase): ]) @override_settings(FEATURES=FEATURES_WITH_ENABLED) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.call_command', autospec=True) + @mock.patch(_VIEW_MOCK_FMT.format('call_command'), autospec=True) def test_reset_without_idp(self, mock_call_command): - organization = OrganizationFactory() + organization = LMSOrganizationFactory() programs = [str(uuid4()), str(uuid4())] self.set_org_in_catalog_cache(organization, programs) @@ -2087,14 +2030,14 @@ class EnrollmentDataResetViewTests(ProgramCacheTestCaseMixin, APITestCase): ]) @override_settings(FEATURES=FEATURES_WITH_ENABLED) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.call_command', autospec=True) + @mock.patch(_VIEW_MOCK_FMT.format('call_command'), autospec=True) def test_organization_not_found(self, mock_call_command): response = self.request('yyz') self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) mock_call_command.assert_has_calls([]) @override_settings(FEATURES=FEATURES_WITH_ENABLED) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.call_command', autospec=True) + @mock.patch(_VIEW_MOCK_FMT.format('call_command'), autospec=True) def test_no_programs_doesnt_break(self, mock_call_command): programs = [] self.set_org_in_catalog_cache(self.organization, programs) @@ -2106,7 +2049,7 @@ class EnrollmentDataResetViewTests(ProgramCacheTestCaseMixin, APITestCase): ]) @override_settings(FEATURES=FEATURES_WITH_ENABLED) - @mock.patch('lms.djangoapps.program_enrollments.api.v1.views.call_command', autospec=True) + @mock.patch(_VIEW_MOCK_FMT.format('call_command'), autospec=True) def test_missing_body_content(self, mock_call_command): response = self.client.post( reverse('programs_api:v1:reset_enrollment_data'), diff --git a/lms/djangoapps/program_enrollments/api/v1/urls.py b/lms/djangoapps/program_enrollments/rest_api/v1/urls.py similarity index 88% rename from lms/djangoapps/program_enrollments/api/v1/urls.py rename to lms/djangoapps/program_enrollments/rest_api/v1/urls.py index cf8ca082ba..3aa3087587 100644 --- a/lms/djangoapps/program_enrollments/api/v1/urls.py +++ b/lms/djangoapps/program_enrollments/rest_api/v1/urls.py @@ -3,18 +3,19 @@ from __future__ import absolute_import from django.conf.urls import url -from lms.djangoapps.program_enrollments.api.v1.constants import PROGRAM_UUID_PATTERN -from lms.djangoapps.program_enrollments.api.v1.views import ( - EnrollmentDataResetView, - ProgramEnrollmentsView, - ProgramCourseEnrollmentsView, - ProgramCourseGradesView, - ProgramCourseEnrollmentOverviewView, - UserProgramReadOnlyAccessView, -) from openedx.core.constants import COURSE_ID_PATTERN -app_name = 'lms.djangoapps.program_enrollments' +from .constants import PROGRAM_UUID_PATTERN +from .views import ( + EnrollmentDataResetView, + ProgramCourseEnrollmentOverviewView, + ProgramCourseEnrollmentsView, + ProgramCourseGradesView, + ProgramEnrollmentsView, + UserProgramReadOnlyAccessView +) + +app_name = 'v1' urlpatterns = [ url( diff --git a/lms/djangoapps/program_enrollments/api/api.py b/lms/djangoapps/program_enrollments/rest_api/v1/utils.py similarity index 97% rename from lms/djangoapps/program_enrollments/api/api.py rename to lms/djangoapps/program_enrollments/rest_api/v1/utils.py index af3ec29e39..cd5504baef 100644 --- a/lms/djangoapps/program_enrollments/api/api.py +++ b/lms/djangoapps/program_enrollments/rest_api/v1/utils.py @@ -1,22 +1,20 @@ # -*- coding: utf-8 -*- """ -ProgramEnrollment internal api +ProgramEnrollment V1 API internal utilities. """ from __future__ import absolute_import, unicode_literals from datetime import datetime, timedelta -from pytz import UTC from django.urls import reverse - +from edx_when.api import get_dates_for_course +from pytz import UTC from six import iteritems from bulk_email.api import is_bulk_email_feature_enabled, is_user_opted_out_for_course -from edx_when.api import get_dates_for_course from xmodule.modulestore.django import modulestore -from lms.djangoapps.program_enrollments.api.v1.constants import ( - CourseRunProgressStatuses, -) + +from .constants import CourseRunProgressStatuses def get_due_dates(request, course_key, user): diff --git a/lms/djangoapps/program_enrollments/api/v1/views.py b/lms/djangoapps/program_enrollments/rest_api/v1/views.py similarity index 92% rename from lms/djangoapps/program_enrollments/api/v1/views.py rename to lms/djangoapps/program_enrollments/rest_api/v1/views.py index 0571950c5e..8ee5f29033 100644 --- a/lms/djangoapps/program_enrollments/api/v1/views.py +++ b/lms/djangoapps/program_enrollments/rest_api/v1/views.py @@ -12,7 +12,6 @@ from django.conf import settings from django.core.exceptions import PermissionDenied from django.core.management import call_command from django.db import transaction -from django.http import Http404 from django.utils.functional import cached_property from edx_rest_framework_extensions import permissions from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication @@ -30,29 +29,6 @@ from course_modes.models import CourseMode from lms.djangoapps.certificates.api import get_certificate_for_user from lms.djangoapps.grades.api import CourseGradeFactory, clear_prefetched_course_grades, prefetch_course_grades from lms.djangoapps.grades.rest_api.v1.utils import CourseEnrollmentPagination -from lms.djangoapps.program_enrollments.api.api import ( - get_course_run_status, - get_course_run_url, - get_due_dates, - get_emails_enabled -) -from lms.djangoapps.program_enrollments.api.v1.constants import ( - ENABLE_ENROLLMENT_RESET_FLAG, - MAX_ENROLLMENT_RECORDS, - CourseEnrollmentResponseStatuses, - ProgramEnrollmentResponseStatuses, -) -from lms.djangoapps.program_enrollments.api.v1.serializers import ( - CourseRunOverviewListSerializer, - ProgramCourseEnrollmentListSerializer, - ProgramCourseEnrollmentRequestSerializer, - ProgramCourseGradeErrorResult, - ProgramCourseGradeResult, - ProgramCourseGradeResultSerializer, - ProgramEnrollmentCreateRequestSerializer, - ProgramEnrollmentListSerializer, - ProgramEnrollmentModifyRequestSerializer -) from lms.djangoapps.program_enrollments.models import ProgramCourseEnrollment, ProgramEnrollment from lms.djangoapps.program_enrollments.utils import ( ProviderDoesNotExistException, @@ -64,6 +40,7 @@ from openedx.core.djangoapps.catalog.utils import ( get_programs, get_programs_by_type, get_programs_for_organization, + is_course_run_in_program, normalize_program_type ) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -74,6 +51,25 @@ from student.models import CourseEnrollment from student.roles import CourseInstructorRole, CourseStaffRole, UserBasedRole from util.query import use_read_replica_if_available +from .constants import ( + ENABLE_ENROLLMENT_RESET_FLAG, + MAX_ENROLLMENT_RECORDS, + ProgramCourseResponseStatuses, + ProgramResponseStatuses +) +from .serializers import ( + CourseRunOverviewListSerializer, + ProgramCourseEnrollmentRequestSerializer, + ProgramCourseEnrollmentSerializer, + ProgramCourseGradeError, + ProgramCourseGradeOk, + ProgramCourseGradeSerializer, + ProgramEnrollmentCreateRequestSerializer, + ProgramEnrollmentModifyRequestSerializer, + ProgramEnrollmentSerializer +) +from .utils import get_course_run_status, get_course_run_url, get_due_dates, get_emails_enabled + logger = logging.getLogger(__name__) @@ -82,15 +78,15 @@ def verify_program_exists(view_func): Raises: An API error if the `program_uuid` kwarg in the wrapped function does not exist in the catalog programs cache. + + Expects to be used within a ProgramSpecificViewMixin subclass. """ @wraps(view_func) def wrapped_function(self, request, **kwargs): """ Wraps the given view_function. """ - program_uuid = kwargs['program_uuid'] - program = get_programs(uuid=program_uuid) - if not program: + if self.program is None: raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message='no program exists with given key', @@ -103,45 +99,29 @@ def verify_program_exists(view_func): def verify_course_exists_and_in_program(view_func): """ Raises: - An api error if the course run specified by the `course_key` kwarg + An api error if the course run specified by the `course_id` kwarg in the wrapped function is not part of the curriculum of the program specified by the `program_uuid` kwarg - Assumes that the program exists and that a program has exactly one active curriculum + This decorator guarantees existance of the program and course, so wrapping + alongside `verify_{program,course}_exists` is redundant. + + Expects to be used within a subclass of ProgramCourseSpecificViewMixin. """ @wraps(view_func) + @verify_program_exists @verify_course_exists def wrapped_function(self, request, **kwargs): """ Wraps view function """ - course_key = CourseKey.from_string(kwargs['course_id']) - program_uuid = kwargs['program_uuid'] - program = get_programs(uuid=program_uuid) - active_curricula = [c for c in program['curricula'] if c['is_active']] - if not active_curricula: - raise self.api_error( - status_code=status.HTTP_404_NOT_FOUND, - developer_message="the program does not have an active curriculum", - error_code='no_active_curriculum' - ) - - curriculum = active_curricula[0] - - if not is_course_in_curriculum(curriculum, course_key): + if not is_course_run_in_program(self.course_key, self.program): raise self.api_error( status_code=status.HTTP_404_NOT_FOUND, developer_message="the program's curriculum does not contain the given course", error_code='course_not_in_program' ) return view_func(self, request, **kwargs) - - def is_course_in_curriculum(curriculum, course_key): - for course in curriculum['courses']: - for course_run in course['course_runs']: - if CourseKey.from_string(course_run["key"]) == course_key: - return True - return wrapped_function @@ -152,7 +132,44 @@ class ProgramEnrollmentPagination(CourseEnrollmentPagination): page_size = 100 -class ProgramEnrollmentsView(DeveloperErrorViewMixin, PaginatedAPIView): +class ProgramSpecificViewMixin(object): + """ + A mixin for views that operate on or within a specific program. + + Requires `program_uuid` to be one of the kwargs to the view. + """ + + @cached_property + def program(self): + """ + The program specified by the `program_uuid` URL parameter. + """ + return get_programs(uuid=self.program_uuid) + + @property + def program_uuid(self): + """ + The program specified by the `program_uuid` URL parameter. + """ + return self.kwargs['program_uuid'] + + +class ProgramCourseSpecificViewMixin(ProgramSpecificViewMixin): + """ + A mixin for views that operate on or within a specific course run in a program + + Requires `course_id` to be one of the kwargs to the view. + """ + + @cached_property + def course_key(self): + """ + The course key for the course run specified by the `course_id` URL parameter. + """ + return CourseKey.from_string(self.kwargs['course_id']) + + +class ProgramEnrollmentsView(ProgramSpecificViewMixin, DeveloperErrorViewMixin, PaginatedAPIView): """ A view for Create/Read/Update methods on Program Enrollment data. @@ -345,7 +362,7 @@ class ProgramEnrollmentsView(DeveloperErrorViewMixin, PaginatedAPIView): ProgramEnrollment.objects.filter(program_uuid=program_uuid) ) paginated_enrollments = self.paginate_queryset(enrollments) - serializer = ProgramEnrollmentListSerializer(paginated_enrollments, many=True) + serializer = ProgramEnrollmentSerializer(paginated_enrollments, many=True) return self.get_paginated_response(serializer.data) @verify_program_exists @@ -393,16 +410,16 @@ class ProgramEnrollmentsView(DeveloperErrorViewMixin, PaginatedAPIView): """ student_key = enrollment['student_key'] if student_key in seen_student_keys: - return CourseEnrollmentResponseStatuses.DUPLICATED + return ProgramResponseStatuses.DUPLICATED seen_student_keys.add(student_key) enrollment_serializer = serializer_class(data=enrollment) try: enrollment_serializer.is_valid(raise_exception=True) - except ValidationError as e: + except ValidationError: if enrollment_serializer.has_invalid_status(): - return CourseEnrollmentResponseStatuses.INVALID_STATUS + return ProgramResponseStatuses.INVALID_STATUS else: - raise e + raise def create_or_modify_enrollments(self, request, program_uuid, serializer_class, operation, success_status): """ @@ -438,7 +455,7 @@ class ProgramEnrollmentsView(DeveloperErrorViewMixin, PaginatedAPIView): program_enrollments = self.get_existing_program_enrollments(program_uuid, enrollments) for enrollment in enrollments: student_key = enrollment["student_key"] - if student_key in results and results[student_key] == ProgramEnrollmentResponseStatuses.DUPLICATED: + if student_key in results and results[student_key] == ProgramResponseStatuses.DUPLICATED: continue try: program_enrollment = program_enrollments[student_key] @@ -453,7 +470,7 @@ class ProgramEnrollmentsView(DeveloperErrorViewMixin, PaginatedAPIView): Create new ProgramEnrollment, unless the learner is already enrolled in the program """ if program_enrollment: - return ProgramEnrollmentResponseStatuses.CONFLICT + return ProgramResponseStatuses.CONFLICT student_key = request_data.get('student_key') try: @@ -477,7 +494,7 @@ class ProgramEnrollmentsView(DeveloperErrorViewMixin, PaginatedAPIView): Change the status of an existing program enrollment """ if not program_enrollment: - return ProgramEnrollmentResponseStatuses.NOT_IN_PROGRAM + return ProgramResponseStatuses.NOT_IN_PROGRAM program_enrollment.status = request_data.get('status') program_enrollment.save() @@ -504,7 +521,7 @@ class ProgramEnrollmentsView(DeveloperErrorViewMixin, PaginatedAPIView): response_status = default_status good_count = len([ v for v in response_data.values() - if v not in CourseEnrollmentResponseStatuses.ERROR_STATUSES + if v in ProgramResponseStatuses.__OK__ ]) if not good_count: response_status = status.HTTP_422_UNPROCESSABLE_ENTITY @@ -637,37 +654,8 @@ class UserProgramReadOnlyAccessView(DeveloperErrorViewMixin, PaginatedAPIView): return program_list -class ProgramSpecificViewMixin(object): - """ - A mixin for views that operate on or within a specific program. - """ - - @cached_property - def program(self): - """ - The program specified by the `program_uuid` URL parameter. - """ - program = get_programs(uuid=self.kwargs['program_uuid']) - if program is None: - raise Http404() - return program - - -class ProgramCourseRunSpecificViewMixin(ProgramSpecificViewMixin): - """ - A mixin for views that operate on or within a specific course run in a program - """ - - @property - def course_key(self): - """ - The course key for the course run specified by the `course_id` URL parameter. - """ - return CourseKey.from_string(self.kwargs['course_id']) - - # pylint: disable=line-too-long -class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpecificViewMixin, PaginatedAPIView): +class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseSpecificViewMixin, PaginatedAPIView): """ A view for enrolling students in a course through a program, modifying program course enrollments, and listing program course @@ -748,23 +736,23 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec permission_classes = (permissions.JWT_RESTRICTED_APPLICATION_OR_USER_ACCESS,) pagination_class = ProgramEnrollmentPagination - @verify_course_exists - @verify_program_exists + @verify_course_exists_and_in_program def get(self, request, program_uuid=None, course_id=None): - """ Defines the GET list endpoint for ProgramCourseEnrollment objects. """ - course_key = CourseKey.from_string(course_id) + """ + Get a list of students enrolled in a course within a program. + """ enrollments = use_read_replica_if_available( ProgramCourseEnrollment.objects.filter( - program_enrollment__program_uuid=program_uuid, course_key=course_key + program_enrollment__program_uuid=program_uuid, + course_key=self.course_key ).select_related( 'program_enrollment' ) ) paginated_enrollments = self.paginate_queryset(enrollments) - serializer = ProgramCourseEnrollmentListSerializer(paginated_enrollments, many=True) + serializer = ProgramCourseEnrollmentSerializer(paginated_enrollments, many=True) return self.get_paginated_response(serializer.data) - @verify_program_exists @verify_course_exists_and_in_program def post(self, request, program_uuid=None, course_id=None): """ @@ -776,7 +764,6 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec self.enroll_learner_in_course ) - @verify_program_exists @verify_course_exists_and_in_program # pylint: disable=unused-argument def patch(self, request, program_uuid=None, course_id=None): @@ -789,7 +776,6 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec self.modify_learner_enrollment_status ) - @verify_program_exists @verify_course_exists_and_in_program # pylint: disable=unused-argument def put(self, request, program_uuid=None, course_id=None): @@ -835,17 +821,20 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec program_enrollments = self.get_existing_program_enrollments(program_uuid, enrollments) for enrollment in enrollments: student_key = enrollment["student_key"] - if student_key in results and results[student_key] == CourseEnrollmentResponseStatuses.DUPLICATED: + if student_key in results and results[student_key] == ProgramCourseResponseStatuses.DUPLICATED: continue try: program_enrollment = program_enrollments[student_key] except KeyError: - results[student_key] = CourseEnrollmentResponseStatuses.NOT_IN_PROGRAM + results[student_key] = ProgramCourseResponseStatuses.NOT_IN_PROGRAM else: program_course_enrollment = program_enrollment.get_program_course_enrollment(self.course_key) results[student_key] = operation(enrollment, program_enrollment, program_course_enrollment) - good_count = sum(1 for _, v in results.items() if v not in CourseEnrollmentResponseStatuses.ERROR_STATUSES) + good_count = sum( + 1 for _, v in results.items() + if v in ProgramCourseResponseStatuses.__OK__ + ) if not good_count: return Response(results, status.HTTP_422_UNPROCESSABLE_ENTITY) if good_count != len(results): @@ -859,14 +848,14 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec """ student_key = enrollment['student_key'] if student_key in seen_student_keys: - return CourseEnrollmentResponseStatuses.DUPLICATED + return ProgramCourseResponseStatuses.DUPLICATED seen_student_keys.add(student_key) enrollment_serializer = ProgramCourseEnrollmentRequestSerializer(data=enrollment) try: enrollment_serializer.is_valid(raise_exception=True) except ValidationError as e: if enrollment_serializer.has_invalid_status(): - return CourseEnrollmentResponseStatuses.INVALID_STATUS + return ProgramCourseResponseStatuses.INVALID_STATUS else: raise e @@ -894,7 +883,7 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec Returns the actual status """ if program_course_enrollment: - return CourseEnrollmentResponseStatuses.CONFLICT + return ProgramCourseResponseStatuses.CONFLICT return ProgramCourseEnrollment.create_program_course_enrollment( program_enrollment, @@ -909,7 +898,7 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec in the given program """ if program_course_enrollment is None: - return CourseEnrollmentResponseStatuses.NOT_FOUND + return ProgramCourseResponseStatuses.NOT_FOUND return program_course_enrollment.change_status(enrollment_request['status']) def create_or_update_learner_enrollment(self, enrollment_request, program_enrollment, program_course_enrollment): @@ -1034,8 +1023,10 @@ class ProgramCourseEnrollmentOverviewView(DeveloperErrorViewMixin, ProgramSpecif user = request.user self._check_program_enrollment_exists(user, program_uuid) - program = get_programs(uuid=program_uuid) - course_run_keys = [CourseKey.from_string(key) for key in course_run_keys_for_program(program)] + course_run_keys = [ + CourseKey.from_string(key) + for key in course_run_keys_for_program(self.program) + ] course_enrollments = CourseEnrollment.objects.filter( user=user, @@ -1106,7 +1097,7 @@ class ProgramCourseEnrollmentOverviewView(DeveloperErrorViewMixin, ProgramSpecif class ProgramCourseGradesView( DeveloperErrorViewMixin, - ProgramCourseRunSpecificViewMixin, + ProgramCourseSpecificViewMixin, PaginatedAPIView, ): """ @@ -1178,15 +1169,14 @@ class ProgramCourseGradesView( permission_classes = (permissions.JWT_RESTRICTED_APPLICATION_OR_USER_ACCESS,) pagination_class = ProgramEnrollmentPagination - @verify_course_exists - @verify_program_exists + @verify_course_exists_and_in_program def get(self, request, program_uuid=None, course_id=None): """ Defines the GET list endpoint for ProgramCourseGrade objects. """ course_key = CourseKey.from_string(course_id) grade_results = self._load_grade_results(program_uuid, course_key) - serializer = ProgramCourseGradeResultSerializer(grade_results, many=True) + serializer = ProgramCourseGradeSerializer(grade_results, many=True) response_code = self._calc_response_code(grade_results) return self.get_paginated_response(serializer.data, status_code=response_code) @@ -1198,7 +1188,7 @@ class ProgramCourseGradesView( program_uuid (str) course_key (CourseKey) - Returns: list[ProgramCourseGradeResult|ProgramCourseGradeErrorResult] + Returns: list[BaseProgramCourseGrade] """ enrollments_qs = use_read_replica_if_available( ProgramCourseEnrollment.objects.filter( @@ -1224,9 +1214,9 @@ class ProgramCourseGradesView( ) grade_results = [ ( - ProgramCourseGradeResult(enrollment, grade) + ProgramCourseGradeOk(enrollment, grade) if grade - else ProgramCourseGradeErrorResult(enrollment, exception) + else ProgramCourseGradeError(enrollment, exception) ) for enrollment, (grade, exception) in enrollment_grade_pairs ] @@ -1270,7 +1260,7 @@ class ProgramCourseGradesView( which may be grades or errors. Arguments: - enrollment_grade_results: list[ProgramCourseGradeResult] + enrollment_grade_results: list[BaseProgramCourseGrade] Returns: int * 200 for all success diff --git a/lms/djangoapps/program_enrollments/signals.py b/lms/djangoapps/program_enrollments/signals.py index 82ed3f8985..9d9ef769b4 100644 --- a/lms/djangoapps/program_enrollments/signals.py +++ b/lms/djangoapps/program_enrollments/signals.py @@ -4,9 +4,11 @@ Signal handlers for program enrollments from __future__ import absolute_import import logging + from django.db.models.signals import post_save from django.dispatch import receiver from social_django.models import UserSocialAuth + from lms.djangoapps.program_enrollments.models import ProgramEnrollment from openedx.core.djangoapps.catalog.utils import get_programs from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_MISC @@ -62,7 +64,9 @@ def matriculate_learner(user, uid): logger.info(u'Ignoring non-saml social auth entry for user=%s', user.id) return except SAMLProviderConfig.DoesNotExist: - logger.warning(u'Got incoming social auth for provider=%s but no such provider exists', provider_slug) + logger.warning( + u'Got incoming social auth for provider=%s but no such provider exists', provider_slug + ) return except SAMLProviderConfig.MultipleObjectsReturned: logger.warning( diff --git a/lms/djangoapps/program_enrollments/tests/test_admin.py b/lms/djangoapps/program_enrollments/tests/test_admin.py index e565476007..2cc5b7a0bf 100644 --- a/lms/djangoapps/program_enrollments/tests/test_admin.py +++ b/lms/djangoapps/program_enrollments/tests/test_admin.py @@ -25,7 +25,9 @@ class ProgramEnrollmentAdminTests(TestCase): def test_program_enrollment_admin(self): request = mock.Mock() - expected_list_display = ('id', 'user', 'external_user_key', 'program_uuid', 'curriculum_uuid', 'status') + expected_list_display = ( + 'id', 'user', 'external_user_key', 'program_uuid', 'curriculum_uuid', 'status' + ) assert expected_list_display == self.program_admin.get_list_display(request) expected_raw_id_fields = ('user',) assert expected_raw_id_fields == self.program_admin.raw_id_fields @@ -33,7 +35,9 @@ class ProgramEnrollmentAdminTests(TestCase): def test_program_course_enrollment_admin(self): request = mock.Mock() - expected_list_display = ('id', 'program_enrollment', 'course_enrollment', 'course_key', 'status') + expected_list_display = ( + 'id', 'program_enrollment', 'course_enrollment', 'course_key', 'status' + ) assert expected_list_display == self.program_course_admin.get_list_display(request) expected_raw_id_fields = ('program_enrollment', 'course_enrollment') assert expected_raw_id_fields == self.program_course_admin.raw_id_fields diff --git a/lms/djangoapps/program_enrollments/tests/test_models.py b/lms/djangoapps/program_enrollments/tests/test_models.py index 8fb29ec301..524a75c56a 100644 --- a/lms/djangoapps/program_enrollments/tests/test_models.py +++ b/lms/djangoapps/program_enrollments/tests/test_models.py @@ -44,7 +44,9 @@ class ProgramEnrollmentModelTests(TestCase): ) def test_unique_external_key_program_curriculum(self): - """ A record with the same (external_user_key, program_uuid, curriculum_uuid) cannot be duplicated. """ + """ + A record with the same (external_user_key, program_uuid, curriculum_uuid) cannot be duplicated. + """ with self.assertRaises(IntegrityError): _ = ProgramEnrollment.objects.create( user=None, @@ -55,7 +57,9 @@ class ProgramEnrollmentModelTests(TestCase): ) def test_unique_user_program_curriculum(self): - """ A record with the same (user, program_uuid, curriculum_uuid) cannot be duplicated. """ + """ + A record with the same (user, program_uuid, curriculum_uuid) cannot be duplicated. + """ with self.assertRaises(IntegrityError): _ = ProgramEnrollment.objects.create( user=self.user, @@ -108,7 +112,8 @@ class ProgramEnrollmentModelTests(TestCase): def test_user_retirement(self): """ - Test that the external_user_key is successfully retired for a user's program enrollments and history. + Test that the external_user_key is successfully retired for a user's program enrollments + and history. """ new_status = 'canceled' @@ -258,7 +263,9 @@ class ProgramCourseEnrollmentModelTests(TestCase): with LogCapture() as capture: with self.assertRaises(NonExistentCourseError): program_course_enrollment.enroll(self.user) - expected = "User {} failed to enroll in non-existent course {}".format(self.user.id, nonexistent_key) + expected = "User {} failed to enroll in non-existent course {}".format( + self.user.id, nonexistent_key + ) capture.check( ('lms.djangoapps.program_enrollments.models', 'WARNING', expected) ) diff --git a/lms/djangoapps/program_enrollments/tests/test_signals.py b/lms/djangoapps/program_enrollments/tests/test_signals.py index e4fb1f6cdb..ff32153fd7 100644 --- a/lms/djangoapps/program_enrollments/tests/test_signals.py +++ b/lms/djangoapps/program_enrollments/tests/test_signals.py @@ -4,30 +4,29 @@ Test signal handlers for program_enrollments from __future__ import absolute_import -from django.core.cache import cache import mock -from opaque_keys.edx.keys import CourseKey import pytest +from django.core.cache import cache +from edx_django_utils.cache import RequestCache +from opaque_keys.edx.keys import CourseKey +from organizations.tests.factories import OrganizationFactory from social_django.models import UserSocialAuth from testfixtures import LogCapture from course_modes.models import CourseMode -from edx_django_utils.cache import RequestCache from lms.djangoapps.program_enrollments.signals import _listen_for_lms_retire, logger from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory -from organizations.tests.factories import OrganizationFactory from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL -from openedx.core.djangoapps.catalog.tests.factories import ( - OrganizationFactory as CatalogOrganizationFactory, ProgramFactory -) +from openedx.core.djangoapps.catalog.tests.factories import OrganizationFactory as CatalogOrganizationFactory +from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import fake_completed_retirement from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.models import CourseEnrollmentException from student.tests.factories import CourseEnrollmentFactory, UserFactory -from third_party_auth.tests.factories import SAMLProviderConfigFactory from third_party_auth.models import SAMLProviderConfig +from third_party_auth.tests.factories import SAMLProviderConfigFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -122,7 +121,9 @@ class SocialAuthEnrollmentCompletionSignalTest(CacheIsolationTestCase): for course_key in cls.course_keys: CourseOverviewFactory(id=course_key) - cls.provider_config = SAMLProviderConfigFactory.create(organization=cls.organization, slug=cls.provider_slug) + cls.provider_config = SAMLProviderConfigFactory.create( + organization=cls.organization, slug=cls.provider_slug + ) def setUp(self): super(SocialAuthEnrollmentCompletionSignalTest, self).setUp() @@ -248,7 +249,9 @@ class SocialAuthEnrollmentCompletionSignalTest(CacheIsolationTestCase): self._assert_program_enrollment_user(program_enrollment, self.user) duplicate_program_course_enrollment = program_course_enrollments[0] - self._assert_program_course_enrollment(duplicate_program_course_enrollment, CourseMode.VERIFIED) + self._assert_program_course_enrollment( + duplicate_program_course_enrollment, CourseMode.VERIFIED + ) program_course_enrollment = program_course_enrollments[1] self._assert_program_course_enrollment(program_course_enrollment) @@ -326,7 +329,7 @@ class SocialAuthEnrollmentCompletionSignalTest(CacheIsolationTestCase): user=self.user, uid='{0}:{1}'.format(self.provider_slug, self.external_id) ) - error_tmpl = ( + error_template = ( u'Failed to complete waiting enrollments for organization={}.' u' No catalog programs with matching authoring_organization exist.' ) @@ -334,7 +337,7 @@ class SocialAuthEnrollmentCompletionSignalTest(CacheIsolationTestCase): ( logger.name, 'WARNING', - error_tmpl.format('UoX') + error_template.format('UoX') ) ) @@ -350,12 +353,14 @@ class SocialAuthEnrollmentCompletionSignalTest(CacheIsolationTestCase): user=self.user, uid='{0}:{1}'.format(self.provider_slug, self.external_id) ) - error_tmpl = u'Failed to enroll user={} with waiting program_course_enrollment={}: {}' + error_template = u'Failed to enroll user={} with waiting program_course_enrollment={}: {}' log.check_present( ( logger.name, 'WARNING', - error_tmpl.format(self.user.id, program_course_enrollments[0].id, 'something has gone wrong') + error_template.format( + self.user.id, program_course_enrollments[0].id, 'something has gone wrong' + ) ) ) @@ -374,11 +379,11 @@ class SocialAuthEnrollmentCompletionSignalTest(CacheIsolationTestCase): user=self.user, uid='{0}:{1}'.format(self.provider_slug, self.external_id), ) - error_tmpl = u'Unable to link waiting enrollments for user {}, social auth creation failed: {}' + error_template = u'Unable to link waiting enrollments for user {}, social auth creation failed: {}' log.check_present( ( logger.name, 'WARNING', - error_tmpl.format(self.user.id, 'unexpected error') + error_template.format(self.user.id, 'unexpected error') ) ) diff --git a/lms/djangoapps/program_enrollments/tests/test_utils.py b/lms/djangoapps/program_enrollments/tests/test_utils.py index 28dcb25fea..8c8f13b1ef 100644 --- a/lms/djangoapps/program_enrollments/tests/test_utils.py +++ b/lms/djangoapps/program_enrollments/tests/test_utils.py @@ -141,11 +141,17 @@ class GetPlatformUserTests(CacheIsolationTestCase): self.create_social_auth_entry(self.user, provider, self.external_user_id) # create a second active config for the same organization - SAMLProviderConfigFactory.create(organization=organization, slug='foox', enabled=second_config_enabled) + SAMLProviderConfigFactory.create( + organization=organization, slug='foox', enabled=second_config_enabled + ) try: get_user_by_program_id(self.external_user_id, self.program_uuid) except ProviderConfigurationException: - self.assertTrue(second_config_enabled, 'Unexpected error when second config is disabled') + self.assertTrue( + second_config_enabled, 'Unexpected error when second config is disabled' + ) else: - self.assertFalse(second_config_enabled, 'Expected error was not raised when second config is enabled') + self.assertFalse( + second_config_enabled, 'Expected error was not raised when second config is enabled' + ) diff --git a/lms/djangoapps/program_enrollments/utils.py b/lms/djangoapps/program_enrollments/utils.py index e3e6dbd376..052e00d65f 100644 --- a/lms/djangoapps/program_enrollments/utils.py +++ b/lms/djangoapps/program_enrollments/utils.py @@ -39,12 +39,14 @@ def get_user_by_program_id(external_user_id, program_uuid): program_uuid: a program this user is/will be enrolled in Returns: - A User object or None, if no user with the given external id for the given organization exists. + A User object or None, if no user with the given external id for the given organization + exists. Raises: ProgramDoesNotExistException if no such program exists. OrganizationDoesNotExistException if no organization exists. - ProviderDoesNotExistException if there is no SAML provider configured for the related organization. + ProviderDoesNotExistException if there is no SAML provider configured for the related + organization. """ program = get_programs(uuid=program_uuid) if program is None: @@ -55,7 +57,9 @@ def get_user_by_program_id(external_user_id, program_uuid): org_key = program['authoring_organizations'][0]['key'] organization = Organization.objects.get(short_name=org_key) except (KeyError, IndexError): - log.error(u'Cannot determine authoring organization key for catalog program [%s]', program_uuid) + log.error( + u'Cannot determine authoring organization key for catalog program [%s]', program_uuid + ) raise OrganizationDoesNotExistException except Organization.DoesNotExist: log.error(u'Unable to find organization for short_name [%s]', org_key) @@ -76,10 +80,12 @@ def get_user_by_organization(external_user_id, organization): organization: organization providing saml authentication for this user Returns: - A User object or None, if no user with the given external id for the given organization exists. + A User object or None, if no user with the given external id for the given organization + exists. Raises: - ProviderDoesNotExistException if there is no SAML provider configured for the related organization. + ProviderDoesNotExistException if there is no SAML provider configured for the related + organization. """ provider_slug = get_provider_slug(organization) try: @@ -98,7 +104,7 @@ def get_provider_slug(organization): ProviderConfigurationException """ try: - return organization.samlproviderconfig_set.current_set().get(enabled=True).provider_id.strip('saml-') + provider_config = organization.samlproviderconfig_set.current_set().get(enabled=True) except SAMLProviderConfig.DoesNotExist: log.error(u'No SAML provider found for organization id [%s]', organization.id) raise ProviderDoesNotExistException @@ -108,3 +114,4 @@ def get_provider_slug(organization): organization.short_name, ) raise ProviderConfigurationException + return provider_config.provider_id.strip('saml-') diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index 6f6dabad32..1f9c4dfb54 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -39,6 +39,7 @@ from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.catalog.utils import ( child_programs, course_run_keys_for_program, + is_course_run_in_program, get_course_run_details, get_course_runs, get_course_runs_for_course, @@ -790,6 +791,11 @@ class TestProgramCourseRunCrawling(TestCase): } self.assertEqual(expected_course_runs, course_run_keys_for_program(self.complex_program)) + def test_is_course_run_in_program(self): + self.assertTrue(is_course_run_in_program('course-run-4', self.complex_program)) + self.assertFalse(is_course_run_in_program('course-run-5', self.complex_program)) + self.assertFalse(is_course_run_in_program('course-run-4', self.simple_program)) + @skip_unless_lms class TestGetProgramsByType(CacheIsolationTestCase): diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index ce5299fcc2..c5604de59d 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -573,6 +573,29 @@ def get_course_run_details(course_run_key, fields): return course_run_details +def is_course_run_in_program(course_run_key, program): + """ + Check if a course run is part of a program. + + Arguments: + program (dict): program data, as returned by get_programs() + course_run_key (CourseKey|str) + + Returns: bool + Whether the program exists AND the course run is part of it. + """ + # Right now, this function simply loads all the program data from the cache, + # walks the structure to collect the set of course run keys, + # and then sees if `course_run_key` is in that set. + # If we need to optimize this later, we can. + course_run_key_str = ( + str(course_run_key) if isinstance(course_run_key, CourseKey) + else course_run_key + ) + course_run_keys = course_run_keys_for_program(program) + return course_run_key_str in course_run_keys + + def course_run_keys_for_program(parent_program): """ All of the course run keys associated with this ``parent_program``, either