Uses http or https appropriately in bulk email urls

and adds system tests for affected methods.

lms/djangoapps/bulk_email/tasks.py

@@ -97,25 +97,31 @@ BULK_EMAIL_FAILURE_ERRORS = (
 )
 
 
-def _get_course_email_context(course):
+def _get_course_email_context(course, is_secure):
     """
     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)
-    course_url = 'https://{}{}'.format(
-        settings.SITE_NAME,
+    scheme = u'https' if is_secure else u'http'
+    base_url = '{}://{}'.format(scheme, settings.SITE_NAME)
+    course_url = '{}{}'.format(
+        base_url,
         reverse('course_root', kwargs={'course_id': course_id})
     )
-    image_url = u'https://{}{}'.format(settings.SITE_NAME, course_image_url(course))
+    image_url = u'{}{}'.format(base_url, course_image_url(course))
     email_context = {
         'course_title': course_title,
         'course_url': course_url,
         'course_image_url': image_url,
         'course_end_date': course_end_date,
-        'account_settings_url': 'https://{}{}'.format(settings.SITE_NAME, reverse('account_settings')),
-        'email_settings_url': 'https://{}{}'.format(settings.SITE_NAME, reverse('dashboard')),
+        'account_settings_url': '{}{}'.format(base_url, reverse('account_settings')),
+        'email_settings_url': '{}{}'.format(base_url, reverse('dashboard')),
         'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
     }
     return email_context
@@ -170,9 +176,12 @@ 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)
+    global_email_context = _get_course_email_context(course, is_secure)
 
     recipient_qsets = [
         target.get_users(course_id, user_id)

lms/djangoapps/bulk_email/tests/test_email.py

@@ -3,24 +3,25 @@
 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
 import os
 from unittest import skipIf
 
-from django.conf import settings
 from django.core import mail
 from django.core.mail.message import forbid_multi_line_headers
 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
-from bulk_email.tasks import _get_source_address
+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 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
@@ -476,3 +477,87 @@ class TestEmailSendFromDashboard(EmailSendFromDashboardTestCase):
 
         message_body = mail.outbox[0].body
         self.assertIn(uni_message, message_body)
+
+
+@ddt.ddt
+class TestCourseEmailContext(SharedModuleStoreTestCase):
+    """
+    Test the course email context hash used to send bulk emails.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        """
+        Create a course, InstructorTask, and CourseEmail shared by all tests.
+        """
+        super(TestCourseEmailContext, cls).setUpClass()
+        cls.course_title = u"Финансовое программирование и политика, часть 1: макроэкономические счета и анализ"
+        cls.course_org = 'IMF'
+        cls.course_number = "FPP.1x"
+        cls.course_run = "2016"
+        cls.course = CourseFactory.create(
+            display_name=cls.course_title,
+            org=cls.course_org,
+            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):
+        """
+        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'],
+                          '{}://edx.org/courses/{}/{}/{}/'.format(scheme,
+                                                                  self.course_org,
+                                                                  self.course_number,
+                                                                  self.course_run))
+        self.assertEquals(email_context['course_image_url'],
+                          '{}://edx.org/c4x/{}/{}/asset/images_course_image.jpg'.format(scheme,
+                                                                                        self.course_org,
+                                                                                        self.course_number))
+        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):
+        """
+        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.
+        """
+        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
+
+        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)

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}
+    task_input = {'email_id': email_id, 'to_option': targets, 'is_secure': request.is_secure()}
     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

 """
 Test for LMS instructor background task queue management
 """
-from mock import patch, Mock, MagicMock
+from mock import patch, Mock, MagicMock, ANY
 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
+from instructor_task.tasks import export_ora2_data, send_bulk_course_email
 from instructor_task.tests.test_base import (
     InstructorTaskTestCase,
     InstructorTaskCourseTestCase,
@@ -221,6 +221,23 @@ 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),