Merge pull request #1051 from jfunez/1050_emails_editorialmanager

aprimorar envío de emails do editorialmanager #1050

Author: jamilatta

scielomanager/articletrack/notifications.py

@@ -3,6 +3,7 @@
 import logging
 
 from scielomanager import notifications
+from scielomanager.tools import user_receive_emails
 
 
 logger = logging.getLogger(__name__)
@@ -48,7 +49,7 @@ def set_recipients(self, checkin):
         and the submitter (checkin.submitted_by).
         """
         if checkin.team_members:
-            send_to = set([member.email for member in checkin.team_members])
+            send_to = set([member.email for member in checkin.team_members if user_receive_emails(member)])
             # the submitter already belong to a related team
             self.recipients = list(send_to)
         else:
@@ -88,11 +89,14 @@ def set_recipients(self, ticket):
         and the submitter (checkin.submitted_by, already belong to a team) of each checkin,
         and the author to the ticket related.
         """
-        send_to = set([ticket.author.email, ])
+        send_to = set()
+        if user_receive_emails(ticket.author):
+            send_to.update([unicode(ticket.author.email), ])
+
         for checkin in ticket.article.checkins.all():
             # the submitter already belong to a related team
             if checkin.team_members:
-                send_to.update([member.email for member in checkin.team_members])
+                send_to.update([member.email for member in checkin.team_members if user_receive_emails(member)])
             else:
                 logger.info("[TicketMessage.set_recipients] Can't prepare a message, checkin.team_members is empty. Checkin pk == %s" % checkin.pk)
         self.recipients = list(send_to)

scielomanager/articletrack/tests/tests_notifications.py

@@ -129,6 +129,36 @@ def test_each_action(self):
             for recipient in self.expected_recipients:
                 self.assertIn(recipient, new_email.to)
 
