Merge pull request #495 from edx/tasawer/ecom-6526-email-on-comment-update

Tasawer/ecom 6526 email on comment update

course_discovery/apps/publisher/models.py

@@ -160,8 +160,12 @@ class Course(TimeStampedModel, ChangedByMixin):
         """ Returns the list of users emails with enable email notifications
         against a course group. By default if attribute value does not exists
         then user will be eligible for emails.
+        Also get users from course-user-role table.
         """
-        users_list = get_users_with_perms(self)
+        users_list_perms = get_users_with_perms(self)
+        users_list_roles = [obj.user for obj in self.course_user_roles.all()]
+
+        users_list = set(list(users_list_perms) + list(users_list_roles))
         emails = [user.email for user in users_list if is_email_notification_enabled(user)]
 
         return emails

course_discovery/apps/publisher/tests/test_model.py

@@ -129,9 +129,19 @@ class CourseTests(TestCase):
         self.course.assign_permission_by_group(self.org_extension_1.group)
         self.assertListEqual(self.course.get_group_users_emails(), [self.user1.email, self.user3.email])
 
+        # add user in course-user-role table
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.PartnerCoordinator, user=self.user2
+        )
+
+        self.assertListEqual(
+            self.course.get_group_users_emails(),
+            [self.user1.email, self.user2.email, self.user3.email]
+        )
+
         # The email addresses of users who have disabled email notifications should NOT be returned.
         factories.UserAttributeFactory(user=self.user1, enable_email_notification=False)
-        self.assertListEqual(self.course.get_group_users_emails(), [self.user3.email])
+        self.assertListEqual(self.course.get_group_users_emails(), [self.user2.email, self.user3.email])
 
     def test_keywords_data(self):
         """ Verify that the property returns the keywords as comma separated string. """

course_discovery/apps/publisher_comments/emails.py

@@ -12,21 +12,32 @@ from course_discovery.apps.publisher.models import CourseRun
 log = logging.getLogger(__name__)
 
 
-def send_email_for_comment(comment):
+def send_email_for_comment(comment, created=False):
     """ Send the emails for a comment.
 
         Arguments:
             comment (Comment): Comment object
+            created (Bool): Value indicating comment is created or updated
     """
     try:
         object_pk = comment.object_pk
         publisher_obj = comment.content_type.get_object_for_this_type(pk=object_pk)
         comment_class = comment.content_type.model_class()
 
+        subject_desc = _('New comment added')
+
+        if not created:
+            subject_desc = _('Comment updated')
+
         if comment_class == CourseRun:
             course = publisher_obj.course
             object_path = reverse('publisher:publisher_course_run_detail', args=[publisher_obj.id])
