Merge pull request #6930 from edx/hotfix/2015-02-09b

get the raw DescriptorSystem, not the CombinedSystem when instantiating ...

cms/djangoapps/contentstore/views/preview.py

@@ -185,7 +185,8 @@ def _preview_module_system(request, descriptor, field_data):
         wrappers=wrappers,
         error_descriptor_class=ErrorDescriptor,
         get_user_role=lambda: get_user_role(request.user, course_id),
-        descriptor_runtime=descriptor.runtime,
+        # Get the raw DescriptorSystem, not the CombinedSystem
+        descriptor_runtime=descriptor._runtime,  # pylint: disable=protected-access
         services={
             "i18n": ModuleI18nService(),
             "field-data": field_data,

common/djangoapps/student/views.py

@@ -1370,7 +1370,7 @@ def _do_create_account(post_vars, extended_profile=None):
     return (user, profile, registration)
 
 
-@ensure_csrf_cookie
+@csrf_exempt
 def create_account(request, post_override=None):  # pylint: disable-msg=too-many-statements
     """
     JSON call to create new edX account.

common/lib/xmodule/xmodule/tests/__init__.py

@@ -94,7 +94,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')):
     """
     user = Mock(name='get_test_system.user', is_staff=False)
 
-    descriptor_system = get_test_descriptor_system(),
+    descriptor_system = get_test_descriptor_system()
 
     def get_module(descriptor):
         """Mocks module_system get_module function"""
@@ -107,7 +107,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')):
 
         # Descriptors can all share a single DescriptorSystem.
         # So, bind to the same one as the current descriptor.
-        module_system.descriptor_runtime = descriptor.runtime._descriptor_system
+        module_system.descriptor_runtime = descriptor._runtime  # pylint: disable=protected-access
 
         descriptor.bind_for_student(module_system, descriptor._field_data)
 

common/lib/xmodule/xmodule/tests/test_library_content.py

@@ -54,14 +54,14 @@ class LibraryContentTest(MixedSplitTestCase):
         Bind a module (part of self.course) so we can access student-specific data.
         """
         module_system = get_test_system(course_id=self.course.location.course_key)
-        module_system.descriptor_runtime = module.runtime
+        module_system.descriptor_runtime = module.runtime._descriptor_system  # pylint: disable=protected-access
         module_system._services['library_tools'] = self.tools  # pylint: disable=protected-access
 
         def get_module(descriptor):
             """Mocks module_system get_module function"""
             sub_module_system = get_test_system(course_id=self.course.location.course_key)
             sub_module_system.get_module = get_module
-            sub_module_system.descriptor_runtime = descriptor.runtime
+            sub_module_system.descriptor_runtime = descriptor._runtime  # pylint: disable=protected-access
             descriptor.bind_for_student(sub_module_system, descriptor._field_data)  # pylint: disable=protected-access
             return descriptor
 

common/lib/xmodule/xmodule/tests/test_split_test_module.py

@@ -78,7 +78,7 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase):
         self.course_sequence = self.course.get_children()[0]
         self.module_system = get_test_system()
 
-        self.module_system.descriptor_runtime = self.course.runtime._descriptor_system  # pylint: disable=protected-access
+        self.module_system.descriptor_runtime = self.course._runtime  # pylint: disable=protected-access
         self.course.runtime.export_fs = MemoryFS()
 
         self.partitions_service = StaticPartitionService(

common/lib/xmodule/xmodule/tests/test_vertical.py

@@ -27,7 +27,7 @@ class BaseVerticalModuleTest(XModuleXmlImportTest):
         course_seq = self.course.get_children()[0]
         self.module_system = get_test_system()
 
-        self.module_system.descriptor_runtime = self.course.runtime._descriptor_system  # pylint: disable=protected-access
+        self.module_system.descriptor_runtime = self.course._runtime  # pylint: disable=protected-access
         self.course.runtime.export_fs = MemoryFS()
 
         self.vertical = course_seq.get_children()[0]

common/lib/xmodule/xmodule/x_module.py

@@ -3,6 +3,7 @@ import os
 import sys
 import yaml
 
+from contracts import contract, new_contract
 from functools import partial
 from lxml import etree
 from collections import namedtuple
@@ -1307,6 +1308,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # p
         pass
 
 
+new_contract('DescriptorSystem', DescriptorSystem)
+
+
 class XMLParsingSystem(DescriptorSystem):
     def __init__(self, process_xml, **kwargs):
         """
@@ -1427,6 +1431,8 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # pylin
     Note that these functions can be closures over e.g. a django request
     and user, or other environment-specific info.
     """
+
+    @contract(descriptor_runtime='DescriptorSystem')
     def __init__(
             self, static_url, track_function, get_module, render_template,
             replace_urls, descriptor_runtime, user=None, filestore=None,

lms/djangoapps/bulk_email/tasks.py

@@ -6,6 +6,7 @@ import re
 import random
 import json
 from time import sleep
+from collections import Counter
 
 import dogstats_wrapper as dog_stats_api
 from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPException
@@ -418,12 +419,31 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
         'failed' count above.
     """
     # Get information from current task's request:
+    parent_task_id = InstructorTask.objects.get(pk=entry_id).task_id
     task_id = subtask_status.task_id
+    total_recipients = len(to_list)
+    recipient_num = 0
+    total_recipients_successful = 0
+    total_recipients_failed = 0
+    recipients_info = Counter()
+
+    log.info(
+        "BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, TotalRecipients: %s",
+        parent_task_id,
+        task_id,
+        email_id,
+        total_recipients
+    )
 
     try:
         course_email = CourseEmail.objects.get(id=email_id)
     except CourseEmail.DoesNotExist as exc:
-        log.exception("Task %s: could not find email id:%s to send.", task_id, email_id)
+        log.exception(
+            "BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Could not find email to send.",
+            parent_task_id,
+            task_id,
+            email_id
+        )
         raise
 
     # Exclude optouts (if not a retry):
@@ -459,6 +479,7 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
             # That way, the to_list will always contain the recipients remaining to be emailed.
             # This is convenient for retries, which will need to send to those who haven't
             # yet been emailed, but not send to those who have already been sent to.
+            recipient_num += 1
             current_recipient = to_list[-1]
             email = current_recipient['email']
             email_context['email'] = email
@@ -489,29 +510,81 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
                 sleep(settings.BULK_EMAIL_RETRY_DELAY_BETWEEN_SENDS)
 
             try:
-                log.debug('Email with id %s to be sent to %s', email_id, email)
-
+                log.info(
+                    "BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Recipient num: %s/%s, \
+                    Recipient name: %s, Email address: %s",
+                    parent_task_id,
+                    task_id,
+                    email_id,
+                    recipient_num,
+                    total_recipients,
+                    current_recipient['profile__name'],
+                    email
+                )
                 with dog_stats_api.timer('course_email.single_send.time.overall', tags=[_statsd_tag(course_title)]):
                     connection.send_messages([email_msg])
 
             except SMTPDataError as exc:
                 # According to SMTP spec, we'll retry error codes in the 4xx range.  5xx range indicates hard failure.
+                total_recipients_failed += 1
+                log.error(
+                    "BulkEmail ==> Status: Failed(SMTPDataError), Task: %s, SubTask: %s, EmailId: %s, \
+                    Recipient num: %s/%s, Email address: %s",
+                    parent_task_id,
+                    task_id,
+                    email_id,
+                    recipient_num,
+                    total_recipients,
+                    email
+                )
                 if exc.smtp_code >= 400 and exc.smtp_code < 500:
                     # This will cause the outer handler to catch the exception and retry the entire task.
                     raise exc
                 else:
                     # This will fall through and not retry the message.
-                    log.warning('Task %s: email with id %s not delivered to %s due to error %s', task_id, email_id, email, exc.smtp_error)
+                    log.warning(
+                        'BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Recipient num: %s/%s, \
+                        Email not delivered to %s due to error %s',
+                        parent_task_id,
+                        task_id,
+                        email_id,
+                        recipient_num,
+                        total_recipients,
+                        email,
+                        exc.smtp_error
+                    )
                     dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)])
                     subtask_status.increment(failed=1)
 
             except SINGLE_EMAIL_FAILURE_ERRORS as exc:
                 # This will fall through and not retry the message.
