diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index f7be9c2450..ace9f1a3b5 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -97,18 +97,14 @@ BULK_EMAIL_FAILURE_ERRORS = ( ) -def _get_course_email_context(course, is_secure): +def _get_course_email_context(course): """ Returns context arguments to apply to all emails, independent of recipient. - - Inputs are: - * `course`: Course related to the current email - * `is_secure`: Set to True to return https URLs, False to use http. """ course_id = course.id.to_deprecated_string() course_title = course.display_name course_end_date = get_default_time_display(course.end) - scheme = u'https' if is_secure else u'http' + scheme = u'https' if settings.HTTPS == "on" else u'http' base_url = '{}://{}'.format(scheme, settings.SITE_NAME) course_url = '{}{}'.format( base_url, @@ -176,12 +172,9 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name) # Fetch the course object. course = get_course(course_id) - # Emails use https URLs by default, to maintain backwards compatibility. - is_secure = task_input.get('is_secure', True) - # Get arguments that will be passed to every subtask. targets = email_obj.targets.all() - global_email_context = _get_course_email_context(course, is_secure) + global_email_context = _get_course_email_context(course) recipient_qsets = [ target.get_users(course_id, user_id) diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index be33e2b94f..2835a6959d 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -3,7 +3,6 @@ Unit tests for sending course email """ import json -import ddt from markupsafe import escape from mock import patch, Mock from nose.plugins.attrib import attr @@ -16,12 +15,11 @@ from django.core.urlresolvers import reverse from django.core.management import call_command from django.test.utils import override_settings -from bulk_email.models import Optout, BulkEmailFlag, CourseEmail, SEND_TO_MYSELF -from bulk_email.tasks import _get_source_address, _get_course_email_context, perform_delegate_email_batches +from bulk_email.models import Optout, BulkEmailFlag +from bulk_email.tasks import _get_source_address, _get_course_email_context from openedx.core.djangoapps.course_groups.models import CourseCohort from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort from courseware.tests.factories import StaffFactory, InstructorFactory -from instructor_task.models import InstructorTask from instructor_task.subtasks import update_subtask_status from student.roles import CourseStaffRole from student.models import CourseEnrollment @@ -479,7 +477,6 @@ class TestEmailSendFromDashboard(EmailSendFromDashboardTestCase): self.assertIn(uni_message, message_body) -@ddt.ddt class TestCourseEmailContext(SharedModuleStoreTestCase): """ Test the course email context hash used to send bulk emails. @@ -488,7 +485,7 @@ class TestCourseEmailContext(SharedModuleStoreTestCase): @classmethod def setUpClass(cls): """ - Create a course, InstructorTask, and CourseEmail shared by all tests. + Create a course shared by all tests. """ super(TestCourseEmailContext, cls).setUpClass() cls.course_title = u"Финансовое программирование и политика, часть 1: макроэкономические счета и анализ" @@ -501,32 +498,11 @@ class TestCourseEmailContext(SharedModuleStoreTestCase): number=cls.course_number, run=cls.course_run, ) - cls.user = UserFactory() - cls.course_email = CourseEmail.create( - course_id=cls.course.id, - sender=cls.user, - targets=[SEND_TO_MYSELF], - subject='subject', - html_message='message', - ) - cls.email_task = InstructorTask.create( - course_id=cls.course.id, - task_type='bulk_course_email', - task_key='key', - task_input={}, - requester=cls.user, - ) - @ddt.data( - (False, 'http'), - (True, 'https'), - ) - @ddt.unpack - def test_context_urls(self, is_secure, scheme): + def verify_email_context(self, email_context, scheme): """ This test tests that the bulk email context uses http or https urls as appropriate. """ - email_context = _get_course_email_context(self.course, is_secure) self.assertEquals(email_context['platform_name'], 'edX') self.assertEquals(email_context['course_title'], self.course_title) self.assertEquals(email_context['course_url'], @@ -541,23 +517,18 @@ class TestCourseEmailContext(SharedModuleStoreTestCase): self.assertEquals(email_context['email_settings_url'], '{}://edx.org/dashboard'.format(scheme)) self.assertEquals(email_context['account_settings_url'], '{}://edx.org/account/settings'.format(scheme)) - @ddt.data( - None, - False, - True, - ) - @patch('bulk_email.tasks._get_course_email_context') - def test_task_input_is_secure(self, is_secure, get_course_email_context): + @override_settings(HTTPS="off") + def test_insecure_email_context(self): """ - Test the effect of different task_input hashes on perform_delegate_email_batches, - to ensure that the correct http or https URL schemes are used in generated emails. + This test tests that the bulk email context uses http urls """ - task_input = {'email_id': self.course_email.id} - if is_secure is not None: - task_input['is_secure'] = is_secure - else: - # https URLs used by default to maintain backwards compatibility - is_secure = True + email_context = _get_course_email_context(self.course) + self.verify_email_context(email_context, 'http') - perform_delegate_email_batches(self.email_task.id, self.course.id, task_input, 'emailed') - get_course_email_context.assert_called_with(self.course, is_secure) + @override_settings(HTTPS="on") + def test_secure_email_context(self): + """ + This test tests that the bulk email context uses https urls + """ + email_context = _get_course_email_context(self.course) + self.verify_email_context(email_context, 'https') diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 6d1c8e6c8c..6301b49bf0 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -290,7 +290,7 @@ def submit_bulk_course_email(request, course_key, email_id): task_type = 'bulk_course_email' task_class = send_bulk_course_email - task_input = {'email_id': email_id, 'to_option': targets, 'is_secure': request.is_secure()} + task_input = {'email_id': email_id, 'to_option': targets} task_key_stub = str(email_id) # create the key value by using MD5 hash: task_key = hashlib.md5(task_key_stub).hexdigest() diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index 84bc65ea63..51ec2b5dff 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -1,7 +1,7 @@ """ Test for LMS instructor background task queue management """ -from mock import patch, Mock, MagicMock, ANY +from mock import patch, Mock, MagicMock from nose.plugins.attrib import attr from bulk_email.models import CourseEmail, SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_LEARNERS from courseware.tests.factories import UserFactory @@ -30,7 +30,7 @@ from instructor_task.api import ( from instructor_task.api_helper import AlreadyRunningError from instructor_task.models import InstructorTask, PROGRESS -from instructor_task.tasks import export_ora2_data, send_bulk_course_email +from instructor_task.tasks import export_ora2_data from instructor_task.tests.test_base import ( InstructorTaskTestCase, InstructorTaskCourseTestCase, @@ -221,23 +221,6 @@ class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCa ) self._test_resubmission(api_call) - def test_submit_bulk_email_task_input(self): - """ - Tests the task_input hash created by submit_bulk_course_email. - """ - request = self.create_task_request(self.instructor) - email_id = self._define_course_email() - task_input = dict(email_id=email_id, - to_option=[SEND_TO_MYSELF, SEND_TO_LEARNERS, SEND_TO_STAFF], - is_secure=request.is_secure()) - - with patch('instructor_task.api.submit_task') as mock_submit_task: - mock_submit_task.return_value = MagicMock() - submit_bulk_course_email(request, self.course.id, email_id) - - mock_submit_task.assert_called_once_with( - request, 'bulk_course_email', send_bulk_course_email, self.course.id, task_input, ANY) - def test_submit_calculate_problem_responses(self): api_call = lambda: submit_calculate_problem_responses_csv( self.create_task_request(self.instructor),