Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Corrects issue with refund logic and certificates.

Browse Source 
[MICROBA-1307]

Before this change a user would not be auto refunded if they had a
certificate in a course with any status. This had unintended
consequences. This change updates the logic to only block auto refund
for statuses that we do not want to refund on such as downloadable.
This commit is contained in:
Albert (AJ) St. Aubin
2021-06-24 08:18:17 -04:00
parent 9616a1c367
commit b23169560f
10 changed files with 109 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
common/djangoapps/entitlements/models.py
View File

@@ -18,7 +18,8 @@ from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.entitlements.utils import is_course_run_entitlement_fulfillable
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentException
from common.djangoapps.util.date_utils import strftime_localized
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.certificates import api as certificates_api
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.commerce.utils import refund_entitlement
from openedx.core.djangoapps.catalog.utils import get_course_uuid_for_course
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
@@ -95,8 +96,11 @@ class CourseEntitlementPolicy(models.Model):
return False
if entitlement.enrollment_course_run:
if GeneratedCertificate.certificate_for_student(
entitlement.user_id, entitlement.enrollment_course_run.course_id) is not None:
certificate = certificates_api.get_certificate_for_user_id(
entitlement.user,
entitlement.enrollment_course_run.course_id
)
if certificate and not CertificateStatuses.is_refundable_status(certificate.status):
return False
# This is >= because a days_until_expiration 0 means that the expiration day has not fully passed yet

9
common/djangoapps/entitlements/tests/test_models.py
View File