-                log.warning('Task %s: email with id %s not delivered to %s due to error %s', task_id, email_id, email, exc)
+                total_recipients_failed += 1
+                log.error(
+                    "BulkEmail ==> Status: Failed(SINGLE_EMAIL_FAILURE_ERRORS), Task: %s, SubTask: %s, \
+                    EmailId: %s, Recipient num: %s/%s, Email address: %s, Exception: %s",
+                    parent_task_id,
+                    task_id,
+                    email_id,
+                    recipient_num,
+                    total_recipients,
+                    email,
+                    exc
+                )
                 dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)])
                 subtask_status.increment(failed=1)
 
             else:
+                total_recipients_successful += 1
+                log.info(
+                    "BulkEmail ==> Status: Success, Task: %s, SubTask: %s, EmailId: %s, \
+                    Recipient num: %s/%s, Email address: %s,",
+                    parent_task_id,
+                    task_id,
+                    email_id,
+                    recipient_num,
+                    total_recipients,
+                    email
+                )
                 dog_stats_api.increment('course_email.sent', tags=[_statsd_tag(course_title)])
                 if settings.BULK_EMAIL_LOG_SENT_EMAILS:
                     log.info('Email with id %s sent to %s', email_id, email)
@@ -522,8 +595,32 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
             # Pop the user that was emailed off the end of the list only once they have
             # successfully been processed.  (That way, if there were a failure that
             # needed to be retried, the user is still on the list.)
