From c955fd0bce93934be07a917d3f73d083d8dac4b2 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Fri, 2 May 2014 17:50:13 +0500 Subject: [PATCH] When sending bulk email check if course is authorized. Also the idash_mode property is stored seperately for each course. LMS-2565 --- .../bulk_email/tests/test_course_optout.py | 2 +- lms/djangoapps/bulk_email/tests/test_email.py | 2 +- .../instructor/tests/test_legacy_email.py | 2 +- lms/djangoapps/instructor/views/api.py | 5 ++ .../instructor/views/instructor_dashboard.py | 4 +- lms/djangoapps/instructor/views/legacy.py | 59 ++++++++++--------- lms/djangoapps/instructor/views/tools.py | 23 ++++++++ 7 files changed, 64 insertions(+), 33 deletions(-) diff --git a/lms/djangoapps/bulk_email/tests/test_course_optout.py b/lms/djangoapps/bulk_email/tests/test_course_optout.py index 5cd54686b1..23bcbde954 100644 --- a/lms/djangoapps/bulk_email/tests/test_course_optout.py +++ b/lms/djangoapps/bulk_email/tests/test_course_optout.py @@ -55,7 +55,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase): # Select the Email view of the instructor dash session = self.client.session - session['idash_mode'] = 'Email' + session[u'idash_mode:{0}'.format(self.course.location.course_id)] = 'Email' session.save() response = self.client.get(url) selected_email_link = 'Email' diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index c96727dfcf..4531c7a374 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -76,7 +76,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): # Select the Email view of the instructor dash session = self.client.session - session['idash_mode'] = 'Email' + session[u'idash_mode:{0}'.format(self.course.location.course_id)] = 'Email' session.save() response = self.client.get(self.url) selected_email_link = 'Email' diff --git a/lms/djangoapps/instructor/tests/test_legacy_email.py b/lms/djangoapps/instructor/tests/test_legacy_email.py index c91d1cce2f..0e2642c829 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_email.py +++ b/lms/djangoapps/instructor/tests/test_legacy_email.py @@ -50,7 +50,7 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase): # Select the Email view of the instructor dash session = self.client.session - session['idash_mode'] = 'Email' + session[u'idash_mode:{0}'.format(self.course.location.course_id)] = 'Email' session.save() response = self.client.get(self.url) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 293d141efe..479a246f02 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -66,6 +66,7 @@ from .tools import ( parse_datetime, set_due_date_extension, strip_if_string, + bulk_email_is_enabled_for_course, ) from xmodule.modulestore import Location @@ -1036,6 +1037,10 @@ def send_email(request, course_id): - 'subject' specifies email's subject - 'message' specifies email's content """ + + if not bulk_email_is_enabled_for_course(course_id): + return HttpResponseForbidden("Email is not enabled for this course.") + send_to = request.POST.get("send_to") subject = request.POST.get("subject") message = request.POST.get("message") diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index fef3e17be4..ed9520420b 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -25,7 +25,7 @@ from student.models import CourseEnrollment from bulk_email.models import CourseAuthorization from class_dashboard.dashboard_data import get_section_display_name, get_array_section_has_problem -from .tools import get_units_with_due_date, title_or_url +from .tools import get_units_with_due_date, title_or_url, bulk_email_is_enabled_for_course @ensure_csrf_cookie @@ -60,7 +60,7 @@ def instructor_dashboard_2(request, course_id): sections.insert(3, _section_extensions(course)) # Gate access to course email by feature flag & by course-specific authorization - if settings.FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and is_studio_course and CourseAuthorization.instructor_email_enabled(course_id): + if bulk_email_is_enabled_for_course(course_id): sections.append(_section_send_email(course_id, access, course)) # Gate access to Metrics tab by featue flag and staff authorization diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index adf9eaa6c7..7f62501a12 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -48,7 +48,7 @@ from django_comment_common.models import ( ) from django_comment_client.utils import has_forum_access from instructor.offline_gradecalc import student_grades, offline_grades_available -from instructor.views.tools import strip_if_string +from instructor.views.tools import strip_if_string, bulk_email_is_enabled_for_course from instructor_task.api import ( get_running_instructor_tasks, get_instructor_task_history, @@ -110,10 +110,11 @@ def instructor_dashboard(request, course_id): # the instructor dashboard page is modal: grades, psychometrics, admin # keep that state in request.session (defaults to grades mode) idash_mode = request.POST.get('idash_mode', '') + idash_mode_key = u'idash_mode:{0}'.format(course_id) if idash_mode: - request.session['idash_mode'] = idash_mode + request.session[idash_mode_key] = idash_mode else: - idash_mode = request.session.get('idash_mode', 'Grades') + idash_mode = request.session.get(idash_mode_key, 'Grades') enrollment_number = CourseEnrollment.num_enrolled_in(course_id) @@ -760,33 +761,36 @@ def instructor_dashboard(request, course_id): email_subject = request.POST.get("subject") html_message = request.POST.get("message") - try: - # Create the CourseEmail object. This is saved immediately, so that - # any transaction that has been pending up to this point will also be - # committed. - email = CourseEmail.create(course_id, request.user, email_to_option, email_subject, html_message) + if bulk_email_is_enabled_for_course(course_id): + try: + # Create the CourseEmail object. This is saved immediately, so that + # any transaction that has been pending up to this point will also be + # committed. + email = CourseEmail.create(course_id, request.user, email_to_option, email_subject, html_message) - # Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes) - submit_bulk_course_email(request, course_id, email.id) # pylint: disable=E1101 + # Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes) + submit_bulk_course_email(request, course_id, email.id) # pylint: disable=E1101 - except Exception as err: - # Catch any errors and deliver a message to the user - error_msg = "Failed to send email! ({0})".format(err) - msg += "" + error_msg + "" - log.exception(error_msg) + except Exception as err: + # Catch any errors and deliver a message to the user + error_msg = "Failed to send email! ({0})".format(err) + msg += "" + error_msg + "" + log.exception(error_msg) - else: - # If sending the task succeeds, deliver a success message to the user. - if email_to_option == "all": - text = _( - "Your email was successfully queued for sending. " - "Please note that for large classes, it may take up to an hour " - "(or more, if other courses are simultaneously sending email) " - "to send all emails." - ) else: - text = _('Your email was successfully queued for sending.') - email_msg = '

