Skip to content

E
edx-platform
Commit 2320d422 by Matt Drayer
Options

Merge pull request #10748 from edx/saleem-latif/SOL-1389 

SOL-1389: Cert Exception: Add error states and messages
parents abcf33ab a8e44b30
Showing with 128 additions and 34 deletions
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(){
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment