diff --git a/lms/djangoapps/bulk_email/admin.py b/lms/djangoapps/bulk_email/admin.py index 3550e1eb42..f4529a9e70 100644 --- a/lms/djangoapps/bulk_email/admin.py +++ b/lms/djangoapps/bulk_email/admin.py @@ -3,8 +3,8 @@ Django admin page for bulk email models """ from django.contrib import admin -from bulk_email.models import CourseEmail, Optout, CourseEmailTemplate -from bulk_email.forms import CourseEmailTemplateForm +from bulk_email.models import CourseEmail, Optout, CourseEmailTemplate, CourseAuthorization +from bulk_email.forms import CourseEmailTemplateForm, CourseAuthorizationAdminForm class CourseEmailAdmin(admin.ModelAdmin): @@ -57,6 +57,23 @@ unsupported tags will cause email sending to fail. return False +class CourseAuthorizationAdmin(admin.ModelAdmin): + """Admin for enabling email on a course-by-course basis.""" + form = CourseAuthorizationAdminForm + fieldsets = ( + (None, { + 'fields': ('course_id', 'email_enabled'), + 'description': ''' +Enter a course id in the following form: Org/Course/CourseRun, eg MITx/6.002x/2012_Fall +Do not enter leading or trailing slashes. There is no need to surround the course ID with quotes. +Validation will be performed on the course name, and if it is invalid, an error message will display. + +To enable email for the course, check the "Email enabled" box, then click "Save". +''' + }), + ) + admin.site.register(CourseEmail, CourseEmailAdmin) admin.site.register(Optout, OptoutAdmin) admin.site.register(CourseEmailTemplate, CourseEmailTemplateAdmin) +admin.site.register(CourseAuthorization, CourseAuthorizationAdmin) diff --git a/lms/djangoapps/bulk_email/forms.py b/lms/djangoapps/bulk_email/forms.py index fc75d4e4b6..bec05fba52 100644 --- a/lms/djangoapps/bulk_email/forms.py +++ b/lms/djangoapps/bulk_email/forms.py @@ -6,12 +6,16 @@ import logging from django import forms from django.core.exceptions import ValidationError -from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG +from bulk_email.models import CourseEmailTemplate, COURSE_EMAIL_MESSAGE_BODY_TAG, CourseAuthorization + +from courseware.courses import get_course_by_id +from xmodule.modulestore import MONGO_MODULESTORE_TYPE +from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) -class CourseEmailTemplateForm(forms.ModelForm): +class CourseEmailTemplateForm(forms.ModelForm): # pylint: disable=R0924 """Form providing validation of CourseEmail templates.""" class Meta: # pylint: disable=C0111 @@ -43,3 +47,32 @@ class CourseEmailTemplateForm(forms.ModelForm): template = self.cleaned_data["plain_template"] self._validate_template(template) return template + + +class CourseAuthorizationAdminForm(forms.ModelForm): # pylint: disable=R0924 + """Input form for email enabling, allowing us to verify data.""" + + class Meta: # pylint: disable=C0111 + model = CourseAuthorization + + def clean_course_id(self): + """Validate the course id""" + course_id = self.cleaned_data["course_id"] + try: + # Just try to get the course descriptor. + # If we can do that, it's a real course. + get_course_by_id(course_id, depth=1) + except Exception as exc: + msg = 'Error encountered ({0})'.format(str(exc).capitalize()) + msg += ' --- Entered course id was: "{0}". '.format(course_id) + msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN' + raise forms.ValidationError(msg) + + # Now, try and discern if it is a Studio course - HTML editor doesn't work with XML courses + is_studio_course = modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE + if not is_studio_course: + msg = "Course Email feature is only available for courses authored in Studio. " + msg += '"{0}" appears to be an XML backed course.'.format(course_id) + raise forms.ValidationError(msg) + + return course_id diff --git a/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py b/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py new file mode 100644 index 0000000000..b200ea2bde --- /dev/null +++ b/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py @@ -0,0 +1,95 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'CourseAuthorization' + db.create_table('bulk_email_courseauthorization', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('course_id', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('email_enabled', self.gf('django.db.models.fields.BooleanField')(default=False)), + )) + db.send_create_signal('bulk_email', ['CourseAuthorization']) + + + def backwards(self, orm): + # Deleting model 'CourseAuthorization' + db.delete_table('bulk_email_courseauthorization') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'bulk_email.courseauthorization': { + 'Meta': {'object_name': 'CourseAuthorization'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'email_enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'bulk_email.courseemail': { + 'Meta': {'object_name': 'CourseEmail'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'html_message': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}), + 'sender': ('django.db.models.fields.related.ForeignKey', [], {'default': '1', 'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}), + 'slug': ('django.db.models.fields.CharField', [], {'max_length': '128', 'db_index': 'True'}), + 'subject': ('django.db.models.fields.CharField', [], {'max_length': '128', 'blank': 'True'}), + 'text_message': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'to_option': ('django.db.models.fields.CharField', [], {'default': "'myself'", 'max_length': '64'}) + }, + 'bulk_email.courseemailtemplate': { + 'Meta': {'object_name': 'CourseEmailTemplate'}, + 'html_template': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'plain_template': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}) + }, + 'bulk_email.optout': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'Optout'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + } + } + + complete_apps = ['bulk_email'] \ No newline at end of file diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index 8bbca579a5..83a691f18e 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/lms/djangoapps/bulk_email/models.py @@ -16,8 +16,17 @@ from django.db import models, transaction from django.contrib.auth.models import User from html_to_text import html_to_text +from django.conf import settings + log = logging.getLogger(__name__) +# Bulk email to_options - the send to options that users can +# select from when they send email. +SEND_TO_MYSELF = 'myself' +SEND_TO_STAFF = 'staff' +SEND_TO_ALL = 'all' +TO_OPTIONS = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_ALL] + class Email(models.Model): """ @@ -35,12 +44,6 @@ class Email(models.Model): abstract = True -SEND_TO_MYSELF = 'myself' -SEND_TO_STAFF = 'staff' -SEND_TO_ALL = 'all' -TO_OPTIONS = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_ALL] - - class CourseEmail(Email): """ Stores information for an email to a course. @@ -209,3 +212,38 @@ class CourseEmailTemplate(models.Model): stored HTML template and the provided `context` dict. """ return CourseEmailTemplate._render(self.html_template, htmltext, context) + + +class CourseAuthorization(models.Model): + """ + Enable the course email feature on a course-by-course basis. + """ + # The course that these features are attached to. + course_id = models.CharField(max_length=255, db_index=True) + + # Whether or not to enable instructor email + email_enabled = models.BooleanField(default=False) + + @classmethod + def instructor_email_enabled(cls, course_id): + """ + Returns whether or not email is enabled for the given course id. + + If email has not been explicitly enabled, returns False. + """ + # If settings.MITX_FEATURES['REQUIRE_COURSE_EMAIL_AUTH'] is + # set to False, then we enable email for every course. + if not settings.MITX_FEATURES['REQUIRE_COURSE_EMAIL_AUTH']: + return True + + try: + record = cls.objects.get(course_id=course_id) + return record.email_enabled + except cls.DoesNotExist: + return False + + def __unicode__(self): + not_en = "Not " + if self.email_enabled: + not_en = "" + return u"Course '{}': Instructor Email {}Enabled".format(self.course_id, not_en) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 8a4499aef1..a2f376cdcc 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -9,9 +9,6 @@ import json from uuid import uuid4 from time import sleep -from sys import exc_info -from traceback import format_exc - from dogapi import dog_stats_api from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPException from boto.ses.exceptions import ( @@ -31,7 +28,6 @@ from celery.exceptions import RetryTaskError from django.conf import settings from django.contrib.auth.models import User, Group from django.core.mail import EmailMultiAlternatives, get_connection -from django.http import Http404 from django.core.urlresolvers import reverse from bulk_email.models import ( @@ -405,8 +401,8 @@ def _get_source_address(course_id, course_title): # in an email address, by substituting a '_' anywhere a non-(ascii, period, or dash) # character appears. course_num = course_id.split('/')[1] - INVALID_CHARS = re.compile(r"[^\w.-]") - course_num = INVALID_CHARS.sub('_', course_num) + invalid_chars = re.compile(r"[^\w.-]") + course_num = invalid_chars.sub('_', course_num) from_addr = '"{0}" Course Staff <{1}-{2}>'.format(course_title_no_quotes, course_num, settings.BULK_EMAIL_DEFAULT_FROM_EMAIL) return from_addr diff --git a/lms/djangoapps/bulk_email/tests/test_course_optout.py b/lms/djangoapps/bulk_email/tests/test_course_optout.py index 0adf119527..378610eeb3 100644 --- a/lms/djangoapps/bulk_email/tests/test_course_optout.py +++ b/lms/djangoapps/bulk_email/tests/test_course_optout.py @@ -59,7 +59,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase): selected_email_link = 'Email' self.assertTrue(selected_email_link in response.content) - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_optout_course(self): """ Make sure student does not receive course email after opting out. @@ -88,7 +88,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase): # Assert that self.student.email not in mail.to, outbox should be empty self.assertEqual(len(mail.outbox), 0) - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_optin_course(self): """ Make sure student receives course email after opting in. diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 80fc692a4a..9da787dbaa 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -44,7 +44,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): Test that emails send correctly. """ - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def setUp(self): self.course = CourseFactory.create() self.instructor = UserFactory.create(username="instructor", email="robot+instructor@edx.org") diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py new file mode 100644 index 0000000000..60a8abf95e --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_forms.py @@ -0,0 +1,105 @@ +""" +Unit tests for bulk-email-related forms. +""" +from django.test.utils import override_settings +from django.conf import settings + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE +from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE + +from xmodule.modulestore.django import modulestore +from xmodule.modulestore import MONGO_MODULESTORE_TYPE + +from mock import patch + +from bulk_email.models import CourseAuthorization +from bulk_email.forms import CourseAuthorizationAdminForm + + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class CourseAuthorizationFormTest(ModuleStoreTestCase): + """Test the CourseAuthorizationAdminForm form for Mongo-backed courses.""" + + def setUp(self): + # Make a mongo course + self.course = CourseFactory.create() + + def tearDown(self): + """ + Undo all patches. + """ + patch.stopall() + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_authorize_mongo_course(self): + # Initially course shouldn't be authorized + self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id)) + # Test authorizing the course, which should totally work + form_data = {'course_id': self.course.id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation should work + self.assertTrue(form.is_valid()) + form.save() + # Check that this course is authorized + self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id)) + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_form_typo(self): + # Munge course id + bad_id = self.course.id + '_typo' + + form_data = {'course_id': bad_id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation shouldn't work + self.assertFalse(form.is_valid()) + + msg = u'Error encountered (Course not found.)' + msg += ' --- Entered course id was: "{0}". '.format(bad_id) + msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN' + self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access + + with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): + form.save() + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_course_name_only(self): + # Munge course id - common + bad_id = self.course.id.split('/')[-1] + + form_data = {'course_id': bad_id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation shouldn't work + self.assertFalse(form.is_valid()) + + msg = u'Error encountered (Need more than 1 value to unpack)' + msg += ' --- Entered course id was: "{0}". '.format(bad_id) + msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN' + self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access + + with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): + form.save() + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class CourseAuthorizationXMLFormTest(ModuleStoreTestCase): + """Check that XML courses cannot be authorized for email.""" + + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_xml_course_authorization(self): + course_id = 'edX/toy/2012_Fall' + # Assert this is an XML course + self.assertTrue(modulestore().get_modulestore_type(course_id) != MONGO_MODULESTORE_TYPE) + + form_data = {'course_id': course_id, 'email_enabled': True} + form = CourseAuthorizationAdminForm(data=form_data) + # Validation shouldn't work + self.assertFalse(form.is_valid()) + + msg = u"Course Email feature is only available for courses authored in Studio. " + msg += '"{0}" appears to be an XML backed course.'.format(course_id) + self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access + + with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): + form.save() diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email/tests/test_models.py index 737bc36845..30713e178c 100644 --- a/lms/djangoapps/bulk_email/tests/test_models.py +++ b/lms/djangoapps/bulk_email/tests/test_models.py @@ -3,10 +3,13 @@ Unit tests for bulk-email-related models. """ from django.test import TestCase from django.core.management import call_command +from django.conf import settings from student.tests.factories import UserFactory -from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate +from mock import patch + +from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate, CourseAuthorization class CourseEmailTest(TestCase): @@ -99,3 +102,46 @@ class CourseEmailTemplateTest(TestCase): template = CourseEmailTemplate.get_template() context = self._get_sample_plain_context() template.render_plaintext("My new plain text.", context) + + +class CourseAuthorizationTest(TestCase): + """Test the CourseAuthorization model.""" + + @patch.dict(settings.MITX_FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_creation_auth_on(self): + course_id = 'abc/123/doremi' + # Test that course is not authorized by default + self.assertFalse(CourseAuthorization.instructor_email_enabled(course_id)) + + # Authorize + cauth = CourseAuthorization(course_id=course_id, email_enabled=True) + cauth.save() + # Now, course should be authorized + self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id)) + self.assertEquals( + cauth.__unicode__(), + "Course 'abc/123/doremi': Instructor Email Enabled" + ) + + # Unauthorize by explicitly setting email_enabled to False + cauth.email_enabled = False + cauth.save() + # Test that course is now unauthorized + self.assertFalse(CourseAuthorization.instructor_email_enabled(course_id)) + self.assertEquals( + cauth.__unicode__(), + "Course 'abc/123/doremi': Instructor Email Not Enabled" + ) + + @patch.dict(settings.MITX_FEATURES, {'REQUIRE_COURSE_EMAIL_AUTH': False}) + def test_creation_auth_off(self): + course_id = 'blahx/blah101/ehhhhhhh' + # Test that course is authorized by default, since auth is turned off + self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id)) + + # Use the admin interface to unauthorize the course + cauth = CourseAuthorization(course_id=course_id, email_enabled=False) + cauth.save() + + # Now, course should STILL be authorized! + self.assertTrue(CourseAuthorization.instructor_email_enabled(course_id)) diff --git a/lms/djangoapps/instructor/tests/test_email.py b/lms/djangoapps/instructor/tests/test_email.py index ae21f85faf..de8832a625 100644 --- a/lms/djangoapps/instructor/tests/test_email.py +++ b/lms/djangoapps/instructor/tests/test_email.py @@ -16,6 +16,8 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from mock import patch +from bulk_email.models import CourseAuthorization + @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) class TestNewInstructorDashboardEmailViewMongoBacked(ModuleStoreTestCase): @@ -44,8 +46,11 @@ class TestNewInstructorDashboardEmailViewMongoBacked(ModuleStoreTestCase): # In order for bulk email to work, we must have both the ENABLE_INSTRUCTOR_EMAIL_FLAG # set to True and for the course to be Mongo-backed. # The flag is enabled and the course is Mongo-backed (should work) - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_email_flag_true_mongo_true(self): + # Assert that instructor email is enabled for this course - since REQUIRE_COURSE_EMAIL_AUTH is False, + # all courses should be authorized to use email. + self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id)) # Assert that the URL for the email view is in the response response = self.client.get(self.url) self.assertIn(self.email_link, response.content) @@ -61,6 +66,47 @@ class TestNewInstructorDashboardEmailViewMongoBacked(ModuleStoreTestCase): response = self.client.get(self.url) self.assertFalse(self.email_link in response.content) + # Flag is enabled, but we require course auth and haven't turned it on for this course + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_course_not_authorized(self): + # Assert that instructor email is not enabled for this course + self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id)) + # 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) + + # Flag is enabled, we require course auth and turn it on for this course + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_course_authorized(self): + # Assert that instructor email is not enabled for this course + self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id)) + # 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) + + # Authorize the course to use email + cauth = CourseAuthorization(course_id=self.course.id, email_enabled=True) + cauth.save() + + # Assert that instructor email is enabled for this course + self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id)) + # Assert that the URL for the email view is not in the response + response = self.client.get(self.url) + self.assertTrue(self.email_link in response.content) + + # Flag is disabled, but course is authorized + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': False, 'REQUIRE_COURSE_EMAIL_AUTH': True}) + def test_course_authorized_feature_off(self): + # Authorize the course to use email + cauth = CourseAuthorization(course_id=self.course.id, email_enabled=True) + cauth.save() + + # Assert that instructor email IS enabled for this course + self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id)) + # 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) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestNewInstructorDashboardEmailViewXMLBacked(ModuleStoreTestCase): @@ -79,14 +125,15 @@ class TestNewInstructorDashboardEmailViewXMLBacked(ModuleStoreTestCase): # URL for email view self.email_link = 'Email' - # The flag is enabled but the course is not Mongo-backed (should not work) - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + # The flag is enabled, and since REQUIRE_COURSE_EMAIL_AUTH is False, all courses should + # be authorized to use email. But the course is not Mongo-backed (should not work) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_email_flag_true_mongo_false(self): response = self.client.get(self.url) self.assertFalse(self.email_link in response.content) # The flag is disabled and the course is not Mongo-backed (should not work) - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': False}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': False, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_email_flag_false_mongo_false(self): response = self.client.get(self.url) self.assertFalse(self.email_link in response.content) diff --git a/lms/djangoapps/instructor/tests/test_legacy_email.py b/lms/djangoapps/instructor/tests/test_legacy_email.py index c89641c98d..20b8271bdf 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_email.py +++ b/lms/djangoapps/instructor/tests/test_legacy_email.py @@ -41,7 +41,7 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase): """ patch.stopall() - @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) + @patch.dict(settings.MITX_FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) def test_email_flag_true(self): # Assert that the URL for the email view is in the response response = self.client.get(self.url) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index b38d06f00a..759d288ff3 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -22,6 +22,7 @@ from courseware.courses import get_course_by_id from django_comment_client.utils import has_forum_access from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR from student.models import CourseEnrollment +from bulk_email.models import CourseAuthorization @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @@ -51,7 +52,9 @@ def instructor_dashboard_2(request, course_id): _section_analytics(course_id), ] - if settings.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and is_studio_course: + # Gate access by feature flag & by course-specific authorization + if settings.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and \ + is_studio_course and CourseAuthorization.instructor_email_enabled(course_id): sections.append(_section_send_email(course_id, access, course)) context = { diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 8bdc7548f4..517d8be1d2 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -30,7 +30,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.html_module import HtmlDescriptor -from bulk_email.models import CourseEmail +from bulk_email.models import CourseEmail, CourseAuthorization from courseware import grades from courseware.access import (has_access, get_access_group_name, course_beta_test_group_name) @@ -798,23 +798,25 @@ def instructor_dashboard(request, course_id): else: instructor_tasks = None - # determine if this is a studio-backed course so we can 1) provide a link to edit this course in studio - # 2) enable course email + # determine if this is a studio-backed course so we can provide a link to edit this course in studio is_studio_course = modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE + studio_url = None + if is_studio_course: + studio_url = get_cms_course_link_by_id(course_id) + email_editor = None # HTML editor for email if idash_mode == 'Email' and is_studio_course: html_module = HtmlDescriptor(course.system, DictFieldData({'data': html_message}), ScopeIds(None, None, None, None)) email_editor = wrap_xmodule(html_module.get_html, html_module, 'xmodule_edit.html')() - studio_url = None - if is_studio_course: - studio_url = get_cms_course_link_by_id(course_id) - - # Flag for whether or not we display the email tab (depending upon - # what backing store this course using (Mongo vs. XML)) - if settings.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and is_studio_course: + # Enable instructor email only if the following conditions are met: + # 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.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and \ + CourseAuthorization.instructor_email_enabled(course_id) and is_studio_course: show_email_tab = True # display course stats only if there is no other table to display: diff --git a/lms/envs/common.py b/lms/envs/common.py index dc902a9ec8..b2e302a5be 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -114,8 +114,12 @@ MITX_FEATURES = { # analytics experiments 'ENABLE_INSTRUCTOR_ANALYTICS': False, - # bulk email available to instructors: - 'ENABLE_INSTRUCTOR_EMAIL': False, + # Enables the LMS bulk email feature for course staff + 'ENABLE_INSTRUCTOR_EMAIL': True, + # If True and ENABLE_INSTRUCTOR_EMAIL: Forces email to be explicitly turned on + # for each course via django-admin interface. + # If False and ENABLE_INSTRUCTOR_EMAIL: Email will be turned on by default for all courses. + 'REQUIRE_COURSE_EMAIL_AUTH': True, # enable analytics server. # WARNING: THIS SHOULD ALWAYS BE SET TO FALSE UNDER NORMAL