{text}

'.format(text=text) + # If sending the task succeeds, deliver a success message to the user. + if email_to_option == "all": + text = _( + "Your email was successfully queued for sending. " + "Please note that for large classes, it may take up to an hour " + "(or more, if other courses are simultaneously sending email) " + "to send all emails." + ) + else: + text = _('Your email was successfully queued for sending.') + email_msg = '

{text}

'.format(text=text) + else: + msg += "Email is not enabled for this course." elif "Show Background Email Task History" in action: message, datatable = get_background_task_table(course_id, task_type='bulk_course_email') @@ -895,8 +899,7 @@ def instructor_dashboard(request, course_id): # 1. Feature flag is on # 2. We have explicitly enabled email for the given course via django-admin # 3. It is NOT an XML course - if settings.FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and \ - CourseAuthorization.instructor_email_enabled(course_id) and is_studio_course: + if bulk_email_is_enabled_for_course(course_id): show_email_tab = True # display course stats only if there is no other table to display: diff --git a/lms/djangoapps/instructor/views/tools.py b/lms/djangoapps/instructor/views/tools.py index 703610cb64..322ef2c898 100644 --- a/lms/djangoapps/instructor/views/tools.py +++ b/lms/djangoapps/instructor/views/tools.py @@ -4,6 +4,7 @@ Tools for the instructor dashboard import dateutil import json +from django.conf import settings from django.contrib.auth.models import User from django.http import HttpResponseBadRequest from django.utils.timezone import utc @@ -11,6 +12,10 @@ from django.utils.translation import ugettext as _ from courseware.models import StudentModule from xmodule.fields import Date +from xmodule.modulestore import XML_MODULESTORE_TYPE +from xmodule.modulestore.django import modulestore + +from bulk_email.models import CourseAuthorization DATE_FIELD = Date() @@ -45,6 +50,24 @@ def handle_dashboard_error(view): return wrapper +def bulk_email_is_enabled_for_course(course_id): + """ + Staff can only send bulk email for a course if all the following conditions are true: + 1. Bulk email feature flag is on. + 2. It is a studio course. + 3. Bulk email is enabled for the course. + """ + + bulk_email_enabled_globally = (settings.FEATURES['ENABLE_INSTRUCTOR_EMAIL'] == True) + is_studio_course = (modulestore().get_modulestore_type(course_id) != XML_MODULESTORE_TYPE) + bulk_email_enabled_for_course = CourseAuthorization.instructor_email_enabled(course_id) + + if bulk_email_enabled_globally and is_studio_course and bulk_email_enabled_for_course: + return True + + return False + + def strip_if_string(value): if isinstance(value, basestring): return value.strip()