From 55f2cbc92faf64f526f48ea40a3fcda357f26ac4 Mon Sep 17 00:00:00 2001 From: cahrens Date: Tue, 6 Aug 2013 15:53:07 -0400 Subject: [PATCH 1/3] Split up event notification, make sorting on username work. --- cms/djangoapps/course_creators/admin.py | 52 +++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 0f9a189bb7..8256c5e8bf 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/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,6 +82,12 @@ def update_creator_group_callback(sender, **kwargs): updated_state = kwargs['state'] update_course_creator_group(kwargs['caller'], user, updated_state == CourseCreator.GRANTED) + +@receiver(send_user_notification, sender=CourseCreator) +def send_user_notification_callback(sender, **kwargs): + user = kwargs['user'] + updated_state = kwargs['state'] + studio_request_email = settings.MITX_FEATURES.get('STUDIO_REQUEST_EMAIL','') context = {'studio_request_email': studio_request_email} @@ -88,3 +106,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) From e6f15bf611a271d6713a32eb0c926e782364b03d Mon Sep 17 00:00:00 2001 From: cahrens Date: Tue, 6 Aug 2013 15:58:23 -0400 Subject: [PATCH 2/3] Send notification to admin when a user enters 'pending' state. Also fixes a bug where message was not sent to user when entering 'denied' state unless the user was previously in 'granted'. Send notification to admin when a user enters 'pending' state. Pylint cleanup. --- CHANGELOG.rst | 3 + cms/djangoapps/course_creators/admin.py | 7 +- cms/djangoapps/course_creators/models.py | 27 +++++- .../course_creators/tests/test_admin.py | 90 ++++++++++++++----- .../emails/course_creator_admin_subject.txt | 2 + .../course_creator_admin_user_pending.txt | 3 + 6 files changed, 108 insertions(+), 24 deletions(-) create mode 100644 cms/templates/emails/course_creator_admin_subject.txt create mode 100644 cms/templates/emails/course_creator_admin_user_pending.txt diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fecccafdca..897ea3ae3a 100644 --- a/CHANGELOG.rst +++ b/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. diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 8256c5e8bf..df2baa1aa2 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -85,10 +85,13 @@ def update_creator_group_callback(sender, **kwargs): @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','') + 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) @@ -115,7 +118,7 @@ def send_admin_notification_callback(sender, **kwargs): """ user = kwargs['user'] - studio_request_email = settings.MITX_FEATURES.get('STUDIO_REQUEST_EMAIL','') + 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) diff --git a/cms/djangoapps/course_creators/models.py b/cms/djangoapps/course_creators/models.py index ba434c9140..5138916c88 100644 --- a/cms/djangoapps/course_creators/models.py +++ b/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() diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index f28fec5a44..aa293e008e 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/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(CourseCreator.DENIED, False) + change_state_and_verify_email(CourseCreator.DENIED, False) - change_state(CourseCreator.GRANTED, True) + change_state_and_verify_email(CourseCreator.GRANTED, True) - change_state(CourseCreator.PENDING, False) + change_state_and_verify_email(CourseCreator.PENDING, False) - change_state(CourseCreator.GRANTED, True) + change_state_and_verify_email(CourseCreator.GRANTED, True) - change_state(CourseCreator.UNREQUESTED, False) + change_state_and_verify_email(CourseCreator.UNREQUESTED, False) + + change_state_and_verify_email(CourseCreator.DENIED, False) + + @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. + """ + + 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): """ diff --git a/cms/templates/emails/course_creator_admin_subject.txt b/cms/templates/emails/course_creator_admin_subject.txt new file mode 100644 index 0000000000..13c7c3094d --- /dev/null +++ b/cms/templates/emails/course_creator_admin_subject.txt @@ -0,0 +1,2 @@ +<%! 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 diff --git a/cms/templates/emails/course_creator_admin_user_pending.txt b/cms/templates/emails/course_creator_admin_user_pending.txt new file mode 100644 index 0000000000..99787fe399 --- /dev/null +++ b/cms/templates/emails/course_creator_admin_user_pending.txt @@ -0,0 +1,3 @@ +<%! 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.")} \ No newline at end of file From bbd8d84a28947b143722f5a788235e7104e0fd72 Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 7 Aug 2013 11:34:51 -0400 Subject: [PATCH 3/3] Include the URL to the course creator admin table. --- .../emails/course_creator_admin_user_pending.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cms/templates/emails/course_creator_admin_user_pending.txt b/cms/templates/emails/course_creator_admin_user_pending.txt index 99787fe399..37464d3e9c 100644 --- a/cms/templates/emails/course_creator_admin_user_pending.txt +++ b/cms/templates/emails/course_creator_admin_user_pending.txt @@ -1,3 +1,9 @@ <%! 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.")} \ No newline at end of file +${_("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