-            subject = _('New comment added in course run: {title}-{pacing_type}-{start}').format(
+
+            # Translators: subject_desc will be choice from ('New comment added', 'Comment updated'),
+            # 'pacing_type' will be choice from ('instructor-paced', 'self-paced'),
+            # 'title' and 'start' will be the value of course title & start date fields.
+            subject = _('{subject_desc} in course run: {title}-{pacing_type}-{start}').format(
+                subject_desc=subject_desc,
                 title=course.title,
                 pacing_type=publisher_obj.get_pacing_type_display(),
                 start=publisher_obj.start.strftime('%B %d, %Y') if publisher_obj.start else ''
@@ -34,7 +45,13 @@ def send_email_for_comment(comment):
         else:
             course = publisher_obj
             object_path = reverse('publisher:publisher_courses_edit', args=[publisher_obj.id])
-            subject = _('New comment added in Course: {title}').format(title=course.title)
+
+            # Translators: 'subject_desc' will be choice from ('New comment added', 'Comment updated')
+            # and 'title' will be the value of course title field.
+            subject = _('{subject_desc} in Course: {title}').format(
+                subject_desc=subject_desc,
+                title=course.title
+            )
 
         to_addresses = course.get_group_users_emails()
         from_address = settings.PUBLISHER_FROM_EMAIL

course_discovery/apps/publisher_comments/models.py

@@ -16,4 +16,4 @@ class Comments(CommentAbstractModel):
 def send_email(sender, instance, **kwargs):    # pylint: disable=unused-argument
     """ Send email on new comment. """
     if waffle.switch_is_active('enable_publisher_email_notifications'):
-        send_email_for_comment(instance)
+        send_email_for_comment(instance, kwargs.get('created', False))

course_discovery/apps/publisher_comments/tests/test_emails.py

@@ -5,11 +5,11 @@ from django.contrib.sites.models import Site
 from django.core import mail
 from django.core.urlresolvers import reverse
 from django.test import TestCase
-from guardian.shortcuts import assign_perm
 
 from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.course_metadata.tests import toggle_switch
-from course_discovery.apps.publisher.models import Course
+from course_discovery.apps.publisher.choices import PublisherUserRole
+from course_discovery.apps.publisher.models import CourseUserRole
 from course_discovery.apps.publisher.tests import factories
 from course_discovery.apps.publisher.tests.factories import UserAttributeFactory
 from course_discovery.apps.publisher_comments.tests.factories import CommentFactory
@@ -30,16 +30,23 @@ class CommentsEmailTests(TestCase):
 
         self.organization_extension = factories.OrganizationExtensionFactory()
 
-        self.user.groups.add(self.organization_extension.group)
-        self.user_2.groups.add(self.organization_extension.group)
-        self.user_3.groups.add(self.organization_extension.group)
-
         self.seat = factories.SeatFactory()
         self.course_run = self.seat.course_run
         self.course = self.course_run.course
 
+        # assign the role against a course
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.MarketingReviewer, user=self.user
+        )
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.PartnerCoordinator, user=self.user_2
+        )
+
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.Publisher, user=self.user_3
+        )
+
         self.course.organizations.add(self.organization_extension.organization)
-        assign_perm(Course.VIEW_PERMISSION, self.organization_extension.group, self.course)
 
         # NOTE: We intentionally do NOT create an attribute for user_2.
         # By default this user WILL receive email notifications.
@@ -79,7 +86,7 @@ class CommentsEmailTests(TestCase):
     def test_email_with_enable_waffle_switch(self, send_email_for_comment):
         """ Verify that send_email_for_comment called with enable waffle switch.. """
         comment = self.create_comment(content_object=self.course)
-        send_email_for_comment.assert_called_once_with(comment)
+        send_email_for_comment.assert_called_once_with(comment, True)
 
     @mock.patch('course_discovery.apps.publisher_comments.models.send_email_for_comment')
     def test_email_with_disable_waffle_switch(self, send_email_for_comment):
@@ -88,11 +95,10 @@ class CommentsEmailTests(TestCase):
         self.create_comment(content_object=self.course)
         send_email_for_comment.assert_not_called()
 
-    def test_email_without_different_group(self):
-        """ Verify the emails behaviour if course group has no users. """
-        self.user.groups.remove(self.organization_extension.group)
-        self.user_2.groups.remove(self.organization_extension.group)
-        self.user_3.groups.remove(self.organization_extension.group)
+    def test_email_without_any_role(self):
+        """ Verify the emails behaviour if course role has no users. """
+        CourseUserRole.objects.all().delete()
+
         self.create_comment(content_object=self.course)
         self.assertEqual(len(mail.outbox), 0)
 
@@ -131,6 +137,64 @@ class CommentsEmailTests(TestCase):
         self.assertIn(page_url, body)
         self.assertIn('The edX team', body)
 