@@ -189,14 +189,14 @@ class TestModels(TestCase):
def test_is_entitlement_regainable(self):
"""
Test that the entitlement is not expired when created now, and is expired when created20 days
Test that the entitlement is not expired when created now, and is expired when created 20 days
ago with a policy that sets the expiration period to 14 days
"""
entitlement = CourseEntitlementFactory.create(enrollment_course_run=self.enrollment)
assert entitlement.is_entitlement_regainable() is True
# Create and associate a GeneratedCertificate for a user and course and make sure it isn't regainable
GeneratedCertificateFactory(
certificate = GeneratedCertificateFactory(
user=entitlement.user,
course_id=entitlement.enrollment_course_run.course_id,
mode=MODES.verified,
@@ -205,6 +205,11 @@ class TestModels(TestCase):
assert entitlement.is_entitlement_regainable() is False
certificate.status = CertificateStatuses.notpassing
certificate.save()
assert entitlement.is_entitlement_regainable() is True
# Create a date 20 days in the past (greater than the policy expire period of 14 days)
# and apply it to both the entitlement and the course
past_datetime = now() - timedelta(days=20)

26
common/djangoapps/student/models.py
View File

@@ -66,7 +66,7 @@ from common.djangoapps.student.signals import ENROLL_STATUS_CHANGE, ENROLLMENT_T
from common.djangoapps.track import contexts, segment
from common.djangoapps.util.model_utils import emit_field_changed_events, get_changed_fields_dict
from common.djangoapps.util.query import use_read_replica_if_available
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.courseware.models import (
CourseDynamicUpgradeDeadlineConfiguration,
DynamicUpgradeDeadlineConfiguration,
@@ -1835,7 +1835,7 @@ class CourseEnrollment(models.Model):
"""Changes this `CourseEnrollment` record's mode to `mode`. Saves immediately."""
self.update_enrollment(mode=mode)
def refundable(self, user_already_has_certs_for=None):
def refundable(self):
"""
For paid/verified certificates, students may always receive a refund if
this CourseEnrollment's `can_refund` attribute is not `None` (that
@@ -1848,11 +1848,6 @@ class CourseEnrollment(models.Model):
* We are not past the refund cutoff date
* There exists a 'verified' CourseMode for this course.
Arguments:
`user_already_has_certs_for` (set of `CourseKey`):
An optional param that is a set of `CourseKeys` that the user
has already been issued certificates in.
Returns:
bool: Whether is CourseEnrollment can be refunded.
"""
@@ -1865,13 +1860,16 @@ class CourseEnrollment(models.Model):
if getattr(self, 'can_refund', None) is not None:
return True
# If the student has already been given a certificate they should not be refunded
if user_already_has_certs_for is not None:
if self.course_id in user_already_has_certs_for:
return False
else:
if GeneratedCertificate.certificate_for_student(self.user, self.course_id) is not None:
return False
# Due to circular import issues this import was placed close to usage. To move this to the
# top of the file would require a large scale refactor of the refund code.
import lms.djangoapps.certificates.api
# If the student has already been given a certificate in a non refundable status they should not be refunded
certificate = lms.djangoapps.certificates.api.get_certificate_for_user_id(
self.user,
self.course_id
)
if certificate and not CertificateStatuses.is_refundable_status(certificate.status):
return False
# If it is after the refundable cutoff date they should not be refunded.
refund_cutoff_date = self.refund_cutoff_date()

16
common/djangoapps/student/tests/test_refunds.py
View File

@@ -25,7 +25,6 @@ from common.djangoapps.course_modes.tests.factories import CourseModeFactory
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAttribute, EnrollmentRefundConfiguration
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
from openedx.core.djangoapps.commerce.utils import ECOMMERCE_DATE_FORMAT
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
@@ -88,7 +87,7 @@ class RefundableTest(SharedModuleStoreTestCase):
assert self.enrollment.refundable()
GeneratedCertificateFactory.create(
certificate = GeneratedCertificateFactory.create(
user=self.user,
course_id=self.course.id,
status=CertificateStatuses.downloadable,
@@ -96,15 +95,18 @@ class RefundableTest(SharedModuleStoreTestCase):
)
assert not self.enrollment.refundable()
assert not self.enrollment.\
refundable(user_already_has_certs_for=GeneratedCertificate.course_ids_with_certs_for_user(self.user))
# Certificates that are not in the PASSED_STATUSES should allow a refund
certificate.status = CertificateStatuses.notpassing
certificate.save()
assert self.enrollment.refundable()
# Assert that can_refund overrides this and allows refund
certificate.status = CertificateStatuses.downloadable
certificate.save()
self.enrollment.can_refund = True
assert self.enrollment.refundable()
assert self.enrollment.refundable(
user_already_has_certs_for=GeneratedCertificate.course_ids_with_certs_for_user(self.user)
)
@patch('common.djangoapps.student.models.CourseEnrollment.refund_cutoff_date')
def test_refundable_with_cutoff_date(self, cutoff_date):

24
lms/djangoapps/certificates/api.py
View File

@@ -54,15 +54,6 @@ User = get_user_model()
MODES = GeneratedCertificate.MODES
def is_passing_status(cert_status):
"""
Given the status of a certificate, return a boolean indicating whether
the student passed the course. This just proxies to the classmethod
defined in models.py
"""
return CertificateStatuses.is_passing_status(cert_status)
def _format_certificate_for_user(username, cert):
"""
Helper function to serialize an user certificate.
@@ -82,7 +73,7 @@ def _format_certificate_for_user(username, cert):
"grade": cert.grade,
"created": cert.created_date,
"modified": cert.modified_date,
"is_passing": is_passing_status(cert.status),
"is_passing": CertificateStatuses.is_passing_status(cert.status),
"is_pdf_certificate": bool(cert.download_url),
"download_url": (
cert.download_url or get_certificate_url(cert.user.id, cert.course_id, uuid=cert.verify_uuid,
@@ -154,6 +145,19 @@ def get_certificate_for_user(username, course_key, format_results=True):
return cert
def get_certificate_for_user_id(user, course_id):
"""
Retrieve certificate information for a user in a specific course.
Arguments:
user (User): A Django User.
course_id (CourseKey): Course ID
Returns:
A GeneratedCertificate object.
"""
return GeneratedCertificate.certificate_for_student(user, course_id)
def get_certificates_for_user_by_course_keys(user, course_keys):
"""
Retrieve certificate information for a particular user for a set of courses.

15
lms/djangoapps/certificates/data.py
View File

@@ -58,6 +58,7 @@ class CertificateStatuses:
}
PASSED_STATUSES = (downloadable, generating)
NON_REFUNDABLE_STATUSES = (downloadable, generating, unavailable)
@classmethod
def is_passing_status(cls, status):
@@ -66,3 +67,17 @@ class CertificateStatuses:
the student passed the course.
"""
return status in cls.PASSED_STATUSES
@classmethod
def is_refundable_status(cls, status):
"""
Given the status of a certificate, check to see if that certificate status can
be refunded.
Arguments:
status (str): The status of the certificate that you are checking
Returns:
bool: True if the status is refundable.
"""
return status not in cls.NON_REFUNDABLE_STATUSES

13
lms/djangoapps/certificates/tests/test_api.py
View File

@@ -44,6 +44,7 @@ from lms.djangoapps.certificates.api import (
get_allowlisted_users,
get_certificate_footer_context,
get_certificate_for_user,
get_certificate_for_user_id,
get_certificate_header_context,
get_certificate_invalidation_entry,
get_certificate_url,
@@ -429,6 +430,18 @@ class CertificateGetTests(SharedModuleStoreTestCase):
assert cert['is_passing'] is True
assert cert['download_url'] == 'www.google.com'
def test_get_certificate_for_user_id(self):
"""
Test to get a certificate for a user id for a specific course.
"""
cert = get_certificate_for_user_id(self.student, self.web_cert_course.id)
assert cert is not None
assert cert.course_id == self.web_cert_course.id
assert cert.mode == CourseMode.VERIFIED
assert cert.status == CertificateStatuses.downloadable
assert cert.grade == '0.88'
def test_get_certificates_for_user(self):
"""
Test to get all the certificates for a user

27
lms/djangoapps/certificates/tests/test_data.py Normal file
View File

@@ -0,0 +1,27 @@
"""Tests for the certificates Python Data Class. """
from django.test import TestCase
from lms.djangoapps.certificates.data import CertificateStatuses
class CertificateStatusAPITests(TestCase):
"""
Test the APIs related to certificate status.
"""
def test_is_refundable_status(self):
"""
Test is a certificate has a refundable status.
"""
assert not CertificateStatuses.is_refundable_status(CertificateStatuses.downloadable)
assert CertificateStatuses.is_refundable_status(CertificateStatuses.notpassing)
def test_is_passing_status(self):
"""
Test is a certificate has a refundable status.
"""
assert not CertificateStatuses.is_passing_status(
CertificateStatuses.notpassing
)
assert CertificateStatuses.is_passing_status(
CertificateStatuses.downloadable
)

5
openedx/core/djangoapps/programs/utils.py
View File

@@ -25,6 +25,7 @@ from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.entitlements.api import get_active_entitlement_list_for_user
from common.djangoapps.entitlements.models import CourseEntitlement
from lms.djangoapps.certificates import api as certificate_api
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.commerce.utils import EcommerceService
from openedx.core.djangoapps.catalog.api import get_programs_by_type
@@ -328,7 +329,7 @@ class ProgramProgressMeter:
modes_match = course_run_mode == certificate_mode
# Grab the available date and keep it if it's the earliest one for this catalog course.
if modes_match and certificate_api.is_passing_status(certificate.status):
if modes_match and CertificateStatuses.is_passing_status(certificate.status):
course_overview = CourseOverview.get_from_id(key)
available_date = available_date_for_certificate(course_overview, certificate)
earliest_course_run_date = min(
@@ -447,7 +448,7 @@ class ProgramProgressMeter:
except CourseOverview.DoesNotExist:
may_certify = True
if (
certificate_api.is_passing_status(certificate['status'])
CertificateStatuses.is_passing_status(certificate['status'])
and may_certify
):
completed_runs.append(course_data)

4
openedx/features/learner_profile/tests/views/test_learner_profile.py
View File

@@ -14,7 +14,7 @@ from opaque_keys.edx.locator import CourseLocator
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from common.djangoapps.util.testing import UrlResetMixin
from lms.djangoapps.certificates.api import is_passing_status
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
from lms.envs.test import CREDENTIALS_PUBLIC_SERVICE_URL
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
@@ -171,7 +171,7 @@ class LearnerProfileViewTest(SiteMixin, UrlResetMixin, ModuleStoreTestCase):
cert.save()
# Ensure that this test is actually using both passing and non-passing certs.
assert is_passing_status(cert.status) == is_passed_status
assert CertificateStatuses.is_passing_status(cert.status) == is_passed_status
response = self.client.get(f'/u/{self.user.username}')