From 764ceb00c81bd4b7457549d461d538523803e42f Mon Sep 17 00:00:00 2001 From: Saleem Latif Date: Tue, 17 Nov 2015 12:49:34 +0500 Subject: [PATCH] Cert Exceptions: View and Edit Exception list --- .../pages/lms/instructor_dashboard.py | 15 + .../lms/test_lms_instructor_dashboard.py | 110 ++-- lms/djangoapps/certificates/models.py | 23 +- .../certificates/tests/factories.py | 2 +- .../instructor/tests/test_certificates.py | 544 ++++++++++++------ lms/djangoapps/instructor/views/api.py | 219 ++++--- lms/djangoapps/instructor/views/api_urls.py | 10 +- .../instructor/views/instructor_dashboard.py | 13 +- .../collections/certificate_whitelist.js | 15 +- .../certificate_whitelist_factory.js | 17 +- .../models/certificate_exception.js | 4 + .../views/certificate_whitelist.js | 45 +- .../views/certificate_whitelist_editor.js | 67 ++- .../certificates_exception_spec.js | 101 +++- .../sass/course/instructor/_instructor_2.scss | 15 +- .../certificate-white-list.underscore | 57 +- .../instructor_dashboard_2/certificates.html | 5 +- 17 files changed, 881 insertions(+), 381 deletions(-) diff --git a/common/test/acceptance/pages/lms/instructor_dashboard.py b/common/test/acceptance/pages/lms/instructor_dashboard.py index 486fa27146..f89910a80e 100644 --- a/common/test/acceptance/pages/lms/instructor_dashboard.py +++ b/common/test/acceptance/pages/lms/instructor_dashboard.py @@ -1032,17 +1032,32 @@ class CertificatesPage(PageObject): self.get_selector('#notes').fill(free_text_note) self.get_selector('#add-exception').click() + self.wait_for_ajax() self.wait_for( lambda: student in self.get_selector('div.white-listed-students table tr:last-child td').text, description='Certificate Exception added to list' ) + def remove_first_certificate_exception(self): + """ + Remove Certificate Exception from the white list. + """ + self.wait_for_element_visibility('#add-exception', 'Add Exception button is visible') + self.get_selector('div.white-listed-students table tr td .delete-exception').first.click() + self.wait_for_ajax() + def click_generate_certificate_exceptions_button(self): # pylint: disable=invalid-name """ Click 'Generate Exception Certificates' button in 'Certificates Exceptions' section """ self.get_selector('#generate-exception-certificates').click() + def fill_user_name_field(self, student): + """ + Fill username/email field with given text + """ + self.get_selector('#certificate-exception').fill(student) + def click_add_exception_button(self): """ Click 'Add Exception' button in 'Certificates Exceptions' section diff --git a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py index 0b2201c5e6..21a87ec03e 100644 --- a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py @@ -661,16 +661,65 @@ class CertificatesTest(BaseInstructorDashboardTest): def test_instructor_can_add_certificate_exception(self): """ - Scenario: On the Certificates tab of the Instructor Dashboard, Instructor can added new certificate - exception to list + Scenario: On the Certificates tab of the Instructor Dashboard, Instructor can add new certificate + exception to list. Given that I am on the Certificates tab on the Instructor Dashboard - When I fill in student username and click 'Add Exception' button + When I fill in student username and notes fields and click 'Add Exception' button Then new certificate exception should be visible in certificate exceptions list """ + notes = 'Test Notes' # Add a student to Certificate exception list - self.certificates_section.add_certificate_exception(self.user_name, '') + self.certificates_section.add_certificate_exception(self.user_name, notes) self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) + self.assertIn(notes, self.certificates_section.last_certificate_exception.text) + self.assertIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) + + # Verify that added exceptions are also synced with backend + # Revisit Page + self.certificates_section.refresh() + + # wait for the certificate exception section to render + self.certificates_section.wait_for_certificate_exceptions_section() + + # validate certificate exception synced with server is visible in certificate exceptions list + self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) + self.assertIn(notes, self.certificates_section.last_certificate_exception.text) + self.assertIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) + + def test_instructor_can_remove_certificate_exception(self): + """ + Scenario: On the Certificates tab of the Instructor Dashboard, Instructor can remove added certificate + exceptions from the list. + + Given that I am on the Certificates tab on the Instructor Dashboard + When I fill in student username and notes fields and click 'Add Exception' button + Then new certificate exception should be visible in certificate exceptions list + """ + notes = 'Test Notes' + # Add a student to Certificate exception list + self.certificates_section.add_certificate_exception(self.user_name, notes) + self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) + self.assertIn(notes, self.certificates_section.last_certificate_exception.text) + self.assertIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) + + # Remove Certificate Exception + self.certificates_section.remove_first_certificate_exception() + self.assertNotIn(self.user_name, self.certificates_section.last_certificate_exception.text) + self.assertNotIn(notes, self.certificates_section.last_certificate_exception.text) + self.assertNotIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) + + # Verify that added exceptions are also synced with backend + # Revisit Page + self.certificates_section.refresh() + + # wait for the certificate exception section to render + self.certificates_section.wait_for_certificate_exceptions_section() + + # validate certificate exception synced with server is visible in certificate exceptions list + self.assertNotIn(self.user_name, self.certificates_section.last_certificate_exception.text) + self.assertNotIn(notes, self.certificates_section.last_certificate_exception.text) + self.assertNotIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) def test_error_on_duplicate_certificate_exception(self): """ @@ -711,6 +760,29 @@ class CertificatesTest(BaseInstructorDashboardTest): self.certificates_section.message.text ) + def test_error_on_non_existing_user(self): + """ + Scenario: On the Certificates tab of the Instructor Dashboard, + Error message appears if username/email does not exists in the system while clicking "Add Exception" button + + Given that I am on the Certificates tab on the Instructor Dashboard + When I click on 'Add Exception' button + AND student username/email does not exists + Then Error Message should say 'Student username/email is required.' + """ + invalid_user = 'test_user_non_existent' + # Click 'Add Exception' button with invalid username/email field + self.certificates_section.wait_for_certificate_exceptions_section() + + self.certificates_section.fill_user_name_field(invalid_user) + self.certificates_section.click_add_exception_button() + self.certificates_section.wait_for_ajax() + + self.assertIn( + 'Student (username/email={}) does not exist'.format(invalid_user), + self.certificates_section.message.text + ) + def test_generate_certificate_exception(self): """ Scenario: On the Certificates tab of the Instructor Dashboard, when user clicks @@ -727,35 +799,7 @@ class CertificatesTest(BaseInstructorDashboardTest): self.certificates_section.click_generate_certificate_exceptions_button() self.certificates_section.wait_for_ajax() - # Revisit Page - self.certificates_section.refresh() - - # wait for the certificate exception section to render - self.certificates_section.wait_for_certificate_exceptions_section() - - # validate certificate exception synced with server is visible in certificate exceptions list - self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) - - def test_invalid_user_on_generate_certificate_exception(self): - """ - Scenario: On the Certificates tab of the Instructor Dashboard, when user clicks - 'Generate Exception Certificates' error message should appear if user does not exist - - Given that I am on the Certificates tab on the Instructor Dashboard - When I click 'Generate Exception Certificates' - AND the user specified by instructor does not exist - Then an error message "Student (username/email=test_user) does not exist" is displayed - """ - invalid_user = 'test_user_non_existent' - # Add a student to Certificate exception list - self.certificates_section.add_certificate_exception(invalid_user, '') - - # Click 'Generate Exception Certificates' button - self.certificates_section.click_generate_certificate_exceptions_button() - self.certificates_section.wait_for_ajax() - - # validate certificate exception synced with server is visible in certificate exceptions list self.assertIn( - 'Student (username/email={}) does not exist'.format(invalid_user), + 'Certificate generation started for white listed students.', self.certificates_section.message.text ) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index e4ec4e1592..cef5482e25 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -116,7 +116,7 @@ class CertificateWhitelist(models.Model): notes = models.TextField(default=None, null=True) @classmethod - def get_certificate_white_list(cls, course_id): + def get_certificate_white_list(cls, course_id, student=None): """ Return certificate white list for the given course as dict object, returned dictionary will have the following key-value pairs @@ -133,6 +133,8 @@ class CertificateWhitelist(models.Model): """ white_list = cls.objects.filter(course_id=course_id, whitelist=True) + if student: + white_list = white_list.filter(user=student) result = [] for item in white_list: @@ -214,6 +216,25 @@ class GeneratedCertificate(models.Model): else: return query.values('status').annotate(count=Count('status')) + 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' + """ + self.verify_uuid = '' + self.download_uuid = '' + self.download_url = '' + self.grade = '' + self.status = CertificateStatuses.unavailable + + self.save() + @receiver(post_save, sender=GeneratedCertificate) def handle_post_cert_generated(sender, instance, **kwargs): # pylint: disable=unused-argument diff --git a/lms/djangoapps/certificates/tests/factories.py b/lms/djangoapps/certificates/tests/factories.py index eb185c50e4..0c68994c69 100644 --- a/lms/djangoapps/certificates/tests/factories.py +++ b/lms/djangoapps/certificates/tests/factories.py @@ -30,7 +30,7 @@ class CertificateWhitelistFactory(DjangoModelFactory): course_id = None whitelist = True - notes = None + notes = 'Test Notes' class BadgeAssertionFactory(DjangoModelFactory): diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index a10efa03f0..c3622536bc 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -6,15 +6,18 @@ import json from nose.plugins.attrib import attr from django.core.urlresolvers import reverse +from django.core.exceptions import ObjectDoesNotExist from django.test.utils import override_settings from django.conf import settings from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from config_models.models import cache from courseware.tests.factories import GlobalStaffFactory, InstructorFactory, UserFactory -from certificates.tests.factories import GeneratedCertificateFactory -from certificates.models import CertificateGenerationConfiguration, CertificateStatuses +from certificates.tests.factories import GeneratedCertificateFactory, CertificateWhitelistFactory +from certificates.models import CertificateGenerationConfiguration, CertificateStatuses, CertificateWhitelist, \ + GeneratedCertificate from certificates import api as certs_api +from student.models import CourseEnrollment @attr('shard_1') @@ -206,18 +209,9 @@ class CertificatesInstructorApiTest(SharedModuleStoreTestCase): self.global_staff = GlobalStaffFactory() self.instructor = InstructorFactory(course_key=self.course.id) self.user = UserFactory() + CourseEnrollment.enroll(self.user, self.course.id) # Enable certificate generation - self.certificate_exception_data = [ - dict( - created="Wednesday, October 28, 2015", - notes="Test Notes for Test Certificate Exception", - user_email='', - user_id='', - user_name=unicode(self.user.username) - ), - ] - cache.clear() CertificateGenerationConfiguration.objects.create(enabled=True) @@ -314,180 +308,6 @@ class CertificatesInstructorApiTest(SharedModuleStoreTestCase): self.assertIsNotNone(res_json['message']) self.assertIsNotNone(res_json['task_id']) - def test_certificate_exception_added_successfully(self): - """ - Test certificates exception addition api endpoint returns success status and updated certificate exception data - when called with valid course key and certificate exception data - """ - self.client.login(username=self.global_staff.username, password='test') - url = reverse( - 'create_certificate_exception', - kwargs={'course_id': unicode(self.course.id), 'white_list_student': ''} - ) - - response = self.client.post( - url, - data=json.dumps(self.certificate_exception_data), - content_type='application/json' - ) - - # Assert successful request processing - self.assertEqual(response.status_code, 200) - res_json = json.loads(response.content) - - # Assert Request was successful - self.assertTrue(res_json['success']) - - # Assert Success Message - self.assertEqual(res_json['message'], u'Students added to Certificate white list successfully') - - # Assert Certificate Exception Updated data - certificate_exception = json.loads(res_json['data'])[0] - self.assertEqual(certificate_exception['user_email'], self.user.email) - self.assertEqual(certificate_exception['user_name'], self.user.username) - self.assertEqual(certificate_exception['user_id'], self.user.id) # pylint: disable=no-member - - def test_certificate_exception_invalid_username_error(self): - """ - Test certificates exception addition api endpoint returns failure when called with - invalid username. - """ - invalid_user = 'test_invalid_user_name' - self.certificate_exception_data[0].update({'user_name': invalid_user}) - - self.client.login(username=self.global_staff.username, password='test') - url = reverse( - 'create_certificate_exception', - kwargs={'course_id': unicode(self.course.id), 'white_list_student': ''} - ) - - response = self.client.post( - url, - data=json.dumps(self.certificate_exception_data), - content_type='application/json') - - # Assert 400 status code in response - self.assertEqual(response.status_code, 400) - res_json = json.loads(response.content) - - # Assert Request not successful - self.assertFalse(res_json['success']) - - # Assert Error Message - self.assertEqual( - res_json['message'], - u'Student (username/email={user}) does not exist'.format(user=invalid_user) - ) - - def test_certificate_exception_missing_username_and_email_error(self): - """ - Test certificates exception addition api endpoint returns failure when called with - missing username/email. - """ - self.certificate_exception_data[0].update({'user_name': '', 'user_email': ''}) - - self.client.login(username=self.global_staff.username, password='test') - url = reverse( - 'create_certificate_exception', - kwargs={'course_id': unicode(self.course.id), 'white_list_student': ''} - ) - - response = self.client.post( - url, - data=json.dumps(self.certificate_exception_data), - content_type='application/json') - - # Assert 400 status code in response - self.assertEqual(response.status_code, 400) - res_json = json.loads(response.content) - - # Assert Request not successful - self.assertFalse(res_json['success']) - - # Assert Error Message - self.assertEqual( - res_json['message'], - u'Student username/email is required.' - ) - - def test_certificate_exception_duplicate_user_error(self): - """ - Test certificates exception addition api endpoint returns failure when called with - username/email that already exists in 'CertificateWhitelist' table. - """ - - self.client.login(username=self.global_staff.username, password='test') - url = reverse( - 'create_certificate_exception', - kwargs={'course_id': unicode(self.course.id), 'white_list_student': ''} - ) - - self.client.post( - url, - data=json.dumps(self.certificate_exception_data), - content_type='application/json' - ) - - # Make some request again to simulate duplicate user scenario - response = self.client.post( - url, - data=json.dumps(self.certificate_exception_data), - content_type='application/json' - ) - - # Assert 400 status code in response - self.assertEqual(response.status_code, 400) - res_json = json.loads(response.content) - - # Assert Request not successful - self.assertFalse(res_json['success']) - - user = self.certificate_exception_data[0]['user_name'] - # Assert Error Message - self.assertEqual( - res_json['message'], - u"Student (username/email={user_id} already in certificate exception list)".format(user_id=user) - ) - - def test_certificate_exception_same_user_in_two_different_courses(self): - """ - Test certificates exception addition api endpoint in scenario when same - student is added to two different courses. - """ - - self.client.login(username=self.global_staff.username, password='test') - - url_course1 = reverse( - 'create_certificate_exception', - kwargs={'course_id': unicode(self.course.id), 'white_list_student': ''} - ) - - response = self.client.post( - url_course1, - data=json.dumps(self.certificate_exception_data), - content_type='application/json' - ) - self.assertEqual(response.status_code, 200) - res_json = json.loads(response.content) - self.assertTrue(res_json['success']) - - course2 = CourseFactory.create() - url_course2 = reverse( - 'create_certificate_exception', - kwargs={'course_id': unicode(course2.id), 'white_list_student': ''} - ) - - # add certificate exception for same user in a different course - self.client.post( - url_course2, - data=json.dumps(self.certificate_exception_data), - content_type='application/json' - ) - - self.assertEqual(response.status_code, 200) - res_json = json.loads(response.content) - self.assertTrue(res_json['success']) - def test_certificate_regeneration_success(self): """ Test certificate regeneration is successful when accessed with 'certificate_statuses' @@ -562,3 +382,355 @@ class CertificatesInstructorApiTest(SharedModuleStoreTestCase): # Assert Error Message self.assertEqual(res_json['message'], u'Please select certificate statuses from the list only.') + + +@attr('shard_1') +@override_settings(CERT_QUEUE='certificates') +@ddt.ddt +class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase): + """Tests for the generate certificates end-points in the instructor dash API. """ + @classmethod + def setUpClass(cls): + super(CertificateExceptionViewInstructorApiTest, cls).setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super(CertificateExceptionViewInstructorApiTest, self).setUp() + self.global_staff = GlobalStaffFactory() + self.instructor = InstructorFactory(course_key=self.course.id) + self.user = UserFactory() + self.user2 = UserFactory() + CourseEnrollment.enroll(self.user, self.course.id) + CourseEnrollment.enroll(self.user2, self.course.id) + self.url = reverse('certificate_exception_view', kwargs={'course_id': unicode(self.course.id)}) + + certificate_white_list_item = CertificateWhitelistFactory.create( + user=self.user2, + course_id=self.course.id, + ) + + self.certificate_exception = dict( + created="", + notes="Test Notes for Test Certificate Exception", + user_email='', + user_id='', + user_name=unicode(self.user.username) + ) + + self.certificate_exception_in_db = dict( + id=certificate_white_list_item.id, + user_name=certificate_white_list_item.user.username, + notes=certificate_white_list_item.notes, + user_email=certificate_white_list_item.user.email, + user_id=certificate_white_list_item.user.id, + ) + + # Enable certificate generation + cache.clear() + CertificateGenerationConfiguration.objects.create(enabled=True) + self.client.login(username=self.global_staff.username, password='test') + + def test_certificate_exception_added_successfully(self): + """ + Test certificates exception addition api endpoint returns success status and updated certificate exception data + when called with valid course key and certificate exception data + """ + response = self.client.post( + self.url, + data=json.dumps(self.certificate_exception), + content_type='application/json' + ) + # Assert successful request processing + self.assertEqual(response.status_code, 200) + certificate_exception = json.loads(response.content) + + # Assert Certificate Exception Updated data + self.assertEqual(certificate_exception['user_email'], self.user.email) + self.assertEqual(certificate_exception['user_name'], self.user.username) + self.assertEqual(certificate_exception['user_id'], self.user.id) # pylint: disable=no-member + + def test_certificate_exception_invalid_username_error(self): + """ + Test certificates exception addition api endpoint returns failure when called with + invalid username. + """ + invalid_user = 'test_invalid_user_name' + self.certificate_exception.update({'user_name': invalid_user}) + response = self.client.post( + self.url, + data=json.dumps(self.certificate_exception), + content_type='application/json' + ) + + # Assert 400 status code in response + self.assertEqual(response.status_code, 400) + res_json = json.loads(response.content) + + # Assert Request not successful + self.assertFalse(res_json['success']) + + # Assert Error Message + self.assertEqual( + res_json['message'], + u'Student (username/email={user}) does not exist'.format(user=invalid_user) + ) + + def test_certificate_exception_missing_username_and_email_error(self): + """ + Test certificates exception addition api endpoint returns failure when called with + missing username/email. + """ + self.certificate_exception.update({'user_name': '', 'user_email': ''}) + response = self.client.post( + self.url, + data=json.dumps(self.certificate_exception), + content_type='application/json' + ) + + # Assert 400 status code in response + self.assertEqual(response.status_code, 400) + res_json = json.loads(response.content) + + # Assert Request not successful + self.assertFalse(res_json['success']) + + # Assert Error Message + self.assertEqual( + res_json['message'], + u'Student username/email is required.' + ) + + def test_certificate_exception_duplicate_user_error(self): + """ + Test certificates exception addition api endpoint returns failure when called with + username/email that already exists in 'CertificateWhitelist' table. + """ + response = self.client.post( + self.url, + data=json.dumps(self.certificate_exception_in_db), + content_type='application/json' + ) + + # Assert 400 status code in response + self.assertEqual(response.status_code, 400) + res_json = json.loads(response.content) + + # Assert Request not successful + self.assertFalse(res_json['success']) + + user = self.certificate_exception_in_db['user_name'] + # Assert Error Message + self.assertEqual( + res_json['message'], + u"Student (username/email={user_name}) already in certificate exception list.".format(user_name=user) + ) + + def test_certificate_exception_same_user_in_two_different_courses(self): + """ + Test certificates exception addition api endpoint in scenario when same + student is added to two different courses. + """ + response = self.client.post( + self.url, + data=json.dumps(self.certificate_exception), + content_type='application/json' + ) + self.assertEqual(response.status_code, 200) + certificate_exception = json.loads(response.content) + + # Assert Certificate Exception Updated data + self.assertEqual(certificate_exception['user_email'], self.user.email) + self.assertEqual(certificate_exception['user_name'], self.user.username) + self.assertEqual(certificate_exception['user_id'], self.user.id) # pylint: disable=no-member + + course2 = CourseFactory.create() + url_course2 = reverse( + 'certificate_exception_view', + kwargs={'course_id': unicode(course2.id)} + ) + + # add certificate exception for same user in a different course + self.client.post( + url_course2, + data=json.dumps(self.certificate_exception), + content_type='application/json' + ) + + self.assertEqual(response.status_code, 200) + certificate_exception = json.loads(response.content) + + # Assert Certificate Exception Updated data + self.assertEqual(certificate_exception['user_email'], self.user.email) + self.assertEqual(certificate_exception['user_name'], self.user.username) + self.assertEqual(certificate_exception['user_id'], self.user.id) # pylint: disable=no-member + + def test_certificate_exception_removed_successfully(self): + """ + Test certificates exception removal api endpoint returns success status + when called with valid course key and certificate exception id + """ + GeneratedCertificateFactory.create( + user=self.user2, + course_id=self.course.id, + status=CertificateStatuses.downloadable, + grade='1.0' + ) + response = self.client.post( + self.url, + data=json.dumps(self.certificate_exception_in_db), + content_type='application/json', + REQUEST_METHOD='DELETE' + ) + # Assert successful request processing + self.assertEqual(response.status_code, 204) + + # Verify that certificate exception successfully removed from CertificateWhitelist and GeneratedCertificate + with self.assertRaises(ObjectDoesNotExist): + CertificateWhitelist.objects.get(user=self.user2, course_id=self.course.id) + GeneratedCertificate.objects.get( + user=self.user2, course_id=self.course.id, status__not=CertificateStatuses.unavailable + ) + + def test_remove_certificate_exception_invalid_request_error(self): + """ + Test certificates exception removal api endpoint returns error + when called without certificate exception id + """ + # Try to delete certificate exception without passing valid data + response = self.client.post( + self.url, + data='Test Invalid data', + content_type='application/json', + REQUEST_METHOD='DELETE' + ) + # Assert error on request + self.assertEqual(response.status_code, 400) + + res_json = json.loads(response.content) + + # Assert Request not successful + self.assertFalse(res_json['success']) + # Assert Error Message + self.assertEqual( + res_json['message'], + u"Invalid Json data" + ) + + def test_remove_certificate_exception_non_existing_error(self): + """ + Test certificates exception removal api endpoint returns error + when called with non existing certificate exception id + """ + response = self.client.post( + self.url, + data=json.dumps(self.certificate_exception), + content_type='application/json', + REQUEST_METHOD='DELETE' + ) + # Assert error on request + self.assertEqual(response.status_code, 400) + + res_json = json.loads(response.content) + + # Assert Request not successful + self.assertFalse(res_json['success']) + # Assert Error Message + self.assertEqual( + res_json['message'], + u"Certificate exception [user={}] does not exist in " + u"certificate white list.".format(self.certificate_exception['user_name']) + ) + + +@attr('shard_1') +@override_settings(CERT_QUEUE='certificates') +@ddt.ddt +class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): + """Tests for the generate certificates end-points in the instructor dash API. """ + @classmethod + def setUpClass(cls): + super(GenerateCertificatesInstructorApiTest, cls).setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super(GenerateCertificatesInstructorApiTest, self).setUp() + self.global_staff = GlobalStaffFactory() + self.instructor = InstructorFactory(course_key=self.course.id) + self.user = UserFactory() + CourseEnrollment.enroll(self.user, self.course.id) + certificate_exception = CertificateWhitelistFactory.create( + user=self.user, + course_id=self.course.id, + ) + + self.certificate_exception = dict( + id=certificate_exception.id, + user_name=certificate_exception.user.username, + notes=certificate_exception.notes, + user_email=certificate_exception.user.email, + user_id=certificate_exception.user.id, + ) + + # Enable certificate generation + cache.clear() + CertificateGenerationConfiguration.objects.create(enabled=True) + self.client.login(username=self.global_staff.username, password='test') + + def test_generate_certificate_exceptions_all_students(self): + """ + Test generate certificates exceptions api endpoint returns success + when called with existing certificate exception + """ + url = reverse( + 'generate_certificate_exceptions', + kwargs={'course_id': unicode(self.course.id), 'generate_for': 'all'} + ) + + response = self.client.post( + url, + data=json.dumps([self.certificate_exception]), + content_type='application/json' + ) + # Assert Success + self.assertEqual(response.status_code, 200) + + res_json = json.loads(response.content) + + # Assert Request is successful + self.assertTrue(res_json['success']) + # Assert Message + self.assertEqual( + res_json['message'], + u"Certificate generation started for white listed students." + ) + + def test_generate_certificate_exceptions_invalid_user_list_error(self): + """ + Test generate certificates exceptions api endpoint returns error + when called with certificate exceptions with empty 'user_id' field + """ + url = reverse( + 'generate_certificate_exceptions', + kwargs={'course_id': unicode(self.course.id), 'generate_for': 'new'} + ) + + # assign empty user_id + self.certificate_exception.update({'user_id': ''}) + + response = self.client.post( + url, + data=json.dumps([self.certificate_exception]), + content_type='application/json' + ) + # Assert Failure + self.assertEqual(response.status_code, 400) + + res_json = json.loads(response.content) + + # Assert Request is not successful + self.assertFalse(res_json['success']) + # Assert Message + self.assertEqual( + res_json['message'], + u"Invalid data, user_id must be present for all certificate exceptions." + ) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index ec976be5c1..2d42076863 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -13,7 +13,7 @@ import time import requests from django.conf import settings from django.views.decorators.csrf import ensure_csrf_cookie -from django.views.decorators.http import require_POST +from django.views.decorators.http import require_POST, require_http_methods from django.views.decorators.cache import cache_control from django.core.exceptions import ValidationError, PermissionDenied from django.core.mail.message import EmailMessage @@ -2731,10 +2731,140 @@ def start_certificate_regeneration(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_global_staff -@require_POST -def create_certificate_exception(request, course_id, white_list_student=None): +@require_http_methods(['POST', 'DELETE']) +def certificate_exception_view(request, course_id): """ - Add Students to certificate white list. + Add/Remove students to/from certificate white list. + + :param request: HttpRequest object + :param course_id: course identifier of the course for whom to add/remove certificates exception. + :return: JsonResponse object with success/error message or certificate exception data. + """ + course_key = CourseKey.from_string(course_id) + # Validate request data and return error response in case of invalid data + try: + certificate_exception, student = parse_request_data_and_get_user(request) + except ValueError as error: + return JsonResponse({'success': False, 'message': error.message}, status=400) + + # Add new Certificate Exception for the student passed in request data + if request.method == 'POST': + try: + exception = add_certificate_exception(course_key, student, certificate_exception) + except ValueError as error: + return JsonResponse({'success': False, 'message': error.message}, status=400) + return JsonResponse(exception) + + # Remove Certificate Exception for the student passed in request data + elif request.method == 'DELETE': + try: + remove_certificate_exception(course_key, student) + except ValueError as error: + return JsonResponse({'success': False, 'message': error.message}, status=400) + + return JsonResponse({}, status=204) + + +def add_certificate_exception(course_key, student, certificate_exception): + """ + Add a certificate exception to CertificateWhitelist table. + Raises ValueError in case Student is already white listed. + + :param course_key: identifier of the course whose certificate exception will be added. + :param student: User object whose certificate exception will be added. + :param certificate_exception: A dict object containing certificate exception info. + :return: CertificateWhitelist item in dict format containing certificate exception info. + """ + if len(CertificateWhitelist.get_certificate_white_list(course_key, student)) > 0: + raise ValueError( + _("Student (username/email={user}) already in certificate exception list.").format(user=student.username) + ) + + certificate_white_list, __ = CertificateWhitelist.objects.get_or_create( + user=student, + course_id=course_key, + defaults={ + 'whitelist': True, + 'notes': certificate_exception.get('notes', '') + } + ) + + exception = dict({ + 'id': certificate_white_list.id, + 'user_email': student.email, + 'user_name': student.username, + 'user_id': student.id, + 'created': certificate_white_list.created.strftime("%A, %B %d, %Y"), + }) + + return exception + + +def remove_certificate_exception(course_key, student): + """ + Remove certificate exception for given course and student from CertificateWhitelist table and + invalidate its GeneratedCertificate if present. + Raises ValueError in case no exception exists for the student in the given course. + + :param course_key: identifier of the course whose certificate exception needs to be removed. + :param student: User object whose certificate exception needs to be removed. + :return: + """ + try: + certificate_exception = CertificateWhitelist.objects.get(user=student, course_id=course_key) + except ObjectDoesNotExist: + raise ValueError( + _('Certificate exception [user={}] does not exist in ' + 'certificate white list.').format(student.username) + ) + + try: + generated_certificate = GeneratedCertificate.objects.get(user=student, course_id=course_key) + generated_certificate.invalidate() + except ObjectDoesNotExist: + # Certificate has not been generated yet, so just remove the certificate exception from white list + pass + certificate_exception.delete() + + +def parse_request_data_and_get_user(request): + """ + Parse request data into Certificate Exception and User object. + Certificate Exception is the dict object containing information about certificate exception. + + :param request: + :return: key-value pairs containing certificate exception data and User object + """ + try: + certificate_exception = json.loads(request.body or '{}') + except ValueError: + raise ValueError(_('Invalid Json data')) + + user = certificate_exception.get('user_name', '') or certificate_exception.get('user_email', '') + if not user: + raise ValueError(_('Student username/email is required.')) + try: + db_user = get_user_by_username_or_email(user) + except ObjectDoesNotExist: + raise ValueError(_('Student (username/email={user}) does not exist').format(user=user)) + + return certificate_exception, db_user + + +@transaction.non_atomic_requests +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@require_global_staff +@require_POST +def generate_certificate_exceptions(request, course_id, generate_for=None): + """ + Generate Certificate for students in the Certificate White List. + + :param request: HttpRequest object, + :param course_id: course identifier of the course for whom to generate certificates + :param generate_for: string to identify whether to generate certificates for 'all' or 'new' + additions to the certificate white-list + :return: JsonResponse object containing success/failure message and certificate exception data """ course_key = CourseKey.from_string(course_id) @@ -2746,21 +2876,29 @@ def create_certificate_exception(request, course_id, white_list_student=None): 'message': _('Invalid Json data') }, status=400) - with outer_atomic(): - try: - certificate_white_list, students = process_certificate_exceptions(certificate_white_list, course_key) - except ValueError as error: - return JsonResponse( - {'success': False, 'message': error.message, 'data': json.dumps(certificate_white_list)}, - status=400 - ) + users = [exception.get('user_id', False) for exception in certificate_white_list] - if white_list_student == 'all': + if generate_for == 'all': # Generate Certificates for all white listed students students = User.objects.filter( certificatewhitelist__course_id=course_key, certificatewhitelist__whitelist=True ) + elif not all(users): + # Invalid data, user_id must be present for all certificate exceptions + return JsonResponse( + { + 'success': False, + 'message': _('Invalid data, user_id must be present for all certificate exceptions.'), + }, + status=400 + ) + else: + students = User.objects.filter( + id__in=users, + certificatewhitelist__course_id=course_key, + certificatewhitelist__whitelist=True + ) if students: # generate certificates for students if 'students' list is not empty @@ -2768,60 +2906,7 @@ def create_certificate_exception(request, course_id, white_list_student=None): response_payload = { 'success': True, - 'message': _('Students added to Certificate white list successfully'), - 'data': json.dumps(certificate_white_list) + 'message': _('Certificate generation started for white listed students.'), } return JsonResponse(response_payload) - - -def process_certificate_exceptions(data_list, course_key): - """ - Validate user data for certificate exceptions, raise ValueError in case of invalid data and create - 'CertificateWhitelist' record for students in data_list. - - return updated data_list after creating 'CertificateWhitelist' records in db. - """ - students = [] - users = [data.get('user_name', False) or data.get('user_email', False) for data in data_list] - - if not all(users): - # Username and email can not both be empty - raise ValueError(_('Student username/email is required.')) - - if len(users) != len(set(users)): - # Duplicate Student username/email is not allowed - raise ValueError(_('Duplicate Student Username/password.')) - - for data in data_list: - user = data.get('user_name', '') or data.get('user_email', '') - try: - db_user = get_user_by_username_or_email(user) - except ObjectDoesNotExist: - raise ValueError(_('Student (username/email={user}) does not exist').format(user=user)) - except MultipleObjectsReturned: - raise ValueError(_('Multiple Students found with username/email={user}').format(user=user)) - - if CertificateWhitelist.objects.filter(user=db_user, course_id=course_key, whitelist=True).count() > 0: - raise ValueError( - _("Student (username/email={user_id} already in certificate exception list)").format(user_id=user) - ) - - certificate_white_list = CertificateWhitelist.objects.create( - user=db_user, - course_id=course_key, - whitelist=True, - notes=data.get('notes', '') - ) - - data.update({ - 'id': certificate_white_list.id, - 'user_email': db_user.email, - 'user_name': db_user.username, - 'user_id': db_user.id, - 'created': certificate_white_list.created.strftime("%A, %B %d, %Y"), - }) - - students.append(db_user) - - return data_list, students diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 6bb918d729..7b7d6535ca 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -150,7 +150,11 @@ urlpatterns = patterns( 'instructor.views.api.start_certificate_regeneration', name='start_certificate_regeneration'), - url(r'^create_certificate_exception/(?P[^/]*)', - 'instructor.views.api.create_certificate_exception', - name='create_certificate_exception'), + url(r'^certificate_exception_view/$', + 'instructor.views.api.certificate_exception_view', + name='certificate_exception_view'), + + url(r'^generate_certificate_exceptions/(?P[^/]*)', + 'instructor.views.api.generate_certificate_exceptions', + name='generate_certificate_exceptions'), ) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 7207a536f8..3582e5f7c3 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -165,9 +165,13 @@ def instructor_dashboard_2(request, course_id): disable_buttons = not _is_small_course(course_key) certificate_white_list = CertificateWhitelist.get_certificate_white_list(course_key) - certificate_exception_url = reverse( - 'create_certificate_exception', - kwargs={'course_id': unicode(course_key), 'white_list_student': ''} + generate_certificate_exceptions_url = reverse( # pylint: disable=invalid-name + 'generate_certificate_exceptions', + kwargs={'course_id': unicode(course_key), 'generate_for': ''} + ) + certificate_exception_view_url = reverse( + 'certificate_exception_view', + kwargs={'course_id': unicode(course_key)} ) context = { @@ -178,7 +182,8 @@ def instructor_dashboard_2(request, course_id): 'disable_buttons': disable_buttons, 'analytics_dashboard_message': analytics_dashboard_message, 'certificate_white_list': certificate_white_list, - 'certificate_exception_url': certificate_exception_url + 'generate_certificate_exceptions_url': generate_certificate_exceptions_url, + 'certificate_exception_view_url': certificate_exception_view_url } return render_to_response('instructor/instructor_dashboard_2/instructor_dashboard_2.html', context) diff --git a/lms/static/js/certificates/collections/certificate_whitelist.js b/lms/static/js/certificates/collections/certificate_whitelist.js index 1831be387b..0604582561 100644 --- a/lms/static/js/certificates/collections/certificate_whitelist.js +++ b/lms/static/js/certificates/collections/certificate_whitelist.js @@ -15,7 +15,7 @@ model: CertificateExceptionModel, initialize: function(attrs, options){ - this.url = options.url; + this.generate_certificates_url = options.generate_certificates_url; }, getModel: function(attrs){ @@ -33,13 +33,16 @@ }, sync: function(options, appended_url){ - var filtered = this.filter(function(model){ - return model.isNew(); - }); - + var filtered = []; + if(appended_url === 'new'){ + filtered = this.filter(function(model){ + return model.get('new'); + }); + } + var url = this.generate_certificates_url + appended_url; Backbone.sync( 'create', - new CertificateWhiteList(filtered, {url: this.url + appended_url}), + new CertificateWhiteList(filtered, {url: url, generate_certificates_url: url}), options ); }, diff --git a/lms/static/js/certificates/factories/certificate_whitelist_factory.js b/lms/static/js/certificates/factories/certificate_whitelist_factory.js index 1eee6e3cf9..38e37c89f6 100644 --- a/lms/static/js/certificates/factories/certificate_whitelist_factory.js +++ b/lms/static/js/certificates/factories/certificate_whitelist_factory.js @@ -12,21 +12,26 @@ ], function($, CertificateWhiteListListView, CertificateExceptionModel, CertificateWhiteListEditorView , CertificateWhiteListCollection){ - return function(certificate_white_list_json, certificate_exception_url){ + return function(certificate_white_list_json, generate_certificate_exceptions_url, + certificate_exception_view_url){ var certificateWhiteList = new CertificateWhiteListCollection(JSON.parse(certificate_white_list_json), { parse: true, canBeEmpty: true, - url: certificate_exception_url + url: certificate_exception_view_url, + generate_certificates_url: generate_certificate_exceptions_url }); - new CertificateWhiteListListView({ + var certificateWhiteListEditorView = new CertificateWhiteListEditorView({ collection: certificateWhiteList + }); + certificateWhiteListEditorView.render(); + + new CertificateWhiteListListView({ + collection: certificateWhiteList, + certificateWhiteListEditorView: certificateWhiteListEditorView }).render(); - new CertificateWhiteListEditorView({ - collection: certificateWhiteList - }).render(); }; } ); diff --git a/lms/static/js/certificates/models/certificate_exception.js b/lms/static/js/certificates/models/certificate_exception.js index ecca48e213..617832193e 100644 --- a/lms/static/js/certificates/models/certificate_exception.js +++ b/lms/static/js/certificates/models/certificate_exception.js @@ -24,6 +24,10 @@ notes: '' }, + url: function() { + return this.get('url'); + }, + validate: function(attrs){ if (!_.str.trim(attrs.user_name) && !_.str.trim(attrs.user_email)) { return gettext('Student username/email is required.'); diff --git a/lms/static/js/certificates/views/certificate_whitelist.js b/lms/static/js/certificates/views/certificate_whitelist.js index c14e32d938..ec6d6a7fb7 100644 --- a/lms/static/js/certificates/views/certificate_whitelist.js +++ b/lms/static/js/certificates/views/certificate_whitelist.js @@ -14,16 +14,19 @@ function($, _, gettext, Backbone){ return Backbone.View.extend({ el: "#white-listed-students", + message_div: '#certificate-white-list-editor .message', generate_exception_certificates_radio: 'input:radio[name=generate-exception-certificates-radio]:checked', events: { - 'click #generate-exception-certificates': 'generateExceptionCertificates' + 'click #generate-exception-certificates': 'generateExceptionCertificates', + 'click .delete-exception': 'removeException' }, - initialize: function(){ + initialize: function(options){ + this.certificateWhiteListEditorView = options.certificateWhiteListEditorView; // Re-render the view when an item is added to the collection - this.listenTo(this.collection, 'change add', this.render); + this.listenTo(this.collection, 'change add remove', this.render); }, render: function(){ @@ -38,6 +41,14 @@ return _.template(templateText); }, + removeException: function(event){ + // Delegate remove exception event to certificate white-list editor view + this.certificateWhiteListEditorView.trigger('removeException', $(event.target).data()); + + // avoid default click behavior of link by returning false. + return false; + }, + generateExceptionCertificates: function(){ this.collection.sync( {success: this.showSuccess(this), error: this.showError(this)}, @@ -45,25 +56,29 @@ ); }, + showMessage: function(message, messageClass){ + $(this.message_div).text(message). + removeClass('msg-error msg-success').addClass(messageClass).focus(); + $('html, body').animate({ + scrollTop: $(this.message_div).offset().top - 20 + }, 1000); + }, + showSuccess: function(caller_object){ return function(xhr){ - var response = xhr; - $(".message").text(response.message).removeClass('msg-error').addClass('msg-success').focus(); - caller_object.collection.update(JSON.parse(response.data)); - $('html, body').animate({ - scrollTop: $("#certificate-exception").offset().top - 10 - }, 1000); + caller_object.showMessage(xhr.message, 'msg-success'); }; }, showError: function(caller_object){ return function(xhr){ - var response = JSON.parse(xhr.responseText); - $(".message").text(response.message).removeClass('msg-success').addClass("msg-error").focus(); - caller_object.collection.update(JSON.parse(response.data)); - $('html, body').animate({ - scrollTop: $("#certificate-exception").offset().top - 10 - }, 1000); + try{ + var response = JSON.parse(xhr.responseText); + caller_object.showMessage(response.message, 'msg-error'); + } + catch(exception){ + caller_object.showMessage("Server Error, Please try again later.", 'msg-error'); + } }; } }); diff --git a/lms/static/js/certificates/views/certificate_whitelist_editor.js b/lms/static/js/certificates/views/certificate_whitelist_editor.js index d7b3b598b8..074a5861a0 100644 --- a/lms/static/js/certificates/views/certificate_whitelist_editor.js +++ b/lms/static/js/certificates/views/certificate_whitelist_editor.js @@ -19,6 +19,11 @@ 'click #add-exception': 'addException' }, + initialize: function(){ + this.on('removeException', this.removeException); + }, + + render: function(){ var template = this.loadTemplate('certificate-white-list-editor'); this.$el.html(template()); @@ -45,23 +50,58 @@ } var certificate_exception = new CertificateExceptionModel({ + url: this.collection.url, user_name: user_name, user_email: user_email, - notes: notes + notes: notes, + new: true }); if(this.collection.findWhere(model)){ this.showMessage("username/email already in exception list", 'msg-error'); } else if(certificate_exception.isValid()){ - this.collection.add(certificate_exception, {validate: true}); - this.showMessage("Student Added to exception list", 'msg-success'); + certificate_exception.save( + null, + { + success: this.showSuccess( + this, + true, + 'Students added to Certificate white list successfully' + ), + error: this.showError(this) + } + ); + } else{ this.showMessage(certificate_exception.validationError, 'msg-error'); } }, + removeException: function(certificate){ + var model = this.collection.findWhere(certificate); + if(model){ + model.destroy( + { + success: this.showSuccess( + this, + false, + 'Student Removed from certificate white list successfully.' + ), + error: this.showError(this), + wait: true, + //emulateJSON: true, + data: JSON.stringify(model.attributes) + } + ); + this.showMessage('Exception is being removed from server.', 'msg-success'); + } + else{ + this.showMessage('Could not find Certificate Exception in white list.', 'msg-error'); + } + }, + isEmailAddress: function validateEmail(email) { var re = /^([\w-]+(?:\.[\w-]+)*)@((?:[\w-]+\.)*\w[\w-]{0,66})\.([a-z]{2,6}(?:\.[a-z]{2})?)$/i; return re.test(email); @@ -73,6 +113,27 @@ $('html, body').animate({ scrollTop: this.$el.offset().top - 20 }, 1000); + }, + + showSuccess: function(caller, add_model, message){ + return function(model){ + if(add_model){ + caller.collection.add(model); + } + caller.showMessage(message, 'msg-success'); + }; + }, + + showError: function(caller){ + return function(model, response){ + try{ + var response_data = JSON.parse(response.responseText); + caller.showMessage(response_data.message, 'msg-error'); + } + catch(exception){ + caller.showMessage("Server Error, Please try again later.", 'msg-error'); + } + }; } }); } diff --git a/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js b/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js index 39597395f4..eb60f56c6c 100644 --- a/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js +++ b/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js @@ -54,8 +54,8 @@ define([ certificate_exception_url = 'test/url/'; var certificates_exceptions_json = [ { - id: "1", - user_id: "1", + id: 1, + user_id: 1, user_name: "test1", user_email: "test1@test.com", course_id: "edX/test/course", @@ -63,8 +63,8 @@ define([ notes: "test notes for test certificate exception" }, { - id: "2", - user_id : "2", + id: 2, + user_id : 2, user_name: "test2", user_email : "test2@test.com", course_id: "edX/test/course", @@ -77,7 +77,8 @@ define([ certificate_white_list = new CertificateWhiteListCollection(certificates_exceptions_json, { parse: true, canBeEmpty: true, - url: certificate_exception_url + url: certificate_exception_url, + generate_certificates_url: certificate_exception_url }); }); @@ -94,7 +95,7 @@ define([ expect(certificate_white_list.getModel({user_name: 'test1'}).attributes).toEqual( { - id: '1', user_id: '1', user_name: 'test1', user_email: 'test1@test.com', + id: 1, user_id: 1, user_name: 'test1', user_email: 'test1@test.com', course_id: 'edX/test/course', created: "Thursday, October 29, 2015", notes: 'test notes for test certificate exception' } @@ -102,7 +103,7 @@ define([ expect(certificate_white_list.getModel({user_email: 'test2@test.com'}).attributes).toEqual( { - id: '2', user_id: '2', user_name: 'test2', user_email: 'test2@test.com', + id: 2, user_id: 2, user_name: 'test2', user_email: 'test2@test.com', course_id: 'edX/test/course', created: "Thursday, October 29, 2015", notes: 'test notes for test certificate exception' } @@ -129,7 +130,7 @@ define([ requests = AjaxHelpers.requests(this), add_students = 'new'; - certificate_white_list.add({user_name: 'test3', notes: 'test3 notes'}); + certificate_white_list.add({user_name: 'test3', notes: 'test3 notes', new: true}); certificate_white_list.sync({success: successCallback, error: errorCallback}, add_students); var expected = { @@ -139,7 +140,8 @@ define([ user_name: "test3", user_email: "", created: "", - notes: "test3 notes"} + notes: "test3 notes", + new: true} ] }; AjaxHelpers.expectJsonRequest(requests, 'POST', expected.url, expected.postData); @@ -152,8 +154,8 @@ define([ var certificates_exceptions_json = [ { - id: "1", - user_id: "1", + id: 1, + user_id: 1, user_name: "test1", user_email: "test1@test.com", course_id: "edX/test/course", @@ -161,8 +163,8 @@ define([ notes: "test notes for test certificate exception" }, { - id: "2", - user_id : "2", + id: 2, + user_id : 2, user_name: "test2", user_email : "test2@test.com", course_id: "edX/test/course", @@ -181,7 +183,9 @@ define([ var certificate_white_list = new CertificateWhiteListCollection(certificates_exceptions_json, { parse: true, canBeEmpty: true, - url: certificate_exception_url + url: certificate_exception_url, + generate_certificates_url: certificate_exception_url + }); view = new CertificateWhiteListView({collection: certificate_white_list}); @@ -252,20 +256,24 @@ define([ describe("edx.certificates.views.certificate_whitelist_editor.CertificateWhiteListEditorView", function() { var view = null, + list_view= null, certificate_exception_url = 'test/url/'; var certificates_exceptions_json = [ { - id: "1", - user_id: "1", + url: certificate_exception_url, + id: 1, + user_id: 1, user_name: "test1", user_email: "test1@test.com", course_id: "edX/test/course", created: "Thursday, October 29, 2015", - notes: "test notes for test certificate exception" + notes: "test notes for test certificate exception", + new: true }, { - id: "2", - user_id : "2", + url: certificate_exception_url, + id: 2, + user_id : 2, user_name: "test2", user_email : "test2@test.com", course_id: "edX/test/course", @@ -281,19 +289,35 @@ define([ "templates/instructor/instructor_dashboard_2/certificate-white-list-editor.underscore" ); + var fixture_2 = readFixtures( + "templates/instructor/instructor_dashboard_2/certificate-white-list.underscore" + ); + setFixtures( "" + - "
" + "" + + "
" + + "
" ); var certificate_white_list = new CertificateWhiteListCollection(certificates_exceptions_json, { parse: true, canBeEmpty: true, - url: certificate_exception_url + url: certificate_exception_url, + generate_certificates_url: certificate_exception_url }); - view = new CertificateWhiteListEditorView({collection: certificate_white_list}); + view = new CertificateWhiteListEditorView({ + collection: certificate_white_list, + url: certificate_exception_url + }); view.render(); + + list_view = new CertificateWhiteListView({ + collection: certificate_white_list, + certificateWhiteListEditorView: view + }); + list_view.render(); }); it("verifies view is initialized and rendered successfully", function() { @@ -307,7 +331,8 @@ define([ var message_selector='.message', error_class = 'msg-error', success_class = 'msg-success', - success_message = 'Student Added to exception list'; + success_message = 'Students added to Certificate white list successfully', + requests = AjaxHelpers.requests(this); var error_messages = { empty_user_name_email: 'Student username/email is required.', @@ -315,6 +340,7 @@ define([ }; // click 'Add Exception' button with empty username/email field + view.$el.find('#certificate-exception').val(""); view.$el.find('#add-exception').click(); // Verify error message for missing username/email @@ -326,6 +352,19 @@ define([ view.$el.find('#notes').val("test user notes"); view.$el.find('#add-exception').click(); + AjaxHelpers.respondWithJson( + requests, + { + id: 3, + user_id : 3, + user_name: "test_user", + user_email : "test2@test.com", + course_id: "edX/test/course", + created: "Thursday, October 29, 2015", + notes: "test user notes" + } + ); + // Verify success message expect(view.$el.find(message_selector)).toHaveClass(success_class); expect(view.$el.find(message_selector).html()).toMatch(success_message); @@ -339,6 +378,22 @@ define([ expect(view.$el.find(message_selector)).toHaveClass(error_class); expect(view.$el.find(message_selector).html()).toMatch(error_messages.duplicate_user); }); + + it('verifies certificate exception can be deleted by clicking "delete" ', function(){ + var user_name = 'test1', + certificate_exception_selector = "div.white-listed-students table tr:contains('" + user_name + "')", + delete_btn_selector = + certificate_exception_selector + " td .delete-exception", + requests = AjaxHelpers.requests(this); + + $(delete_btn_selector).click(); + AjaxHelpers.respondWithJson(requests, {}); + + // Verify the certificate exception is removed from the list + expect($(certificate_exception_selector).length).toBe(0); + + }); + }); } ); diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index 744696f8c4..2966bf02dc 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -2169,9 +2169,22 @@ input[name="subject"] { text-align: left; color: $gray; - &.date-column{ + &.date, &.email{ width: 230px; } + + &.user-id{ + width: 60px; + } + + &.user-name{ + width: 150px; + } + + &.action{ + width: 150px; + } + } td { diff --git a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore index 4cd6cdaa20..c6f0108041 100644 --- a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore @@ -1,31 +1,3 @@ -<% if (certificates.length === 0) { %> -

