launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-639703-pg-bugs into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #639703 project group bug page is active when no sub projects use bugs
  https://bugs.launchpad.net/bugs/639703


= Summary =

Currently if a project group has sub-projects but none use Launchpad for
bug tracking a user will be allowed to attempt to file a bug.  At the
next step the user is informed there are no projects that track bugs in LP.

== Proposed fix ==

Prevent such a project group from displaying the portlets and providing
links to bug reporting and other bug activities.

== Pre-implementation notes ==

Talk with Curtis

== Implementation details ==

As above

== Tests ==

bin/test -vvm lp.bugs -t test_bugtask

== Demo and Q/A ==

Create a project group and see that it states it does not use Launchpad
for bug tracking.  Add a project that does not use bug tracking and
ensure the message is the same.  Set one of the project to do bug
tracking and observe the change in the page.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/testing/views.py
  lib/lp/bugs/templates/buglisting-default.pt
  lib/lp/bugs/browser/tests/test_bugtask.py
  lib/lp/bugs/browser/bugtask.py

./lib/lp/bugs/browser/bugtask.py
    2201: Line exceeds 78 characters.
-- 
https://code.launchpad.net/~bac/launchpad/bug-639703-pg-bugs/+merge/37463
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-639703-pg-bugs into lp:launchpad/devel.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2010-09-28 14:50:19 +0000
+++ lib/lp/bugs/browser/bugtask.py	2010-10-04 12:55:59 +0000
@@ -168,6 +168,7 @@
     ObjectImageDisplayAPI,
     PersonFormatterAPI,
     )
+
 from canonical.lazr.interfaces import IObjectPrivacy
 from canonical.lazr.utils import smartquote
 from canonical.widgets.bug import BugTagsWidget
@@ -2987,8 +2988,17 @@
             return False
 
     @property
+    def should_show_bug_information(self):
+        """Return False if a project group that does not use Launchpad."""
+        if not self._projectContext():
+            return True
+        involvement = getMultiAdapter((self.context, self.request),
+                                      name="+get-involved")
+        return involvement.official_malone
+
+    @property
     def form_has_errors(self):
-        """Return True of the form has errors, otherwise False."""
+        """Return True if the form has errors, otherwise False."""
         return len(self.errors) > 0
 
     def validateVocabulariesAdvancedForm(self):

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2010-09-23 14:51:48 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2010-10-04 12:55:59 +0000
@@ -19,6 +19,7 @@
     login,
     login_person,
     )
+from canonical.launchpad.testing.pages import find_tag_by_id
 from canonical.launchpad.testing.systemdocs import (
     LayeredDocFileSuite,
     setUp,
@@ -33,7 +34,10 @@
     BugTasksAndNominationsView,
     )
 from lp.bugs.interfaces.bugtask import BugTaskStatus
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 from lp.testing._webservice import QueryCollector
 from lp.testing.matchers import HasQueryCount
 from lp.testing.sampledata import (
@@ -41,6 +45,7 @@
     NO_PRIVILEGE_EMAIL,
     USER_EMAIL,
     )
+from lp.testing.views import create_initialized_view
 
 
 class TestBugTaskView(TestCaseWithFactory):
@@ -69,7 +74,8 @@
         self.addCleanup(recorder.unregister)
         self.invalidate_caches(task)
         self.getUserBrowser(url, person_no_teams)
-        # This may seem large: it is; there is easily another 30% fat in there.
+        # This may seem large: it is; there is easily another 30% fat in
+        # there.
         self.assertThat(recorder, HasQueryCount(LessThan(62)))
         count_with_no_teams = recorder.count
         # count with many teams
@@ -418,6 +424,88 @@
             view.form_fields['assignee'].field.vocabularyName)
 
 
