diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py
index b98240c193161d9b2b026cd4d261c72bbf74a3de..b2a00e65434aefa6aa06d9b14df79dc0313e7565 100644
--- a/lms/djangoapps/certificates/models.py
+++ b/lms/djangoapps/certificates/models.py
@@ -350,24 +350,31 @@ class CertificateGenerationHistory(TimeStampedModel):
students.
"""
task_input = self.instructor_task.task_input
- try:
- task_input_json = json.loads(task_input)
- except ValueError:
+ if not task_input.strip():
# if task input is empty, it means certificates were generated for all learners
# Translators: This string represents task was executed for all learners.
return _("All learners")
+ task_input_json = json.loads(task_input)
+
# get statuses_to_regenerate from task_input convert statuses to human readable strings and return
statuses = task_input_json.get('statuses_to_regenerate', None)
if statuses:
- return ", ".join(
- [CertificateStatuses.readable_statuses.get(status, "") for status in statuses]
- )
-
- # If students is present in task_input then, certificate generation task was run to
- # generate certificates for white listed students otherwise it is for all students.
- # Translators: This string represents task was executed for students having exceptions.
- return _("For exceptions") if 'students' in task_input_json else _("All learners")
+ readable_statuses = [
+ CertificateStatuses.readable_statuses.get(status) for status in statuses
+ if CertificateStatuses.readable_statuses.get(status) is not None
+ ]
+ return ", ".join(readable_statuses)
+
+ # If "student_set" is present in task_input, then this task only
+ # generates certificates for white listed students. Note that
+ # this key used to be "students", so we include that in this conditional
+ # for backwards compatibility.
+ if 'student_set' in task_input_json or 'students' in task_input_json:
+ # Translators: This string represents task was executed for students having exceptions.
+ return _("For exceptions")
+ else:
+ return _("All learners")
class Meta(object):
app_label = "certificates"
diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py
index 2227f78554a6e8729af4998f6851920b0519e46b..92f62f40263b50f25247013aec2babbf87afd96a 100644
--- a/lms/djangoapps/certificates/tests/test_models.py
+++ b/lms/djangoapps/certificates/tests/test_models.py
@@ -1,10 +1,13 @@
"""Tests for certificate Django models. """
+import ddt
from django.conf import settings
from django.core.exceptions import ValidationError
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase
from django.test.utils import override_settings
from nose.plugins.attrib import attr
+import json
+from mock import Mock
from path import Path as path
from certificates.models import (
@@ -14,10 +17,12 @@ from certificates.models import (
CertificateTemplateAsset,
GeneratedCertificate,
CertificateStatuses,
+ CertificateGenerationHistory,
)
from certificates.tests.factories import GeneratedCertificateFactory
+from instructor_task.tests.factories import InstructorTaskFactory
from opaque_keys.edx.locator import CourseLocator
-from student.tests.factories import UserFactory
+from student.tests.factories import AdminFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -229,3 +234,69 @@ class EligibleCertificateManagerTest(SharedModuleStoreTestCase):
list(GeneratedCertificate.objects.filter(user=self.user)), # pylint: disable=no-member
[self.eligible_cert, self.ineligible_cert]
)
+
+
+@attr('shard_1')
+@ddt.ddt
+class TestCertificateGenerationHistory(TestCase):
+ """
+ Test the CertificateGenerationHistory model's methods
+ """
+ @ddt.data(
+ ({"student_set": "whitelisted_not_generated"}, "For exceptions", True),
+ ({"student_set": "whitelisted_not_generated"}, "For exceptions", False),
+ # check "students" key for backwards compatibility
+ ({"students": [1, 2, 3]}, "For exceptions", True),
+ ({"students": [1, 2, 3]}, "For exceptions", False),
+ ({}, "All learners", True),
+ ({}, "All learners", False),
+ # test single status to regenerate returns correctly
+ ({"statuses_to_regenerate": ['downloadable']}, 'already received', True),
+ ({"statuses_to_regenerate": ['downloadable']}, 'already received', False),
+ # test that list of > 1 statuses render correctly
+ ({"statuses_to_regenerate": ['downloadable', 'error']}, 'already received, error states', True),
+ ({"statuses_to_regenerate": ['downloadable', 'error']}, 'already received, error states', False),
+ # test that only "readable" statuses are returned
+ ({"statuses_to_regenerate": ['downloadable', 'not_readable']}, 'already received', True),
+ ({"statuses_to_regenerate": ['downloadable', 'not_readable']}, 'already received', False),
+ )
+ @ddt.unpack
+ def test_get_certificate_generation_candidates(self, task_input, expected, is_regeneration):
+ staff = AdminFactory.create()
+ instructor_task = InstructorTaskFactory.create(
+ task_input=json.dumps(task_input),
+ requester=staff,
+ task_key=Mock(),
+ task_id=Mock(),
+ )
+ certificate_generation_history = CertificateGenerationHistory(
+ course_id=instructor_task.course_id,
+ generated_by=staff,
+ instructor_task=instructor_task,
+ is_regeneration=is_regeneration,
+ )
+ self.assertEqual(
+ certificate_generation_history.get_certificate_generation_candidates(),
+ expected
+ )
+
+ @ddt.data((True, "regenerated"), (False, "generated"))
+ @ddt.unpack
+ def test_get_task_name(self, is_regeneration, expected):
+ staff = AdminFactory.create()
+ instructor_task = InstructorTaskFactory.create(
+ task_input=json.dumps({}),
+ requester=staff,
+ task_key=Mock(),
+ task_id=Mock(),
+ )
+ certificate_generation_history = CertificateGenerationHistory(
+ course_id=instructor_task.course_id,
+ generated_by=staff,
+ instructor_task=instructor_task,
+ is_regeneration=is_regeneration,
+ )
+ self.assertEqual(
+ certificate_generation_history.get_task_name(),
+ expected
+ )