diff --git a/common/djangoapps/entitlements/models.py b/common/djangoapps/entitlements/models.py index eeb028066a..5090832637 100644 --- a/common/djangoapps/entitlements/models.py +++ b/common/djangoapps/entitlements/models.py @@ -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 diff --git a/common/djangoapps/entitlements/tests/test_models.py b/common/djangoapps/entitlements/tests/test_models.py index 3fb1d8e3b8..da15c09e26 100644 --- a/common/djangoapps/entitlements/tests/test_models.py +++ b/common/djangoapps/entitlements/tests/test_models.py @@ -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) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 9b20142039..c529df7c1d 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -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() diff --git a/common/djangoapps/student/tests/test_refunds.py b/common/djangoapps/student/tests/test_refunds.py index 67fc4f64da..ccc1a27fc3 100644 --- a/common/djangoapps/student/tests/test_refunds.py +++ b/common/djangoapps/student/tests/test_refunds.py @@ -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): diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 180f67b2ff..f0d3d3d51e 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -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. diff --git a/lms/djangoapps/certificates/data.py b/lms/djangoapps/certificates/data.py index 5d6381d55f..f8c670fffb 100644 --- a/lms/djangoapps/certificates/data.py +++ b/lms/djangoapps/certificates/data.py @@ -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 diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index bf737c1881..b3d18f89cb 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -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 diff --git a/lms/djangoapps/certificates/tests/test_data.py b/lms/djangoapps/certificates/tests/test_data.py new file mode 100644 index 0000000000..828633e812 --- /dev/null +++ b/lms/djangoapps/certificates/tests/test_data.py @@ -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 + ) diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index 62ac9fb314..133c05e19a 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -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) diff --git a/openedx/features/learner_profile/tests/views/test_learner_profile.py b/openedx/features/learner_profile/tests/views/test_learner_profile.py index 63299a7695..97d42a5cb9 100644 --- a/openedx/features/learner_profile/tests/views/test_learner_profile.py +++ b/openedx/features/learner_profile/tests/views/test_learner_profile.py @@ -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}')