removed certificates delivered dependency on certificate eligibility

split variables on multiple lines

changes after feedback from Afzal

lms/djangoapps/certificates/models.py

@@ -304,22 +304,18 @@ def certificate_info_for_user(user, course_id, grade, user_is_whitelisted=None):
             user=user, course_id=course_id, whitelist=True
         ).exists()
 
-    eligible_for_certificate = (user_is_whitelisted or grade is not None) and user.profile.allow_certificate
-
-    if eligible_for_certificate:
-        user_is_eligible = 'Y'
-
-        certificate_status = certificate_status_for_student(user, course_id)
-        certificate_generated = certificate_status['status'] == CertificateStatuses.downloadable
-        certificate_is_delivered = 'Y' if certificate_generated else 'N'
-
-        certificate_type = certificate_status['mode'] if certificate_generated else 'N/A'
-    else:
-        user_is_eligible = 'N'
-        certificate_is_delivered = 'N'
-        certificate_type = 'N/A'
-
-    return [user_is_eligible, certificate_is_delivered, certificate_type]
+    certificate_is_delivered = 'N'
+    certificate_type = 'N/A'
+    eligible_for_certificate = 'Y' if (user_is_whitelisted or grade is not None) and user.profile.allow_certificate \
+        else 'N'
+
+    certificate_status = certificate_status_for_student(user, course_id)
+    certificate_generated = certificate_status['status'] == CertificateStatuses.downloadable
+    if certificate_generated:
+        certificate_is_delivered = 'Y'
+        certificate_type = certificate_status['mode']
+
+    return [eligible_for_certificate, certificate_is_delivered, certificate_type]
 
 
 class ExampleCertificateSet(TimeStampedModel):

lms/djangoapps/certificates/tests/tests.py

@@ -60,6 +60,36 @@ class CertificatesModelTest(ModuleStoreTestCase):
         certificate_info = certificate_info_for_user(student, course.id, grade, whitelisted)
         self.assertEqual(certificate_info, output)
 
+    @unpack
+    @data(
+        {'allow_certificate': False, 'whitelisted': False, 'grade': None, 'output': ['N', 'Y', 'honor']},
+        {'allow_certificate': True, 'whitelisted': True, 'grade': None, 'output': ['Y', 'Y', 'honor']},
+        {'allow_certificate': True, 'whitelisted': False, 'grade': 0.9, 'output': ['Y', 'Y', 'honor']},
+        {'allow_certificate': False, 'whitelisted': True, 'grade': 0.8, 'output': ['N', 'Y', 'honor']},
+        {'allow_certificate': False, 'whitelisted': None, 'grade': 0.8, 'output': ['N', 'Y', 'honor']}
+    )
+    def test_certificate_info_for_user_when_grade_changes(self, allow_certificate, whitelisted, grade, output):
+        """
+        Verify that certificate_info_for_user works as expect in scenario when grading of problems
+        changes after certificates already generated. In such scenario `Certificate delivered` should not depend
+        on student's eligibility to get certificates since in above scenario eligibility can change over period
+        of time.
+        """
+        student = UserFactory()
+        course = CourseFactory.create(org='edx', number='verified', display_name='Verified Course')
+        student.profile.allow_certificate = allow_certificate
+        student.profile.save()
+
+        GeneratedCertificateFactory.create(
+            user=student,
+            course_id=course.id,
+            status=CertificateStatuses.downloadable,
+            mode='honor'
+        )
+
+        certificate_info = certificate_info_for_user(student, course.id, grade, whitelisted)
+        self.assertEqual(certificate_info, output)
+
     @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True})
     def test_course_milestone_collected(self):
         seed_milestone_relationship_types()