Merge pull request #10656 from edx/saleem-latif/SOL-1390

SOL-1390: Cert Exceptions: View and Edit Exception list

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

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,51 +760,46 @@ class CertificatesTest(BaseInstructorDashboardTest):
             self.certificates_section.message.text
         )
 
-    def test_generate_certificate_exception(self):
+    def test_error_on_non_existing_user(self):
         """
-        Scenario: On the Certificates tab of the Instructor Dashboard, when user clicks
-        'Generate Exception Certificates' newly added certificate exceptions should be synced on server
+        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 'Generate Exception Certificates'
-            Then newly added certificate exceptions should be synced on server
+            When I click on 'Add Exception' button
+            AND student username/email does not exists
+            Then Error Message should say 'Student username/email is required.'
         """
-        # Add a student to Certificate exception list
-        self.certificates_section.add_certificate_exception(self.user_name, '')
+        invalid_user = 'test_user_non_existent'
+        # Click 'Add Exception' button with invalid username/email field
+        self.certificates_section.wait_for_certificate_exceptions_section()
 
-        # Click 'Generate Exception Certificates' button
-        self.certificates_section.click_generate_certificate_exceptions_button()
+        self.certificates_section.fill_user_name_field(invalid_user)
+        self.certificates_section.click_add_exception_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)
+        self.assertIn(
+            'Student (username/email={}) does not exist'.format(invalid_user),
+            self.certificates_section.message.text
+        )
 
-    def test_invalid_user_on_generate_certificate_exception(self):
+    def test_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
+        'Generate Exception Certificates' newly added certificate exceptions should be synced on server
 
             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
+            Then newly added certificate exceptions should be synced on server
         """
-        invalid_user = 'test_user_non_existent'
         # Add a student to Certificate exception list
-        self.certificates_section.add_certificate_exception(invalid_user, '')
+        self.certificates_section.add_certificate_exception(self.user_name, '')
 
         # 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
         )

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

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):

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<white_list_student>[^/]*)',
-        '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<generate_for>[^/]*)',
+        'instructor.views.api.generate_certificate_exceptions',
+        name='generate_certificate_exceptions'),
 )

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)
 

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
                     );
                 },

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
-                }).render();
+                });
+                certificateWhiteListEditorView.render();
 
-                new CertificateWhiteListEditorView({
-                    collection: certificateWhiteList
+                new CertificateWhiteListListView({
+                    collection: certificateWhiteList,
+                    certificateWhiteListEditorView: certificateWhiteListEditorView
                 }).render();
+
             };
         }
     );

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.');

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');
+                        }
                     };
                 }
             });

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');
+                        }
+                    };
                 }
             });
         }

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 {

lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore

+<label>
+    <input type='radio' name='generate-exception-certificates-radio' checked="checked" value='new' aria-describedby='generate-exception-certificates-radio-new-tip'>
+    <span id='generate-exception-certificates-radio-new-tip'><%- gettext('Generate a Certificate for all ') %><strong><%- gettext('New') %></strong> <%- gettext('additions to the Exception list') %></span>
+</label>
+<br/>
+<label>
+    <input type='radio' name='generate-exception-certificates-radio' value='all' aria-describedby='generate-exception-certificates-radio-all-tip'>
+    <span id='generate-exception-certificates-radio-all-tip'><%- gettext('Generate a Certificate for all users on the Exception list') %></span>
+</label>
+<br/>
+<input type="button" id="generate-exception-certificates" value="<%- gettext('Generate Exception Certificates') %>" />
+<br/>
 <% if (certificates.length === 0) { %>
     <p><%- gettext("No results") %></p>
 <% } else { %>
     <table>
         <thead>
-            <th><%- gettext("Name") %></th>
-            <th><%- gettext("User ID") %></th>
-            <th><%- gettext("User Email") %></th>
-            <th class='date-column'><%- gettext("Date Exception Granted") %></th>
-            <th><%- gettext("Notes") %></th>
+            <th class='user-name'><%- gettext("Name") %></th>
+            <th class='user-id'><%- gettext("User ID") %></th>
+            <th class='user-email'><%- gettext("User Email") %></th>
+            <th class='date'><%- gettext("Date Exception Granted") %></th>
+            <th class='notes'><%- gettext("Notes") %></th>
+            <th class='action'><%- gettext("Action") %></th>
         </thead>
         <tbody>
         <% for (var i = 0; i < certificates.length; i++) {
@@ -19,21 +32,9 @@
             <td><%- cert.get("user_email") %></td>
             <td><%- cert.get("created") %></td>
             <td><%- cert.get("notes") %></td>
+            <td><button class='delete-exception' data-user_id='<%- cert.get("user_id") %>'><%- gettext("Remove from List") %></button></td>
         </tr>
         <% } %>
         </tbody>
     </table>
 <% } %>
-
-<br/>
-<label>
-    <input type='radio' name='generate-exception-certificates-radio' checked="checked" value='new' aria-describedby='generate-exception-certificates-radio-new-tip'>
-    <span id='generate-exception-certificates-radio-new-tip'><%- gettext('Generate a Certificate for all ') %><strong><%- gettext('New') %></strong> <%- gettext('additions to the Exception list') %></span>
-</label>
-<br/>
-<label>
-    <input type='radio' name='generate-exception-certificates-radio' value='all' aria-describedby='generate-exception-certificates-radio-all-tip'>
-    <span id='generate-exception-certificates-radio-all-tip'><%- gettext('Generate a Certificate for all users on the Exception list') %></span>
-</label>
-<br/>
-<input type="button" id="generate-exception-certificates" value="<%- gettext('Generate Exception Certificates') %>" />

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}");
 </%static:require_module>
 
 <%page args="section_data"/>
@@ -123,11 +123,8 @@ import json
             <p>${_("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.")} </p>
             <br />
             <div id="certificate-white-list-editor"></div>
-            <br/>
-            <br/>
             <div class="white-listed-students" id="white-listed-students"></div>
             <br/>
-            <br/>
         </div>
         <div class="no-pending-tasks-message"></div>
     </div>