<%- gettext("No results") %>

-<% } else { %> - - - - - - - - - - <% for (var i = 0; i < certificates.length; i++) { - var cert = certificates[i]; - %> - - - - - - - - <% } %> - -
<%- gettext("Name") %><%- gettext("User ID") %><%- gettext("User Email") %><%- gettext("Date Exception Granted") %><%- gettext("Notes") %>
<%- cert.get("user_name") %><%- cert.get("user_id") %><%- cert.get("user_email") %><%- cert.get("created") %><%- cert.get("notes") %>
-<% } %> - -

+
+<% if (certificates.length === 0) { %> +

<%- gettext("No results") %>

+<% } else { %> + + + + + + + + + + + <% for (var i = 0; i < certificates.length; i++) { + var cert = certificates[i]; + %> + + + + + + + + + <% } %> + +
<%- gettext("Name") %><%- gettext("User ID") %><%- gettext("User Email") %><%- gettext("Date Exception Granted") %><%- gettext("Notes") %><%- gettext("Action") %>
<%- cert.get("user_name") %><%- cert.get("user_id") %><%- cert.get("user_email") %><%- cert.get("created") %><%- cert.get("notes") %>
+<% } %> diff --git a/lms/templates/instructor/instructor_dashboard_2/certificates.html b/lms/templates/instructor/instructor_dashboard_2/certificates.html index bc3060f060..68ac9ad3c5 100644 --- a/lms/templates/instructor/instructor_dashboard_2/certificates.html +++ b/lms/templates/instructor/instructor_dashboard_2/certificates.html @@ -5,7 +5,7 @@ import json %> <%static:require_module module_name="js/certificates/factories/certificate_whitelist_factory" class_name="CertificateWhitelistFactory"> - CertificateWhitelistFactory('${json.dumps(certificate_white_list)}', "${certificate_exception_url}"); + CertificateWhitelistFactory('${json.dumps(certificate_white_list)}', "${generate_certificate_exceptions_url}", "${certificate_exception_view_url}"); <%page args="section_data"/> @@ -123,11 +123,8 @@ import json

${_("Use this to generate certificates for users who did not pass the course but have been given an exception by the Course Team to earn a certificate.")}


-
-

-