Fixed an issue with mail sending logic.

Summary:
Here is the issue that we faced:
Consider two projects: ProjectA (notify author on failure = True), ProjectB (notify author on failure = False). If both projects were triggered by a commit, and ProjectA passed, ProjectB failed, the author would be notified anyway, because the setting from ProjectA was combined with the failure of ProjectB.

Test Plan: unit tests

Reviewers: kylec

Reviewed By: kylec

Subscribers: changesbot, anupc, kylec

Differential Revision: https://tails.corp.dropbox.com/D227634

changes/listeners/mail.py

@@ -105,14 +105,17 @@ def get_build_recipients(self, build):
         The build author is included unless the build and all failing jobs
         have turned off the mail.notify-author option.
 
-        Successful builds will only return the author.
+        Successful builds will return the empty list.
 
         Recipients are also collected from each failing job's
         mail.notify-addresses and mail.notify-addresses-revisions options.
         Should there be no failing jobs (is that possible?), recipients are
         collected from the build's own mail.notify-addresses and
         mail.notify-addresses-revisions options.
         """
+        if build.result == Result.passed:
+            return []
+
         recipients = []
         options = self.get_build_options(build)
 
@@ -121,9 +124,6 @@ def get_build_recipients(self, build):
             if author:
                 recipients.append(u'%s <%s>' % (author.name, author.email))
 
-        if build.result == Result.passed:
-            return recipients
-
         recipients.extend(options['mail.notify-addresses'])
         if build_type.is_initial_commit_build(build):
             recipients.extend(options['mail.notify-addresses-revisions'])

tests/changes/listeners/test_mail.py

@@ -76,6 +76,23 @@ def test_with_addressees(self):
             'bar@example.com',
         ]
 
+    def test_build_passed(self):
+        project = self.create_project()
+        db.session.add(ProjectOption(
+            project=project, name='mail.notify-author', value='1'))
+        db.session.add(ProjectOption(
+            project=project, name='mail.notify-addresses',
+            value='test@example.com, bar@example.com'))
+
+        author = self.create_author('foo@example.com')
+        build = self.create_build(project, result=Result.passed, author=author)
+        db.session.commit()
+
+        handler = MailNotificationHandler()
+        recipients = handler.get_build_recipients(build)
+
+        assert recipients == []
+
     def test_with_revision_addressees(self):
         project = self.create_project()
         db.session.add(ProjectOption(
@@ -122,6 +139,87 @@ def test_with_revision_addressees(self):
         ]
 
 
+class GetCollectionRecipientsTestCase(TestCase):
+
+    def test_diff_passed_and_failed(self):
+        project = self.create_project()
+        db.session.add(ProjectOption(
+            project=project, name='mail.notify-author', value='1'))
+        db.session.add(ProjectOption(
+            project=project, name='mail.notify-addresses-revisions',
+            value='test@example.com, bar@example.com'))
+
+        author = self.create_author('foo@example.com')
+        author_recipient = '{0} <{1}>'.format(author.name, author.email)
+
+        patch_build = self.create_build(
+            project=project,
+            source=self.create_source(project, patch=self.create_patch(repository=project.repository)),
+            author=author,
+            result=Result.passed,
+        )
+
+        project2 = self.create_project()
+        db.session.add(ProjectOption(
+            project=project2, name='mail.notify-author', value='0'))
+        db.session.add(ProjectOption(
+            project=project2, name='mail.notify-addresses-revisions',
+            value='test2@example.com, bar2@example.com'))
+
+        author2 = self.create_author('foo2@example.com')
+        author2_recipient = '{0} <{1}>'.format(author2.name, author2.email)
+
+        patch_build2 = self.create_build(
+            project=project2,
+            source=self.create_source(project2, patch=self.create_patch(repository=project2.repository)),
+            author=author2,
+            result=Result.failed,
+        )
+        db.session.commit()
+
+        recipients = MailNotificationHandler().get_collection_recipients({"builds": [{'build': patch_build}, {'build': patch_build2}]})
+        assert recipients == []
+
+    def test_diff_all_failed(self):
+        project = self.create_project()
+        db.session.add(ProjectOption(
+            project=project, name='mail.notify-author', value='1'))
+        db.session.add(ProjectOption(
+            project=project, name='mail.notify-addresses-revisions',
+            value='test@example.com, bar@example.com'))
+
+        author = self.create_author('foo@example.com')
+        author_recipient = '{0} <{1}>'.format(author.name, author.email)
+
+        patch_build = self.create_build(
+            project=project,
+            source=self.create_source(project, patch=self.create_patch(repository=project.repository)),
+            author=author,
+            result=Result.failed,
+        )
+
+        project2 = self.create_project()
+        db.session.add(ProjectOption(
+            project=project2, name='mail.notify-author', value='0'))
+        db.session.add(ProjectOption(
+            project=project2, name='mail.notify-addresses-revisions',
+            value='test2@example.com, bar2@example.com'))
+
+        author2 = self.create_author('foo2@example.com')
+        author2_recipient = '{0} <{1}>'.format(author2.name, author2.email)
+
+        patch_build2 = self.create_build(
+            project=project2,
+            source=self.create_source(project2, patch=self.create_patch(repository=project2.repository)),
+            author=author2,
+            result=Result.failed,
+        )
+        db.session.commit()
+
+        recipients = MailNotificationHandler().get_collection_recipients({"builds": [{'build': patch_build}, {'build': patch_build2}]})
+        assert recipients == [author_recipient]
+
+
 class SendTestCase(TestCase):
     @mock.patch.object(MailNotificationHandler, 'get_collection_recipients')
     def test_simple(self, get_collection_recipients):