diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 6e86c40a2e..3a981090f4 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -692,6 +692,80 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase) self.assertEqual(response.status_code, 200) +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class TestInstructorSendEmail(ModuleStoreTestCase, LoginEnrollmentTestCase): + """ + fill this out + """ + def setUp(self): + self.instructor = AdminFactory.create() + self.course = CourseFactory.create() + self.client.login(username=self.instructor.username, password='test') + + def test_send_email_as_logged_in_instructor(self): + url = reverse('send_email', kwargs={'course_id': self.course.id}) + response = self.client.post(url,{ + 'send_to': 'staff', + 'subject': 'test subject', + 'message': 'test message', + }) + self.assertEqual(response.status_code, 200) + + def test_send_email_but_not_logged_in(self): + self.client.logout() + url = reverse('send_email', kwargs={'course_id': self.course.id}) + response = self.client.post(url, { + 'send_to': 'staff', + 'subject': 'test subject', + 'message': 'test message', + }) + self.assertEqual(response.status_code, 403) + + def test_send_email_but_not_staff(self): + self.client.logout() + self.student = UserFactory() + self.client.login(username=self.student.username, password='test') + url = reverse('send_email', kwargs={'course_id': self.course.id}) + response = self.client.post(url, { + 'send_to': 'staff', + 'subject': 'test subject', + 'message': 'test message', + }) + self.assertEqual(response.status_code, 403) + + def test_send_email_but_course_not_exist(self): + url = reverse('send_email', kwargs={'course_id': 'GarbageCourse/DNE/NoTerm'}) + response = self.client.post(url, { + 'send_to': 'staff', + 'subject': 'test subject', + 'message': 'test message', + }) + self.assertNotEqual(response.status_code, 200) + + def test_send_email_no_sendto(self): + url = reverse('send_email', kwargs={'course_id': self.course.id}) + response = self.client.post(url, { + 'subject': 'test subject', + 'message': 'test message', + }) + self.assertEqual(response.status_code, 400) + + def test_send_email_no_subject(self): + url = reverse('send_email', kwargs={'course_id': self.course.id}) + response = self.client.post(url, { + 'send_to': 'staff', + 'message': 'test message', + }) + self.assertEqual(response.status_code, 400) + + def test_send_email_no_message(self): + url = reverse('send_email', kwargs={'course_id': self.course.id}) + response = self.client.post(url, { + 'send_to': 'staff', + 'subject': 'test subject', + }) + self.assertEqual(response.status_code, 400) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestInstructorSendEmail(ModuleStoreTestCase, LoginEnrollmentTestCase): """ diff --git a/lms/djangoapps/instructor/tests/test_email.py b/lms/djangoapps/instructor/tests/test_email.py index 1150c575fe..0bc8140c14 100644 --- a/lms/djangoapps/instructor/tests/test_email.py +++ b/lms/djangoapps/instructor/tests/test_email.py @@ -41,9 +41,9 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase): self.client.login(username=instructor.username, password="test") # URL for instructor dash - self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id}) + self.url = reverse('instructor_dashboard_2', kwargs={'course_id': self.course.id}) # URL for email view - self.email_link = 'Email' + self.email_link = 'Email' def tearDown(self): """ @@ -51,30 +51,34 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase): """ patch.stopall() + # Enabled and IS mongo @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) def test_email_flag_true(self): + from nose.tools import set_trace; set_trace() # Assert that the URL for the email view is in the response response = self.client.get(self.url) self.assertTrue(self.email_link in response.content) - # Select the Email view of the instructor dash - session = self.client.session - session['idash_mode'] = 'Email' - session.save() - response = self.client.get(self.url) - - # Ensure we've selected the view properly and that the send_to field is present. - selected_email_link = 'Email' - self.assertTrue(selected_email_link in response.content) send_to_label = '' self.assertTrue(send_to_label in response.content) + self.assertEqual(response.status_code,200) + # Disabled but IS mongo @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': False}) def test_email_flag_false(self): # Assert that the URL for the email view is not in the response response = self.client.get(self.url) self.assertFalse(self.email_link in response.content) + # Enabled but NOT mongo + @patch.dict(settings.MITX_FEATURES,{'ENABLE_INSTRUCTOR_EMAIL': True}) + def test_email_flag_false(self): + with patch('xmodule.modulestore.mongo.base.MongoModuleStore.get_modulestore_type') as mock_modulestore: + mock_modulestore.return_value = XML_MODULESTORE_TYPE + + response = self.client.get(self.url) + self.assertFalse(self.email_link in response.content) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) def test_email_flag_true_xml_store(self): # If the enable email setting is enabled, but this is an XML backed course, @@ -92,49 +96,8 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase): response = self.client.get(self.url) self.assertFalse(self.email_link in response.content) - -@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) -class TestStudentDashboardEmailView(ModuleStoreTestCase): - """ - Check for email view displayed with flag - """ - def setUp(self): - self.course = CourseFactory.create() - - # Create student account - student = UserFactory.create() - CourseEnrollmentFactory.create(user=student, course_id=self.course.id) - self.client.login(username=student.username, password="test") - - # URL for dashboard - self.url = reverse('dashboard') - # URL for email settings modal - self.email_modal_link = (('Email Settings') - .format(self.course.org, - self.course.number, - self.course.display_name.replace(' ', '_'))) - - def tearDown(self): - """ - Undo all patches. - """ - patch.stopall() - - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) - def test_email_flag_true(self): - # Assert that the URL for the email view is in the response - response = self.client.get(self.url) - self.assertTrue(self.email_modal_link in response.content) - + # Disabled and IS Mongo @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': False}) - def test_email_flag_false(self): - # Assert that the URL for the email view is not in the response - response = self.client.get(self.url) - self.assertFalse(self.email_modal_link in response.content) - - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) def test_email_flag_true_xml_store(self): # If the enable email setting is enabled, but this is an XML backed course, # the email view shouldn't be available on the instructor dashboard. @@ -149,4 +112,4 @@ class TestStudentDashboardEmailView(ModuleStoreTestCase): # Assert that the URL for the email view is not in the response response = self.client.get(self.url) - self.assertFalse(self.email_modal_link in response.content) + self.assertFalse(self.email_link in response.content) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index e7f394cea1..b184ad4ae8 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -43,6 +43,8 @@ from bulk_email.models import CourseEmail from html_to_text import html_to_text from bulk_email import tasks +log = logging.getLogger(__name__) + def common_exceptions_400(func): """ @@ -743,9 +745,9 @@ def list_forum_members(request, course_id): } return JsonResponse(response_payload) + +@ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -# todo check if staff is the desired access level -# todo do html and plaintext messages @require_level('staff') @require_query_params(send_to="sending to whom", subject="subject line", message="message text") def send_email(request, course_id): @@ -756,17 +758,13 @@ def send_email(request, course_id): - 'subject' specifies email's subject - 'message' specifies email's content """ - set_trace() course = get_course_by_id(course_id) - has_instructor_access = has_access(request.user, course, 'instructor') send_to = request.GET.get("send_to") subject = request.GET.get("subject") message = request.GET.get("message") text_message = html_to_text(message) - if subject == "": - return HttpResponseBadRequest("Operation requires instructor access.") email = CourseEmail( - course_id = course_id, + course_id=course_id, sender=request.user, to_option=send_to, subject=subject, @@ -783,6 +781,7 @@ def send_email(request, course_id): } return JsonResponse(response_payload) + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') diff --git a/lms/static/coffee/src/instructor_dashboard/send_email.coffee b/lms/static/coffee/src/instructor_dashboard/send_email.coffee index 0868d10cd3..3ed7622e0e 100644 --- a/lms/static/coffee/src/instructor_dashboard/send_email.coffee +++ b/lms/static/coffee/src/instructor_dashboard/send_email.coffee @@ -22,7 +22,6 @@ class SendEmail # attach click handlers @$btn_send.click => - if @$subject.val() == "" alert gettext("Your message must have a subject.") else if @$emailEditor.save()['data'] == "" @@ -46,8 +45,11 @@ class SendEmail dataType: 'json' url: @$btn_send.data 'endpoint' data: send_data - success: (data) => @display_response gettext('Your email was successfully queued for sending.') + success: (data) => + @display_response gettext('Your email was successfully queued for sending.') + $(".msg-confirm").css({"display":"block"}) error: std_ajax_err => @fail_with_error gettext('Error sending email.') + $(".msg-confirm").css({"display":"none"}) fail_with_error: (msg) -> console.warn msg diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index c9a4c79aa8..2862de27dc 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -260,19 +260,17 @@ section.instructor-dashboard-content-2 { // system feedback - messages .msg { - - - .copy { - font-weight: 600; - } + border-radius: 1px; + padding: 10px 15px; + margin-bottom: 20px; + font-weight: 600; + color: green; } .msg-confirm { + border-top: 2px solid green; background: tint(green,90%); - - .copy { - color: green; - } + display: none; } .list-advice { @@ -289,7 +287,7 @@ section.instructor-dashboard-content-2 { } } } - .msg .copy { + .copy { font-weight: 600; } .msg-confirm { background: #e5f2e5; } diff --git a/lms/templates/instructor/instructor_dashboard_2/email.html b/lms/templates/instructor/instructor_dashboard_2/email.html deleted file mode 100644 index 3ede65e7fb..0000000000 --- a/lms/templates/instructor/instructor_dashboard_2/email.html +++ /dev/null @@ -1,63 +0,0 @@ -<%! from django.utils.translation import ugettext as _ %> -<%page args="section_data"/> - -

Email

- -
- - -
- ${_("Please try not to email students more than once a day. Important things to consider before sending:")} - - -
- -
diff --git a/lms/templates/instructor/instructor_dashboard_2/send_email.html b/lms/templates/instructor/instructor_dashboard_2/send_email.html index 115cdeddd4..b19cd1f587 100644 --- a/lms/templates/instructor/instructor_dashboard_2/send_email.html +++ b/lms/templates/instructor/instructor_dashboard_2/send_email.html @@ -6,7 +6,7 @@

${_("Send Email")}

-
+
+
- \ No newline at end of file +