Skip to content

Commit

Permalink
fix: don't log error if course-effort is missing
Browse files Browse the repository at this point in the history
we'll log an error for invalid effort values only
  • Loading branch information
shadinaif committed Dec 22, 2024
1 parent 1362a0b commit 77534d6
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from lms.djangoapps.certificates.models import GeneratedCertificate
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers.exceptions import FXCodedException, FXExceptionCodes
from futurex_openedx_extensions.helpers.querysets import get_base_queryset_courses

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -68,7 +69,10 @@ def parse_course_effort(effort: str, course_id: str) -> float:
"""Parses course effort in HH:MM format and returns total hours as a float."""
try:
if not effort:
raise ValueError('Course effort is not set.')
raise FXCodedException(
FXExceptionCodes.COURSE_EFFORT_NOT_FOUND,
f'Course effort not found for course {course_id}'
)

parts = effort.split(':')
hours = int(parts[0])
Expand All @@ -86,6 +90,9 @@ def parse_course_effort(effort: str, course_id: str) -> float:

return round(total_hours, 1)

except FXCodedException:
return settings.FX_DEFAULT_COURSE_EFFORT

except (ValueError, IndexError) as exc:
log.exception(
'Invalid course-effort for course %s. Assuming default value (%s hours). Error: %s',
Expand Down
1 change: 1 addition & 0 deletions futurex_openedx_extensions/helpers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class FXExceptionCodes(Enum):
INVALID_INPUT = 4001

COURSE_CREATOR_NOT_FOUND = 5001
COURSE_EFFORT_NOT_FOUND = 5002

EXPORT_CSV_VIEW_RESPONSE_FAILURE = 6001
EXPORT_CSV_MISSING_REQUIRED_PARAMS = 6002
Expand Down
21 changes: 18 additions & 3 deletions tests/test_dashboard/test_statistics/test_certificates.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,27 @@ def test_get_learning_hours_count_for_default_course_effort(
('-55', 10 * 2, 'Invalid, neg hour, use default value'),
('0:10', 10 * 2, 'Invalid, course effort value is too small less then 0.5, use default value'),
])
def test_get_learning_hours_count_for_different_course_effor_format(
base_data, fx_permission_info, effort, expected_result, usecase
): # pylint: disable=unused-argument
def test_get_learning_hours_count_for_different_course_effort_format(
base_data, fx_permission_info, effort, expected_result, usecase, caplog,
): # pylint: disable=unused-argument, too-many-arguments
"""Verify get_learning_hours_count function for different course format."""
fx_permission_info['view_allowed_full_access_orgs'] = get_tenants_orgs([8])
fx_permission_info['view_allowed_any_access_orgs'] = get_tenants_orgs([8])
CourseOverview.objects.filter(id='course-v1:ORG8+1+1').update(effort=effort)
result = certificates.get_learning_hours_count(fx_permission_info)
assert result == expected_result, f'Wrong learning hours count usecase: {usecase}'
if usecase.startswith('Invalid'):
assert 'Invalid course-effort for course course-v1:ORG8+1+1. Assuming default value' in caplog.text


@pytest.mark.django_db
@override_settings(FX_DEFAULT_COURSE_EFFORT=10)
def test_get_learning_hours_count_for_different_course_effort_not_set(
base_data, fx_permission_info, caplog,
): # pylint: disable=unused-argument
"""Verify get_learning_hours_count function when the effort is not set."""
fx_permission_info['view_allowed_full_access_orgs'] = get_tenants_orgs([8])
fx_permission_info['view_allowed_any_access_orgs'] = get_tenants_orgs([8])
result = certificates.get_learning_hours_count(fx_permission_info)
assert result == 10 * 2
assert 'Invalid course-effort for course course-v1:ORG8+1+1. Assuming default value' not in caplog.text

0 comments on commit 77534d6

Please sign in to comment.