Skip to content

Commit

Permalink
fix: catch a possible exception in beta course configuration
Browse files Browse the repository at this point in the history
when the learner is a beta tester, and the beta test has been set up with a large duration, the courseware djangoapp can return an overflow error on a timedelta call. In those circumstances, this defaults to returning an unmodified date.

FIXES: APER-3848
  • Loading branch information
deborahgu committed Jan 24, 2025
1 parent cb55799 commit 2626afc
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 60 deletions.
45 changes: 24 additions & 21 deletions lms/djangoapps/courseware/access_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
It allows us to share code between access.py and block transformers.
"""


from datetime import datetime, timedelta
from logging import getLogger

Expand All @@ -21,7 +20,7 @@
EnrollmentRequiredAccessError,
IncorrectActiveEnterpriseAccessError,
StartDateEnterpriseLearnerError,
StartDateError
StartDateError,
)
from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_student
from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, COURSE_PRE_START_ACCESS_FLAG
Expand Down Expand Up @@ -58,9 +57,12 @@ def adjust_start_date(user, days_early_for_beta, start, course_key):

if CourseBetaTesterRole(course_key).has_user(user):
debug("Adjust start time: user in beta role for %s", course_key)
delta = timedelta(days_early_for_beta)
effective = start - delta
return effective
# timedelta.max days from now is in the year 2739931, so that's probably pretty safe
delta = timedelta(min(days_early_for_beta, timedelta.max.days))
try:
return start - delta
except OverflowError:
return start

return start

Expand Down Expand Up @@ -93,7 +95,7 @@ def enterprise_learner_enrolled(request, user, course_key):
# enterprise_customer_data is either None (if learner is not linked to any customer) or a serialized
# EnterpriseCustomer representing the learner's active linked customer.
enterprise_customer_data = enterprise_customer_from_session_or_learner_data(request)
learner_portal_enabled = enterprise_customer_data and enterprise_customer_data['enable_learner_portal']
learner_portal_enabled = enterprise_customer_data and enterprise_customer_data["enable_learner_portal"]
if not learner_portal_enabled:
return False

Expand All @@ -102,18 +104,18 @@ def enterprise_learner_enrolled(request, user, course_key):
enterprise_enrollments = EnterpriseCourseEnrollment.objects.filter(
course_id=course_key,
enterprise_customer_user__user_id=user.id,
enterprise_customer_user__enterprise_customer__uuid=enterprise_customer_data['uuid'],
enterprise_customer_user__enterprise_customer__uuid=enterprise_customer_data["uuid"],
)
enterprise_enrollment_exists = enterprise_enrollments.exists()
log.info(
(
'[enterprise_learner_enrolled] Checking for an enterprise enrollment for '
'lms_user_id=%s in course_key=%s via enterprise_customer_uuid=%s. '
'Exists: %s'
"[enterprise_learner_enrolled] Checking for an enterprise enrollment for "
"lms_user_id=%s in course_key=%s via enterprise_customer_uuid=%s. "
"Exists: %s"
),
user.id,
course_key,
enterprise_customer_data['uuid'],
enterprise_customer_data["uuid"],
enterprise_enrollment_exists,
)
return enterprise_enrollment_exists
Expand All @@ -130,7 +132,7 @@ def check_start_date(user, days_early_for_beta, start, course_key, display_error
Returns:
AccessResponse: Either ACCESS_GRANTED or StartDateError.
"""
start_dates_disabled = settings.FEATURES['DISABLE_START_DATES']
start_dates_disabled = settings.FEATURES["DISABLE_START_DATES"]
masquerading_as_student = is_masquerading_as_student(user, course_key)

if start_dates_disabled and not masquerading_as_student:
Expand Down Expand Up @@ -161,8 +163,8 @@ def in_preview_mode():
Returns whether the user is in preview mode or not.
"""
hostname = get_current_request_hostname()
preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE', None)
return bool(preview_lms_base and hostname and hostname.split(':')[0] == preview_lms_base.split(':')[0])
preview_lms_base = settings.FEATURES.get("PREVIEW_LMS_BASE", None)
return bool(preview_lms_base and hostname and hostname.split(":")[0] == preview_lms_base.split(":")[0])


def check_course_open_for_learner(user, course):
Expand Down Expand Up @@ -233,18 +235,19 @@ def check_public_access(course, visibilities):

def check_data_sharing_consent(course_id):
"""
Grants access if the user is do not need DataSharing consent, otherwise returns data sharing link.
Grants access if the user is do not need DataSharing consent, otherwise returns data sharing link.
Returns:
AccessResponse: Either ACCESS_GRANTED or DataSharingConsentRequiredAccessError
"""
Returns:
AccessResponse: Either ACCESS_GRANTED or DataSharingConsentRequiredAccessError
"""
from openedx.features.enterprise_support.api import get_enterprise_consent_url