+    @override_settings(CELERY_EAGER_PROPAGATES_EXCEPTIONS=True, CELERY_ALWAYS_EAGER=True, BROKER_BACKEND='memory')
+    def test_each_action_with_disable_notifications_for_one_team_member(self):
+        """
+        for each action, test notifications are sent, but for one team member will have disable notifications
+        """
+        # disable email notifications for one team member (excludes submitter)
+        non_exclusion_users_pk = [self.ticket.author.pk , self.submitter.pk, ]
+        target_team_member_profile = self.team.member.all().exclude(pk__in=non_exclusion_users_pk)[0].get_profile()
+        target_team_member_profile.email_notifications = False
+        target_team_member_profile.save()
+
+        # update self.expected_recipients
+        self.expected_recipients.remove(target_team_member_profile.user.email)
+
+        email_count = 0
+        for action in self.ACTIONS:
+            email_count += 1
+            # when
+            result = notifications.ticket_send_mail_by_action(ticket=self.ticket, action=action)
+            # then
+            expected_subject = self._make_subject(action)
+
+            self.assertTrue(result)
+            self.assertEqual(len(mail.outbox), email_count)
+            new_email = mail.outbox[-1]
+            self.assertEqual(new_email.subject, expected_subject)
+            self.assertEqual(len(self.expected_recipients), len(new_email.to))
+            for recipient in self.expected_recipients:
+                self.assertIn(recipient, new_email.to)
+
 
 class CommentMessageTests(TestCase):
     ACTIONS = [
@@ -184,3 +214,33 @@ def test_each_action(self):
             self.assertEqual(len(self.expected_recipients), len(new_email.to))
             for recipient in self.expected_recipients:
                 self.assertIn(recipient, new_email.to)
+
+    @override_settings(CELERY_EAGER_PROPAGATES_EXCEPTIONS=True, CELERY_ALWAYS_EAGER=True, BROKER_BACKEND='memory')
+    def test_each_action_with_disable_notifications_for_one_team_member(self):
+        """
+        for each action, test notifications are sent, but for one team member will have disable notifications
+        """
+        # disable email notifications for one team member (excludes submitter and author for no particular reason)
+        non_exclusion_users_pk = [self.ticket.author.pk , self.submitter.pk, ]
+        target_team_member_profile = self.team.member.all().exclude(pk__in=non_exclusion_users_pk)[0].get_profile()
+        target_team_member_profile.email_notifications = False
+        target_team_member_profile.save()
+
+        # update self.expected_recipients
+        self.expected_recipients.remove(target_team_member_profile.user.email)
+
+        email_count = 0
+        for action in self.ACTIONS:
+            email_count += 1
+            # when
+            result = notifications.comment_send_mail_by_action(comment=self.comment, action=action)
+            # then
+            expected_subject = self._make_subject(action)
+
+            self.assertTrue(result)
+            self.assertEqual(len(mail.outbox), email_count)
+            new_email = mail.outbox[-1]
+            self.assertEqual(new_email.subject, expected_subject)
+            self.assertEqual(len(self.expected_recipients), len(new_email.to))
+            for recipient in self.expected_recipients:
+                self.assertIn(recipient, new_email.to)

scielomanager/editorialmanager/notifications.py

@@ -1,7 +1,8 @@
 # coding: utf-8
 
 import logging
-
+from django.core.exceptions import ObjectDoesNotExist
+from scielomanager.tools import get_users_by_group_by_collections, user_receive_emails
 from scielomanager import notifications
 
 
@@ -24,9 +25,12 @@ class IssueBoardMessage(notifications.Message):
     def set_recipients(self, issue):
         editor = getattr(issue.journal, 'editor', None)
         if editor:
-            self.recipients = [editor.email, ]
+            if user_receive_emails(editor):
+                self.recipients = [editor.email, ]
+            else:
+                logger.info("[IssueBoardMessage.set_recipients] editor (user.pk: %s) does not have a profile or decides to not receive emails." % editor.pk)
         else:
-            logger.info("[IssueBoardMessage.set_recipients] Can't prepare a message, issue.journal.editor is None or empty. Issue pk == %s" % issue.pk)
+            logger.error("[IssueBoardMessage.set_recipients] Can't prepare a message, issue.journal.editor is None or empty. Issue pk == %s" % issue.pk)
 
 
 class BoardMembersMessage(notifications.Message):
@@ -46,14 +50,25 @@ class BoardMembersMessage(notifications.Message):
         }
     }
 
-    def set_recipients(self, member):
-        from scielomanager.tools import get_users_by_group
-        from django.core.exceptions import ObjectDoesNotExist
-        try:
-            librarians = get_users_by_group('Librarian')
-            self.recipients = [user.email for user in librarians if user.email]
-        except ObjectDoesNotExist:
-            logger.info("[BoardMembersMessage.set_recipients] Can't prepare a message, Can't retrieve a list of Librarian Users.")
+    def set_recipients(self):
+        """ emails must be sent as BCC """
+        self.recipients = []
+
+    def set_bcc_recipients(self, member):
+        """ recipients must belong to the same collection as member """
+        collections_of_board_member = member.board.issue.journal.collections.all()
+
+        if collections_of_board_member:
+            librarians = get_users_by_group_by_collections('Librarian', collections_of_board_member)
+        else:
+            logger.error("[BoardMembersMessage.set_bcc_recipients] Can't define the collection of member (pk: %s), to filter bcc_recipients" % member.pk)
+            return
+
+        if librarians:
+            filtered_librarians = [librarian for librarian in librarians if user_receive_emails(librarian)]
+            self.bcc_recipients = map(lambda u: u.email, filtered_librarians)
+        else:
+            logger.error("[BoardMembersMessage.set_bcc_recipients] Can't prepare a message, Can't retrieve a list of Librarian Users.")
 
 
 def issue_board_replica(issue, action):
@@ -64,14 +79,15 @@ def issue_board_replica(issue, action):
     return message.send_mail()
 
 
-def board_members_send_email_by_action(member, user, message, action):
+def board_members_send_email_by_action(member, user, audit_log_msg, action):
     message = BoardMembersMessage(action=action)
