Merge pull request #17730 from edx/ri/EDUCATOR-2428-certificate-delivered-column

EDUCATOR-2428 add acceptance criteria for certificate delivered column in grade report

lms/djangoapps/certificates/models.py

@@ -66,6 +66,7 @@ from opaque_keys.edx.django.models import CourseKeyField
 from badges.events.course_complete import course_badge_check
 from badges.events.course_meta import completion_check, course_group_check
 from lms.djangoapps.instructor_task.models import InstructorTask
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.signals.signals import COURSE_CERT_AWARDED
 from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager
 from util.milestones_helpers import fulfill_course_milestone, is_prerequisite_courses_enabled
@@ -563,7 +564,7 @@ def certificate_status(generated_certificate):
         return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor, 'uuid': None}
 
 
-def certificate_info_for_user(user, grade, user_is_whitelisted, user_certificate):
+def certificate_info_for_user(user, course_id, grade, user_is_whitelisted, user_certificate):
     """
     Returns the certificate info for a user for grade report.
     """
@@ -573,8 +574,9 @@ def certificate_info_for_user(user, grade, user_is_whitelisted, user_certificate
         else 'N'
 
     status = certificate_status(user_certificate)
+    can_have_certificate = CourseOverview.get_from_id(course_id).may_certify()
     certificate_generated = status['status'] == CertificateStatuses.downloadable
-    if certificate_generated:
+    if certificate_generated and can_have_certificate:
         certificate_is_delivered = 'Y'
         certificate_type = status['mode']
 

lms/djangoapps/certificates/tests/tests.py

@@ -1,7 +1,9 @@
 """
 Tests for the certificates models.
 """
+from datetime import datetime, timedelta
 from ddt import data, ddt, unpack
+from pytz import UTC
 from django.conf import settings
 from milestones.tests.utils import MilestonesTestCaseMixin
 from mock import patch
@@ -28,6 +30,22 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
     Tests for the GeneratedCertificate model
     """
 
+    def setUp(self):
+        super(CertificatesModelTest, self).setUp()
+
+        today = datetime.now(UTC)
+        self.instructor_paced_course = CourseFactory.create(
+            org='edx', number='instructor', display_name='Instructor Paced Course',
+            start=today - timedelta(days=30),
+            end=today - timedelta(days=2),
+            certificate_available_date=today - timedelta(days=1),
+            self_paced=False
+        )
+        self.self_paced_course = CourseFactory.create(
+            org='edx', number='self',
+            display_name='Self Paced Course', self_paced=True
+        )
+
     def test_certificate_status_for_student(self):
         student = UserFactory()
         course = CourseFactory.create(org='edx', number='verified', display_name='Verified Course')
@@ -49,11 +67,21 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
         Verify that certificate_info_for_user works.
         """
         student = UserFactory()
-        _ = CourseFactory.create(org='edx', number='verified', display_name='Verified Course')
         student.profile.allow_certificate = allow_certificate
         student.profile.save()
 
-        certificate_info = certificate_info_for_user(student, grade, whitelisted, user_certificate=None)
+        # for instructor paced course
+        certificate_info = certificate_info_for_user(
+            student, self.instructor_paced_course.id, grade,
+            whitelisted, user_certificate=None
+        )
+        self.assertEqual(certificate_info, output)
+
+        # for self paced course
+        certificate_info = certificate_info_for_user(
+            student, self.self_paced_course.id, grade,
+            whitelisted, user_certificate=None
+        )
         self.assertEqual(certificate_info, output)
 
     @unpack
@@ -72,17 +100,35 @@ class CertificatesModelTest(ModuleStoreTestCase, MilestonesTestCaseMixin):
         of time.
         """
         student = UserFactory()
-        course = CourseFactory.create(org='edx', number='verified', display_name='Verified Course')
         student.profile.allow_certificate = allow_certificate
         student.profile.save()
 
-        certificate = GeneratedCertificateFactory.create(
+        certificate1 = GeneratedCertificateFactory.create(
             user=student,
-            course_id=course.id,
+            course_id=self.instructor_paced_course.id,
             status=CertificateStatuses.downloadable,
             mode='honor'
         )
-        certificate_info = certificate_info_for_user(student, grade, whitelisted, certificate)
+
+        certificate2 = GeneratedCertificateFactory.create(
+            user=student,
+            course_id=self.self_paced_course.id,
+            status=CertificateStatuses.downloadable,
+            mode='honor'
+        )
+
+        # for instructor paced course
+        certificate_info = certificate_info_for_user(
+            student, self.instructor_paced_course.id, grade,
+            whitelisted, certificate1
+        )
+        self.assertEqual(certificate_info, output)
+
+        # for self paced course
+        certificate_info = certificate_info_for_user(
+            student, self.self_paced_course.id, grade,
+            whitelisted, certificate2
+        )
         self.assertEqual(certificate_info, output)
 
     def test_course_ids_with_certs_for_user(self):

lms/djangoapps/instructor_task/tasks_helper/grades.py

@@ -401,6 +401,7 @@ class CourseGradeReport(object):
         is_whitelisted = user.id in bulk_certs.whitelisted_user_ids
         certificate_info = certificate_info_for_user(
             user,
+            context.course_id,
             course_grade.letter_grade,
             is_whitelisted,
             bulk_certs.certificates_by_user.get(user.id),

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

@@ -12,7 +12,7 @@ import os
 import shutil
 import tempfile
 import urllib
-from datetime import datetime
+from datetime import datetime, timedelta
 
 import ddt
 import unicodecsv
@@ -396,7 +396,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
 
         RequestCache.clear_request_cache()
 
-        expected_query_count = 36
+        expected_query_count = 41
         with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
             with check_mongo_calls(mongo_count):
                 with self.assertNumQueries(expected_query_count):
@@ -1809,12 +1809,19 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase):
 @patch('lms.djangoapps.instructor_task.tasks_helper.misc.DefaultStorage', new=MockDefaultStorage)
 class TestGradeReportEnrollmentAndCertificateInfo(TestReportMixin, InstructorTaskModuleTestCase):
     """
-    Test that grade report has correct user enrolment, verification, and certificate information.
+    Test that grade report has correct user enrollment, verification, and certificate information.
     """
     def setUp(self):
         super(TestGradeReportEnrollmentAndCertificateInfo, self).setUp()
 
-        self.initialize_course()
+        today = datetime.now(UTC)
+        course_factory_kwargs = {
+            'start': today - timedelta(days=30),
+            'end': today - timedelta(days=2),
+            'certificate_available_date': today - timedelta(days=1)
+        }
+
+        self.initialize_course(course_factory_kwargs)
 
         self.create_problem()