+    def test_email_with_roles(self):
+        """ Verify that emails send to the users against course-user-roles also."""
+        user_4 = UserFactory()
+        user_5 = UserFactory()
+
+        # assign the role against a course
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.MarketingReviewer, user=user_4
+        )
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.PartnerCoordinator, user=user_5
+        )
+        self.create_comment(content_object=self.course_run)
+        self.assertEqual(len(mail.outbox), 1)
+        self.assertEqual([self.user.email, self.user_2.email, user_4.email, user_5.email], mail.outbox[0].to)
+
+    def test_email_for_roles_only(self):
+        """ Verify the emails send to the course roles users even if groups has no users. """
+        user_4 = UserFactory()
+        # assign the role against a course
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.MarketingReviewer, user=user_4
+        )
+
+        self.create_comment(content_object=self.course)
+        self.assertEqual(len(mail.outbox), 1)
+
+    def test_email_with_course_comment_editing(self):
+        """ Verify that after editing a comment against a course emails send
+        to multiple users.
+        """
+        comment = self.create_comment(content_object=self.course)
+        subject = 'New comment added in Course: {title}'.format(title=self.course.title)
+        self.assertEqual(str(mail.outbox[0].subject), subject)
+        self.assertIn(comment.comment, str(mail.outbox[0].body.strip()))
+
+        comment.comment = 'update the comment'
+        comment.save()  # pylint: disable=no-member
+        subject = 'Comment updated in Course: {title}'.format(title=self.course.title)
+        self.assertEqual(str(mail.outbox[1].subject), subject)
+        self.assertIn(comment.comment, str(mail.outbox[1].body.strip()), 'update the comment')
+
+    def test_email_with_course_run_comment_editing(self):
+        """ Verify that after editing a comment against a course emails send
+        to multiple users.
+        """
+        comment = self.create_comment(content_object=self.course_run)
+        comment.comment = 'Update the comment'
+        comment.save()  # pylint: disable=no-member
+
+        subject = 'Comment updated in course run: {title}-{pacing_type}-{start}'.format(
+            title=self.course_run.course.title,
+            pacing_type=self.course_run.get_pacing_type_display(),
+            start=self.course_run.start.strftime('%B %d, %Y')
+        )
+        self.assertEqual(str(mail.outbox[1].subject), subject)
+        self.assertIn(comment.comment, str(mail.outbox[1].body.strip()), 'Update the comment')
+
     def create_comment(self, content_object):
         """ DRY method to create the comment for a given content type."""
         return CommentFactory(

course_discovery/apps/publisher_comments/tests/test_views.py

@@ -7,6 +7,7 @@ from django.test import TestCase
 
 from course_discovery.apps.core.tests.factories import UserFactory, USER_PASSWORD
 from course_discovery.apps.course_metadata.tests import toggle_switch
+from course_discovery.apps.publisher.choices import PublisherUserRole
 from course_discovery.apps.publisher.models import Seat
 from course_discovery.apps.publisher.tests import factories
 from course_discovery.apps.publisher_comments.tests.factories import CommentFactory
@@ -20,8 +21,6 @@ class CommentsTests(TestCase):
         self.user = UserFactory(is_staff=True, is_superuser=True)
         self.organization_extension = factories.OrganizationExtensionFactory()
 
-        self.user.groups.add(self.organization_extension.group)
-
         self.client.login(username=self.user.username, password=USER_PASSWORD)
         self.site = Site.objects.get(pk=settings.SITE_ID)
         self.course_edit_page = 'publisher:publisher_courses_edit'
@@ -34,7 +33,12 @@ class CommentsTests(TestCase):
         self.course = self.course_run.course
 
         self.course.organizations.add(self.organization_extension.organization)
-        self.course.assign_permission_by_group(self.organization_extension.group)
+
+        # assign the role against a course
+        factories.CourseUserRoleFactory(
+            course=self.course, role=PublisherUserRole.MarketingReviewer, user=self.user
+        )
+
         toggle_switch('enable_publisher_email_notifications', True)
 
     def test_course_edit_page_with_multiple_comments(self):

course_discovery/conf/locale/en/LC_MESSAGES/django.po

@@ -7,14 +7,14 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-12-19 14:36+0500\n"
+"POT-Creation-Date: 2016-12-21 15:19+0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
-"Language: \n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
+"Language: \n"
 
 #: apps/api/filters.py
 #, python-brace-format
@@ -647,13 +647,28 @@ msgid "Publish"
 msgstr ""
 
 #: apps/publisher_comments/emails.py
+msgid "New comment added"
+msgstr ""
+
+#: apps/publisher_comments/emails.py
+msgid "Comment updated"
+msgstr ""
+
+#. Translators: subject_desc will be choice from ('New comment added',
+#. 'Comment updated'),
+#. 'pacing_type' will be choice from ('instructor-paced', 'self-paced'),
+#. 'title' and 'start' will be the value of course title & start date fields.
+#: apps/publisher_comments/emails.py
 #, python-brace-format
-msgid "New comment added in course run: {title}-{pacing_type}-{start}"
+msgid "{subject_desc} in course run: {title}-{pacing_type}-{start}"
 msgstr ""
 
+#. Translators: 'subject_desc' will be choice from ('New comment added',
+#. 'Comment updated')
+#. and 'title' will be the value of course title field.
 #: apps/publisher_comments/emails.py
 #, python-brace-format
-msgid "New comment added in Course: {title}"
+msgid "{subject_desc} in Course: {title}"
 msgstr ""
 
 #: apps/publisher_comments/models.py

course_discovery/conf/locale/en/LC_MESSAGES/djangojs.po

@@ -7,14 +7,14 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-12-19 14:36+0500\n"
+"POT-Creation-Date: 2016-12-21 15:19+0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
-"Language: \n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
+"Language: \n"
 
 #: static/js/catalogs-change-form.js
 msgid "Preview"

course_discovery/conf/locale/eo/LC_MESSAGES/django.po

@@ -7,14 +7,14 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-12-19 14:36+0500\n"
+"POT-Creation-Date: 2016-12-21 15:19+0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
-"Language: \n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
+"Language: \n"
 "Plural-Forms: nplurals=2; plural=(n != 1);\n"
 
 #: apps/api/filters.py
@@ -778,17 +778,31 @@ msgid "Publish"
 msgstr "Püßlïsh Ⱡ'σяєм ιρѕυм #"
 
 #: apps/publisher_comments/emails.py
+msgid "New comment added"
+msgstr "Néw çömmént äddéd Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмє#"
+
+#: apps/publisher_comments/emails.py
+msgid "Comment updated"
+msgstr "Çömmént üpdätéd Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт α#"
+
+#. Translators: subject_desc will be choice from ('New comment added',
+#. 'Comment updated'),
+#. 'pacing_type' will be choice from ('instructor-paced', 'self-paced'),
+#. 'title' and 'start' will be the value of course title & start date fields.
+#: apps/publisher_comments/emails.py
 #, python-brace-format
-msgid "New comment added in course run: {title}-{pacing_type}-{start}"
+msgid "{subject_desc} in course run: {title}-{pacing_type}-{start}"
 msgstr ""
-"Néw çömmént äddéd ïn çöürsé rün: {title}-{pacing_type}-{start} Ⱡ'σяєм ιρѕυм "
-"∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя #"
+"{subject_desc} ïn çöürsé rün: {title}-{pacing_type}-{start} Ⱡ'σяєм ιρѕυм "
+"∂σłσя ѕιт αмєт, ¢σηѕє¢т#"
 
+#. Translators: 'subject_desc' will be choice from ('New comment added',
+#. 'Comment updated')
+#. and 'title' will be the value of course title field.
 #: apps/publisher_comments/emails.py
 #, python-brace-format
-msgid "New comment added in Course: {title}"
-msgstr ""
-"Néw çömmént äddéd ïn Çöürsé: {title} Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тє#"
+msgid "{subject_desc} in Course: {title}"
+msgstr "{subject_desc} ïn Çöürsé: {title} Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт#"
 
 #: apps/publisher_comments/models.py
 msgid "modified"

course_discovery/conf/locale/eo/LC_MESSAGES/djangojs.po

@@ -7,14 +7,14 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-12-19 14:36+0500\n"
+"POT-Creation-Date: 2016-12-21 15:19+0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
-"Language: \n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
+"Language: \n"
 "Plural-Forms: nplurals=2; plural=(n != 1);\n"
 
 #: static/js/catalogs-change-form.js