Merge pull request #598 from edx/christina/email-studio-request

Send e-mail to STUDIO_REQUEST_EMAIL when user requests course creator privileges

CHANGELOG.rst

@@ -5,6 +5,9 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio: Email will be sent to admin address when a user requests course creator
+privileges for Studio (edge only).
+
 Studio: Studio course authors (both instructors and staff) will be auto-enrolled
 for their courses so that "View Live" works.
 

cms/djangoapps/course_creators/admin.py

@@ -2,13 +2,15 @@
 django admin page for the course creators table
 """
 
-from course_creators.models import CourseCreator, update_creator_state
+from course_creators.models import CourseCreator, update_creator_state, send_user_notification, send_admin_notification
 from course_creators.views import update_course_creator_group
 
 from ratelimitbackend import admin
 from django.conf import settings
 from django.dispatch import receiver
 from mitxmako.shortcuts import render_to_string
+from django.core.mail import send_mail
+from smtplib import SMTPException
 
 import logging
 
@@ -28,12 +30,12 @@ class CourseCreatorAdmin(admin.ModelAdmin):
     """
 
     # Fields to display on the overview page.
-    list_display = ['user', get_email, 'state', 'state_changed', 'note']
-    readonly_fields = ['user', 'state_changed']
+    list_display = ['username', get_email, 'state', 'state_changed', 'note']
+    readonly_fields = ['username', 'state_changed']
     # Controls the order on the edit form (without this, read-only fields appear at the end).
     fieldsets = (
         (None, {
-            'fields': ['user', 'state', 'state_changed', 'note']
+            'fields': ['username', 'state', 'state_changed', 'note']
         }),
     )
     # Fields that filtering support
@@ -43,6 +45,16 @@ class CourseCreatorAdmin(admin.ModelAdmin):
     # Turn off the action bar (we have no bulk actions)
     actions = None
 
+    def username(self, inst):
+        """
+        Returns the username for a given user.
+
+        Implemented to make sorting by username instead of by user object.
+        """
+        return inst.user.username
+
+    username.admin_order_field = 'user__username'
+
     def has_add_permission(self, request):
         return False
 
@@ -70,7 +82,16 @@ def update_creator_group_callback(sender, **kwargs):
     updated_state = kwargs['state']
     update_course_creator_group(kwargs['caller'], user, updated_state == CourseCreator.GRANTED)
 
-    studio_request_email = settings.MITX_FEATURES.get('STUDIO_REQUEST_EMAIL','')
+
+@receiver(send_user_notification, sender=CourseCreator)
+def send_user_notification_callback(sender, **kwargs):
+    """
+    Callback for notifying user about course creator status change.
+    """
+    user = kwargs['user']
+    updated_state = kwargs['state']
+
+    studio_request_email = settings.MITX_FEATURES.get('STUDIO_REQUEST_EMAIL', '')
     context = {'studio_request_email': studio_request_email}
 
     subject = render_to_string('emails/course_creator_subject.txt', context)
@@ -88,3 +109,29 @@ def update_creator_group_callback(sender, **kwargs):
         user.email_user(subject, message, studio_request_email)
     except:
         log.warning("Unable to send course creator status e-mail to %s", user.email)
+
+
+@receiver(send_admin_notification, sender=CourseCreator)
+def send_admin_notification_callback(sender, **kwargs):
+    """
+    Callback for notifying admin of a user in the 'pending' state.
+    """
+    user = kwargs['user']
+
+    studio_request_email = settings.MITX_FEATURES.get('STUDIO_REQUEST_EMAIL', '')
+    context = {'user_name': user.username, 'user_email': user.email}
+
+    subject = render_to_string('emails/course_creator_admin_subject.txt', context)
+    subject = ''.join(subject.splitlines())
+    message = render_to_string('emails/course_creator_admin_user_pending.txt', context)
+
+    try:
+        send_mail(
+            subject,
+            message,
+            studio_request_email,
+            [studio_request_email],
+            fail_silently=False
+        )
+    except SMTPException:
+        log.warning("Failure sending 'pending state' e-mail for %s to %s", user.email, studio_request_email)

cms/djangoapps/course_creators/models.py

@@ -10,7 +10,13 @@ from django.utils import timezone
 from django.utils.translation import ugettext as _
 
 # A signal that will be sent when users should be added or removed from the creator group
