diff --git a/AUTHORS b/AUTHORS index 9ebfdbca689d..d451b1d16eb7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -180,4 +180,5 @@ Eugeny Kolpakov Omar Al-Ithawi Louis Pilfold Akiva Leffert -Mike Bifulco \ No newline at end of file +Mike Bifulco +Se Won Jang diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 5062450cf561..cf437ca3e763 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -6,10 +6,12 @@ import copy import mock from mock import patch +from smtplib import SMTPException from django.utils.timezone import UTC from django.test.utils import override_settings from django.conf import settings +from django.core.management import call_command from models.settings.course_details import (CourseDetails, CourseSettingsEncoder) from models.settings.course_grading import CourseGradingModel @@ -47,6 +49,7 @@ def test_virgin_fetch(self): self.assertIsNone(details.syllabus, "syllabus somehow initialized" + str(details.syllabus)) self.assertIsNone(details.intro_video, "intro_video somehow initialized" + str(details.intro_video)) self.assertIsNone(details.effort, "effort somehow initialized" + str(details.effort)) + self.assertFalse(details.enable_enrollment_email, "Enrollment Email should be initialized as false") def test_encoder(self): details = CourseDetails.fetch(self.course.id) @@ -166,6 +169,17 @@ def test_regular_site_fetch(self): self.assertContains(response, "Course Introduction Video") self.assertContains(response, "Requirements") + def test_send_test_enrollment_email(self): + call_command("loaddata", "course_email_template.json") + response = self.client.post(reverse_course_url('send_test_enrollment_email', self.course.id), {'subject': 'Test subject', 'message': 'Test body'}) + self.assertEquals(response.status_code, 200) + + def test_send_test_enrollment_email_failure(self): + call_command("loaddata", "course_email_template.json") + with patch('django.contrib.auth.models.User.email_user', side_effect=SMTPException): + response = self.client.post(reverse_course_url('send_test_enrollment_email', self.course.id), {'subject': 'Test subject', 'message': 'Test body'}) + self.assertContains(response, 'Error while sending test email.', status_code=400) + class CourseDetailsViewTest(CourseTestCase): """ diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 8698feb6d9c6..183e6f71cab9 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -86,6 +86,38 @@ def lms_link_test(self): link = utils.get_lms_link_for_item(location) self.assertEquals(link, "//localhost:8000/courses/mitX/101/test/jump_to/i4x://mitX/101/course/test") + def test_dashboard_link(self): + """ Tests get_lms_link_for_dashboard. """ + dashboard_link = utils.get_lms_link_for_dashboard() + self.assertEquals(dashboard_link, u"https://localhost:8000/dashboard") + + @override_settings(LMS_BASE=None) + def test_dashboard_link_no_lms_base(self): + """ Tests get_lms_link_for_dashboard with no LMS_BASE. """ + self.assertEquals(utils.get_lms_link_for_dashboard(), None) + + def test_login_link(self): + """ Tests get_lms_link_for_login. """ + login_link = utils.get_lms_link_for_login() + self.assertEquals(login_link, u"https://localhost:8000/login") + + @override_settings(LMS_BASE=None) + def test_login_link_no_lms_base(self): + """ Tests get_lms_link_for_login with no LMS_BASE. """ + self.assertEquals(utils.get_lms_link_for_login(), None) + + def test_course_link(self): + """ Tests get_lms_link_for_course. """ + course_key = SlashSeparatedCourseKey('mitX', '101', 'test') + course_link = utils.get_lms_link_for_course(course_key) + self.assertEquals(course_link, u"https://localhost:8000/courses/mitX/101/test/") + + @override_settings(LMS_BASE=None) + def test_course_link_no_lms_base(self): + """ Tests get_lms_link_for_course with no LMS_BASE. """ + course_key = SlashSeparatedCourseKey('mitX', '101', 'test') + self.assertEquals(utils.get_lms_link_for_course(course_key), None) + class ExtraPanelTabTestCase(TestCase): """ Tests adding and removing extra course tabs. """ diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 914ad1ec65d4..23cc086b0c61 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -146,6 +146,30 @@ def get_lms_link_for_about_page(course_key): ) +def get_lms_link_for_dashboard(): + """Returns the url to the lms dashboard.""" + if settings.LMS_BASE is None: + return None + return u"https://{lms_base}/dashboard".format(lms_base=settings.LMS_BASE) + + +def get_lms_link_for_login(): + """Returns the url to the lms login page.""" + if settings.LMS_BASE is None: + return None + return u"https://{lms_base}/login".format(lms_base=settings.LMS_BASE) + + +def get_lms_link_for_course(course_key): + """Returns the url to the lms course.""" + if settings.LMS_BASE is None: + return None + return u"https://{lms_base}/courses/{course_key}/".format( + lms_base=settings.LMS_BASE, + course_key=course_key + ) + + def course_image_url(course): """Returns the image url for the course.""" loc = StaticContent.compute_location(course.location.course_key, course.course_image) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ba14898e694b..bf87024afa88 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -13,6 +13,7 @@ from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.http import HttpResponseBadRequest, HttpResponseNotFound, HttpResponse, Http404 +from smtplib import SMTPException from util.json_request import JsonResponse, JsonResponseBadRequest from util.date_utils import get_default_time_display from edxmako.shortcuts import render_to_response @@ -35,6 +36,10 @@ add_instructor, initialize_permissions, get_lms_link_for_item, + get_lms_link_for_about_page, + get_lms_link_for_dashboard, + get_lms_link_for_course, + course_image_url, add_extra_panel_tab, remove_extra_panel_tab, reverse_course_url, @@ -58,7 +63,6 @@ from contentstore.tasks import rerun_course from .item import create_xblock_info from course_creators.views import get_course_creator_status, add_user_with_status_unrequested -from contentstore import utils from student.roles import ( CourseInstructorRole, CourseStaffRole, CourseCreatorRole, GlobalStaff, UserBasedRole ) @@ -67,6 +71,7 @@ from course_action_state.managers import CourseActionStateItemNotFoundError from microsite_configuration import microsite from xmodule.course_module import CourseFields +from bulk_email.models import CourseEmailTemplate __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler', @@ -76,7 +81,8 @@ 'advanced_settings_handler', 'course_notifications_handler', 'textbooks_list_handler', 'textbooks_detail_handler', - 'group_configurations_list_handler', 'group_configurations_detail_handler'] + 'group_configurations_list_handler', 'group_configurations_detail_handler', + 'send_test_enrollment_email'] log = logging.getLogger(__name__) @@ -730,6 +736,38 @@ def course_info_update_handler(request, course_key_string, provided_id=None): ) +@require_http_methods("POST") +def send_test_enrollment_email(request, course_key_string): + """ + Handles ajax request for sending test enrollment emails to the instructor + """ + course_key = CourseKey.from_string(course_key_string) + course = modulestore().get_course(course_key) + user = request.user + subject = request.POST.get('subject') + subject = ''.join(subject.splitlines()) + message = request.POST.get('message') + + template_name = microsite.get_value('course_email_template_name') + from_address = microsite.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + + template = CourseEmailTemplate.get_template(template_name) + email_context = { + 'course_title': course.display_name, + 'course_url': get_lms_link_for_course(course_key_string), + 'account_settings_url': get_lms_link_for_dashboard(), + 'platform_name': settings.PLATFORM_NAME, + 'email': user.email + } + message = template.render_plaintext(message, email_context) + + try: + user.email_user(subject, message, from_address) + except SMTPException: + return HttpResponseBadRequest(_("Error while sending test email.")) + return HttpResponse() + + @login_required @ensure_csrf_cookie @require_http_methods(("GET", "PUT", "POST")) @@ -762,12 +800,15 @@ def settings_handler(request, course_key_string): return render_to_response('settings.html', { 'context_course': course_module, 'course_locator': course_key, - 'lms_link_for_about_page': utils.get_lms_link_for_about_page(course_key), - 'course_image_url': utils.course_image_url(course_module), + 'lms_link_for_about_page': get_lms_link_for_about_page(course_key), + 'course_image_url': course_image_url(course_module), 'details_url': reverse_course_url('settings_handler', course_key), 'about_page_editable': about_page_editable, 'short_description_editable': short_description_editable, - 'upload_asset_url': upload_asset_url + 'upload_asset_url': upload_asset_url, + 'test_email_url': reverse_course_url('send_test_enrollment_email', course_key), + 'default_pre_template': CourseDetails.get_default_pre_enrollment_email(course_key), + 'default_post_template': CourseDetails.get_default_post_enrollment_email(), }) elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): if request.method == 'GET': diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 21f344f706b4..413092bdcbcf 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -4,12 +4,21 @@ import json from json.encoder import JSONEncoder +from django.conf import settings +from mako.template import Template +from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import Location from xmodule.modulestore.exceptions import ItemNotFoundError -from contentstore.utils import course_image_url +from contentstore.utils import ( + course_image_url, + get_lms_link_for_about_page, + get_lms_link_for_dashboard, + get_lms_link_for_login, +) from models.settings import course_grading from xmodule.fields import Date from xmodule.modulestore.django import modulestore +from edxmako.shortcuts import render_to_string # This list represents the attribute keys for a course's 'about' info. # Note: The 'video' attribute is intentionally excluded as it must be @@ -19,6 +28,10 @@ 'short_description', 'overview', 'effort', + 'pre_enrollment_email', + 'post_enrollment_email', + 'pre_enrollment_email_subject', + 'post_enrollment_email_subject', ] @@ -26,7 +39,7 @@ class CourseDetails(object): def __init__(self, org, course_id, run): # still need these for now b/c the client's screen shows these 3 fields self.org = org - self.course_id = course_id + self.course_id = course_id # This actually holds the course number. self.run = run self.start_date = None # 'start' self.end_date = None # 'end' @@ -35,10 +48,15 @@ def __init__(self, org, course_id, run): self.syllabus = None # a pdf file asset self.short_description = "" self.overview = "" # html to render as the overview + self.pre_enrollment_email = CourseDetails.get_default_pre_enrollment_email(CourseKey.from_string(u'/'.join([self.org, self.course_id, self.run]))) + self.post_enrollment_email = CourseDetails.get_default_post_enrollment_email() + self.pre_enrollment_email_subject = "Thanks for Enrolling in {}".format(self.course_id) + self.post_enrollment_email_subject = "Thanks for Enrolling in {}".format(self.course_id) self.intro_video = None # a video pointer self.effort = None # int hours/week self.course_image_name = "" self.course_image_asset_path = "" # URL of the course image + self.enable_enrollment_email = False @classmethod def _fetch_about_attribute(cls, course_key, attribute): @@ -66,6 +84,7 @@ def fetch(cls, course_key): course_details.enrollment_end = descriptor.enrollment_end course_details.course_image_name = descriptor.course_image course_details.course_image_asset_path = course_image_url(descriptor) + course_details.enable_enrollment_email = descriptor.enable_enrollment_email for attribute in ABOUT_ATTRIBUTES: value = cls._fetch_about_attribute(course_key, attribute) @@ -116,6 +135,11 @@ def update_from_json(cls, course_key, jsondict, user): # setter expects as input. date = Date() + # Added to allow admins to enable/disable enrollment emails + if 'enable_enrollment_email' in jsondict: + descriptor.enable_enrollment_email = jsondict['enable_enrollment_email'] + dirty = True + if 'start_date' in jsondict: converted = date.from_json(jsondict['start_date']) else: @@ -200,6 +224,34 @@ def recompose_video_tag(video_key): video_key + '?rel=0" frameborder="0" allowfullscreen="">' return result + @staticmethod + def get_default_pre_enrollment_email(course_key): + """ + Returns the rendered default email body on enrolling before course starts. + """ + enroll_email_dict = { + 'dashboard_url': get_lms_link_for_dashboard(), + 'about_url': u"https:{}".format(get_lms_link_for_about_page(course_key)), + } + if settings.DEFAULT_PRE_ENROLLMENT_EMAIL: + return Template(settings.DEFAULT_PRE_ENROLLMENT_EMAIL).render_unicode(**enroll_email_dict) + else: + return render_to_string('emails/default_pre_enrollment_message.txt', enroll_email_dict) + + @staticmethod + def get_default_post_enrollment_email(): + """ + Returns the rendered default email body on enrolling after course starts. + """ + enroll_email_dict = { + 'dashboard_url': get_lms_link_for_dashboard(), + 'signin_url': get_lms_link_for_login(), + } + if settings.DEFAULT_POST_ENROLLMENT_EMAIL: + return Template(settings.DEFAULT_POST_ENROLLMENT_EMAIL).render_unicode(**enroll_email_dict) + else: + return render_to_string('emails/default_post_enrollment_message.txt', enroll_email_dict) + # TODO move to a more general util? class CourseSettingsEncoder(json.JSONEncoder): diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 052ecfeb5700..cc6a6b8598fb 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -33,6 +33,7 @@ class CourseMetadata(object): 'tags', # from xblock 'visible_to_staff_only', 'group_access', + 'enable_enrollment_email', ] @classmethod diff --git a/lms/djangoapps/bulk_email/__init__.py b/cms/djangoapps/models/settings/tests/__init__.py similarity index 100% rename from lms/djangoapps/bulk_email/__init__.py rename to cms/djangoapps/models/settings/tests/__init__.py diff --git a/cms/djangoapps/models/settings/tests/test_enrollment_email_theming.py b/cms/djangoapps/models/settings/tests/test_enrollment_email_theming.py new file mode 100644 index 000000000000..a5f4c2d16b7e --- /dev/null +++ b/cms/djangoapps/models/settings/tests/test_enrollment_email_theming.py @@ -0,0 +1,30 @@ +""" +Test that theming works (for enrollment email template). +""" +from django.test import TestCase +from django.test.utils import override_settings + +from opaque_keys.edx.locations import SlashSeparatedCourseKey +from models.settings.course_details import CourseDetails + + +class EnrollmentEmailThemingTestCase(TestCase): + """ + Tests that theming for enrollment email works. + """ + @override_settings(DEFAULT_PRE_ENROLLMENT_EMAIL='This is a test pre enrollment email template.') + def test_pre_enrollment_email_theming(self): + """ + Test that settings override template is used for default email on enrolling before course start. + """ + course_key = SlashSeparatedCourseKey('mitX', '101', 'test') + template = CourseDetails.get_default_pre_enrollment_email(course_key) + self.assertIn(u'This is a test pre enrollment email template.', template) + + @override_settings(DEFAULT_POST_ENROLLMENT_EMAIL='This is a test post enrollment email template.') + def test_post_enrollment_email_theming(self): + """ + Test that settings override template is used for default email on enrolling after course start. + """ + template = CourseDetails.get_default_post_enrollment_email() + self.assertIn(u'This is a test post enrollment email template.', template) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 3530fa728312..f644f32061e7 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -190,6 +190,10 @@ if "TRACKING_IGNORE_URL_PATTERNS" in ENV_TOKENS: TRACKING_IGNORE_URL_PATTERNS = ENV_TOKENS.get("TRACKING_IGNORE_URL_PATTERNS") +# Enrollment email overrides +DEFAULT_PRE_ENROLLMENT_EMAIL = ENV_TOKENS.get("DEFAULT_PRE_ENROLLMENT_EMAIL", None) +DEFAULT_POST_ENROLLMENT_EMAIL = ENV_TOKENS.get("DEFAULT_POST_ENROLLMENT_EMAIL", None) + # Django CAS external authentication settings CAS_EXTRA_LOGIN_PARAMS = ENV_TOKENS.get("CAS_EXTRA_LOGIN_PARAMS", None) if FEATURES.get('AUTH_USE_CAS'): diff --git a/cms/envs/common.py b/cms/envs/common.py index 9243f3b5145c..40cf14053887 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -107,6 +107,9 @@ # Modulestore to use for new courses 'DEFAULT_STORE_FOR_NEW_COURSE': None, + + # Toggle option to send email confirmation of course enrollment + 'ENABLE_ENROLLMENT_EMAIL': False, } ENABLE_JASMINE = False @@ -528,6 +531,13 @@ DEFAULT_PRIORITY_QUEUE: {} } +############################# Enrollment Email ############################# +# Override for the default email template to use for users enrolling before +# and after a course starts. Leave empty to use templates provided in +# cms/templates/emails/ +# Set these values in your environment settings files, not here. +DEFAULT_PRE_ENROLLMENT_EMAIL = '' +DEFAULT_POST_ENROLLMENT_EMAIL = '' ############################## Video ########################################## @@ -620,6 +630,9 @@ # Additional problem types 'edx_jsme', # Molecular Structure + + # For email template footer + 'bulk_email' ) diff --git a/cms/envs/test.py b/cms/envs/test.py index 8ee1bcb62554..7650684e5341 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -224,3 +224,6 @@ # For consistency in user-experience, keep the value of this setting in sync with # the one in lms/envs/test.py FEATURES['ENABLE_DISCUSSION_SERVICE'] = False + +DEFAULT_PRE_ENROLLMENT_EMAIL = '' +DEFAULT_POST_ENROLLMENT_EMAIL = '' diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index 3957940b5dc2..7425a4d54887 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -5,17 +5,18 @@ var CourseDetails = Backbone.Model.extend({ org : '', course_id: '', run: '', - start_date: null, // maps to 'start' - end_date: null, // maps to 'end' + start_date: null, // maps to 'start' + end_date: null, // maps to 'end' enrollment_start: null, enrollment_end: null, syllabus: null, short_description: "", overview: "", intro_video: null, - effort: null, // an int or null, + effort: null, // an int or null, course_image_name: '', // the filename - course_image_asset_path: '' // the full URL (/c4x/org/course/num/asset/filename) + course_image_asset_path: '', // the full URL (/c4x/org/course/num/asset/filename) + enable_enrollment_email: false }, validate: function(newattrs) { @@ -43,6 +44,18 @@ var CourseDetails = Backbone.Model.extend({ } // TODO check if key points to a real video using google's youtube api } + if (newattrs.pre_enrollment_email_subject === "") { + errors.pre_enrollment_email_subject = gettext("Subject cannot be empty"); + } + if (newattrs.post_enrollment_email_subject === "") { + errors.post_enrollment_email_subject = gettext("Subject cannot be empty"); + } + if (newattrs.pre_enrollment_email === "") { + errors.pre_enrollment_email = gettext("Body cannot be empty"); + } + if (newattrs.post_enrollment_email === "") { + errors.post_enrollment_email = gettext("Body cannot be empty"); + } if (!_.isEmpty(errors)) return errors; // NOTE don't return empty errors as that will be interpreted as an error state }, diff --git a/cms/static/js/spec/views/settings/main_spec.js b/cms/static/js/spec/views/settings/main_spec.js index c3066f4fa614..0e13417fcad7 100644 --- a/cms/static/js/spec/views/settings/main_spec.js +++ b/cms/static/js/spec/views/settings/main_spec.js @@ -19,7 +19,8 @@ define([ intro_video : null, effort : null, course_image_name : '', - course_image_asset_path : '' + course_image_asset_path : '', + enable_enrollment_email: false }, mockSettingsPage = readFixtures('mock/mock-settings-page.underscore'); diff --git a/cms/static/js/views/settings/main.js b/cms/static/js/views/settings/main.js index 47f88d423470..fe1ff3756b3e 100644 --- a/cms/static/js/views/settings/main.js +++ b/cms/static/js/views/settings/main.js @@ -12,6 +12,13 @@ var DetailsView = ValidatingView.extend({ "change textarea" : "updateModel", 'click .remove-course-introduction-video' : "removeVideo", 'focus #course-overview' : "codeMirrorize", + 'click #enable-enrollment-email' : "toggleEnrollmentEmails", + 'focus #pre-enrollment-email' : "codeMirrorize", + 'focus #post-enrollment-email' : "codeMirrorize", + 'click #test_email_pre': "sendTestEmail", + 'click #test_email_post': "sendTestEmail", + 'click #fill_default_email_pre': "showDefaultTemplate", + 'click #fill_default_email_post': "showDefaultTemplate", 'mouseover .timezone' : "updateTime", // would love to move to a general superclass, but event hashes don't inherit in backbone :-( 'focus :input' : "inputFocus", @@ -38,6 +45,24 @@ var DetailsView = ValidatingView.extend({ this.listenTo(this.model, 'invalid', this.handleValidationError); this.listenTo(this.model, 'change', this.showNotificationBar); this.selectorToField = _.invert(this.fieldToSelectorMap); + + /* Memoize html elements for enrollment emails */ + this.enrollment_email_settings = this.$el.find('#enrollment-email-settings'); + + this.pre_enrollment_email_elem = this.$el.find('#' + this.fieldToSelectorMap['pre_enrollment_email']); + this.pre_enrollment_email_subject_elem = this.$el.find('#' + this.fieldToSelectorMap['pre_enrollment_email_subject']); + this.pre_enrollment_email_field = this.$el.find('#field-pre-enrollment-email'); + this.pre_enrollment_email_subject_field = this.$el.find('#field-pre-enrollment-email-subject'); + + this.post_enrollment_email_elem = this.$el.find('#' + this.fieldToSelectorMap['post_enrollment_email']); + this.post_enrollment_email_subject_elem = this.$el.find('#' + this.fieldToSelectorMap['post_enrollment_email_subject']); + this.post_enrollment_email_field = this.$el.find('#field-post-enrollment-email'); + this.post_enrollment_email_subject_field = this.$el.find('#field-post-enrollment-email-subject'); + + this.enable_enrollment_email_box = this.$el.find('#' + this.fieldToSelectorMap['enable_enrollment_email']); + + this.default_pre_template = this.$el.find('#default_pre_enrollment_email_template'); + this.default_post_template = this.$el.find('#default_post_enrollment_email_template'); }, render: function() { @@ -49,6 +74,23 @@ var DetailsView = ValidatingView.extend({ this.$el.find('#' + this.fieldToSelectorMap['overview']).val(this.model.get('overview')); this.codeMirrorize(null, $('#course-overview')[0]); + this.pre_enrollment_email_subject_elem.val(this.model.get('pre_enrollment_email_subject')); + this.post_enrollment_email_subject_elem.val(this.model.get('post_enrollment_email_subject')); + + this.pre_enrollment_email_elem.val(this.model.get('pre_enrollment_email')); + this.codeMirrorize(null, $('#pre-enrollment-email')[0]); + + this.post_enrollment_email_elem.val(this.model.get('post_enrollment_email')); + this.codeMirrorize(null, $('#post-enrollment-email')[0]); + + this.enable_enrollment_email_box.prop('checked', this.model.get('enable_enrollment_email')); + + if (this.enable_enrollment_email_box.prop('checked')) { + this.enrollment_email_settings.show(); + } else { + this.enrollment_email_settings.hide(); + } + this.$el.find('#' + this.fieldToSelectorMap['short_description']).val(this.model.get('short_description')); this.$el.find('.current-course-introduction-video iframe').attr('src', this.model.videosourceSample()); @@ -72,10 +114,15 @@ var DetailsView = ValidatingView.extend({ 'enrollment_start' : 'enrollment-start', 'enrollment_end' : 'enrollment-end', 'overview' : 'course-overview', + 'pre_enrollment_email' : 'pre-enrollment-email', + 'post_enrollment_email' : 'post-enrollment-email', 'short_description' : 'course-short-description', 'intro_video' : 'course-introduction-video', 'effort' : "course-effort", - 'course_image_asset_path': 'course-image-url' + 'course_image_asset_path': 'course-image-url', + 'enable_enrollment_email': 'enable-enrollment-email', + 'pre_enrollment_email_subject' :'pre-enrollment-email-subject', + 'post_enrollment_email_subject':'post-enrollment-email-subject', }, updateTime : function(e) { @@ -151,6 +198,12 @@ var DetailsView = ValidatingView.extend({ case 'course-effort': this.setField(event); break; + case 'pre-enrollment-email-subject': + this.setField(event); + break; + case 'post-enrollment-email-subject': + this.setField(event); + break; case 'course-short-description': this.setField(event); break; @@ -184,6 +237,23 @@ var DetailsView = ValidatingView.extend({ this.$el.find('.remove-course-introduction-video').hide(); } }, + + toggleEnrollmentEmails: function(event) { + var isChecked = this.enable_enrollment_email_box.prop('checked'); + + /* enable & disable default will show the template */ + if(isChecked) { + this.enrollment_email_settings.slideDown(); + } else { + this.enrollment_email_settings.slideUp(); + } + + var field = this.selectorToField['enable-enrollment-email']; + if (this.model.get(field) != isChecked) { + this.setAndValidate(field, isChecked); + } + }, + codeMirrors : {}, codeMirrorize: function (e, forcedTarget) { var thisTarget; @@ -224,13 +294,13 @@ var DetailsView = ValidatingView.extend({ var self = this; this.model.fetch({ success: function() { - self.render(); _.each(self.codeMirrors, function(mirror) { var ele = mirror.getTextArea(); var field = self.selectorToField[ele.id]; mirror.setValue(self.model.get(field)); }); + self.render(); }, reset: true, silent: true}); @@ -276,6 +346,55 @@ var DetailsView = ValidatingView.extend({ } }); modal.show(); + }, + + sendTestEmail: function (event) { + event.preventDefault(); + var email_type = event.target.id; + var subject = ""; + var message = ""; + if (email_type === "test_email_pre") { + subject = this.pre_enrollment_email_subject_elem.val(); + message = this.pre_enrollment_email_elem.val(); + } else { + subject = this.post_enrollment_email_subject_elem.val(); + message = this.post_enrollment_email_elem.val(); + } + + $.post($(event.target).data('endpoint'), + { + subject: subject, + message:message + }, + function (data) { + alert(gettext("Test email sent! Please check your inbox. Don't forget to save!")); + } + ); + }, + + showDefaultTemplate: function (event) { + event.preventDefault(); + + var content = ""; + var codeMirrorItem; + var oldContent = ""; + var target_id = event.target.id; + + if (target_id === "fill_default_email_pre") { + content = $('#default_pre_enrollment_email_template').text(); + codeMirrorItem = this.codeMirrors[this.pre_enrollment_email_elem[0].id]; + oldContent = codeMirrorItem.getValue(); + } else { + content = $('#default_post_enrollment_email_template').text(); + codeMirrorItem = this.codeMirrors[this.post_enrollment_email_elem[0].id]; + oldContent = codeMirrorItem.getValue(); + } + + if (oldContent.trim() !== "") { + var confirmed = confirm(gettext("This will overwrite the current message with the default one. Do you wish to continue?")); + if (!confirmed) return; + } + codeMirrorItem.setValue(content); } }); diff --git a/cms/static/js/views/validation.js b/cms/static/js/views/validation.js index bbca2e67a072..6669b95cde4c 100644 --- a/cms/static/js/views/validation.js +++ b/cms/static/js/views/validation.js @@ -76,11 +76,23 @@ var ValidatingView = BaseView.extend({ getInputElements: function(ele) { var inputElements = 'input, textarea'; if ($(ele).is(inputElements)) { + // put error on CodeMirror sibling of textarea if it exists + if ($(ele).is('textarea') && $(ele).next().hasClass('CodeMirror')) { + return $(ele).next(); + } return $(ele); } else { // put error on the contained inputs - return $(ele).find(inputElements); + var elements = $(ele).find(inputElements); + // put error on CodeMirror sibling of textareas if it exists + for (var i=0; i${_("Hi there!")} + +${_("Thank you for enrolling in a {platform_name} course. You are receiving this " +"email from {platform_name} as a confirmation that you have " +"successfully enrolled in one of our online courses. Your chosen course will " +"now appear on your dashboard ({dashboard_url}). This course is already under " +"way, so make sure to sign in ({signin_url}) and click \"View Course\" from your " +"dashboard in order to get started. Doing this will take you to the Course " +"Info page for more information.").format( + platform_name=settings.PLATFORM_NAME, + dashboard_url=dashboard_url, + signin_url=signin_url +)} + +${_("See you soon!")} diff --git a/cms/templates/emails/default_pre_enrollment_message.txt b/cms/templates/emails/default_pre_enrollment_message.txt new file mode 100644 index 000000000000..7f5c4c724c43 --- /dev/null +++ b/cms/templates/emails/default_pre_enrollment_message.txt @@ -0,0 +1,20 @@ +<%! from django.utils.translation import ugettext as _ %>${_("Hi there!")} + +${_("Thank you for enrolling in a {platform_name} course. You are receiving this " +"email from {platform_name} as a confirmation that you have " +"successfully enrolled in one of our online courses. Your chosen course will " +"now appear on your dashboard ({dashboard_url}). As it has not yet started, " +"you'll notice that it isn't yet possible to access the course site. Once the " +"course begins, the course team will send out a welcome email to everyone " +"enrolled with more information.").format( + platform_name=settings.PLATFORM_NAME, + dashboard_url=dashboard_url +)} + +${_("For the time being, you can return to your course's About page at {about_url} " +"for more information, such as when the course will start, and what topics will be " +"covered in the course.").format( + about_url=about_url +)} + +${_("See you soon!")} diff --git a/cms/templates/settings.html b/cms/templates/settings.html index ea12f2453a7c..d25a58eccc4e 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -203,7 +203,7 @@

${_("Introducing Your Course")}

    % if short_description_editable:
  1. - + ${_("Appears on the course catalog page when students roll over the course name. Limit to ~150 characters")}
    2. @@ -273,6 +273,65 @@

    ${_("Introducing Your Course")}

+ % if settings.FEATURES.get('ENABLE_ENROLLMENT_EMAIL'): +
+
+
+

${_("Course Enrollment Email")}

+ ${_("Settings for emails that students receive upon course enrollment")} +
+
+

By default, this feature is disabled. You can either use the default email content or create your own.
+ Students enrolling right on the start date will receive the post-course-start email.
+

+ + +
+
+
+

+ ${_("Message for students who enroll {strong_start}before the start date{strong_end}").format(strong_start="", strong_end="")} + ${_("Send me a test email")} + ${_("Reset to default content")} +

+
    +
  1. +

    + + +

    +

    + + + ${_("This email will be sent to any student who enrolls in the course before its start date.")} +

    +
    2. +
+
+
+

+ ${_("Message for students who enroll {strong_start}on or after the start date{strong_end}").format(strong_start="", strong_end="")} + ${_("Send me a test email")} + ${_("Reset to default content")} +

+
    +
  1. +

    + + +

    +

    + + + ${_("This email will be sent to any student who enrolls in the course after its start date.")} +

    +
    2. +
+
+
+
+ % endif + % if about_page_editable:
@@ -323,5 +382,7 @@

${_("Other Course Settings")}

+ + diff --git a/cms/urls.py b/cms/urls.py index 7e06f5033909..5db2216e94fd 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -89,6 +89,7 @@ url(r'^settings/details/{}$'.format(settings.COURSE_KEY_PATTERN), 'settings_handler'), url(r'^settings/grading/{}(/)?(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), 'grading_handler'), url(r'^settings/advanced/{}$'.format(settings.COURSE_KEY_PATTERN), 'advanced_settings_handler'), + url(r'^settings/send_test_enrollment_email/{}$'.format(settings.COURSE_KEY_PATTERN), 'send_test_enrollment_email', name='send_test_enrollment_email'), url(r'^textbooks/{}$'.format(settings.COURSE_KEY_PATTERN), 'textbooks_list_handler'), url(r'^textbooks/{}/(?P\d[^/]*)$'.format(settings.COURSE_KEY_PATTERN), 'textbooks_detail_handler'), url(r'^group_configurations/{}$'.format(settings.COURSE_KEY_PATTERN), 'group_configurations_list_handler'), diff --git a/lms/djangoapps/bulk_email/migrations/__init__.py b/common/djangoapps/bulk_email/__init__.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/__init__.py rename to common/djangoapps/bulk_email/__init__.py diff --git a/lms/djangoapps/bulk_email/fixtures/course_email_template.json b/common/djangoapps/bulk_email/fixtures/course_email_template.json similarity index 100% rename from lms/djangoapps/bulk_email/fixtures/course_email_template.json rename to common/djangoapps/bulk_email/fixtures/course_email_template.json diff --git a/lms/djangoapps/bulk_email/fixtures/plain-html-no-newlines-or-tabs.txt b/common/djangoapps/bulk_email/fixtures/plain-html-no-newlines-or-tabs.txt similarity index 100% rename from lms/djangoapps/bulk_email/fixtures/plain-html-no-newlines-or-tabs.txt rename to common/djangoapps/bulk_email/fixtures/plain-html-no-newlines-or-tabs.txt diff --git a/lms/djangoapps/bulk_email/fixtures/plain-html-no-newlines.txt b/common/djangoapps/bulk_email/fixtures/plain-html-no-newlines.txt similarity index 100% rename from lms/djangoapps/bulk_email/fixtures/plain-html-no-newlines.txt rename to common/djangoapps/bulk_email/fixtures/plain-html-no-newlines.txt diff --git a/lms/djangoapps/bulk_email/fixtures/plain-html.txt b/common/djangoapps/bulk_email/fixtures/plain-html.txt similarity index 100% rename from lms/djangoapps/bulk_email/fixtures/plain-html.txt rename to common/djangoapps/bulk_email/fixtures/plain-html.txt diff --git a/lms/djangoapps/bulk_email/migrations/0001_initial.py b/common/djangoapps/bulk_email/migrations/0001_initial.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/0001_initial.py rename to common/djangoapps/bulk_email/migrations/0001_initial.py diff --git a/lms/djangoapps/bulk_email/migrations/0002_change_field_names.py b/common/djangoapps/bulk_email/migrations/0002_change_field_names.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/0002_change_field_names.py rename to common/djangoapps/bulk_email/migrations/0002_change_field_names.py diff --git a/lms/djangoapps/bulk_email/migrations/0003_add_optout_user.py b/common/djangoapps/bulk_email/migrations/0003_add_optout_user.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/0003_add_optout_user.py rename to common/djangoapps/bulk_email/migrations/0003_add_optout_user.py diff --git a/lms/djangoapps/bulk_email/migrations/0004_migrate_optout_user.py b/common/djangoapps/bulk_email/migrations/0004_migrate_optout_user.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/0004_migrate_optout_user.py rename to common/djangoapps/bulk_email/migrations/0004_migrate_optout_user.py diff --git a/lms/djangoapps/bulk_email/migrations/0005_remove_optout_email.py b/common/djangoapps/bulk_email/migrations/0005_remove_optout_email.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/0005_remove_optout_email.py rename to common/djangoapps/bulk_email/migrations/0005_remove_optout_email.py diff --git a/lms/djangoapps/bulk_email/migrations/0006_add_course_email_template.py b/common/djangoapps/bulk_email/migrations/0006_add_course_email_template.py similarity index 99% rename from lms/djangoapps/bulk_email/migrations/0006_add_course_email_template.py rename to common/djangoapps/bulk_email/migrations/0006_add_course_email_template.py index 69ec3fe3b3ec..f12bb071e9f5 100644 --- a/lms/djangoapps/bulk_email/migrations/0006_add_course_email_template.py +++ b/common/djangoapps/bulk_email/migrations/0006_add_course_email_template.py @@ -19,7 +19,6 @@ def backwards(self, orm): # Deleting model 'CourseEmailTemplate' db.delete_table('bulk_email_courseemailtemplate') - models = { 'auth.group': { 'Meta': {'object_name': 'Group'}, diff --git a/lms/djangoapps/bulk_email/migrations/0007_load_course_email_template.py b/common/djangoapps/bulk_email/migrations/0007_load_course_email_template.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/0007_load_course_email_template.py rename to common/djangoapps/bulk_email/migrations/0007_load_course_email_template.py diff --git a/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py b/common/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py similarity index 99% rename from lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py rename to common/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py index a24e48d6e5b5..69b426a683b0 100644 --- a/lms/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py +++ b/common/djangoapps/bulk_email/migrations/0008_add_course_authorizations.py @@ -16,12 +16,10 @@ def forwards(self, orm): )) 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'}, diff --git a/lms/djangoapps/bulk_email/migrations/0009_force_unique_course_ids.py b/common/djangoapps/bulk_email/migrations/0009_force_unique_course_ids.py similarity index 99% rename from lms/djangoapps/bulk_email/migrations/0009_force_unique_course_ids.py rename to common/djangoapps/bulk_email/migrations/0009_force_unique_course_ids.py index d4a329b7277f..57c358fc6ccb 100644 --- a/lms/djangoapps/bulk_email/migrations/0009_force_unique_course_ids.py +++ b/common/djangoapps/bulk_email/migrations/0009_force_unique_course_ids.py @@ -11,12 +11,10 @@ def forwards(self, orm): # Adding unique constraint on 'CourseAuthorization', fields ['course_id'] db.create_unique('bulk_email_courseauthorization', ['course_id']) - def backwards(self, orm): # Removing unique constraint on 'CourseAuthorization', fields ['course_id'] db.delete_unique('bulk_email_courseauthorization', ['course_id']) - models = { 'auth.group': { 'Meta': {'object_name': 'Group'}, diff --git a/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py b/common/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py similarity index 100% rename from lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py rename to common/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py diff --git a/lms/djangoapps/bulk_email/tests/__init__.py b/common/djangoapps/bulk_email/migrations/__init__.py similarity index 100% rename from lms/djangoapps/bulk_email/tests/__init__.py rename to common/djangoapps/bulk_email/migrations/__init__.py diff --git a/lms/djangoapps/bulk_email/models.py b/common/djangoapps/bulk_email/models.py similarity index 99% rename from lms/djangoapps/bulk_email/models.py rename to common/djangoapps/bulk_email/models.py index 045edd99bb62..db7534b54dcd 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/common/djangoapps/bulk_email/models.py @@ -8,7 +8,7 @@ 1. Go to the edx-platform dir 2. ./manage.py lms schemamigration bulk_email --auto description_of_your_change -3. Add the migration file created in edx-platform/lms/djangoapps/bulk_email/migrations/ +3. Add the migration file created in edx-platform/common/djangoapps/bulk_email/migrations/ """ import logging diff --git a/common/djangoapps/bulk_email/tasks.py b/common/djangoapps/bulk_email/tasks.py new file mode 100644 index 000000000000..80381292c030 --- /dev/null +++ b/common/djangoapps/bulk_email/tasks.py @@ -0,0 +1 @@ +"""This file is intentionally blank. Please use lms/djangoapps/bulk_email_lms/tasks.py""" diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index b99126c49438..2f0725e5e048 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -1,13 +1,17 @@ """ Tests for student enrollment. """ +from datetime import datetime import ddt +import pytz import unittest from mock import patch from django.test.utils import override_settings from django.conf import settings +from django.core.management import call_command from django.core.urlresolvers import reverse +from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, mixed_store_config ) @@ -94,6 +98,63 @@ def test_enroll(self, course_modes, next_url, enrollment_mode): self.assertTrue(is_active) self.assertEqual(course_mode, enrollment_mode) + def _create_about_item(self, about_key): + """Create specified about item. Uses key as the data.""" + store = modulestore() + about_item = store.create_xblock(self.course.runtime, self.course.id, 'about', about_key, {'data': about_key}) + store.update_item(about_item, self.user.id, allow_not_found=True) + + def assertEnrollmentEmail(self, expected_subject, expected_msg): + """Assert that enrollment email was sent with expected subject and msg.""" + self.course.enable_enrollment_email = True + self.course = self.update_course(self.course, self.user.id) + call_command('loaddata', 'course_email_template.json') + + with patch('django.contrib.auth.models.User.email_user') as mock_email_user: + resp = self._change_enrollment('enroll') + self.assertEqual(resp.status_code, 200) + (subject, msg, from_addr) = mock_email_user.call_args[0] + self.assertEquals(subject, expected_subject) + self.assertIn(expected_msg, msg) + self.assertEquals(from_addr, settings.DEFAULT_FROM_EMAIL) + + @patch.dict(settings.FEATURES, {'AUTOMATIC_AUTH_FOR_TESTING': False}) + def test_pre_enrollment_email(self): + """ + Test sending automated emails to users upon course enrollment before it starts. + """ + self._create_about_item('pre_enrollment_email_subject') + self._create_about_item('pre_enrollment_email') + + self.assertEnrollmentEmail('pre_enrollment_email_subject', 'pre_enrollment_email') + + @patch.dict(settings.FEATURES, {'AUTOMATIC_AUTH_FOR_TESTING': False}) + def test_post_enrollment_email(self): + """ + Test sending automated emails to users upon course enrollment after it starts. + """ + self.course.start = datetime.now(pytz.UTC) + self._create_about_item('post_enrollment_email_subject') + self._create_about_item('post_enrollment_email') + + self.assertEnrollmentEmail('post_enrollment_email_subject', 'post_enrollment_email') + + @patch.dict(settings.FEATURES, {'AUTOMATIC_AUTH_FOR_TESTING': False}) + @patch('student.views.log.error') + def test_enrollment_email_failure(self, error_log): + """ + Test that enrollment email failure logs an error + """ + self.course.enable_enrollment_email = True + self.course = self.update_course(self.course, self.user.id) + call_command('loaddata', 'course_email_template.json') + + with patch('django.contrib.auth.models.User.email_user', side_effect=Exception): + resp = self._change_enrollment('enroll') + self.assertEqual(resp.status_code, 200) + error_log.assert_called_with('Unable to send course enrollment verification email to user from "{from_address}"'.format( + from_address=settings.DEFAULT_FROM_EMAIL), exc_info=True) + def test_unenroll(self): # Enroll the student in the course CourseEnrollment.enroll(self.user, self.course.id, mode="honor") diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 898d2cc0b9b1..d11ca27354dd 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -593,7 +593,6 @@ def setUp(self): self.course = CourseFactory.create() self.user = UserFactory.create(password='secret') self.client.login(username=self.user.username, password='secret') - self.url = reverse('change_enrollment') def _enroll_through_view(self, course): """ Enroll a student in a course. """ diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index b007e15a31f4..f666e06c4d46 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -68,7 +68,7 @@ from collections import namedtuple -from courseware.courses import get_courses, sort_by_announcement +from courseware.courses import get_courses, sort_by_announcement, get_course_about_section # pylint: disable=import-error from courseware.access import has_access from django_comment_common.models import Role @@ -76,7 +76,7 @@ from external_auth.models import ExternalAuthMap import external_auth.views -from bulk_email.models import Optout, CourseAuthorization +from bulk_email.models import Optout, CourseEmailTemplate, CourseAuthorization import shoppingcart from shoppingcart.models import DonationConfiguration from openedx.core.djangoapps.user_api.models import UserPreference @@ -869,6 +869,11 @@ def change_enrollment(request, check_access=True): except Exception: return HttpResponseBadRequest(_("Could not enroll")) + # notify the user of the enrollment via email + course = modulestore().get_course(course_id) + if (course.enable_enrollment_email and not (settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING'))): + notify_enrollment_by_email(course, user) + # If we have more than one course mode or professional ed is enabled, # then send the user to the choose your track page. # (In the case of professional ed, this will redirect to a page that @@ -903,6 +908,45 @@ def change_enrollment(request, check_access=True): return HttpResponseBadRequest(_("Enrollment action is invalid")) +def notify_enrollment_by_email(course, user): + """ + Updates the user about the course enrollment by email. + + If the Course has already started, use post_enrollment_email + If the Course has not yet started, use pre_enrollment_email + """ + template_name = microsite.get_value('course_email_template_name') + template = CourseEmailTemplate.get_template(template_name) + from_address = microsite.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) + + try: + # Check if the course has already started and set subject & message accordingly + if course.has_started(): + subject = get_course_about_section(course, 'post_enrollment_email_subject') + message = get_course_about_section(course, 'post_enrollment_email') + else: + subject = get_course_about_section(course, 'pre_enrollment_email_subject') + message = get_course_about_section(course, 'pre_enrollment_email') + + subject = ''.join(subject.splitlines()) + email_context = { + 'course_title': course.display_name, + 'course_url': 'https://{}{}'.format( + settings.SITE_NAME, + reverse('course_root', kwargs={'course_id': course.id.to_deprecated_string()}) + ), + 'account_settings_url': 'https://{}{}'.format(settings.SITE_NAME, reverse('dashboard')), + 'platform_name': settings.PLATFORM_NAME, + 'email': user.email + } + message = template.render_plaintext(message, email_context) + user.email_user(subject, message, from_address) + + except Exception: # pylint: disable=broad-except + log.error('Unable to send course enrollment verification email to user from "{from_address}"'.format( + from_address=from_address), exc_info=True) + + # pylint: disable=fixme # TODO: This function is kind of gnarly/hackish/etc and is only used in one location. # It'd be awesome if we could get rid of it; manually parsing course_id strings form larger strings diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 48b3b8922723..492043200ee2 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -173,6 +173,7 @@ class CourseFields(object): default=[], scope=Scope.content) wiki_slug = String(help="Slug that points to the wiki for this course", scope=Scope.content) + enable_enrollment_email = Boolean(help="Whether to send notification email upon enrollment or not", default=False, scope=Scope.settings) enrollment_start = Date(help="Date that enrollment for this class is opened", scope=Scope.settings) enrollment_end = Date(help="Date that enrollment for this class is closed", scope=Scope.settings) start = Date(help="Start time when this module is visible", diff --git a/lms/djangoapps/bulk_email_lms/__init__.py b/lms/djangoapps/bulk_email_lms/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/bulk_email/admin.py b/lms/djangoapps/bulk_email_lms/admin.py similarity index 97% rename from lms/djangoapps/bulk_email/admin.py rename to lms/djangoapps/bulk_email_lms/admin.py index a63c7b029590..68c544db5cd7 100644 --- a/lms/djangoapps/bulk_email/admin.py +++ b/lms/djangoapps/bulk_email_lms/admin.py @@ -4,7 +4,7 @@ from django.contrib import admin from bulk_email.models import CourseEmail, Optout, CourseEmailTemplate, CourseAuthorization -from bulk_email.forms import CourseEmailTemplateForm, CourseAuthorizationAdminForm +from bulk_email_lms.forms import CourseEmailTemplateForm, CourseAuthorizationAdminForm class CourseEmailAdmin(admin.ModelAdmin): diff --git a/lms/djangoapps/bulk_email/forms.py b/lms/djangoapps/bulk_email_lms/forms.py similarity index 99% rename from lms/djangoapps/bulk_email/forms.py rename to lms/djangoapps/bulk_email_lms/forms.py index 49fb840560e8..477817f04f06 100644 --- a/lms/djangoapps/bulk_email/forms.py +++ b/lms/djangoapps/bulk_email_lms/forms.py @@ -37,6 +37,7 @@ def _validate_template(self, template): msg = 'Multiple instances of tag: "{}"'.format(COURSE_EMAIL_MESSAGE_BODY_TAG) log.warning(msg) raise ValidationError(msg) + # pylint: disable=fixme # TODO: add more validation here, including the set of known tags # for which values will be supplied. (Email will fail if the template # uses tags for which values are not supplied.) diff --git a/lms/djangoapps/bulk_email_lms/models.py b/lms/djangoapps/bulk_email_lms/models.py new file mode 100644 index 000000000000..224e5c1c4715 --- /dev/null +++ b/lms/djangoapps/bulk_email_lms/models.py @@ -0,0 +1 @@ +"""This file is intentionally blank. It has been moved to common/djangoapps/bulk_email""" diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email_lms/tasks.py similarity index 99% rename from lms/djangoapps/bulk_email/tasks.py rename to lms/djangoapps/bulk_email_lms/tasks.py index c3b17339c34c..04661a510dfc 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email_lms/tasks.py @@ -33,7 +33,7 @@ from django.core.urlresolvers import reverse from bulk_email.models import ( - CourseEmail, Optout, CourseEmailTemplate, + CourseEmail, Optout, SEND_TO_MYSELF, SEND_TO_ALL, TO_OPTIONS, ) from courseware.courses import get_course, course_image_url @@ -91,7 +91,7 @@ ) -def _get_recipient_queryset(user_id, to_option, course_id, course_location): +def _get_recipient_queryset(user_id, to_option, course_id): """ Returns a query set of email recipients corresponding to the requested to_option category. @@ -229,7 +229,7 @@ def _create_send_email_subtask(to_list, initial_subtask_status): ) return new_subtask - recipient_qset = _get_recipient_queryset(user_id, to_option, course_id, course.location) + recipient_qset = _get_recipient_queryset(user_id, to_option, course_id) recipient_fields = ['profile__name', 'email'] log.info(u"Task %s: Preparing to queue subtasks for sending emails for course %s, email %s, to_option %s", @@ -389,6 +389,7 @@ def _get_source_address(course_id, course_title): return from_addr +# pylint: disable=too-many-statements def _send_course_email(entry_id, email_id, to_list, global_email_context, subtask_status): """ Performs the email sending task. diff --git a/lms/djangoapps/bulk_email_lms/tests/__init__.py b/lms/djangoapps/bulk_email_lms/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/bulk_email/tests/test_course_optout.py b/lms/djangoapps/bulk_email_lms/tests/test_course_optout.py similarity index 100% rename from lms/djangoapps/bulk_email/tests/test_course_optout.py rename to lms/djangoapps/bulk_email_lms/tests/test_course_optout.py diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email_lms/tests/test_email.py similarity index 99% rename from lms/djangoapps/bulk_email/tests/test_email.py rename to lms/djangoapps/bulk_email_lms/tests/test_email.py index dad65d6e66b9..7ccfb46f112a 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email_lms/tests/test_email.py @@ -277,7 +277,7 @@ def test_unicode_students_send_to_all(self): ) @override_settings(BULK_EMAIL_EMAILS_PER_TASK=3) - @patch('bulk_email.tasks.update_subtask_status') + @patch('bulk_email_lms.tasks.update_subtask_status') def test_chunked_queries_send_numerous_emails(self, email_mock): """ Test sending a large number of emails, to test the chunked querying diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email_lms/tests/test_err_handling.py similarity index 95% rename from lms/djangoapps/bulk_email/tests/test_err_handling.py rename to lms/djangoapps/bulk_email_lms/tests/test_err_handling.py index fc889f36aa5a..31b8d713456e 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email_lms/tests/test_err_handling.py @@ -15,7 +15,7 @@ from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError from bulk_email.models import CourseEmail, SEND_TO_ALL -from bulk_email.tasks import perform_delegate_email_batches, send_course_email +from bulk_email_lms.tasks import perform_delegate_email_batches, send_course_email from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from instructor_task.models import InstructorTask from instructor_task.subtasks import ( @@ -62,8 +62,8 @@ def setUp(self): def tearDown(self): patch.stopall() - @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.send_course_email.retry') + @patch('bulk_email_lms.tasks.get_connection', autospec=True) + @patch('bulk_email_lms.tasks.send_course_email.retry') def test_data_err_retry(self, retry, get_conn): """ Test that celery handles transient SMTPDataErrors by retrying. @@ -84,9 +84,9 @@ def test_data_err_retry(self, retry, get_conn): exc = kwargs['exc'] self.assertIsInstance(exc, SMTPDataError) - @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.update_subtask_status') - @patch('bulk_email.tasks.send_course_email.retry') + @patch('bulk_email_lms.tasks.get_connection', autospec=True) + @patch('bulk_email_lms.tasks.update_subtask_status') + @patch('bulk_email_lms.tasks.send_course_email.retry') def test_data_err_fail(self, retry, result, get_conn): """ Test that celery handles permanent SMTPDataErrors by failing and not retrying. @@ -116,8 +116,8 @@ def test_data_err_fail(self, retry, result, get_conn): self.assertEquals(subtask_status.failed, expected_fails) self.assertEquals(subtask_status.succeeded, settings.BULK_EMAIL_EMAILS_PER_TASK - expected_fails) - @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.send_course_email.retry') + @patch('bulk_email_lms.tasks.get_connection', autospec=True) + @patch('bulk_email_lms.tasks.send_course_email.retry') def test_disconn_err_retry(self, retry, get_conn): """ Test that celery handles SMTPServerDisconnected by retrying. @@ -137,8 +137,8 @@ def test_disconn_err_retry(self, retry, get_conn): exc = kwargs['exc'] self.assertIsInstance(exc, SMTPServerDisconnected) - @patch('bulk_email.tasks.get_connection', autospec=True) - @patch('bulk_email.tasks.send_course_email.retry') + @patch('bulk_email_lms.tasks.get_connection', autospec=True) + @patch('bulk_email_lms.tasks.send_course_email.retry') def test_conn_err_retry(self, retry, get_conn): """ Test that celery handles SMTPConnectError by retrying. @@ -159,8 +159,8 @@ def test_conn_err_retry(self, retry, get_conn): exc = kwargs['exc'] self.assertIsInstance(exc, SMTPConnectError) - @patch('bulk_email.tasks.SubtaskStatus.increment') - @patch('bulk_email.tasks.log') + @patch('bulk_email_lms.tasks.SubtaskStatus.increment') + @patch('bulk_email_lms.tasks.log') def test_nonexistent_email(self, mock_log, result): """ Tests retries when the email doesn't exist @@ -328,5 +328,5 @@ def test_send_email_undefined_email(self): with self.assertRaises(CourseEmail.DoesNotExist): # we skip the call that updates subtask status, since we've not set up the InstructorTask # for the subtask, and it's not important to the test. - with patch('bulk_email.tasks.update_subtask_status'): + with patch('bulk_email_lms.tasks.update_subtask_status'): send_course_email(entry_id, bogus_email_id, to_list, global_email_context, subtask_status.to_dict()) diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email_lms/tests/test_forms.py similarity index 98% rename from lms/djangoapps/bulk_email/tests/test_forms.py rename to lms/djangoapps/bulk_email_lms/tests/test_forms.py index 558758f58459..5936a816a922 100644 --- a/lms/djangoapps/bulk_email/tests/test_forms.py +++ b/lms/djangoapps/bulk_email_lms/tests/test_forms.py @@ -7,7 +7,7 @@ from mock import patch from bulk_email.models import CourseAuthorization, CourseEmailTemplate -from bulk_email.forms import CourseAuthorizationAdminForm, CourseEmailTemplateForm +from bulk_email_lms.forms import CourseAuthorizationAdminForm, CourseEmailTemplateForm from xmodule.modulestore.tests.django_utils import ( TEST_DATA_MOCK_MODULESTORE, TEST_DATA_MIXED_TOY_MODULESTORE ) @@ -114,7 +114,7 @@ def test_course_name_only(self): # Validation shouldn't work self.assertFalse(form.is_valid()) - error_msg = form._errors['course_id'][0] + error_msg = form._errors['course_id'][0] # pylint: disable=protected-access self.assertIn(u'--- Entered course id was: "{0}". '.format(self.course.id.run), error_msg) self.assertIn(u'Please recheck that you have supplied a valid course id.', error_msg) diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email_lms/tests/test_models.py similarity index 100% rename from lms/djangoapps/bulk_email/tests/test_models.py rename to lms/djangoapps/bulk_email_lms/tests/test_models.py diff --git a/lms/djangoapps/bulk_email/tests/test_tasks.py b/lms/djangoapps/bulk_email_lms/tests/test_tasks.py similarity index 94% rename from lms/djangoapps/bulk_email/tests/test_tasks.py rename to lms/djangoapps/bulk_email_lms/tests/test_tasks.py index d2b00e03270e..e4c15d5992d5 100644 --- a/lms/djangoapps/bulk_email/tests/test_tasks.py +++ b/lms/djangoapps/bulk_email_lms/tests/test_tasks.py @@ -109,7 +109,7 @@ def _run_task_with_mock_celery(self, task_class, entry_id, task_id): mock_current_task.default_retry_delay = settings.BULK_EMAIL_DEFAULT_RETRY_DELAY task_args = [entry_id, {}] - with patch('bulk_email.tasks._get_current_task') as mock_get_task: + with patch('bulk_email_lms.tasks._get_current_task') as mock_get_task: mock_get_task.return_value = mock_current_task return task_class.apply(task_args, task_id=task_id).get() @@ -133,7 +133,7 @@ def dummy_update_subtask_status(entry_id, _current_task_id, new_subtask_status): update_subtask_status(entry_id, bogus_task_id, new_subtask_status) with self.assertRaises(ValueError): - with patch('bulk_email.tasks.update_subtask_status', dummy_update_subtask_status): + with patch('bulk_email_lms.tasks.update_subtask_status', dummy_update_subtask_status): send_bulk_course_email(task_entry.id, {}) # pylint: disable=no-member def _create_students(self, num_students): @@ -191,7 +191,7 @@ def test_successful(self): num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK # We also send email to the instructor: self._create_students(num_emails - 1) - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: get_conn.return_value.send_messages.side_effect = cycle([None]) self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, num_emails) @@ -200,12 +200,12 @@ def test_successful_twice(self): num_emails = settings.BULK_EMAIL_EMAILS_PER_TASK # We also send email to the instructor: self._create_students(num_emails - 1) - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: get_conn.return_value.send_messages.side_effect = cycle([None]) task_entry = self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, num_emails) # submit the same task a second time, and confirm that it is not run again. - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: get_conn.return_value.send_messages.side_effect = cycle([Exception("This should not happen!")]) parent_status = self._run_task_with_mock_celery(send_bulk_course_email, task_entry.id, task_entry.task_id) self.assertEquals(parent_status.get('total'), num_emails) @@ -221,7 +221,7 @@ def test_unactivated_user(self): student = students[0] student.is_active = False student.save() - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: get_conn.return_value.send_messages.side_effect = cycle([None]) self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails - 1, num_emails - 1) @@ -236,7 +236,7 @@ def test_skipped(self): for index in range(0, num_emails, 4): Optout.objects.create(user=students[index], course_id=self.course.id) # mark some students as opting out - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: get_conn.return_value.send_messages.side_effect = cycle([None]) self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, skipped=expected_skipped) @@ -248,7 +248,7 @@ def _test_email_address_failures(self, exception): self._create_students(num_emails - 1) expected_fails = int((num_emails + 3) / 4.0) expected_succeeds = num_emails - expected_fails - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: # have every fourth email fail due to some address failure: get_conn.return_value.send_messages.side_effect = cycle([exception, None, None, None]) self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, failed=expected_fails) @@ -282,7 +282,7 @@ def _test_retry_after_limited_retry_error(self, exception): self._create_students(num_emails - 1) expected_fails = 0 expected_succeeds = num_emails - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: # Have every other mail attempt fail due to disconnection. get_conn.return_value.send_messages.side_effect = cycle([exception, None]) self._test_run_with_task( @@ -303,10 +303,10 @@ def _test_max_retry_limit_causes_failure(self, exception): self._create_students(num_emails - 1) expected_fails = num_emails expected_succeeds = 0 - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: # always fail to connect, triggering repeated retries until limit is hit: get_conn.return_value.send_messages.side_effect = cycle([exception]) - with patch('bulk_email.tasks.update_subtask_status', my_update_subtask_status): + with patch('bulk_email_lms.tasks.update_subtask_status', my_update_subtask_status): self._test_run_with_task( send_bulk_course_email, 'emailed', @@ -353,7 +353,7 @@ def _test_retry_after_unlimited_retry_error(self, exception): # exceeded"). The maximum recursion depth is 90, so # num_emails * expected_retries < 90. expected_retries = 10 - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: # Cycle through N throttling errors followed by a success. get_conn.return_value.send_messages.side_effect = cycle( chain(repeat(exception, expected_retries), [None]) @@ -382,7 +382,7 @@ def _test_immediate_failure(self, exception): self._create_students(num_emails - 1) expected_fails = num_emails expected_succeeds = 0 - with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: + with patch('bulk_email_lms.tasks.get_connection', autospec=True) as get_conn: # always fail to connect, triggering repeated retries until limit is hit: get_conn.return_value.send_messages.side_effect = cycle([exception]) self._test_run_with_task( diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 616040d13e5f..340ff109b258 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -175,6 +175,10 @@ def get_course_about_section(course, section_key): - faq - more_info - ocw_links + - pre_enrollment_email + - post_enrollment_email + - pre_enrollment_email_subject + - post_enrollment_email_subject """ # Many of these are stored as html files instead of some semantic @@ -186,7 +190,9 @@ def get_course_about_section(course, section_key): 'course_staff_short', 'course_staff_extended', 'requirements', 'syllabus', 'textbook', 'faq', 'more_info', 'number', 'instructors', 'overview', - 'effort', 'end_date', 'prerequisites', 'ocw_links']: + 'effort', 'end_date', 'prerequisites', 'ocw_links', + 'pre_enrollment_email', 'post_enrollment_email', + 'pre_enrollment_email_subject', 'post_enrollment_email_subject']: try: @@ -209,6 +215,8 @@ def get_course_about_section(course, section_key): html = '' if about_module is not None: + if section_key in ['pre_enrollment_email', 'post_enrollment_email', 'pre_enrollment_email_subject', 'post_enrollment_email_subject']: + return about_module.data try: html = about_module.render(STUDENT_VIEW).content except Exception: # pylint: disable=broad-except diff --git a/lms/djangoapps/django_comment_client/models.py b/lms/djangoapps/django_comment_client/models.py index 76d27be3bf18..1047fc076161 100644 --- a/lms/djangoapps/django_comment_client/models.py +++ b/lms/djangoapps/django_comment_client/models.py @@ -1 +1 @@ -# This file is intentionally blank. It has been moved to common/djangoapps/django_comment_common +"""This file is intentionally blank. It has been moved to common/djangoapps/django_comment_common""" diff --git a/lms/djangoapps/instructor_task/tasks.py b/lms/djangoapps/instructor_task/tasks.py index 518d71da0a9d..cfaf7a3c7295 100644 --- a/lms/djangoapps/instructor_task/tasks.py +++ b/lms/djangoapps/instructor_task/tasks.py @@ -33,7 +33,7 @@ upload_grades_csv, upload_students_csv ) -from bulk_email.tasks import perform_delegate_email_batches +from bulk_email_lms.tasks import perform_delegate_email_batches @task(base=BaseInstructorTask) # pylint: disable=not-callable diff --git a/lms/djangoapps/linkedin/management/commands/linkedin_mailusers.py b/lms/djangoapps/linkedin/management/commands/linkedin_mailusers.py index 6e88abc64988..bcf8f009a29d 100644 --- a/lms/djangoapps/linkedin/management/commands/linkedin_mailusers.py +++ b/lms/djangoapps/linkedin/management/commands/linkedin_mailusers.py @@ -36,7 +36,7 @@ from ...models import LinkedIn -# The following is blatantly cribbed from bulk_email/tasks.py +# The following is blatantly cribbed from bulk_email_lms/tasks.py # Errors that an individual email is failing to be sent, and should just # be treated as a fail. diff --git a/lms/djangoapps/shoppingcart/exceptions.py b/lms/djangoapps/shoppingcart/exceptions.py index a5fa8492db99..714cc0323ce8 100644 --- a/lms/djangoapps/shoppingcart/exceptions.py +++ b/lms/djangoapps/shoppingcart/exceptions.py @@ -55,3 +55,11 @@ class ReportException(Exception): class ReportTypeDoesNotExistException(ReportException): pass + + +class InvalidStatusToRetire(Exception): + pass + + +class UnexpectedOrderItemStatus(Exception): + pass diff --git a/lms/djangoapps/shoppingcart/management/__init__.py b/lms/djangoapps/shoppingcart/management/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/shoppingcart/management/commands/__init__.py b/lms/djangoapps/shoppingcart/management/commands/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/shoppingcart/management/commands/retire_order.py b/lms/djangoapps/shoppingcart/management/commands/retire_order.py new file mode 100644 index 000000000000..40932e0591c1 --- /dev/null +++ b/lms/djangoapps/shoppingcart/management/commands/retire_order.py @@ -0,0 +1,44 @@ +""" +Script for retiring order that went through cybersource but weren't +marked as "purchased" in the db +""" + +from django.core.management.base import BaseCommand, CommandError +from shoppingcart.models import Order +from shoppingcart.exceptions import UnexpectedOrderItemStatus, InvalidStatusToRetire + + +class Command(BaseCommand): + """ + Retire orders that went through cybersource but weren't updated + appropriately in the db + """ + help = """ + Retire orders that went through cybersource but weren't updated appropriately in the db. + Takes a file of orders to be retired, one order per line + """ + + def handle(self, *args, **options): + "Execute the command" + if len(args) != 1: + raise CommandError("retire_order requires one argument: ") + + with open(args[0]) as orders_file: + order_ids = [int(line.strip()) for line in orders_file.readlines()] + + orders = Order.objects.filter(id__in=order_ids) + + for order in orders: + old_status = order.status + try: + order.retire() + except (UnexpectedOrderItemStatus, InvalidStatusToRetire) as err: + print "Did not retire order {order}: {message}".format( + order=order.id, message=err.message + ) + else: + print "retired order {order_id} from status {old_status} to status {new_status}".format( + order_id=order.id, + old_status=old_status, + new_status=order.status, + ) diff --git a/lms/djangoapps/shoppingcart/management/tests/__init__.py b/lms/djangoapps/shoppingcart/management/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/shoppingcart/management/tests/test_retire_order.py b/lms/djangoapps/shoppingcart/management/tests/test_retire_order.py new file mode 100644 index 000000000000..40ccdd53fb17 --- /dev/null +++ b/lms/djangoapps/shoppingcart/management/tests/test_retire_order.py @@ -0,0 +1,76 @@ +"""Tests for the retire_order command""" + +from tempfile import NamedTemporaryFile +from django.core.management import call_command + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from shoppingcart.models import Order, CertificateItem +from student.tests.factories import UserFactory + + +class TestRetireOrder(ModuleStoreTestCase): + """Test the retire_order command""" + def setUp(self): + course = CourseFactory.create() + self.course_key = course.id + + # set up test carts + self.cart, __ = self._create_cart() + + self.paying, __ = self._create_cart() + self.paying.start_purchase() + + self.already_defunct_cart, __ = self._create_cart() + self.already_defunct_cart.retire() + + self.purchased, self.purchased_item = self._create_cart() + self.purchased.status = "purchased" + self.purchased.save() + self.purchased_item.status = "purchased" + self.purchased.save() + + def test_retire_order(self): + """Test the retire_order command""" + nonexistent_id = max(order.id for order in Order.objects.all()) + 1 + order_ids = [ + self.cart.id, + self.paying.id, + self.already_defunct_cart.id, + self.purchased.id, + nonexistent_id + ] + + self._create_tempfile_and_call_command(order_ids) + + self.assertEqual( + Order.objects.get(id=self.cart.id).status, "defunct-cart" + ) + self.assertEqual( + Order.objects.get(id=self.paying.id).status, "defunct-paying" + ) + self.assertEqual( + Order.objects.get(id=self.already_defunct_cart.id).status, + "defunct-cart" + ) + self.assertEqual( + Order.objects.get(id=self.purchased.id).status, "purchased" + ) + + def _create_tempfile_and_call_command(self, order_ids): + """ + Takes a list of order_ids, writes them to a tempfile, and then runs the + "retire_order" command on the tempfile + """ + with NamedTemporaryFile() as temp: + temp.write("\n".join(str(order_id) for order_id in order_ids)) + temp.seek(0) + call_command('retire_order', temp.name) + + def _create_cart(self): + """Creates a cart and adds a CertificateItem to it""" + cart = Order.get_cart_for_user(UserFactory.create()) + item = CertificateItem.add_to_order( + cart, self.course_key, 10, 'honor', currency='usd' + ) + return cart, item diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 79f5b5931e5b..7c5851731e8b 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -38,10 +38,17 @@ from verify_student.models import SoftwareSecurePhotoVerification from .exceptions import ( - InvalidCartItem, PurchasedCallbackException, ItemAlreadyInCartException, - AlreadyEnrolledInCourseException, CourseDoesNotExistException, - MultipleCouponsNotAllowedException, RegCodeAlreadyExistException, - ItemDoesNotExistAgainstRegCodeException, ItemNotAllowedToRedeemRegCodeException + InvalidCartItem, + PurchasedCallbackException, + ItemAlreadyInCartException, + AlreadyEnrolledInCourseException, + CourseDoesNotExistException, + MultipleCouponsNotAllowedException, + RegCodeAlreadyExistException, + ItemDoesNotExistAgainstRegCodeException, + ItemNotAllowedToRedeemRegCodeException, + InvalidStatusToRetire, + UnexpectedOrderItemStatus, ) from microsite_configuration import microsite @@ -62,8 +69,22 @@ # The user's order has been refunded. ('refunded', 'refunded'), + + # The user's order went through, but the order was erroneously left + # in 'cart'. + ('defunct-cart', 'defunct-cart'), + + # The user's order went through, but the order was erroneously left + # in 'paying'. + ('defunct-paying', 'defunct-paying'), ) +# maps order statuses to their defunct states +ORDER_STATUS_MAP = { + 'cart': 'defunct-cart', + 'paying': 'defunct-paying', +} + # we need a tuple to represent the primary key of various OrderItem subclasses OrderItemSubclassPK = namedtuple('OrderItemSubclassPK', ['cls', 'pk']) # pylint: disable=invalid-name @@ -484,6 +505,39 @@ def generate_receipt_instructions(self): instruction_set.update(set_of_html) return instruction_dict, instruction_set + def retire(self): + """ + Method to "retire" orders that have gone through to the payment service + but have (erroneously) not had their statuses updated. + This method only works on orders that satisfy the following conditions: + 1) the order status is either "cart" or "paying" (otherwise we raise + an InvalidStatusToRetire error) + 2) the order's order item's statuses match the order's status (otherwise + we throw an UnexpectedOrderItemStatus error) + """ + # if an order is already retired, no-op: + if self.status in ORDER_STATUS_MAP.values(): + return + + if self.status not in ORDER_STATUS_MAP.keys(): + raise InvalidStatusToRetire( + "order status {order_status} is not 'paying' or 'cart'".format( + order_status=self.status + ) + ) + + for item in self.orderitem_set.all(): # pylint: disable=no-member + if item.status != self.status: + raise UnexpectedOrderItemStatus( + "order_item status is different from order status" + ) + + self.status = ORDER_STATUS_MAP[self.status] + self.save() + + for item in self.orderitem_set.all(): # pylint: disable=no-member + item.retire() + class OrderItem(TimeStampedModel): """ @@ -616,6 +670,15 @@ def analytics_data(self): 'category': 'N/A', } + def retire(self): + """ + Called by the `retire` method defined in the `Order` class. Retires + an order item if its (and its order's) status was erroneously not + updated to "purchased" after the order was processed. + """ + self.status = ORDER_STATUS_MAP[self.status] + self.save() + class Invoice(models.Model): """ diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index 0fa9f6cf729e..83d412c4fc53 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -9,6 +9,7 @@ from mock import patch, MagicMock import pytz +import ddt from django.core import mail from django.conf import settings from django.db import DatabaseError @@ -28,8 +29,14 @@ from student.tests.factories import UserFactory from student.models import CourseEnrollment from course_modes.models import CourseMode -from shoppingcart.exceptions import (PurchasedCallbackException, CourseDoesNotExistException, - ItemAlreadyInCartException, AlreadyEnrolledInCourseException) +from shoppingcart.exceptions import ( + PurchasedCallbackException, + CourseDoesNotExistException, + ItemAlreadyInCartException, + AlreadyEnrolledInCourseException, + InvalidStatusToRetire, + UnexpectedOrderItemStatus, +) from opaque_keys.edx.locator import CourseLocator @@ -39,6 +46,7 @@ @override_settings(MODULESTORE=MODULESTORE_CONFIG) +@ddt.ddt class OrderTest(ModuleStoreTestCase): def setUp(self): self.user = UserFactory.create() @@ -153,6 +161,62 @@ def test_start_purchase(self): for item in cart.orderitem_set.all(): self.assertEqual(item.status, 'purchased') + def test_retire_order_cart(self): + """Test that an order in cart can successfully be retired""" + cart = Order.get_cart_for_user(user=self.user) + CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd') + + cart.retire() + self.assertEqual(cart.status, 'defunct-cart') + self.assertEqual(cart.orderitem_set.get().status, 'defunct-cart') + + def test_retire_order_paying(self): + """Test that an order in "paying" can successfully be retired""" + cart = Order.get_cart_for_user(user=self.user) + CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd') + cart.start_purchase() + + cart.retire() + self.assertEqual(cart.status, 'defunct-paying') + self.assertEqual(cart.orderitem_set.get().status, 'defunct-paying') + + @ddt.data( + ("cart", "paying", UnexpectedOrderItemStatus), + ("purchased", "purchased", InvalidStatusToRetire), + ) + @ddt.unpack + def test_retire_order_error(self, order_status, item_status, exception): + """ + Test error cases for retiring an order: + 1) Order item has a different status than the order + 2) The order's status isn't in "cart" or "paying" + """ + cart = Order.get_cart_for_user(user=self.user) + item = CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd') + + cart.status = order_status + cart.save() + item.status = item_status + item.save() + + with self.assertRaises(exception): + cart.retire() + + @ddt.data('defunct-paying', 'defunct-cart') + def test_retire_order_already_retired(self, status): + """ + Check that orders that have already been retired noop when the method + is called on them again. + """ + cart = Order.get_cart_for_user(user=self.user) + item = CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd') + cart.status = item.status = status + cart.save() + item.save() + cart.retire() + self.assertEqual(cart.status, status) + self.assertEqual(item.status, status) + @override_settings( SEGMENT_IO_LMS_KEY="foobar", FEATURES={ @@ -291,20 +355,20 @@ def test_billing_info_storage_off(self, render): ((_, context), _) = render.call_args self.assertFalse(context['has_billing_info']) - mock_gen_inst = MagicMock(return_value=(OrderItemSubclassPK(OrderItem, 1), set([]))) - def test_generate_receipt_instructions_callchain(self): """ This tests the generate_receipt_instructions call chain (ie calling the function on the cart also calls it on items in the cart """ + mock_gen_inst = MagicMock(return_value=(OrderItemSubclassPK(OrderItem, 1), set([]))) + cart = Order.get_cart_for_user(self.user) item = OrderItem(user=self.user, order=cart) item.save() self.assertTrue(cart.has_items()) - with patch.object(OrderItem, 'generate_receipt_instructions', self.mock_gen_inst): + with patch.object(OrderItem, 'generate_receipt_instructions', mock_gen_inst): cart.generate_receipt_instructions() - self.mock_gen_inst.assert_called_with() + mock_gen_inst.assert_called_with() class OrderItemTest(TestCase):