Merge pull request #14547 from edx/ormsbee/all_certs_for_user

Find the courses a user has certs for up front.

common/djangoapps/student/models.py

@@ -1531,10 +1531,26 @@ class CourseEnrollment(models.Model):
         """Changes this `CourseEnrollment` record's mode to `mode`.  Saves immediately."""
         self.update_enrollment(mode=mode)
 
-    def refundable(self):
+    def refundable(self, user_already_has_certs_for=None):
         """
-        For paid/verified certificates, students may receive a refund if they have
-        a verified certificate and the deadline for refunds has not yet passed.
+        For paid/verified certificates, students may always receive a refund if
+        this CourseEnrollment's `can_refund` attribute is not `None` (that
+        overrides all other rules).
+
+        If the `.can_refund` attribute is `None` or doesn't exist, then ALL of
+        the following must be true for this enrollment to be refundable:
+
+            * The user does not have a certificate issued for this course.
+            * 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.
         """
         # In order to support manual refunds past the deadline, set can_refund on this object.
         # On unenrolling, the "UNENROLL_DONE" signal calls CertificateItem.refund_cert_callback(),
@@ -1545,8 +1561,12 @@ class CourseEnrollment(models.Model):
             return True
 
         # If the student has already been given a certificate they should not be refunded
-        if GeneratedCertificate.certificate_for_student(self.user, self.course_id) is not None:
-            return False
+        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
 
         # If it is after the refundable cutoff date they should not be refunded.
         refund_cutoff_date = self.refund_cutoff_date()

common/djangoapps/student/tests/test_refunds.py

@@ -21,7 +21,7 @@ from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 
 # These imports refer to lms djangoapps.
 # Their testcases are only run under lms.
-from certificates.models import CertificateStatuses  # pylint: disable=import-error
+from certificates.models import CertificateStatuses, GeneratedCertificate  # pylint: disable=import-error
 from certificates.tests.factories import GeneratedCertificateFactory  # pylint: disable=import-error
 from openedx.core.djangoapps.commerce.utils import ECOMMERCE_DATE_FORMAT
 
@@ -106,10 +106,20 @@ class RefundableTest(SharedModuleStoreTestCase):
         )
 
         self.assertFalse(self.enrollment.refundable())
+        self.assertFalse(
+            self.enrollment.refundable(
+                user_already_has_certs_for=GeneratedCertificate.course_ids_with_certs_for_user(self.user)
+            )
+        )
 
         # Assert that can_refund overrides this and allows refund
         self.enrollment.can_refund = True
         self.assertTrue(self.enrollment.refundable())
+        self.assertTrue(
+            self.enrollment.refundable(
+                user_already_has_certs_for=GeneratedCertificate.course_ids_with_certs_for_user(self.user)
+            )
+        )
 
     def test_refundable_with_cutoff_date(self):
         """ Assert enrollment is refundable before cutoff and not refundable after."""

common/djangoapps/student/views.py

@@ -61,7 +61,9 @@ from student.tasks import send_activation_email
 from lms.djangoapps.commerce.utils import EcommerceService  # pylint: disable=import-error
 from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification  # pylint: disable=import-error
 from bulk_email.models import Optout, BulkEmailFlag  # pylint: disable=import-error
-from certificates.models import CertificateStatuses, certificate_status_for_student
+from certificates.models import (  # pylint: disable=import-error
+    CertificateStatuses, GeneratedCertificate, certificate_status_for_student
+)
 from certificates.api import (  # pylint: disable=import-error
     get_certificate_url,
     has_html_certificates_enabled,
@@ -716,9 +718,12 @@ def dashboard(request):
     statuses = ["approved", "denied", "pending", "must_reverify"]
     reverifications = reverification_info(statuses)
 
+    user_already_has_certs_for = GeneratedCertificate.course_ids_with_certs_for_user(request.user)
     show_refund_option_for = frozenset(
         enrollment.course_id for enrollment in course_enrollments
-        if enrollment.refundable()
+        if enrollment.refundable(
+            user_already_has_certs_for=user_already_has_certs_for
+        )
     )
 
     block_courses = frozenset(

lms/djangoapps/certificates/models.py

@@ -260,6 +260,23 @@ class GeneratedCertificate(models.Model):
 
         return None
 
+    @classmethod
+    def course_ids_with_certs_for_user(cls, user):
+        """
+        Return a set of CourseKeys for which the user has certificates.
+
+        Sometimes we just want to check if a user has already been issued a
+        certificate for a given course (e.g. to test refund elibigility).
+        Instead of checking if `certificate_for_student` returns `None` on each
+        course_id individually, we instead just return a set of all CourseKeys
+        for which this student has certificates all at once.
+        """
+        return {
+            cert.course_id
+            for cert
+            in cls.objects.filter(user=user).only('course_id')  # pylint: disable=no-member
+        }
+
     @classmethod
     def get_unique_statuses(cls, course_key=None, flat=False):
         """

lms/djangoapps/certificates/tests/tests.py

@@ -91,6 +91,42 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
         certificate_info = certificate_info_for_user(student, course.id, grade, whitelisted)
         self.assertEqual(certificate_info, output)
 
+    def test_course_ids_with_certs_for_user(self):
+        # Create one user with certs and one without
+        student_no_certs = UserFactory()
+        student_with_certs = UserFactory()
+        student_with_certs.profile.allow_certificate = True
+        student_with_certs.profile.save()
+
+        # Set up a couple of courses
+        course_1 = CourseFactory.create()
+        course_2 = CourseFactory.create()
+
+        # Generate certificates
+        GeneratedCertificateFactory.create(
+            user=student_with_certs,
+            course_id=course_1.id,
+            status=CertificateStatuses.downloadable,
+            mode='honor'
+        )
+        GeneratedCertificateFactory.create(
+            user=student_with_certs,
+            course_id=course_2.id,
+            status=CertificateStatuses.downloadable,
+            mode='honor'
+        )
+
+        # User with no certs should return an empty set.
+        self.assertSetEqual(
+            GeneratedCertificate.course_ids_with_certs_for_user(student_no_certs),
+            set()
+        )
+        # User with certs should return a set with the two course_ids
+        self.assertSetEqual(
+            GeneratedCertificate.course_ids_with_certs_for_user(student_with_certs),
+            {course_1.id, course_2.id}
+        )
+
     @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True})
     def test_course_milestone_collected(self):
         student = UserFactory()