+            recipients_info[email] += 1
             to_list.pop()
 
+        log.info(
+            "BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Total Successful Recipients: %s/%s, \
+            Failed Recipients: %s/%s",
+            parent_task_id,
+            task_id,
+            email_id,
+            total_recipients_successful,
+            total_recipients,
+            total_recipients_failed,
+            total_recipients
+        )
+        duplicate_recipients = ["{0} ({1})".format(email, repetition)
+                                for email, repetition in recipients_info.most_common() if repetition > 1]
+        if duplicate_recipients:
+            log.info(
+                "BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Total Duplicate Recipients [%s]: [%s]",
+                parent_task_id,
+                task_id,
+                email_id,
+                len(duplicate_recipients),
+                ', '.join(duplicate_recipients)
+            )
+
     except INFINITE_RETRY_ERRORS as exc:
         dog_stats_api.increment('course_email.infinite_retry', tags=[_statsd_tag(course_title)])
         # Increment the "retried_nomax" counter, update other counters with progress to date,

lms/djangoapps/courseware/module_render.py

@@ -642,7 +642,7 @@ def get_module_system_for_user(user, field_data_cache,
             'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff),
         },
         get_user_role=lambda: get_user_role(user, course_id),
-        descriptor_runtime=descriptor.runtime,
+        descriptor_runtime=descriptor._runtime,  # pylint: disable=protected-access
         rebind_noauth_module_to_user=rebind_noauth_module_to_user,
         user_location=user_location,
         request_token=request_token,

lms/djangoapps/courseware/tests/test_module_render.py

@@ -41,7 +41,7 @@ from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory, check_mongo_calls
-from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW
+from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW, CombinedSystem
 
 TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
 
@@ -903,13 +903,16 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase):
             _field_data=Mock(spec=FieldData),
             location=location,
             static_asset_path=None,
-            runtime=Mock(
+            _runtime=Mock(
                 spec=Runtime,
                 resources_fs=None,
-                mixologist=Mock(_mixins=())
+                mixologist=Mock(_mixins=(), name='mixologist'),
+                name='runtime',
             ),
             scope_ids=Mock(spec=ScopeIds),
+            name='descriptor'
         )
+        descriptor.runtime = CombinedSystem(descriptor._runtime, None)  # pylint: disable=protected-access
         # Use the xblock_class's bind_for_student method
         descriptor.bind_for_student = partial(xblock_class.bind_for_student, descriptor)
 
@@ -919,10 +922,10 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase):
         return render.get_module_for_descriptor_internal(
             user=self.user,
             descriptor=descriptor,
-            field_data_cache=Mock(spec=FieldDataCache),
+            field_data_cache=Mock(spec=FieldDataCache, name='field_data_cache'),
             course_id=course_id,
-            track_function=Mock(),  # Track Function
-            xqueue_callback_url_prefix=Mock(),  # XQueue Callback Url Prefix
+            track_function=Mock(name='track_function'),  # Track Function
+            xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'),  # XQueue Callback Url Prefix
             request_token='request_token',
         ).xmodule_runtime.anonymous_student_id