fix: Clear error reason when setting a non-error status (#27273)

MICROBA-1106

lms/djangoapps/certificates/generation.py

@@ -80,7 +80,8 @@ def _generate_certificate(user, course_key):
             'grade': course_grade.percent,
             'download_url': '',
             'key': key,
-            'verify_uuid': uuid
+            'verify_uuid': uuid,
+            'error_reason': ''
         }
     )
 

lms/djangoapps/certificates/models.py

@@ -41,7 +41,7 @@ class CertificateStatuses:
     """
     Enum for certificate statuses.
 
-    Not all of these status are currently used. Some are kept for historical reasons and because existing course
+    Not all of these statuses are currently used. Some are kept for historical reasons and because existing course
     certificates may have been granted that status.
 
     audit_notpassing    - User is in the audit track and has not achieved a passing grade.
@@ -342,25 +342,17 @@ class GeneratedCertificate(models.Model):
     def invalidate(self):
         """
         Invalidate Generated Certificate by  marking it 'unavailable'.
-
-        Following is the list of fields with their defaults
-            1 - verify_uuid = '',
-            2 - download_uuid = '',
-            3 - download_url = '',
-            4 - grade = ''
-            5 - status = 'unavailable'
         """
-        log.info('Marking certificate as unavailable for {user} : {course}'.format(
-            user=self.user.id,
-            course=self.course_id
-        ))
+        log.info(f'Marking certificate as unavailable for {self.user.id} : {self.course_id}')
 
+        self.error_reason = ''
         self.verify_uuid = ''
         self.download_uuid = ''
         self.download_url = ''
         self.grade = ''
         self.status = CertificateStatuses.unavailable
         self.save()
+
         COURSE_CERT_REVOKED.send_robust(
             sender=self.__class__,
             user=self.user,
@@ -371,19 +363,18 @@ class GeneratedCertificate(models.Model):
 
     def mark_notpassing(self, grade):
         """
-        Invalidates a Generated Certificate by marking it as not passing
+        Invalidates a Generated Certificate by marking it as notpassing
         """
-        log.info('Marking certificate as notpassing for {user} : {course}'.format(
-            user=self.user.id,
-            course=self.course_id
-        ))
+        log.info(f'Marking certificate as notpassing for {self.user.id} : {self.course_id}')
 
+        self.error_reason = ''
         self.verify_uuid = ''
         self.download_uuid = ''
         self.download_url = ''
         self.grade = grade
         self.status = CertificateStatuses.notpassing
         self.save()
+
         COURSE_CERT_REVOKED.send_robust(
             sender=self.__class__,
             user=self.user,

lms/djangoapps/certificates/tests/test_generation.py

@@ -9,6 +9,7 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, U
 from common.djangoapps.util.testing import EventTestMixin
 from lms.djangoapps.certificates.generation import generate_course_certificate
 from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
+from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
 from openedx.core.djangoapps.certificates.config import waffle
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -42,7 +43,7 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
             user=u,
             course_id=key,
             is_active=True,
-            mode="verified",
+            mode='verified',
         )
         gen_mode = 'batch'
 
@@ -64,3 +65,36 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
             certificate_url='',
             generation_mode=gen_mode
         )
+
+    def test_generation_existing(self):
+        """
+        Test certificate generation when a certificate already exists
+        """
+        # Create user, a course run, and an enrollment
+        u = UserFactory()
+        cr = CourseFactory()
+        key = cr.id  # pylint: disable=no-member
+        CourseEnrollmentFactory(
+            user=u,
+            course_id=key,
+            is_active=True,
+            mode='verified',
+        )
+        error_reason = 'Some PDF error'
+        GeneratedCertificateFactory(
+            user=u,
+            course_id=key,
+            mode='verified',
+            status=CertificateStatuses.error,
+            error_reason=error_reason
+        )
+        gen_mode = 'batch'
+
+        cert = GeneratedCertificate.objects.get(user=u, course_id=key)
+        assert cert.error_reason == error_reason
+
+        generated_cert = generate_course_certificate(u, key, gen_mode)
+        assert generated_cert.status, CertificateStatuses.downloadable
+
+        cert = GeneratedCertificate.objects.get(user=u, course_id=key)
+        assert cert.error_reason == ''