diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 6d6bbccfd6..f121f34f69 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -6,8 +6,11 @@ rather than importing Django models directly. """ import logging +from django.conf import settings from django.core.urlresolvers import reverse +from xmodule.modulestore.django import modulestore + from certificates.models import ( CertificateStatuses as cert_status, certificate_status_for_student, @@ -21,7 +24,7 @@ from certificates.queue import XQueueCertInterface log = logging.getLogger("edx.certificate") -def generate_user_certificates(student, course_key, course=None): +def generate_user_certificates(student, course_key, course=None, insecure=False): """ It will add the add-cert request into the xqueue. @@ -36,9 +39,41 @@ def generate_user_certificates(student, course_key, course=None): Keyword Arguments: course (Course): Optionally provide the course object; if not provided it will be loaded. + insecure - (Boolean) """ xqueue = XQueueCertInterface() - xqueue.add_cert(student, course_key, course=course) + if insecure: + xqueue.use_https = False + generate_pdf = not has_html_certificates_enabled(course_key, course) + return xqueue.add_cert(student, course_key, course=course, generate_pdf=generate_pdf) + + +def regenerate_user_certificates(student, course_key, course=None, + forced_grade=None, template_file=None, insecure=False): + """ + It will add the regen-cert request into the xqueue. + + A new record will be created to track the certificate + generation task. If an error occurs while adding the certificate + to the queue, the task will have status 'error'. + + Args: + student (User) + course_key (CourseKey) + + Keyword Arguments: + course (Course): Optionally provide the course object; if not provided + it will be loaded. + grade_value - The grade string, such as "Distinction" + template_file - The template file used to render this certificate + insecure - (Boolean) + """ + xqueue = XQueueCertInterface() + if insecure: + xqueue.use_https = False + + generate_pdf = not has_html_certificates_enabled(course_key, course) + return xqueue.regen_cert(student, course_key, course, forced_grade, template_file, generate_pdf) def certificate_downloadable_status(student, course_key): @@ -156,6 +191,18 @@ def generate_example_certificates(course_key): xqueue.add_example_cert(cert) +def has_html_certificates_enabled(course_key, course=None): + """ + It determines if course has html certificates enabled + """ + html_certificates_enabled = False + if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): + course = course if course else modulestore().get_course(course_key, depth=0) + if get_active_web_certificate(course) is not None: + html_certificates_enabled = True + return html_certificates_enabled + + def example_certificates_status(course_key): """Check the status of example certificates for a course. diff --git a/lms/djangoapps/certificates/management/commands/regenerate_user.py b/lms/djangoapps/certificates/management/commands/regenerate_user.py index cacc23d512..ff53c41214 100644 --- a/lms/djangoapps/certificates/management/commands/regenerate_user.py +++ b/lms/djangoapps/certificates/management/commands/regenerate_user.py @@ -9,9 +9,8 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore.django import modulestore -from certificates.queue import XQueueCertInterface from certificates.models import BadgeAssertion - +from certificates.api import regenerate_user_certificates LOGGER = logging.getLogger(__name__) @@ -107,14 +106,11 @@ class Command(BaseCommand): ) # Add the certificate request to the queue - xqueue_interface = XQueueCertInterface() - if options['insecure']: - xqueue_interface.use_https = False - - ret = xqueue_interface.regen_cert( + ret = regenerate_user_certificates( student, course_id, course=course, forced_grade=options['grade_value'], - template_file=options['template_file'] + template_file=options['template_file'], + insecure=options['insecure'] ) try: diff --git a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py b/lms/djangoapps/certificates/management/commands/ungenerated_certs.py index 67d019ce80..1c7ddd67ac 100644 --- a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py +++ b/lms/djangoapps/certificates/management/commands/ungenerated_certs.py @@ -7,7 +7,7 @@ import datetime from pytz import UTC from django.core.management.base import BaseCommand, CommandError from certificates.models import certificate_status_for_student -from certificates.queue import XQueueCertInterface +from certificates.api import generate_user_certificates from django.contrib.auth.models import User from optparse import make_option from opaque_keys import InvalidKeyError @@ -108,9 +108,6 @@ class Command(BaseCommand): courseenrollment__course_id=course_key ) - xq = XQueueCertInterface() - if options['insecure']: - xq.use_https = False total = enrolled_students.count() count = 0 start = datetime.datetime.now(UTC) @@ -144,7 +141,12 @@ class Command(BaseCommand): if not options['noop']: # Add the certificate request to the queue - ret = xq.add_cert(student, course_key, course=course) + ret = generate_user_certificates( + student, + course_key, + course=course, + insecure=options['insecure'] + ) if ret == 'generating': LOGGER.info( diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 7926b23fdb..31c9e8e24f 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -104,7 +104,7 @@ class XQueueCertInterface(object): self.restricted = UserProfile.objects.filter(allow_certificate=False) self.use_https = True - def regen_cert(self, student, course_id, course=None, forced_grade=None, template_file=None): + def regen_cert(self, student, course_id, course=None, forced_grade=None, template_file=None, generate_pdf=True): """(Re-)Make certificate for a particular student in a particular course Arguments: @@ -154,7 +154,7 @@ class XQueueCertInterface(object): except GeneratedCertificate.DoesNotExist: pass - return self.add_cert(student, course_id, course, forced_grade, template_file) + return self.add_cert(student, course_id, course, forced_grade, template_file, generate_pdf) def del_cert(self, student, course_id): @@ -173,7 +173,9 @@ class XQueueCertInterface(object): raise NotImplementedError - def add_cert(self, student, course_id, course=None, forced_grade=None, template_file=None, title='None'): + # pylint: disable=too-many-statements + def add_cert(self, student, course_id, course=None, forced_grade=None, template_file=None, + title='None', generate_pdf=True): """ Request a new certificate for a student. @@ -183,6 +185,7 @@ class XQueueCertInterface(object): forced_grade - a string indicating a grade parameter to pass with the certificate request. If this is given, grading will be skipped. + generate_pdf - Boolean should a message be sent in queue to generate certificate PDF Will change the certificate status to 'generating'. @@ -340,35 +343,36 @@ class XQueueCertInterface(object): } if template_file: contents['template_pdf'] = template_file - new_status = status.generating + new_status = status.generating if generate_pdf else status.downloadable cert.status = new_status cert.save() - try: - self._send_to_xqueue(contents, key) - except XQueueAddToQueueError as exc: - new_status = ExampleCertificate.STATUS_ERROR - cert.status = new_status - cert.error_reason = unicode(exc) - cert.save() - LOGGER.critical( - ( - u"Could not add certificate task to XQueue. " - u"The course was '%s' and the student was '%s'." - u"The certificate task status has been marked as 'error' " - u"and can be re-submitted with a management command." - ), student.id, course_id - ) - else: - LOGGER.info( - ( - u"The certificate status has been set to '%s'. " - u"Sent a certificate grading task to the XQueue " - u"with the key '%s'. " - ), - key, - new_status - ) + if generate_pdf: + try: + self._send_to_xqueue(contents, key) + except XQueueAddToQueueError as exc: + new_status = ExampleCertificate.STATUS_ERROR + cert.status = new_status + cert.error_reason = unicode(exc) + cert.save() + LOGGER.critical( + ( + u"Could not add certificate task to XQueue. " + u"The course was '%s' and the student was '%s'." + u"The certificate task status has been marked as 'error' " + u"and can be re-submitted with a management command." + ), course_id, student.id + ) + else: + LOGGER.info( + ( + u"The certificate status has been set to '%s'. " + u"Sent a certificate grading task to the XQueue " + u"with the key '%s'. " + ), + new_status, + key + ) else: new_status = status.notpassing cert.status = new_status diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 2ba4cc11ae..2499deb350 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -4,6 +4,7 @@ import ddt from django.test import TestCase, RequestFactory from django.test.utils import override_settings +from django.conf import settings from mock import patch from nose.plugins.attrib import attr @@ -25,6 +26,9 @@ from certificates.models import ( from certificates.queue import XQueueCertInterface, XQueueAddToQueueError from certificates.tests.factories import GeneratedCertificateFactory +FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() +FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True + @attr('shard_1') class CertificateDownloadableStatusTests(ModuleStoreTestCase): @@ -134,7 +138,7 @@ class GenerateUserCertificatesTest(ModuleStoreTestCase): # Verify that the certificate has status 'generating' cert = GeneratedCertificate.objects.get(user=self.student, course_id=self.course.id) - self.assertEqual(cert.status, 'generating') + self.assertEqual(cert.status, CertificateStatuses.generating) def test_xqueue_submit_task_error(self): with self._mock_passing_grade(): @@ -146,6 +150,19 @@ class GenerateUserCertificatesTest(ModuleStoreTestCase): self.assertEqual(cert.status, 'error') self.assertIn(self.ERROR_REASON, cert.error_reason) + @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + def test_new_cert_requests_returns_generating_for_html_certificate(self): + """ + Test no message sent to Xqueue if HTML certificate view is enabled + """ + self._setup_course_certificate() + with self._mock_passing_grade(): + certs_api.generate_user_certificates(self.student, self.course.id) + + # Verify that the certificate has status 'downloadable' + cert = GeneratedCertificate.objects.get(user=self.student, course_id=self.course.id) + self.assertEqual(cert.status, CertificateStatuses.downloadable) + @contextmanager def _mock_passing_grade(self): """Mock the grading function to always return a passing grade. """ @@ -166,6 +183,26 @@ class GenerateUserCertificatesTest(ModuleStoreTestCase): yield mock_send_to_queue + def _setup_course_certificate(self): + """ + Creates certificate configuration for course + """ + certificates = [ + { + 'id': 1, + 'name': 'Test Certificate Name', + 'description': 'Test Certificate Description', + 'course_title': 'tes_course_title', + 'org_logo_path': '/t4x/orgX/testX/asset/org-logo.png', + 'signatories': [], + 'version': 1, + 'is_active': True + } + ] + self.course.certificates = {'certificates': certificates} + self.course.save() + self.store.update_item(self.course, self.user.id) + @attr('shard_1') @ddt.ddt diff --git a/lms/djangoapps/certificates/tests/test_cert_management.py b/lms/djangoapps/certificates/tests/test_cert_management.py index 0e9d7537db..d015e884e8 100644 --- a/lms/djangoapps/certificates/tests/test_cert_management.py +++ b/lms/djangoapps/certificates/tests/test_cert_management.py @@ -1,5 +1,6 @@ """Tests for the resubmit_error_certificates management command. """ import ddt +from contextlib import contextmanager from django.core.management.base import CommandError from nose.plugins.attrib import attr from django.test.utils import override_settings @@ -10,7 +11,7 @@ from certificates.tests.factories import BadgeAssertionFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls from student.tests.factories import UserFactory, CourseEnrollmentFactory -from certificates.management.commands import resubmit_error_certificates, regenerate_user +from certificates.management.commands import resubmit_error_certificates, regenerate_user, ungenerated_certs from certificates.models import GeneratedCertificate, CertificateStatuses, BadgeAssertion @@ -157,7 +158,7 @@ class RegenerateCertificatesTest(CertificateManagementTest): self.course = self.courses[0] @override_settings(CERT_QUEUE='test-queue') - @patch('certificates.management.commands.regenerate_user.XQueueCertInterface', spec=True) + @patch('certificates.api.XQueueCertInterface', spec=True) def test_clear_badge(self, xqueue): """ Given that I have a user with a badge @@ -174,6 +175,72 @@ class RegenerateCertificatesTest(CertificateManagementTest): grade_value=None ) xqueue.return_value.regen_cert.assert_called_with( - self.user, key, course=self.course, forced_grade=None, template_file=None + self.user, key, self.course, None, None, True ) self.assertFalse(BadgeAssertion.objects.filter(user=self.user, course_id=key)) + + @override_settings(CERT_QUEUE='test-queue') + @patch('capa.xqueue_interface.XQueueInterface.send_to_queue', spec=True) + def test_regenerating_certificate(self, mock_send_to_queue): + """ + Given that I have a user who has not passed course + If I run regeneration for that user + Then certificate generation will be not be requested + """ + key = self.course.location.course_key + self._create_cert(key, self.user, CertificateStatuses.downloadable) + self._run_command( + username=self.user.email, course=unicode(key), noop=False, insecure=True, template_file=None, + grade_value=None + ) + certificate = GeneratedCertificate.objects.get( + user=self.user, + course_id=key + ) + self.assertEqual(certificate.status, CertificateStatuses.notpassing) + self.assertFalse(mock_send_to_queue.called) + + +@attr('shard_1') +class UngenerateCertificatesTest(CertificateManagementTest): + """ + Tests for generating certificates. + """ + command = ungenerated_certs + + def setUp(self): + """ + We just need one course here. + """ + super(UngenerateCertificatesTest, self).setUp() + self.course = self.courses[0] + + @override_settings(CERT_QUEUE='test-queue') + @patch('capa.xqueue_interface.XQueueInterface.send_to_queue', spec=True) + def test_ungenerated_certificate(self, mock_send_to_queue): + """ + Given that I have ended course + If I run ungenerated certs command + Then certificates should be generated for all users who passed course + """ + mock_send_to_queue.return_value = (0, "Successfully queued") + key = self.course.location.course_key + self._create_cert(key, self.user, CertificateStatuses.unavailable) + with self._mock_passing_grade(): + self._run_command( + course=unicode(key), noop=False, insecure=True, force=False + ) + self.assertTrue(mock_send_to_queue.called) + certificate = GeneratedCertificate.objects.get( + user=self.user, + course_id=key + ) + self.assertEqual(certificate.status, CertificateStatuses.generating) + + @contextmanager + def _mock_passing_grade(self): + """Mock the grading function to always return a passing grade. """ + symbol = 'courseware.grades.grade' + with patch(symbol) as mock_grade: + mock_grade.return_value = {'grade': 'Pass', 'percent': 0.75} + yield diff --git a/lms/djangoapps/certificates/tests/test_queue.py b/lms/djangoapps/certificates/tests/test_queue.py index a644aee5f2..5ba7a05f6f 100644 --- a/lms/djangoapps/certificates/tests/test_queue.py +++ b/lms/djangoapps/certificates/tests/test_queue.py @@ -54,6 +54,17 @@ class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase): actual_header = json.loads(kwargs['header']) self.assertIn('https://edx.org/update_certificate?key=', actual_header['lms_callback_url']) + def test_no_create_action_in_queue_for_html_view_certs(self): + """ + Tests there is no certificate create message in the queue if generate_pdf is False + """ + with patch('courseware.grades.grade', Mock(return_value={'grade': 'Pass', 'percent': 0.75})): + with patch.object(XQueueInterface, 'send_to_queue') as mock_send: + self.xqueue.add_cert(self.user, self.course.id, generate_pdf=False) + + # Verify that add_cert method does not add message to queue + self.assertFalse(mock_send.called) + @attr('shard_1') @override_settings(CERT_QUEUE='certificates') diff --git a/lms/djangoapps/certificates/tests/test_views.py b/lms/djangoapps/certificates/tests/test_views.py index 63d5e88223..136872883f 100644 --- a/lms/djangoapps/certificates/tests/test_views.py +++ b/lms/djangoapps/certificates/tests/test_views.py @@ -4,6 +4,7 @@ import json import ddt from uuid import uuid4 from nose.plugins.attrib import attr +from mock import patch from django.conf import settings from django.core.cache import cache @@ -14,13 +15,20 @@ from django.test.utils import override_settings from opaque_keys.edx.locator import CourseLocator from openedx.core.lib.tests.assertions.events import assert_event_matches -from student.tests.factories import UserFactory +from student.tests.factories import UserFactory, CourseEnrollmentFactory from track.tests import EventTrackingTestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from certificates.api import get_certificate_url -from certificates.models import ExampleCertificateSet, ExampleCertificate, GeneratedCertificate, BadgeAssertion +from certificates.models import ( + ExampleCertificateSet, + ExampleCertificate, + GeneratedCertificate, + BadgeAssertion, + CertificateStatuses +) + from certificates.tests.factories import ( CertificateHtmlViewConfigurationFactory, LinkedInAddToProfileConfigurationFactory, @@ -210,6 +218,10 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): mode='honor', name=self.user.profile.name, ) + CourseEnrollmentFactory.create( + user=self.user, + course_id=self.course_id + ) CertificateHtmlViewConfigurationFactory.create() LinkedInAddToProfileConfigurationFactory.create() @@ -454,6 +466,31 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): self.get_event() ) + @override_settings(FEATURES=FEATURES_WITH_CERTS_DISABLED) + def test_request_certificate_without_passing(self): + self.cert.status = CertificateStatuses.unavailable + self.cert.save() + request_certificate_url = reverse('certificates.views.request_certificate') + response = self.client.post(request_certificate_url, {'course_id': unicode(self.course.id)}) + self.assertEqual(response.status_code, 200) + response_json = json.loads(response.content) + self.assertEqual(CertificateStatuses.notpassing, response_json['add_status']) + + @override_settings(FEATURES=FEATURES_WITH_CERTS_DISABLED) + @override_settings(CERT_QUEUE='test-queue') + def test_request_certificate_after_passing(self): + self.cert.status = CertificateStatuses.unavailable + self.cert.save() + request_certificate_url = reverse('certificates.views.request_certificate') + with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_queue: + mock_queue.return_value = (0, "Successfully queued") + with patch('courseware.grades.grade') as mock_grade: + mock_grade.return_value = {'grade': 'Pass', 'percent': 0.75} + response = self.client.post(request_certificate_url, {'course_id': unicode(self.course.id)}) + self.assertEqual(response.status_code, 200) + response_json = json.loads(response.content) + self.assertEqual(CertificateStatuses.generating, response_json['add_status']) + class TrackShareRedirectTest(ModuleStoreTestCase, EventTrackingTestCase): """ diff --git a/lms/djangoapps/certificates/views.py b/lms/djangoapps/certificates/views.py index 5d5dbdb9dd..64b90920d7 100644 --- a/lms/djangoapps/certificates/views.py +++ b/lms/djangoapps/certificates/views.py @@ -16,7 +16,7 @@ from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_POST from capa.xqueue_interface import XQUEUE_METRIC_NAME -from certificates.api import get_active_web_certificate, get_certificate_url +from certificates.api import get_active_web_certificate, get_certificate_url, generate_user_certificates from certificates.models import ( certificate_status_for_student, CertificateStatuses, @@ -56,7 +56,6 @@ def request_certificate(request): """ if request.method == "POST": if request.user.is_authenticated(): - xqci = XQueueCertInterface() username = request.user.username student = User.objects.get(username=username) course_key = SlashSeparatedCourseKey.from_deprecated_string(request.POST.get('course_id')) @@ -66,7 +65,7 @@ def request_certificate(request): if status in [CertificateStatuses.unavailable, CertificateStatuses.notpassing, CertificateStatuses.error]: log_msg = u'Grading and certification requested for user %s in course %s via /request_certificate call' logger.info(log_msg, username, course_key) - status = xqci.add_cert(student, course_key, course=course) + status = generate_user_certificates(student, course_key, course=course) return HttpResponse(json.dumps({'add_status': status}), mimetype='application/json') return HttpResponse(json.dumps({'add_status': 'ERRORANONYMOUSUSER'}), mimetype='application/json')