+class TestProjectGroupBugs(TestCaseWithFactory):
+    """Test the bugs overview page for Project Groups."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestProjectGroupBugs, self).setUp()
+        self.owner = self.factory.makePerson(name='bob')
+        self.projectgroup = self.factory.makeProject(name='container',
+                                                     owner=self.owner)
+
+    def makeSubordinateProduct(self, tracks_bugs_in_lp):
+        """Create a new product and add it to the project group."""
+        product = self.factory.makeProduct(official_malone=tracks_bugs_in_lp)
+        with person_logged_in(product.owner):
+            product.project = self.projectgroup
+        expected = {True: 'LAUNCHPAD',
+                    False: 'UNKNOWN',
+                    }
+        self.assertEqual(
+            expected[tracks_bugs_in_lp], product.bug_tracking_usage.name)
+
+    def test_empty_project_group(self):
+        # An empty project group does not use Launchpad for bugs.
+        view = create_initialized_view(
+            self.projectgroup, name=u'+bugs', rootsite='bugs')
+        self.assertFalse(self.projectgroup.hasProducts())
+        self.assertFalse(view.should_show_bug_information)
+
+    def test_project_group_with_subordinate_not_using_launchpad(self):
+        # A project group with all subordinates not using Launchpad
+        # will itself be marked as not using Launchpad for bugs.
+        self.makeSubordinateProduct(False)
+        self.assertTrue(self.projectgroup.hasProducts())
+        view = create_initialized_view(
+            self.projectgroup, name=u'+bugs', rootsite='bugs')
+        self.assertFalse(view.should_show_bug_information)
+
+    def test_project_group_with_subordinate_using_launchpad(self):
+        # A project group with one subordinate using Launchpad
+        # will itself be marked as using Launchpad for bugs.
+        self.makeSubordinateProduct(True)
+        self.assertTrue(self.projectgroup.hasProducts())
+        view = create_initialized_view(
+            self.projectgroup, name=u'+bugs', rootsite='bugs')
+        self.assertTrue(view.should_show_bug_information)
+
+    def test_project_group_with_mixed_subordinates(self):
+        # A project group with one or more subordinates using Launchpad
+        # will itself be marked as using Launchpad for bugs.
+        self.makeSubordinateProduct(False)
+        self.makeSubordinateProduct(True)
+        self.assertTrue(self.projectgroup.hasProducts())
+        view = create_initialized_view(
+            self.projectgroup, name=u'+bugs', rootsite='bugs')
+        self.assertTrue(view.should_show_bug_information)
+
+    def test_project_group_has_no_portlets_if_not_using_LP(self):
+        # A project group that has no projects using Launchpad will not have a
+        # 'Report a bug' link.
+        self.makeSubordinateProduct(False)
+        view = create_initialized_view(
+            self.projectgroup, name=u'+bugs', rootsite='bugs',
+            current_request=True)
+        self.assertFalse(view.should_show_bug_information)
+        contents = view.render()
+        report_a_bug = find_tag_by_id(contents, 'bug-portlets')
+        self.assertIs(None, report_a_bug)
+
+    def test_project_group_has_portlets_link_if_using_LP(self):
+        # A project group that has no projects using Launchpad will not have a
+        # 'Report a bug' link.
+        self.makeSubordinateProduct(True)
+        view = create_initialized_view(
+            self.projectgroup, name=u'+bugs', rootsite='bugs',
+            current_request=True)
+        self.assertTrue(view.should_show_bug_information)
+        contents = view.render()
+        report_a_bug = find_tag_by_id(contents, 'bug-portlets')
+        self.assertIsNot(None, report_a_bug)
+
+
 def test_suite():
     suite = unittest.TestLoader().loadTestsFromName(__name__)
     suite.addTest(DocTestSuite(bugtask))

=== modified file 'lib/lp/bugs/templates/buglisting-default.pt'
--- lib/lp/bugs/templates/buglisting-default.pt	2009-09-16 17:05:13 +0000
+++ lib/lp/bugs/templates/buglisting-default.pt	2010-10-04 12:55:59 +0000
@@ -21,51 +21,62 @@
             associate an existing project with it.
           </p>
         </tal:block>
+        <tal:has_products condition="context/products">
+          <p condition="not:view/should_show_bug_information">
+            <tal:project replace="context/displayname" /> does not have any
+            projects that use Launchpad for bug tracking so you may not file a
+            bug against this project group.
+          </p>
+        </tal:has_products>
       </tal:block>
-      <h1
-        tal:condition="view/shouldShowAdvancedForm"
-        tal:content="view/getAdvancedSearchPageHeading"
-      >Bugs in Fooix: Advanced search</h1>
-
-      <tal:do_not_show_advanced_form
-        condition="not: view/shouldShowAdvancedForm">
-      <div tal:define="batch_navigator view/search">
-
-        <div metal:use-macro="context/@@+bugtarget-macros-search/simple-search-form" />
-        <tal:buglisting content="structure batch_navigator/@@+table-view" />
-      </div>
-      </tal:do_not_show_advanced_form>
-
-      <tal:show_advanced_form condition="view/shouldShowAdvancedForm">
-        <metal:advanced_form
-          use-macro="context/@@+bugtask-macros-tableview/advanced_search_form" />
-      </tal:show_advanced_form>
+      <tal:show_bug_info condition="view/should_show_bug_information">
+        <h1
+          tal:condition="view/shouldShowAdvancedForm"
+          tal:content="view/getAdvancedSearchPageHeading"
+        >Bugs in Fooix: Advanced search</h1>
+
+        <tal:do_not_show_advanced_form
+          condition="not: view/shouldShowAdvancedForm">
+        <div tal:define="batch_navigator view/search">
+
+          <div metal:use-macro="context/@@+bugtarget-macros-search/simple-search-form" />
+          <tal:buglisting content="structure batch_navigator/@@+table-view" />
+        </div>
+        </tal:do_not_show_advanced_form>
+
+        <tal:show_advanced_form condition="view/shouldShowAdvancedForm">
+          <metal:advanced_form
+            use-macro="context/@@+bugtask-macros-tableview/advanced_search_form" />
+        </tal:show_advanced_form>
+      </tal:show_bug_info>
     </div>
   </div>
   <tal:side metal:fill-slot="side">
-    <div id="involvement" class="portlet involvement">
-      <ul>
-        <li style="border: 0">
-          <a href="+filebug" class="menu-link-filebug sprite bugs">
-            Report a bug
-          </a>
-        </li>
-      </ul>
-    </div>
-    <tal:menu replace="structure view/@@+related-pages" />
-    <tal:do_not_show_portlets_advanced_form
-      condition="not: view/shouldShowAdvancedForm">
-      <tal:block content="structure context/@@+portlet-bugfilters" />
-      <tal:block
-          content="structure context/@@+portlet-publishing-details | nothing"/>
-      <tal:block content="structure context/@@+portlet-bugtags" />
-      <tal:releasecriticalbugs
+    <div id="bug-portlets" tal:condition="view/should_show_bug_information">
+      <div id="involvement" class="portlet involvement">
+        <ul>
+          <li style="border: 0">
+            <a href="+filebug" class="menu-link-filebug sprite bugs">
+              Report a bug
+            </a>
+          </li>
+        </ul>
+      </div>
+      <tal:menu replace="structure view/@@+related-pages" />
+      <tal:do_not_show_portlets_advanced_form
+        condition="not: view/shouldShowAdvancedForm">
+        <tal:block content="structure context/@@+portlet-bugfilters" />
+        <tal:block
+            content="structure context/@@+portlet-publishing-details | nothing"/>
+        <tal:block content="structure context/@@+portlet-bugtags"/>
+        <tal:releasecriticalbugs
+            tal:condition="view/shouldShowReleaseCriticalPortlet"
+            tal:content="structure context/@@+portlet-bugtasklist-seriesbugs" />
+        <tal:milestonecriticalbugs
           tal:condition="view/shouldShowReleaseCriticalPortlet"
-          tal:content="structure context/@@+portlet-bugtasklist-seriesbugs" />
-      <tal:milestonecriticalbugs
-        tal:condition="view/shouldShowReleaseCriticalPortlet"
-        tal:content="structure context/@@+portlet-bugtasklist-milestonebugs" />
-     </tal:do_not_show_portlets_advanced_form>
+          tal:content="structure context/@@+portlet-bugtasklist-milestonebugs" />
+       </tal:do_not_show_portlets_advanced_form>
+    </div>
   </tal:side>
 </body>
 

=== modified file 'lib/lp/testing/views.py'
--- lib/lp/testing/views.py	2010-09-19 03:09:49 +0000
+++ lib/lp/testing/views.py	2010-10-04 12:55:59 +0000
@@ -85,7 +85,8 @@
 def create_initialized_view(context, name, form=None, layer=None,
                             server_url=None, method=None, principal=None,
                             query_string=None, cookie=None, request=None,
-                            path_info='/', rootsite=None):
+                            path_info='/', rootsite=None,
+                            current_request=False):
     """Return a view that has already been initialized."""
     if method is None:
         if form is None:
@@ -94,7 +95,8 @@
             method = 'POST'
     view = create_view(
         context, name, form, layer, server_url, method, principal,
-        query_string, cookie, request, path_info, rootsite=rootsite)
+        query_string, cookie, request, path_info, rootsite=rootsite,
+        current_request=current_request)
     view.initialize()
     return view