consent_url = get_enterprise_consent_url(
request=get_current_request(),
course_id=str(course_id),
return_to='courseware',
return_to="courseware",
enrollment_exists=True,
source='CoursewareAccess'
source="CoursewareAccess",
)
if consent_url:
return DataSharingConsentRequiredAccessError(consent_url=consent_url)
Expand Down Expand Up @@ -274,7 +277,7 @@ def check_correct_active_enterprise_customer(user, course_id):
except (EnterpriseCustomerUser.DoesNotExist, EnterpriseCustomerUser.MultipleObjectsReturned):
# Ideally this should not happen. As there should be only 1 active enterprise customer in our system
log.error("Multiple or No Active Enterprise found for the user %s.", user.id)
active_enterprise_name = 'Incorrect'
active_enterprise_name = "Incorrect"

enrollment_enterprise_name = enterprise_enrollments.first().enterprise_customer_user.enterprise_customer.name
return IncorrectActiveEnterpriseAccessError(enrollment_enterprise_name, active_enterprise_name)
Expand Down
103 changes: 64 additions & 39 deletions lms/djangoapps/courseware/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
"""
Unit test for various Utility functions
"""

import json
from datetime import date, datetime, timedelta
from unittest.mock import patch

import ddt
Expand All @@ -14,13 +16,44 @@
from common.djangoapps.student.tests.factories import GlobalStaffFactory, UserFactory
from lms.djangoapps.courseware.constants import UNEXPECTED_ERROR_IS_ELIGIBLE
from lms.djangoapps.courseware.tests.factories import FinancialAssistanceConfigurationFactory
from lms.djangoapps.courseware.access_utils import adjust_start_date
from lms.djangoapps.courseware.utils import (
create_financial_assistance_application,
get_financial_assistance_application_status,
is_eligible_for_financial_aid
is_eligible_for_financial_aid,
)


@ddt.ddt
class TestAccessUtils(TestCase):
"""Tests for the access_utils functions."""

@ddt.data(
# days_early_for_beta, is_beta_user, expected_date
(None, True, "2025-01-03"),
(2, True, "2025-01-01"),
(timedelta.max.days + 10, True, "2025-01-03"),
(None, False, "2025-01-03"),
(2, False, "2025-01-03"),
(timedelta.max.days + 10, False, "2025-01-03"),
)
@ddt.unpack
def test_adjust_start_date(self, days_early_for_beta, is_beta_user, expected_date):
"""Tests adjust_start_date
Should only modify the date if the user is beta for the course,
and `days_early_for_beta` is sensible number."""
start = date(2025, 1, 3)
expected = date.fromisoformat(expected_date)
user = "princessofpower"
course_key = "edx1+8675"
with patch("lms.djangoapps.courseware.access_utils.CourseBetaTesterRole") as role_mock:
instance = role_mock.return_value
instance.has_user.return_value = is_beta_user
adjusted_date = adjust_start_date(user, days_early_for_beta, start, course_key)
self.assertEqual(expected, adjusted_date)


@ddt.ddt
class TestFinancialAssistanceViews(TestCase):
"""
Expand All @@ -29,17 +62,17 @@ class TestFinancialAssistanceViews(TestCase):

def setUp(self) -> None:
super().setUp()
self.test_course_id = 'course-v1:edX+Test+1'
self.test_course_id = "course-v1:edX+Test+1"
self.user = UserFactory()
self.global_staff = GlobalStaffFactory.create()
_ = FinancialAssistanceConfigurationFactory(
api_base_url='http://financial.assistance.test:1234',
api_base_url="http://financial.assistance.test:1234",
service_username=self.global_staff.username,
fa_backend_enabled_courses_percentage=100,
enabled=True
enabled=True,
)
_ = Application.objects.create(
name='Test Application',
name="Test Application",
user=self.global_staff,
client_type=Application.CLIENT_PUBLIC,
authorization_grant_type=Application.GRANT_CLIENT_CREDENTIALS,
Expand All @@ -51,33 +84,31 @@ def _mock_response(self, status_code, content=None):
"""
mock_response = Response()
mock_response.status_code = status_code
mock_response._content = json.dumps(content).encode('utf-8') # pylint: disable=protected-access
mock_response._content = json.dumps(content).encode("utf-8") # pylint: disable=protected-access
return mock_response

@ddt.data(
{'is_eligible': True, 'reason': None},
{'is_eligible': False, 'reason': 'This course is not eligible for financial aid'}
{"is_eligible": True, "reason": None},
{"is_eligible": False, "reason": "This course is not eligible for financial aid"},
)
def test_is_eligible_for_financial_aid(self, response_data):
"""
Tests the functionality of is_eligible_for_financial_aid which calls edx-financial-assistance backend
to return eligibility status for financial assistance for a given course.
"""
with patch.object(OAuthAPIClient, 'request') as oauth_mock:
with patch.object(OAuthAPIClient, "request") as oauth_mock:
oauth_mock.return_value = self._mock_response(status.HTTP_200_OK, response_data)
is_eligible, reason = is_eligible_for_financial_aid(self.test_course_id)
assert is_eligible is response_data.get('is_eligible')
assert reason == response_data.get('reason')
assert is_eligible is response_data.get("is_eligible")
assert reason == response_data.get("reason")

