launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/no-private-bug-rss into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #302097 in Launchpad itself: "Trying to access rss feeds of a private bug OOPSes"
  https://bugs.launchpad.net/launchpad/+bug/302097

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/no-private-bug-rss/+merge/61572

This branch stops exposing rss feeds for private bugs or branches.

== Implementation ==

Add a class method feed_allowed() to FeedLinkBase, default to returning True. However, this is overridden in BranchFeedLink and BugFeedLink to return true only if the branch/bug is not private.

TODO: when a user uses the inline ajax widget to change the privacy status, the rss feed status is not toggled. This will be done in a subsequent branch.

== Tests ==

Extends the xx-links.txt doctest to check that private bugs and branches pages do not render the rss link element.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/browser/feeds.py
  lib/canonical/launchpad/pagetests/feeds/xx-links.txt

./lib/canonical/launchpad/pagetests/feeds/xx-links.txt
       1: narrative uses a moin header.
       9: narrative uses a moin header.
      33: narrative uses a moin header.
      58: narrative uses a moin header.
      88: narrative uses a moin header.
     122: narrative uses a moin header.
     149: want exceeds 78 characters.
     152: want exceeds 78 characters.
     155: want exceeds 78 characters.
     158: want exceeds 78 characters.
     160: narrative uses a moin header.
     208: narrative uses a moin header.
     235: narrative uses a moin header.
     248: narrative uses a moin header.
     261: narrative uses a moin header.
     270: want exceeds 78 characters.
     274: narrative uses a moin header.
     290: narrative uses a moin header.
     306: narrative uses a moin header.
     322: narrative uses a moin header.
     332: want exceeds 78 characters.
     333: want exceeds 78 characters.
 
-- 
https://code.launchpad.net/~wallyworld/launchpad/no-private-bug-rss/+merge/61572
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/no-private-bug-rss into lp:launchpad.

=== modified file 'lib/canonical/launchpad/browser/feeds.py'
--- lib/canonical/launchpad/browser/feeds.py	2010-11-08 12:52:43 +0000
+++ lib/canonical/launchpad/browser/feeds.py	2011-05-19 13:47:33 +0000
@@ -177,6 +177,14 @@
             "Context %r does not provide interface %r"
             % (context, self.usedfor))
 
+    @classmethod
+    def feed_allowed(cls, context):
+        """Return True if a feed is allowed for the given context.
+
+        Subclasses should override this method as necessary.
+        """
+        return True
+
 
 class BugFeedLink(FeedLinkBase):
     usedfor = IBugTask
@@ -190,6 +198,12 @@
         return urlappend(self.rooturl,
                          'bugs/' + str(self.context.bug.id) + '/bug.atom')
 
+    @classmethod
+    def feed_allowed(cls, context):
+        """See `FeedLinkBase`"""
+        # No feeds for private bugs.
+        return not context.bug.private
+
 
 class BugTargetLatestBugsFeedLink(FeedLinkBase):
     usedfor = IHasBugs
@@ -222,6 +236,7 @@
             return urlappend(canonical_url(self.context, rootsite='feeds'),
                              'announcements.atom')
 
+
 class RootAnnouncementsFeedLink(AnnouncementsFeedLink):
     usedfor = ILaunchpadRoot
 
@@ -296,11 +311,18 @@
     @property
     def title(self):
         return 'Latest Revisions for Branch %s' % self.context.displayname
+
     @property
     def href(self):
         return urlappend(canonical_url(self.context, rootsite="feeds"),
                          'branch.atom')
 
+    @classmethod
+    def feed_allowed(cls, context):
+        """See `FeedLinkBase`"""
+        # No feeds for private branches.
+        return not context.private
+
 
 class PersonRevisionsFeedLink(FeedLinkBase):
     """Feed links for revisions created by a person."""
@@ -346,4 +368,5 @@
     def feed_links(self):
         return [feed_type(self.context)
                 for feed_type in self.feed_types
-                if feed_type.usedfor.providedBy(self.context)]
+                if feed_type.usedfor.providedBy(self.context) and
+                feed_type.feed_allowed(self.context)]

=== modified file 'lib/canonical/launchpad/pagetests/feeds/xx-links.txt'
--- lib/canonical/launchpad/pagetests/feeds/xx-links.txt	2011-05-05 05:46:41 +0000
+++ lib/canonical/launchpad/pagetests/feeds/xx-links.txt	2011-05-19 13:47:33 +0000
@@ -42,6 +42,18 @@
         href="http://feeds.launchpad.dev/bugs/1/bug.atom";
         title="Bug 1 Feed" />]
 
+But if the bug is private, there should be no link.
+
+    >>> from canonical.launchpad.testing.pages import setupBrowserForUser
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> login(ANONYMOUS)
+    >>> user = getUtility(IPersonSet).getByEmail('daf@xxxxxxxxxxxxx')
+    >>> auth_browser = setupBrowserForUser(user, 'daf')
+    >>> auth_browser.open('http://bugs.launchpad.dev/jokosher/+bug/14')
+    >>> soup = BeautifulSoup(auth_browser.contents)
+    >>> soup.head.findAll('link', type='application/atom+xml')
+    []
 
 == Latest Bugs and Branches for a Person ==
 
@@ -319,3 +331,14 @@
     [<link rel="alternate" type="application/atom+xml"
         href="http://feeds.launchpad.dev/~mark/firefox/release--0.9.1/branch.atom";
 	title="Latest Revisions for Branch lp://dev/~mark/firefox/release--0.9.1" />]
+
+But if the branch is private, there should be no link.
+
+    >>> login(ANONYMOUS)
+    >>> user = getUtility(IPersonSet).getByEmail('test@xxxxxxxxxxxxx')
+    >>> auth_browser = setupBrowserForUser(user)
+    >>> auth_browser.open(
+    ... 'https://code.launchpad.dev/~name12/landscape/feature-x')
+    >>> soup = BeautifulSoup(auth_browser.contents)
+    >>> soup.head.findAll('link', type='application/atom+xml')
+    []