From 68b5f98485fb533f0928cb289cd67d7875f40988 Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Thu, 26 Sep 2019 16:10:55 +0500 Subject: [PATCH] Save order date attribute and some refactoring. Save order placement date attribute so that we don't need to call ecommerce again. Also did some refactoring. --- common/djangoapps/student/models.py | 17 ++++- .../djangoapps/student/tests/test_refunds.py | 17 ++++- openedx/core/djangoapps/enrollments/api.py | 64 ++++++++++--------- openedx/core/djangoapps/enrollments/data.py | 44 ++++++------- 4 files changed, 85 insertions(+), 57 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 648bda9a80..c9b0991c06 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -65,7 +65,11 @@ from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from openedx.core.djangoapps.content.course_overviews.models import CourseOverview import openedx.core.djangoapps.django_comment_common.comment_client as cc -from openedx.core.djangoapps.enrollments.api import _default_course_mode +from openedx.core.djangoapps.enrollments.api import ( + _default_course_mode, + get_enrollment_attributes, + set_enrollment_attributes +) from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager from openedx.core.djangolib.model_mixins import DeletableByUserValue @@ -1775,6 +1779,17 @@ class CourseEnrollment(models.Model): try: order = ecommerce_api_client(self.user).orders(order_number).get() date_placed = order['date_placed'] + # also save the attribute so that we don't need to call ecommerce again. + username = self.user.username + enrollment_attributes = get_enrollment_attributes(username, six.text_type(self.course_id)) + enrollment_attributes.append( + { + "namespace": "order", + "name": "date_placed", + "value": date_placed, + } + ) + set_enrollment_attributes(username, six.text_type(self.course_id), enrollment_attributes) except HttpClientError: log.warning( u"Encountered HttpClientError while getting order details from ecommerce. " diff --git a/common/djangoapps/student/tests/test_refunds.py b/common/djangoapps/student/tests/test_refunds.py index 2214946d6f..67de106610 100644 --- a/common/djangoapps/student/tests/test_refunds.py +++ b/common/djangoapps/student/tests/test_refunds.py @@ -22,11 +22,10 @@ from six.moves import range # These imports refer to lms djangoapps. # Their testcases are only run under lms. from course_modes.tests.factories import CourseModeFactory - from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from openedx.core.djangoapps.commerce.utils import ECOMMERCE_DATE_FORMAT -from student.models import CourseEnrollment, EnrollmentRefundConfiguration +from student.models import CourseEnrollment, CourseEnrollmentAttribute, EnrollmentRefundConfiguration from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -141,7 +140,8 @@ class RefundableTest(SharedModuleStoreTestCase): course_start = now + course_start_delta expected_date = now + expected_date_delta refund_period = timedelta(days=days) - expected_content = '{{"date_placed": "{date}"}}'.format(date=order_date.strftime(ECOMMERCE_DATE_FORMAT)) + date_placed = order_date.strftime(ECOMMERCE_DATE_FORMAT) + expected_content = '{{"date_placed": "{date}"}}'.format(date=date_placed) httpretty.register_uri( httpretty.GET, @@ -166,6 +166,17 @@ class RefundableTest(SharedModuleStoreTestCase): expected_date + refund_period ) + expected_date_placed_attr = { + "namespace": "order", + "name": "date_placed", + "value": date_placed, + } + + self.assertIn( + expected_date_placed_attr, + CourseEnrollmentAttribute.get_enrollment_attributes(self.enrollment) + ) + def test_refund_cutoff_date_no_attributes(self): """ Assert that the None is returned when no order number attribute is found.""" self.assertIsNone(self.enrollment.refund_cutoff_date()) diff --git a/openedx/core/djangoapps/enrollments/api.py b/openedx/core/djangoapps/enrollments/api.py index c3b68f2701..f23b697762 100644 --- a/openedx/core/djangoapps/enrollments/api.py +++ b/openedx/core/djangoapps/enrollments/api.py @@ -20,14 +20,14 @@ log = logging.getLogger(__name__) DEFAULT_DATA_API = 'openedx.core.djangoapps.enrollments.data' -def get_enrollments(user_id, include_inactive=False): +def get_enrollments(username, include_inactive=False): """Retrieves all the courses a user is enrolled in. Takes a user and retrieves all relative enrollments. Includes information regarding how the user is enrolled in the the course. Args: - user_id (str): The username of the user we want to retrieve course enrollment information for. + username: The username of the user we want to retrieve course enrollment information for. include_inactive (bool): Determines whether inactive enrollments will be included Returns: @@ -95,16 +95,16 @@ def get_enrollments(user_id, include_inactive=False): ] """ - return _data_api().get_course_enrollments(user_id, include_inactive) + return _data_api().get_course_enrollments(username, include_inactive) -def get_enrollment(user_id, course_id): +def get_enrollment(username, course_id): """Retrieves all enrollment information for the user in respect to a specific course. Gets all the course enrollment information specific to a user in a course. Args: - user_id (str): The user to get course enrollment information for. + username: The user to get course enrollment information for. course_id (str): The course to get enrollment information for. Returns: @@ -142,16 +142,16 @@ def get_enrollment(user_id, course_id): } """ - return _data_api().get_course_enrollment(user_id, course_id) + return _data_api().get_course_enrollment(username, course_id) -def add_enrollment(user_id, course_id, mode=None, is_active=True, enrollment_attributes=None): +def add_enrollment(username, course_id, mode=None, is_active=True, enrollment_attributes=None): """Enrolls a user in a course. Enrolls a user in a course. If the mode is not specified, this will default to `CourseMode.DEFAULT_MODE_SLUG`. Arguments: - user_id (str): The user to enroll. + username: The user to enroll. course_id (str): The course to enroll the user in. mode (str): Optional argument for the type of enrollment to create. Ex. 'audit', 'honor', 'verified', 'professional'. If not specified, this defaults to the default course mode. @@ -196,21 +196,23 @@ def add_enrollment(user_id, course_id, mode=None, is_active=True, enrollment_att if mode is None: mode = _default_course_mode(course_id) validate_course_mode(course_id, mode, is_active=is_active) - enrollment = _data_api().create_course_enrollment(user_id, course_id, mode, is_active) + enrollment = _data_api().create_course_enrollment(username, course_id, mode, is_active) if enrollment_attributes is not None: - set_enrollment_attributes(user_id, course_id, enrollment_attributes) + set_enrollment_attributes(username, course_id, enrollment_attributes) return enrollment -def update_enrollment(user_id, course_id, mode=None, is_active=None, enrollment_attributes=None, include_expired=False): +def update_enrollment( + username, course_id, mode=None, is_active=None, enrollment_attributes=None, include_expired=False +): """Updates the course mode for the enrolled user. Update a course enrollment for the given user and course. Arguments: - user_id (str): The user associated with the updated enrollment. + username: The user associated with the updated enrollment. course_id (str): The course associated with the updated enrollment. Keyword Arguments: @@ -255,22 +257,22 @@ def update_enrollment(user_id, course_id, mode=None, is_active=None, enrollment_ """ log.info(u'Starting Update Enrollment process for user {user} in course {course} to mode {mode}'.format( - user=user_id, + user=username, course=course_id, mode=mode, )) if mode is not None: validate_course_mode(course_id, mode, is_active=is_active, include_expired=include_expired) - enrollment = _data_api().update_course_enrollment(user_id, course_id, mode=mode, is_active=is_active) + enrollment = _data_api().update_course_enrollment(username, course_id, mode=mode, is_active=is_active) if enrollment is None: - msg = u"Course Enrollment not found for user {user} in course {course}".format(user=user_id, course=course_id) + msg = u"Course Enrollment not found for user {user} in course {course}".format(user=username, course=course_id) log.warn(msg) raise errors.EnrollmentNotFoundError(msg) else: if enrollment_attributes is not None: - set_enrollment_attributes(user_id, course_id, enrollment_attributes) + set_enrollment_attributes(username, course_id, enrollment_attributes) log.info(u'Course Enrollment updated for user {user} in course {course} to mode {mode}'.format( - user=user_id, + user=username, course=course_id, mode=mode )) @@ -346,13 +348,13 @@ def get_course_enrollment_details(course_id, include_expired=False): return course_enrollment_details -def set_enrollment_attributes(user_id, course_id, attributes): +def set_enrollment_attributes(username, course_id, attributes): """Set enrollment attributes for the enrollment of given user in the course provided. Args: - course_id (str): The Course to set enrollment attributes for. - user_id (str): The User to set enrollment attributes for. + course_id: The Course to set enrollment attributes for. + username: The User to set enrollment attributes for. attributes (list): Attributes to be set. Example: @@ -368,15 +370,15 @@ def set_enrollment_attributes(user_id, course_id, attributes): ] ) """ - _data_api().add_or_update_enrollment_attr(user_id, course_id, attributes) + _data_api().add_or_update_enrollment_attr(username, course_id, attributes) -def get_enrollment_attributes(user_id, course_id): +def get_enrollment_attributes(username, course_id): """Retrieve enrollment attributes for given user for provided course. Args: - user_id: The User to get enrollment attributes for - course_id (str): The Course to get enrollment attributes for. + username: The User to get enrollment attributes for + course_id: The Course to get enrollment attributes for. Example: >>>get_enrollment_attributes("Bob", "course-v1-edX-DemoX-1T2015") @@ -390,7 +392,7 @@ def get_enrollment_attributes(user_id, course_id): Returns: list """ - return _data_api().get_enrollment_attributes(user_id, course_id) + return _data_api().get_enrollment_attributes(username, course_id) def _default_course_mode(course_id): @@ -459,22 +461,22 @@ def validate_course_mode(course_id, mode, is_active=None, include_expired=False) raise errors.CourseModeNotFoundError(msg, course_enrollment_info) -def unenroll_user_from_all_courses(user_id): +def unenroll_user_from_all_courses(username): """ Unenrolls a specified user from all of the courses they are currently enrolled in. - :param user_id: The id of the user being unenrolled. + :param username: The id of the user being unenrolled. :return: The IDs of all of the organizations from which the learner was unenrolled. """ - return _data_api().unenroll_user_from_all_courses(user_id) + return _data_api().unenroll_user_from_all_courses(username) -def get_user_roles(user_id): +def get_user_roles(username): """ Returns a list of all roles that this user has. - :param user_id: The id of the selected user. + :param username: The id of the selected user. :return: All roles for all courses that this user has. """ - return _data_api().get_user_roles(user_id) + return _data_api().get_user_roles(username) def _data_api(): diff --git a/openedx/core/djangoapps/enrollments/data.py b/openedx/core/djangoapps/enrollments/data.py index c8021c6777..1170f66f0a 100644 --- a/openedx/core/djangoapps/enrollments/data.py +++ b/openedx/core/djangoapps/enrollments/data.py @@ -34,13 +34,13 @@ from student.roles import RoleCache log = logging.getLogger(__name__) -def get_course_enrollments(user_id, include_inactive=False): +def get_course_enrollments(username, include_inactive=False): """Retrieve a list representing all aggregated data for a user's course enrollments. Construct a representation of all course enrollment data for a specific user. Args: - user_id (str): The name of the user to retrieve course enrollment information for. + username: The name of the user to retrieve course enrollment information for. include_inactive (bool): Determines whether inactive enrollments will be included @@ -49,7 +49,7 @@ def get_course_enrollments(user_id, include_inactive=False): """ qset = CourseEnrollment.objects.filter( - user__username=user_id, + user__username=username, ).order_by('created') if not include_inactive: @@ -71,7 +71,7 @@ def get_course_enrollments(user_id, include_inactive=False): ( u"Course enrollments for user %s reference " u"courses that do not exist (this can occur if a course is deleted)." - ), user_id, + ), username, ) return valid @@ -191,13 +191,13 @@ def update_course_enrollment(username, course_id, mode=None, is_active=None): return None -def add_or_update_enrollment_attr(user_id, course_id, attributes): +def add_or_update_enrollment_attr(username, course_id, attributes): """Set enrollment attributes for the enrollment of given user in the course provided. Args: course_id (str): The Course to set enrollment attributes for. - user_id (str): The User to set enrollment attributes for. + username: The User to set enrollment attributes for. attributes (list): Attributes to be set. Example: @@ -214,17 +214,17 @@ def add_or_update_enrollment_attr(user_id, course_id, attributes): ) """ course_key = CourseKey.from_string(course_id) - user = _get_user(user_id) + user = _get_user(username) enrollment = CourseEnrollment.get_enrollment(user, course_key) if not _invalid_attribute(attributes) and enrollment is not None: CourseEnrollmentAttribute.add_enrollment_attr(enrollment, attributes) -def get_enrollment_attributes(user_id, course_id): +def get_enrollment_attributes(username, course_id): """Retrieve enrollment attributes for given user for provided course. Args: - user_id: The User to get enrollment attributes for + username: The User to get enrollment attributes for course_id (str): The Course to get enrollment attributes for. Example: @@ -240,18 +240,18 @@ def get_enrollment_attributes(user_id, course_id): Returns: list """ course_key = CourseKey.from_string(course_id) - user = _get_user(user_id) + user = _get_user(username) enrollment = CourseEnrollment.get_enrollment(user, course_key) return CourseEnrollmentAttribute.get_enrollment_attributes(enrollment) -def unenroll_user_from_all_courses(user_id): +def unenroll_user_from_all_courses(username): """ Set all of a user's enrollments to inactive. - :param user_id: The user being unenrolled. + :param username: The user being unenrolled. :return: A list of all courses from which the user was unenrolled. """ - user = _get_user(user_id) + user = _get_user(username) enrollments = CourseEnrollment.objects.filter(user=user) with transaction.atomic(): for enrollment in enrollments: @@ -260,18 +260,18 @@ def unenroll_user_from_all_courses(user_id): return set([str(enrollment.course_id.org) for enrollment in enrollments]) -def _get_user(user_id): - """Retrieve user with provided user_id +def _get_user(username): + """Retrieve user with provided username Args: - user_id(str): username of the user for which object is to retrieve + username: username of the user for which object is to retrieve Returns: obj """ try: - return User.objects.get(username=user_id) + return User.objects.get(username=username) except User.DoesNotExist: - msg = u"Not user with username '{username}' found.".format(username=user_id) + msg = u"Not user with username '{username}' found.".format(username=username) log.warn(msg) raise UserNotFoundError(msg) @@ -286,7 +286,7 @@ def _invalid_attribute(attributes): """Validate enrollment attribute Args: - attributes(dict): dict of attribute + attributes(List): List of attribute dicts Return: list of invalid attributes @@ -342,14 +342,14 @@ def get_course_enrollment_info(course_id, include_expired=False): return CourseSerializer(course, include_expired=include_expired).data -def get_user_roles(user_id): +def get_user_roles(username): """ Returns a list of all roles that this user has. - :param user_id: The id of the selected user. + :param username: The id of the selected user. :return: All roles for all courses that this user has. """ # pylint: disable=protected-access - user = _get_user(user_id) + user = _get_user(username) if not hasattr(user, '_roles'): user._roles = RoleCache(user) role_cache = user._roles