launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:hide-announcements-inactive-projects into launchpad:master.

Commit message:
Hide announcements from inactive projects, and from users without karma

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1884716 in Launchpad itself: "AnnouncementSet:+index shows announcements from inactive projects"
  https://bugs.launchpad.net/launchpad/+bug/1884716

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/386286
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:hide-announcements-inactive-projects into launchpad:master.

diff --git a/lib/lp/registry/browser/tests/test_announcements.py b/lib/lp/registry/browser/tests/test_announcements.py
index c71e8cb..078ccfb 100644
--- a/lib/lp/registry/browser/tests/test_announcements.py
+++ b/lib/lp/registry/browser/tests/test_announcements.py
@@ -1,16 +1,27 @@
-# Copyright 2011-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for +announcement views."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
 from datetime import datetime
 
 from lxml import html
 from pytz import utc
+import six
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
+from lp.registry.enums import TeamMembershipPolicy
+from lp.registry.interfaces.announcement import IAnnouncementSet
+from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.registry.model.announcement import Announcement
+from lp.services.database.interfaces import IStore
 from lp.testing import (
+    BrowserTestCase,
     normalize_whitespace,
     TestCaseWithFactory,
     )
@@ -45,3 +56,76 @@ class TestAnnouncement(TestCaseWithFactory):
         self.assertEqual(
             "Written for Foo by Bar Baz on 2007-01-12",
             normalize_whitespace(reg_para.text_content()))
+
+
+class TestAnnouncementPage(BrowserTestCase):
+    layer = LaunchpadFunctionalLayer
+
+    def assertShowsAnnouncements(self, context, view_name=None, user=None,
+                                 cleanup_announcements=True):
+        # cleanup announcements from test data to make sure we are not
+        # hiding new announcements because of pagination.
+        if cleanup_announcements:
+            store = IStore(Announcement)
+            for i in store.find(Announcement):
+                store.remove(i)
+            store.flush()
+
+        real_user = self.factory.makePerson(karma=500)
+        team = self.factory.makeTeam(
+            membership_policy=TeamMembershipPolicy.MODERATED)
+        spammer = self.factory.makePerson(karma=0)
+
+        if IProjectGroup.providedBy(context):
+            project_group = context
+        else:
+            project_group = None
+        first_product = self.factory.makeProduct(
+            owner=real_user, projectgroup=project_group)
+        first_announcement = first_product.announce(
+            real_user, "Some real announcement", "Yep, announced here",
+            publication_date=datetime.now(utc))
+
+        second_product = self.factory.makeProduct(
+            owner=team, projectgroup=project_group)
+        second_announcement = second_product.announce(
+            team, "Other real announcement", "Yep too, announced here",
+            publication_date=datetime.now(utc))
+
+        inactive_product = self.factory.makeProduct(
+            owner=real_user, projectgroup=project_group)
+        inactive_announcement = inactive_product.announce(
+            real_user, "Do not show inactive, please", "Nope, not here",
+            publication_date=datetime.now(utc))
+        removeSecurityProxy(inactive_product).active = False
+
+        spam_product = self.factory.makeProduct(
+            owner=spammer, projectgroup=project_group)
+        spam_announcement = spam_product.announce(
+            spammer, "This is just spam", "Buy something now",
+            publication_date=datetime.now(utc))
+
+        browser = self.getViewBrowser(context, view_name, user=user)
+        contents = six.ensure_str(browser.contents, "utf-8")
+
+        self.assertIn(first_announcement.title, contents)
+        self.assertIn(first_announcement.summary, contents)
+
+        self.assertIn(second_announcement.title, contents)
+        self.assertIn(second_announcement.summary, contents)
+
+        self.assertNotIn(inactive_announcement.title, contents)
+        self.assertNotIn(inactive_announcement.summary, contents)
+
+        self.assertNotIn(spam_announcement.title, contents)
+        self.assertNotIn(spam_announcement.summary, contents)
+
+    def test_announcement_page_filter_out_inactive_projects(self):
+        user = self.factory.makePerson()
+        context = getUtility(IAnnouncementSet)
+        self.assertShowsAnnouncements(context, None, user)
+
+    def test_project_group_announcement_filter_out_inactive_projects(self):
+        user = self.factory.makePerson()
+        context = self.factory.makeProject()  # actually, a IProjectGroup
+        self.assertShowsAnnouncements(context, None, user)
diff --git a/lib/lp/registry/model/announcement.py b/lib/lp/registry/model/announcement.py
index addbcad..424895c 100644
--- a/lib/lp/registry/model/announcement.py
+++ b/lib/lp/registry/model/announcement.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes for project news and announcement."""
@@ -169,17 +169,41 @@ class HasAnnouncements:
                     (Announcement.product = %s OR Announcement.project = %s)
                     """ % sqlvalues(self.id, self.projectgroup)
         elif IProjectGroup.providedBy(self):
-            query += """ AND
-                (Announcement.project = %s OR Announcement.product IN
-                    (SELECT id FROM Product WHERE project = %s))
+            query += """ AND (
+                Announcement.project = %s
+                OR Announcement.product IN (
+                    SELECT Product.id FROM Product
+                    INNER JOIN Person
+                        ON Person.id = Product.owner
+                    WHERE
+                        Product.project = %s AND Product.active
+                        AND (
+                            Person.teamOwner IS NOT NULL
+                            OR EXISTS (
+                                SELECT 1 FROM KarmaTotalCache
+                                WHERE person = Product.owner
+                                    AND karma_total > 0))))
                     """ % sqlvalues(self.id, self.id)
         elif IDistribution.providedBy(self):
             query += (' AND Announcement.distribution = %s'
                 % sqlvalues(self.id))
         elif IAnnouncementSet.providedBy(self):
-            # There is no need to filter for pillar if we are looking for
-            # all announcements.
-            pass
+            # Just filter out inactive projects, mostly to exclude spam.
+            query += """ AND (
+                Announcement.product IS NULL
+                OR EXISTS (
+                    SELECT 1 FROM Product
+                    INNER JOIN Person
+                        ON Person.id = Product.owner
+                    WHERE
+                        Product.id = Announcement.product AND Product.active
+                        AND (
+                            Person.teamOwner IS NOT NULL
+                            OR EXISTS (
+                                SELECT 1 FROM KarmaTotalCache
+                                WHERE person = Product.owner
+                                    AND karma_total > 0))))
+                """
         else:
             raise AssertionError('Unsupported announcement target')
         return Announcement.select(query, limit=limit)