def test_is_eligible_for_financial_aid_invalid_course_id(self):
"""
Tests the functionality of is_eligible_for_financial_aid for an invalid course id.
"""
error_message = f"Invalid course id {self.test_course_id} provided"
with patch.object(OAuthAPIClient, 'request') as oauth_mock:
oauth_mock.return_value = self._mock_response(
status.HTTP_400_BAD_REQUEST, {"message": error_message}
)
with patch.object(OAuthAPIClient, "request") as oauth_mock:
oauth_mock.return_value = self._mock_response(status.HTTP_400_BAD_REQUEST, {"message": error_message})
is_eligible, reason = is_eligible_for_financial_aid(self.test_course_id)
assert is_eligible is False
assert reason == error_message
Expand All @@ -86,9 +117,9 @@ def test_is_eligible_for_financial_aid_invalid_unexpected_error(self):
"""
Tests the functionality of is_eligible_for_financial_aid for an unexpected error
"""
with patch.object(OAuthAPIClient, 'request') as oauth_mock:
with patch.object(OAuthAPIClient, "request") as oauth_mock:
oauth_mock.return_value = self._mock_response(
status.HTTP_500_INTERNAL_SERVER_ERROR, {'error': 'unexpected error occurred'}
status.HTTP_500_INTERNAL_SERVER_ERROR, {"error": "unexpected error occurred"}
)
is_eligible, reason = is_eligible_for_financial_aid(self.test_course_id)
assert is_eligible is False
Expand All @@ -99,45 +130,39 @@ def test_get_financial_assistance_application_status(self):
Tests the functionality of get_financial_assistance_application_status against a user id and a course id
edx-financial-assistance backend to return status of a financial assistance application.
"""
test_response = {'id': 123, 'status': 'ACCEPTED', 'coupon_code': 'ABCD..'}
with patch.object(OAuthAPIClient, 'request') as oauth_mock:
test_response = {"id": 123, "status": "ACCEPTED", "coupon_code": "ABCD.."}
with patch.object(OAuthAPIClient, "request") as oauth_mock:
oauth_mock.return_value = self._mock_response(status.HTTP_200_OK, test_response)
has_application, reason = get_financial_assistance_application_status(self.user.id, self.test_course_id)
assert has_application is True
assert reason == test_response

@ddt.data(
{
'status': status.HTTP_400_BAD_REQUEST,
'content': {'message': 'Invalid course id provided'}
},
{
'status': status.HTTP_404_NOT_FOUND,
'content': {'message': 'Application details not found'}
}
{"status": status.HTTP_400_BAD_REQUEST, "content": {"message": "Invalid course id provided"}},
{"status": status.HTTP_404_NOT_FOUND, "content": {"message": "Application details not found"}},
)
def test_get_financial_assistance_application_status_unsuccessful(self, response_data):
"""
Tests unsuccessful scenarios of get_financial_assistance_application_status
against a user id and a course id edx-financial-assistance backend.
"""
with patch.object(OAuthAPIClient, 'request') as oauth_mock:
oauth_mock.return_value = self._mock_response(response_data.get('status'), response_data.get('content'))
with patch.object(OAuthAPIClient, "request") as oauth_mock:
oauth_mock.return_value = self._mock_response(response_data.get("status"), response_data.get("content"))
has_application, reason = get_financial_assistance_application_status(self.user.id, self.test_course_id)
assert has_application is False
assert reason == response_data.get('content').get('message')
assert reason == response_data.get("content").get("message")

def test_create_financial_assistance_application(self):
"""
Tests the functionality of create_financial_assistance_application which calls edx-financial-assistance backend
to create a new financial assistance application given a form data.
"""
test_form_data = {
'lms_user_id': self.user.id,
'course_id': self.test_course_id,
"lms_user_id": self.user.id,
"course_id": self.test_course_id,
}
with patch.object(OAuthAPIClient, 'request') as oauth_mock:
oauth_mock.return_value = self._mock_response(status.HTTP_200_OK, {'success': True})
with patch.object(OAuthAPIClient, "request") as oauth_mock:
oauth_mock.return_value = self._mock_response(status.HTTP_200_OK, {"success": True})
response = create_financial_assistance_application(form_data=test_form_data)
assert response.status_code == status.HTTP_204_NO_CONTENT

Expand All @@ -147,12 +172,12 @@ def test_create_financial_assistance_application_bad_request(self):
to create a new financial assistance application given a form data.
"""
test_form_data = {
'lms_user_id': self.user.id,
'course_id': 'invalid_course_id',
"lms_user_id": self.user.id,
"course_id": "invalid_course_id",
}
error_response = {'message': 'Invalid course id provided'}
with patch.object(OAuthAPIClient, 'request') as oauth_mock:
error_response = {"message": "Invalid course id provided"}
with patch.object(OAuthAPIClient, "request") as oauth_mock:
oauth_mock.return_value = self._mock_response(status.HTTP_400_BAD_REQUEST, error_response)
response = create_financial_assistance_application(form_data=test_form_data)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert json.loads(response.content.decode('utf-8')) == error_response
assert json.loads(response.content.decode("utf-8")) == error_response

0 comments on commit 2626afc

Please sign in to comment.