Cert Exception: Add error states and messages

common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py

@@ -728,7 +728,7 @@ class CertificatesTest(BaseInstructorDashboardTest):
 
             Given that I am on the Certificates tab on the Instructor Dashboard
             When I fill in student username that already is in the list and click 'Add Exception' button
-            Then Error Message should say 'username/email already in exception list'
+            Then Error Message should say 'User (username/email={user}) already in exception list.'
         """
         # Add a student to Certificate exception list
         self.certificates_section.add_certificate_exception(self.user_name, '')
@@ -737,7 +737,7 @@ class CertificatesTest(BaseInstructorDashboardTest):
         self.certificates_section.add_certificate_exception(self.user_name, '')
 
         self.assertIn(
-            'username/email already in exception list',
+            'User (username/email={user}) already in exception list.'.format(user=self.user_name),
             self.certificates_section.message.text
         )
 
@@ -749,14 +749,17 @@ class CertificatesTest(BaseInstructorDashboardTest):
             Given that I am on the Certificates tab on the Instructor Dashboard
             When I click on 'Add Exception' button
             AND student username/email field is empty
-            Then Error Message should say 'Student username/email is required.'
+            Then Error Message should say
+                'Student username/email field is required and can not be empty. '
+                'Kindly fill in username/email and then press "Add Exception" button.'
         """
         # Click 'Add Exception' button without filling username/email field
         self.certificates_section.wait_for_certificate_exceptions_section()
         self.certificates_section.click_add_exception_button()
 
         self.assertIn(
-            'Student username/email is required.',
+            'Student username/email field is required and can not be empty. '
+            'Kindly fill in username/email and then press "Add Exception" button.',
             self.certificates_section.message.text
         )
 
@@ -768,7 +771,9 @@ class CertificatesTest(BaseInstructorDashboardTest):
             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.'
+            Then Error Message should say
+                'Student username/email field is required and can not be empty. '
+                'Kindly fill in username/email and then press "Add Exception" button.
         """
         invalid_user = 'test_user_non_existent'
         # Click 'Add Exception' button with invalid username/email field
@@ -779,7 +784,42 @@ class CertificatesTest(BaseInstructorDashboardTest):
         self.certificates_section.wait_for_ajax()
 
         self.assertIn(
-            'Student (username/email={}) does not exist'.format(invalid_user),
+            "We can't find the user (username/email={user}) you've entered. "
+            "Make sure the username or email address is correct, then try again.".format(user=invalid_user),
+            self.certificates_section.message.text
+        )
+
+    def test_user_not_enrolled_error(self):
+        """
+        Scenario: On the Certificates tab of the Instructor Dashboard,
+        Error message appears if user is not enrolled in the course while trying to add a new exception.
+
+            Given that I am on the Certificates tab on the Instructor Dashboard
+            When I click on 'Add Exception' button
+            AND student is not enrolled in the course
+            Then Error Message should say
+                "The user (username/email={user}) you have entered is not enrolled in this course.
+                Make sure the username or email address is correct, then try again."
+        """
+        new_user = 'test_user_{uuid}'.format(uuid=self.unique_id[6:12])
+        new_email = 'test_user_{uuid}@example.com'.format(uuid=self.unique_id[6:12])
+        # Create a new user who is not enrolled in the course
+        AutoAuthPage(self.browser, username=new_user, email=new_email).visit()
+        # Login as instructor and visit Certificate Section of Instructor Dashboard
+        self.user_name, self.user_id = self.log_in_as_instructor()
+        self.instructor_dashboard_page.visit()
+        self.certificates_section = self.instructor_dashboard_page.select_certificates()
+
+        # Click 'Add Exception' button with invalid username/email field
+        self.certificates_section.wait_for_certificate_exceptions_section()
+
+        self.certificates_section.fill_user_name_field(new_user)
+        self.certificates_section.click_add_exception_button()
+        self.certificates_section.wait_for_ajax()
+
+        self.assertIn(
+            "The user (username/email={user}) you have entered is not enrolled in this course. "
+            "Make sure the username or email address is correct, then try again.".format(user=new_user),
             self.certificates_section.message.text
         )
 

lms/djangoapps/instructor/tests/test_certificates.py

@@ -472,7 +472,8 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase):
         # Assert Error Message
         self.assertEqual(
             res_json['message'],
-            u'Student (username/email={user}) does not exist'.format(user=invalid_user)
+            u"We can't find the user (username/email={user}) you've entered. "
+            u"Make sure the username or email address is correct, then try again.".format(user=invalid_user)
         )
 
     def test_certificate_exception_missing_username_and_email_error(self):
@@ -497,7 +498,8 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase):
         # Assert Error Message
         self.assertEqual(
             res_json['message'],
-            u'Student username/email is required.'
+            u'Student username/email field is required and can not be empty. '
+            u'Kindly fill in username/email and then press "Add Exception" button.'
         )
 
     def test_certificate_exception_duplicate_user_error(self):
@@ -564,6 +566,35 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase):
         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_user_not_enrolled_error(self):
+        """
+        Test certificates exception addition api endpoint returns failure when called with
+        username/email that is not enrolled in the given course.
+        """
+        # Un-enroll student from the course
+        CourseEnrollment.unenroll(self.user, self.course.id)
+        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'],
+            "The user (username/email={user}) you have entered is not enrolled in this course. "
+            "Make sure the username or email address is correct, then try again.".format(
+                user=self.certificate_exception['user_name']
+            )
+        )
+
     def test_certificate_exception_removed_successfully(self):
         """
         Test certificates exception removal api endpoint returns success status
