launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/bugs-view-unauthorised-1060573 into lp:launchpad.

Commit message:
Add back functionality so that +bugs displays a message when unauthorised.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1060573 in Launchpad itself: "+bugs view raises Unauthorised instead of rendering a message when user has insufficient permissions"
  https://bugs.launchpad.net/launchpad/+bug/1060573

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/bugs-view-unauthorised-1060573/+merge/127626

== Implementation ==

With the dynamic bug listing feature flag removed, the +bugs view initialise method now performs a search() unconditionally. This raises Unauthorised if the user does not have view permission on the context, and neuters the display of a user friendly message, since the initialise happens before the TAL macro which displays the user message.

This branch adds a bit of logic to only initialise the +bugs view if the user has permission to view the context. This allows the user message to be displayed when required.

Also remove an incorrect XXX in a doc test since the bug is invalid.

== Tests ==

Re-instate the part of the test_user_without_launchpad_view() test which tests the +bugs view.
Re-run selected doc tests to check that the +bugs view works as expected normally.
xx-front-page-search.txt
xx-listing-basics.txt
xx-distribution-upstream-report.txt
etc

== Lint ==

Fond some lint in buglisting.js so fixed that.

Doc test noise, but otherwise lint free.
-- 
https://code.launchpad.net/~wallyworld/launchpad/bugs-view-unauthorised-1060573/+merge/127626
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/bugs-view-unauthorised-1060573 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2012-10-03 01:38:38 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-10-03 02:37:33 +0000
@@ -150,6 +150,7 @@
     )
 from lp.app.interfaces.launchpad import (
     ILaunchpadCelebrities,
+    IPrivacy,
     IServiceUsage,
     )
 from lp.app.vocabularies import InformationTypeVocabulary
@@ -2715,7 +2716,9 @@
 
         expose_structural_subscription_data_to_js(
             self.context, self.request, self.user)
-        if not FeedsLayer.providedBy(self.request):
+        can_view = (IPrivacy(self.context, None) is None
+            or check_permission('launchpad.View', self.context))
+        if can_view and not FeedsLayer.providedBy(self.request):
             cache = IJSONRequestCache(self.request)
             view_names = set(reg.name for reg
                 in iter_view_registrations(self.__class__))

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2012-09-12 22:09:06 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2012-10-03 02:37:33 +0000
@@ -90,7 +90,7 @@
         ATTRS: {
             field_visibility_defaults: { value: null },
             total: { value: null }
-        }    
+        }
     });
 
     /**
@@ -112,8 +112,8 @@
             },
 
             /**
-             * Return the model to use for rendering the batch.  This will include
-             * updates to field visibility.
+             * Return the model to use for rendering the batch.  This will
+             * include updates to field visibility.
              */
             get_render_model: function(current_batch) {
                 return Y.merge(
@@ -128,8 +128,7 @@
                     field_visibility: cache.field_visibility,
                     field_visibility_defaults: cache.field_visibility_defaults
                 });
-            },
-
+            }
         },{});
 
     /**
@@ -163,7 +162,7 @@
         navigator.clickAction('.last', navigator.last_batch);
         navigator.update_navigation_links();
         return navigator;
-    }
+    };
 
     /**
      * Helper view object for managing the buglisting code on the actual table

=== modified file 'lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.txt'
--- lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.txt	2012-10-02 06:36:44 +0000
+++ lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.txt	2012-10-03 02:37:33 +0000
@@ -203,10 +203,6 @@
 
 We display bug badges for associated branches, specifications, patches, etc.
 
-XXX wallyworld 2012-10-02 bug=1060014
-The dynamic bug listings feature has broken the rendering of branch links.
-Some doc tests have been altered till the functionality is restored.
-
     >>> def names_and_branches(contents):
     ...     listing = find_tag_by_id(contents, 'bugs-table-listing')
     ...     for row in listing.fetch(None, {'class': 'buglisting-row'}):

=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py	2012-10-03 01:34:16 +0000
+++ lib/lp/registry/browser/tests/test_team.py	2012-10-03 02:37:33 +0000
@@ -880,7 +880,7 @@
             archive.newSubscription(user, registrant=owner)
         with person_logged_in(user):
             for rootsite, view_name in [
-                (None, '+index'), ('code', '+branches'),
+                (None, '+index'), ('code', '+branches'), ('bugs', '+bugs'),
                 ('blueprints', '+specs'), ('answers', '+questions'),
                 ('translations', '+translations')]:
                 view = create_initialized_view(