Uses settings.HTTPS to determine appropriate URL scheme for bulk emails

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)

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')

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()

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),