@@ -613,7 +644,7 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase):
         # Assert Error Message
         self.assertEqual(
             res_json['message'],
-            u"Invalid Json data"
+            u"Invalid Json data, Please refresh the page and then try again."
         )
 
     def test_remove_certificate_exception_non_existing_error(self):
@@ -637,8 +668,8 @@ class CertificateExceptionViewInstructorApiTest(SharedModuleStoreTestCase):
         # 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'])
+            u"Certificate exception (user={user}) does not exist in certificate white list. "
+            u"Please refresh the page and try again.".format(user=self.certificate_exception['user_name'])
         )
 
 

lms/djangoapps/instructor/views/api.py

@@ -2743,7 +2743,7 @@ def certificate_exception_view(request, course_id):
     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)
+        certificate_exception, student = parse_request_data_and_get_user(request, course_key)
     except ValueError as error:
         return JsonResponse({'success': False, 'message': error.message}, status=400)
 
@@ -2814,8 +2814,8 @@ def remove_certificate_exception(course_key, student):
         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)
+            _('Certificate exception (user={user}) does not exist in certificate white list. '
+              'Please refresh the page and try again.').format(user=student.username)
         )
 
     try:
@@ -2827,26 +2827,34 @@ def remove_certificate_exception(course_key, student):
     certificate_exception.delete()
 
 