-update_creator_state = Signal(providing_args=["caller", "user", "add"])
+update_creator_state = Signal(providing_args=["caller", "user", "state"])
+
+# A signal that will be sent when admin should be notified of a pending user request
+send_admin_notification = Signal(providing_args=["user"])
+
+# A signal that will be sent when user should be notified of change in course creator privileges
+send_user_notification = Signal(providing_args=["user", "state"])
 
 
 class CourseCreator(models.Model):
@@ -60,9 +66,10 @@ def post_save_callback(sender, **kwargs):
     # We only wish to modify the state_changed time if the state has been modified. We don't wish to
     # modify it for changes to the notes field.
     if instance.state != instance.orig_state:
+        granted_state_change = instance.state == CourseCreator.GRANTED or instance.orig_state == CourseCreator.GRANTED
         # If either old or new state is 'granted', we must manipulate the course creator
         # group maintained by authz. That requires staff permissions (stored admin).
-        if instance.state == CourseCreator.GRANTED or instance.orig_state == CourseCreator.GRANTED:
+        if granted_state_change:
             assert hasattr(instance, 'admin'), 'Must have stored staff user to change course creator group'
             update_creator_state.send(
                 sender=sender,
@@ -71,6 +78,22 @@ def post_save_callback(sender, **kwargs):
                 state=instance.state
             )
 
+        # If user has been denied access, granted access, or previously granted access has been
+        # revoked, send a notification message to the user.
+        if instance.state == CourseCreator.DENIED or granted_state_change:
+            send_user_notification.send(
+                sender=sender,
+                user=instance.user,
+                state=instance.state
+            )
+
+        # If the user has gone into the 'pending' state, send a notification to interested admin.
+        if instance.state == CourseCreator.PENDING:
+            send_admin_notification.send(
+                sender=sender,
+                user=instance.user
+            )
+
         instance.state_changed = timezone.now()
         instance.orig_state = instance.state
         instance.save()

cms/djangoapps/course_creators/tests/test_admin.py

@@ -11,6 +11,7 @@ import mock
 from course_creators.admin import CourseCreatorAdmin
 from course_creators.models import CourseCreator
 from auth.authz import is_user_in_creator_group
+from django.core import mail
 
 
 def mock_render_to_string(template_name, context):
@@ -37,21 +38,25 @@ class CourseCreatorAdminTest(TestCase):
 
         self.creator_admin = CourseCreatorAdmin(self.table_entry, AdminSite())
 
+        self.studio_request_email = 'mark@marky.mark'
+        self.enable_creator_group_patch = {
+            "ENABLE_CREATOR_GROUP": True,
+            "STUDIO_REQUEST_EMAIL": self.studio_request_email
+        }
+
     @mock.patch('course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True))
     @mock.patch('django.contrib.auth.models.User.email_user')
     def test_change_status(self, email_user):
         """
         Tests that updates to state impact the creator group maintained in authz.py and that e-mails are sent.
         """
-        STUDIO_REQUEST_EMAIL = 'mark@marky.mark'
 
-        def change_state(state, is_creator):
-            """ Helper method for changing state """
-            self.table_entry.state = state
-            self.creator_admin.save_model(self.request, self.table_entry, None, True)
+        def change_state_and_verify_email(state, is_creator):
+            """ Changes user state, verifies creator status, and verifies e-mail is sent based on transition """
+            self._change_state(state)
             self.assertEqual(is_creator, is_user_in_creator_group(self.user))
 
-            context = {'studio_request_email': STUDIO_REQUEST_EMAIL}
+            context = {'studio_request_email': self.studio_request_email}
             if state == CourseCreator.GRANTED:
                 template = 'emails/course_creator_granted.txt'
             elif state == CourseCreator.DENIED:
@@ -61,31 +66,76 @@ class CourseCreatorAdminTest(TestCase):
             email_user.assert_called_with(
                 mock_render_to_string('emails/course_creator_subject.txt', context),
                 mock_render_to_string(template, context),
-                STUDIO_REQUEST_EMAIL
+                self.studio_request_email
             )
 
-        with mock.patch.dict(
-            'django.conf.settings.MITX_FEATURES',
-            {
-                "ENABLE_CREATOR_GROUP": True,
-                "STUDIO_REQUEST_EMAIL": STUDIO_REQUEST_EMAIL
-            }
-        ):
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group_patch):
 
             # User is initially unrequested.
             self.assertFalse(is_user_in_creator_group(self.user))
 