-    message.set_recipients(member)
+    message.set_recipients()
+    message.set_bcc_recipients(member)
     extra_context = {
         'user': user,
         'member': member,
         'issue': member.board.issue,
-        'message': message,
+        'message': audit_log_msg,
     }
     message.render_body(extra_context)
     return message.send_mail()

scielomanager/editorialmanager/tests/tests_notifications.py

@@ -5,7 +5,7 @@
 from django.test import TestCase
 from django.test.utils import override_settings
 from django_factory_boy import auth
-
+from journalmanager import models as jmodels
 from journalmanager.tests import modelfactories
 from editorialmanager import notifications
 from . import modelfactories as editorial_modelfactories
@@ -21,7 +21,7 @@ def setUp(self):
         self.editor = auth.UserF(is_active=True)
         self.journal = modelfactories.JournalFactory.create(editor=self.editor)
         self.issue = modelfactories.IssueFactory(journal=self.journal)
-        self.expected_recipients = [self.editor, ]
+        self.expected_recipients = [self.editor.email, ]
         self.expected_subject_sufix_by_action = {
             'issue_add_no_replicated_board': "Issue Board can't be replicated",
             'issue_add_replicated_board': "Issue has a new replicated board",
@@ -53,7 +53,27 @@ def test_each_action(self):
             self.assertEqual(new_email.subject, expected_subject)
             self.assertEqual(len(self.expected_recipients), len(new_email.to))
             for recipient in self.expected_recipients:
-                self.assertIn(recipient.email, new_email.to)
+                self.assertIn(recipient, new_email.to)
+
+    @override_settings(CELERY_EAGER_PROPAGATES_EXCEPTIONS=True, CELERY_ALWAYS_EAGER=True, BROKER_BACKEND='memory')
+    def test_each_action_with_disable_notifications_for_editor(self):
+        """
+        for each action, test notifications are sent, but editor will have disable notifications
+        """
+        editor_profile = self.editor.get_profile()
+        editor_profile.email_notifications = False
+        editor_profile.save()
+        # update self.expected_recipients
+        self.expected_recipients.remove(editor_profile.user.email)
+
+        for action in self.ACTIONS:
+            # with
+            expected_subject = self._make_subject(action)
+            # when
+            result = notifications.issue_board_replica(issue=self.issue, action=action)
+            # then
+            # no mail sent, since the only recipient: editor, "choose" to not receive emails
+            self.assertIsNone(result)
 
 
 class BoardMembersMessageTests(TestCase):
@@ -75,13 +95,22 @@ def setUp(self):
         self.librarian2.groups.add(self.librarian_group)
         self.librarian2.save()
 
+        self.collection = modelfactories.CollectionFactory.create()
         self.editor = auth.UserF(is_active=True)
         self.journal = modelfactories.JournalFactory.create(editor=self.editor)
         self.issue = modelfactories.IssueFactory(journal=self.journal)
         self.board = editorial_modelfactories.EditorialBoardFactory.create(issue=self.issue)
         self.member = editorial_modelfactories.EditorialMemberFactory.create(board=self.board)
 
-        self.expected_recipients = [self.librarian1, self.librarian2, ]
+        # link journal to collection
+        jmodels.Membership.objects.create(journal=self.journal, collection=self.collection, created_by=auth.UserF(is_active=True))
+
+        # link librarians and collection
+        self.collection.add_user(self.librarian1)
+        self.collection.add_user(self.librarian2)
+
+        self.expected_recipients = []
+        self.expected_bcc_recipients = [self.librarian1.email, self.librarian2.email, ]
         self.expected_subject_sufix_by_action = {
             'board_add_member': "Member of the journal board, was added",
             'board_edit_member': "Member of the journal board, was edited",
@@ -99,7 +128,39 @@ def test_each_action(self):
         for action in self.ACTIONS:
             # with
             expected_subject = self._make_subject(action)
-            message = ''
+            message = 'Audit Log change message goes here!'
+            email_count += 1
+            # when
+            result = notifications.board_members_send_email_by_action(self.member, self.editor, message, action)
+            # then
+            self.assertTrue(result)
+            self.assertEqual(len(mail.outbox), email_count)
+            new_email = mail.outbox[-1]
+            self.assertEqual(new_email.subject, expected_subject)
+            self.assertEqual(len(self.expected_recipients), len(new_email.to))
+            self.assertEqual(len(self.expected_bcc_recipients), len(new_email.bcc))
+            for recipient in self.expected_recipients:
+                self.assertIn(recipient, new_email.to)
+            for recipient in self.expected_bcc_recipients:
+                self.assertIn(recipient, new_email.bcc)
+
+
+    @override_settings(CELERY_EAGER_PROPAGATES_EXCEPTIONS=True, CELERY_ALWAYS_EAGER=True, BROKER_BACKEND='memory')
+    def test_each_action_with_disable_notifications_for_one_librarian(self):
+        """
+        for each action, test notifications are sent, but librarian2 will have disable notifications
+        """
+        librarian2_profile = self.librarian2.get_profile()
+        librarian2_profile.email_notifications = False
+        librarian2_profile.save()
+        # remove it from expected_bcc_recipients
+        self.expected_bcc_recipients.remove(librarian2_profile.user.email)
+
+        email_count = 0
+        for action in self.ACTIONS:
+            # with
+            expected_subject = self._make_subject(action)
+            message = 'Audit Log change message goes here!'
             email_count += 1
             # when
             result = notifications.board_members_send_email_by_action(self.member, self.editor, message, action)
@@ -109,5 +170,8 @@ def test_each_action(self):
             new_email = mail.outbox[-1]
             self.assertEqual(new_email.subject, expected_subject)
             self.assertEqual(len(self.expected_recipients), len(new_email.to))
+            self.assertEqual(len(self.expected_bcc_recipients), len(new_email.bcc))
             for recipient in self.expected_recipients:
-                self.assertIn(recipient.email, new_email.to)
+                self.assertIn(recipient, new_email.to)
+            for recipient in self.expected_bcc_recipients:
+                self.assertIn(recipient, new_email.bcc)

scielomanager/journalmanager/admin.py

@@ -2,6 +2,7 @@
 from django.contrib import admin
 from django.contrib.auth.models import User
 from django.contrib.auth.admin import UserAdmin
+from django.contrib.admin.templatetags.admin_static import static
 from tastypie.models import ApiAccess
 
 from .models import *
@@ -91,6 +92,22 @@ def queryset(self, request):
 
 
 class UserAdmin(UserAdmin):
+    def profile_email_notifications(self, user):
+        """
+        display a green/red icon if user accepts or not email notifications.
+        if for some reason, the user don't have a UserProfile, it returns a (?) icon.
+        """
+        if user.get_profile():
+            email_notifications = user.get_profile().email_notifications
+            icon_url = static('admin/img/icon-%s.gif' % {True: 'yes', False: 'no', None: 'unknown'}[email_notifications])
+            return '<img src="{0}" alt="{1}" />'.format(icon_url, email_notifications)
+        else:
+            icon_url = static('admin/img/icon-icon-unknown.gif')
+            return '<img src="{0}" alt="NO PROFILE" />no profile!' % icon_url
+    profile_email_notifications.short_description = 'Email Notifications'
+    profile_email_notifications.allow_tags = True
+
+    list_display = ('username', 'email', 'first_name', 'last_name', 'is_staff', 'profile_email_notifications', )
     inlines = (UserProfileInline, UserCollectionsInline)
     form = UserChangeForm
     add_form = UserCreationForm

scielomanager/journalmanager/forms.py

@@ -755,6 +755,11 @@ def get_all_ahead_pressrelease_forms(post_dict, journal, pressrelease):
     return d
 
 
+class UserProfileForm(ModelForm):
+    class Meta:
+        model = models.UserProfile
+        fields = ('email_notifications',)
+
 class UserCollectionsForm(ModelForm):
     def __init__(self, *args, **kwargs):
         """