-def parse_request_data_and_get_user(request):
+def parse_request_data_and_get_user(request, course_key):
     """
         Parse request data into Certificate Exception and User object.
         Certificate Exception is the dict object containing information about certificate exception.
 
     :param request:
+    :param course_key: Course Identifier of the course for whom to process certificate exception
     :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'))
+        raise ValueError(_('Invalid Json data, Please refresh the page and then try again.'))
 
     user = certificate_exception.get('user_name', '') or certificate_exception.get('user_email', '')
     if not user:
-        raise ValueError(_('Student username/email is required.'))
+        raise ValueError(_('Student username/email field is required and can not be empty. '
+                           'Kindly fill in username/email and then press "Add Exception" button.'))
     try:
         db_user = get_user_by_username_or_email(user)
     except ObjectDoesNotExist:
-        raise ValueError(_('Student (username/email={user}) does not exist').format(user=user))
+        raise ValueError(_("We can't find the user (username/email={user}) you've entered. "
+                           "Make sure the username or email address is correct, then try again.").format(user=user))
+
+    # Make Sure the given student is enrolled in the course
+    if not CourseEnrollment.is_enrolled(db_user, course_key):
+        raise ValueError(_("The user (username/email={user}) you have entered is not enrolled in this course. "
+                           "Make sure the username or email address is correct, then try again.").format(user=user))
 
     return certificate_exception, db_user
 
@@ -2873,7 +2881,7 @@ def generate_certificate_exceptions(request, course_id, generate_for=None):
     except ValueError:
         return JsonResponse({
             'success': False,
-            'message': _('Invalid Json data')
+            'message': _('Invalid Json data, Please refresh the page and then try again.')
         }, status=400)
 
     users = [exception.get('user_id', False) for exception in certificate_white_list]

lms/static/js/certificates/models/certificate_exception.js

@@ -30,7 +30,8 @@
 
                 validate: function(attrs){
                     if (!_.str.trim(attrs.user_name) && !_.str.trim(attrs.user_email)) {
-                        return gettext('Student username/email is required.');
+                        return gettext('Student username/email field is required and can not be empty. ' +
+                            'Kindly fill in username/email and then press "Add Exception" button.');
                     }
 
                 }

lms/static/js/certificates/views/certificate_whitelist.js

@@ -77,7 +77,9 @@
                             caller_object.showMessage(response.message, 'msg-error');
                         }
                         catch(exception){
-                            caller_object.showMessage("Server Error, Please try again later.", 'msg-error');
+                            caller_object.showMessage(
+                                "Server Error, Please refresh the page and try again.", 'msg-error'
+                            );
                         }
                     };
                 }

lms/static/js/certificates/views/certificate_whitelist_editor.js

@@ -58,7 +58,10 @@
                     });
 
                     if(this.collection.findWhere(model)){
-                        this.showMessage("username/email already in exception list", 'msg-error');
+                        this.showMessage(
+                            "User (username/email=" + (user_name || user_email) + ") already in exception list.",
+                            'msg-error'
+                        );
                     }
                     else if(certificate_exception.isValid()){
                         certificate_exception.save(
@@ -67,7 +70,7 @@
                                 success: this.showSuccess(
                                     this,
                                     true,
-                                    'Students added to Certificate white list successfully'
+                                    'Student added to Certificate white list successfully.'
                                 ),
                                 error: this.showError(this)
                             }
@@ -98,7 +101,11 @@
                         this.showMessage('Exception is being removed from server.', 'msg-success');
                     }
                     else{
-                        this.showMessage('Could not find Certificate Exception in white list.', 'msg-error');
+                        this.showMessage(
+                            'Could not find Certificate Exception in white list. ' +
+                            'Please refresh the page and try again',
+                            'msg-error'
+                        );
                     }
                 },
 
@@ -131,7 +138,9 @@
                             caller.showMessage(response_data.message, 'msg-error');
                         }
                         catch(exception){
-                            caller.showMessage("Server Error, Please try again later.", 'msg-error');
+                            caller.showMessage("" +
+                                "Server Error, Please refresh the page and try again.", 'msg-error'
+                            );
                         }
                     };
                 }

lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js

@@ -24,7 +24,8 @@ define([
             };
 
             var EXPECTED_ERRORS = {
-                user_name_or_email_required: "Student username/email is required."
+                user_name_or_email_required: 'Student username/email field is required and can not be empty. ' +
+                'Kindly fill in username/email and then press "Add Exception" button.'
             };
 
             beforeEach(function() {
@@ -331,12 +332,14 @@ define([
                 var message_selector='.message',
                     error_class = 'msg-error',
                     success_class = 'msg-success',
-                    success_message = 'Students added to Certificate white list successfully',
-                    requests = AjaxHelpers.requests(this);
+                    success_message = 'Student added to Certificate white list successfully.',
+                    requests = AjaxHelpers.requests(this),
+                    duplicate_user='test_user';
 
                 var error_messages = {
-                    empty_user_name_email: 'Student username/email is required.',
-                    duplicate_user: 'username/email already in exception list'
+                    empty_user_name_email: 'Student username/email field is required and can not be empty. ' +
+                    'Kindly fill in username/email and then press "Add Exception" button.',
+                    duplicate_user: "User (username/email=" + (duplicate_user) + ") already in exception list."
                 };
 
                 // click 'Add Exception' button with empty username/email field
@@ -348,7 +351,7 @@ define([
                 expect(view.$el.find(message_selector).html()).toMatch(error_messages.empty_user_name_email);
 
                 // Add a new Exception to list
-                view.$el.find('#certificate-exception').val("test_user");
+                view.$el.find('#certificate-exception').val(duplicate_user);
                 view.$el.find('#notes').val("test user notes");
                 view.$el.find('#add-exception').click();
 
@@ -357,7 +360,7 @@ define([
                     {
                         id: 3,
                         user_id : 3,
-                        user_name: "test_user",
+                        user_name: duplicate_user,
                         user_email : "test2@test.com",
                         course_id: "edX/test/course",
                         created: "Thursday, October 29, 2015",
@@ -370,13 +373,13 @@ define([
                 expect(view.$el.find(message_selector).html()).toMatch(success_message);
 
                 // Add a duplicate Certificate Exception
-                view.$el.find('#certificate-exception').val("test_user");
+                view.$el.find('#certificate-exception').val(duplicate_user);
                 view.$el.find('#notes').val("test user notes");
                 view.$el.find('#add-exception').click();
 
                 // Verify success message
                 expect(view.$el.find(message_selector)).toHaveClass(error_class);
-                expect(view.$el.find(message_selector).html()).toMatch(error_messages.duplicate_user);
+                expect(view.$el.find(message_selector).html()).toEqual(error_messages.duplicate_user);
             });
 
             it('verifies certificate exception can be deleted by clicking "delete" ', function(){