launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/dds-req-packagediff-fix-perms-746267 into lp:launchpad with lp:~rvb/launchpad/dds-req-packagediff-fix-flashing as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #746267 in Launchpad itself: "A user without the appropriate permissions should not see the link to sync version on the localpackagediff page."
  https://bugs.launchpad.net/launchpad/+bug/746267

For more details, see:
https://code.launchpad.net/~rvb/launchpad/dds-req-packagediff-fix-perms-746267/+merge/55778

This branch fixes the display of the link to request package diffs so that it appears a link only to users with lp.Edit permission on the dsd.

== Tests ==
{{{
./bin/test -cvv test_distroseriesdifference_views test_package_diff_request_link
}}}

== QA ==
As a non privileged user, make sure there is no link to request package diffs.
(https://dogfood.launchpad.net/deribuntu/dangerous/+localpackagediffs)


-- 
https://code.launchpad.net/~rvb/launchpad/dds-req-packagediff-fix-perms-746267/+merge/55778
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/dds-req-packagediff-fix-perms-746267 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
--- lib/lp/registry/browser/distroseriesdifference.py	2011-03-31 07:18:49 +0000
+++ lib/lp/registry/browser/distroseriesdifference.py	2011-03-31 15:49:21 +0000
@@ -132,11 +132,28 @@
         return self.request.is_ajax and check_permission(
             'launchpad.Edit', self.context)
 
+<<<<<<< TREE
     @property
     def display_child_diff(self):
         """Only show the child diff if we need to."""
         return self.context.source_version != self.context.base_version
 
+=======
+    @property
+    def show_package_diffs_request_link(self):
+        """Return whether package diffs can be requested.
+
+        At least one of the package diffs for this dsd must be missing
+        and the user must have lp.Edit.
+
+        This method is used in the template to show the package diff
+        request link.
+        """
+        return (check_permission('launchpad.Edit', self.context) and
+                (not self.context.package_diff or
+                 not self.context.parent_package_diff))
+
+>>>>>>> MERGE-SOURCE
 
 class DistroSeriesDifferenceDisplayComment:
     """Used simply to provide `IComment` for rendering."""

=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_views.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_views.py	2011-03-31 10:02:21 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_views.py	2011-03-31 15:49:21 +0000
@@ -7,7 +7,14 @@
 
 from BeautifulSoup import BeautifulSoup
 import re
+<<<<<<< TREE
 import transaction
+=======
+
+from BeautifulSoup import BeautifulSoup
+import soupmatchers
+from testtools.matchers import Not
+>>>>>>> MERGE-SOURCE
 from zope.component import getUtility
 
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
@@ -388,3 +395,25 @@
         self.assertEqual(
             DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
             view.initial_values.get('blacklist_options'))
+
+    def test_package_diff_request_link(self):
+        # The link to compute package diffs is only shown to
+        # a user with lp.Edit persmission.
+        ds_diff = self.factory.makeDistroSeriesDifference()
+        package_diff_request_matcher = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Request link', 'a',
+                text=re.compile(
+                    '\s*Compute differences from last common version\s*')))
+
+        with person_logged_in(self.factory.makePerson()):
+            view = create_initialized_view(
+                ds_diff, '+listing-distroseries-extra')
+            self.assertFalse(view.show_package_diffs_request_link)
+            self.assertThat(view(), Not(package_diff_request_matcher))
+
+        with celebrity_logged_in('admin'):
+            view = create_initialized_view(
+                ds_diff, '+listing-distroseries-extra')
+            self.assertThat(view(), package_diff_request_matcher)
+            self.assertTrue(view.show_package_diffs_request_link)

=== modified file 'lib/lp/registry/templates/distroseriesdifference-listing-extra.pt'
--- lib/lp/registry/templates/distroseriesdifference-listing-extra.pt	2011-03-31 15:48:57 +0000
+++ lib/lp/registry/templates/distroseriesdifference-listing-extra.pt	2011-03-31 15:49:21 +0000
@@ -40,7 +40,7 @@
     <dt>Last common version:</dt>
     <dd tal:content="context/base_version">1.2.1</dd>
     <dt
-      tal:condition="python: not(context.package_diff) or not(context.parent_package_diff)"
+      tal:condition="view/show_package_diffs_request_link"
       class="package-diff-placeholder">
         <span class="package-diff-compute-request">
           <a class="js-action sprite add" href="">
@@ -48,7 +48,7 @@
         </span>
     </dt>
     <dt
-      tal:condition="python: context.package_diff and context.parent_package_diff">
+      tal:condition="python: not view.show_package_diffs_request_link">
       Differences from last common version:
     </dt>
     <dd>