-            change_state(CourseCreator.GRANTED, True)
+            change_state_and_verify_email(CourseCreator.GRANTED, True)
+
+            change_state_and_verify_email(CourseCreator.DENIED, False)
+
+            change_state_and_verify_email(CourseCreator.GRANTED, True)
+
+            change_state_and_verify_email(CourseCreator.PENDING, False)
 
-            change_state(CourseCreator.DENIED, False)
+            change_state_and_verify_email(CourseCreator.GRANTED, True)
 
-            change_state(CourseCreator.GRANTED, True)
+            change_state_and_verify_email(CourseCreator.UNREQUESTED, False)
 
-            change_state(CourseCreator.PENDING, False)
+            change_state_and_verify_email(CourseCreator.DENIED, False)
 
-            change_state(CourseCreator.GRANTED, True)
+    @mock.patch('course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True))
+    def test_mail_admin_on_pending(self):
+        """
+        Tests that the admin account is notified when a user is in the 'pending' state.
+        """
 
-            change_state(CourseCreator.UNREQUESTED, False)
+        def check_admin_message_state(state, expect_sent_to_admin, expect_sent_to_user):
+            """ Changes user state and verifies e-mail sent to admin address only when pending. """
+            mail.outbox = []
+            self._change_state(state)
+
+            # If a message is sent to the user about course creator status change, it will be the first
+            # message sent. Admin message will follow.
+            base_num_emails = 1 if expect_sent_to_user else 0
+            if expect_sent_to_admin:
+                context = {'user_name': "test_user", 'user_email': 'test_user+courses@edx.org'}
+                self.assertEquals(base_num_emails + 1, len(mail.outbox), 'Expected admin message to be sent')
+                sent_mail = mail.outbox[base_num_emails]
+                self.assertEquals(
+                    mock_render_to_string('emails/course_creator_admin_subject.txt', context),
+                    sent_mail.subject
+                )
+                self.assertEquals(
+                    mock_render_to_string('emails/course_creator_admin_user_pending.txt', context),
+                    sent_mail.body
+                )
+                self.assertEquals(self.studio_request_email, sent_mail.from_email)
+                self.assertEqual([self.studio_request_email], sent_mail.to)
+            else:
+                self.assertEquals(base_num_emails, len(mail.outbox))
+
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group_patch):
+            # E-mail message should be sent to admin only when new state is PENDING, regardless of what
+            # previous state was (unless previous state was already PENDING).
+            # E-mail message sent to user only on transition into and out of GRANTED state.
+            check_admin_message_state(CourseCreator.UNREQUESTED, expect_sent_to_admin=False, expect_sent_to_user=False)
+            check_admin_message_state(CourseCreator.PENDING, expect_sent_to_admin=True, expect_sent_to_user=False)
+            check_admin_message_state(CourseCreator.GRANTED, expect_sent_to_admin=False, expect_sent_to_user=True)
+            check_admin_message_state(CourseCreator.DENIED, expect_sent_to_admin=False, expect_sent_to_user=True)
+            check_admin_message_state(CourseCreator.GRANTED, expect_sent_to_admin=False, expect_sent_to_user=True)
+            check_admin_message_state(CourseCreator.PENDING, expect_sent_to_admin=True, expect_sent_to_user=True)
+            check_admin_message_state(CourseCreator.PENDING, expect_sent_to_admin=False, expect_sent_to_user=False)
+            check_admin_message_state(CourseCreator.DENIED, expect_sent_to_admin=False, expect_sent_to_user=True)
+
+    def _change_state(self, state):
+        """ Helper method for changing state """
+        self.table_entry.state = state
+        self.creator_admin.save_model(self.request, self.table_entry, None, True)
 
     def test_add_permission(self):
         """

cms/templates/emails/course_creator_admin_subject.txt

+<%! from django.utils.translation import ugettext as _ %>
+${_("{email} has requested Studio course creator privileges on edge".format(email=user_email))}
\ No newline at end of file

cms/templates/emails/course_creator_admin_user_pending.txt

+<%! from django.utils.translation import ugettext as _ %>
+${_("User '{user}' with e-mail {email} has requested Studio course creator privileges on edge.".format(user=user_name, email=user_email))}
+${_("To grant or deny this request, use the course creator admin table.")}
+
+% if is_secure:
+  https://${ site }/admin/course_creators/coursecreator/
+% else:
+  http://${ site }/admin/course_creators/coursecreator/
+% endif