Merge lp:~stevenk/launchpad/filter-spec-lists into lp:launchpad

[Steve Kowalik]

Change IBug.specifications so that it returns a resultset of the linked specifications that the user can view.

Add a new method, IBranch.getSpecificationLinks() that will return a resultset of the specification links themselves (since that's what IBranch.spec_links does) that the user can view. IBranch.spec_links continues to live since it's exported via the API.

Clean up some whitespace that caught my eye.

[William Grant (wgrant)]

I wouldn't use an explicit join in getSpecifications (particularly since you have the join condition and where clause around the wrong way), but otherwise fine.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2012-09-28 01:25:36 +0000
+++ lib/lp/bugs/browser/bug.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """IBug related view classes."""
@@ -536,6 +536,10 @@
         """
         return getUtility(ILaunchBag).bugtask
 
+    @property
+    def specifications(self):
+        return self.context.getSpecifications(self.user)
+
 
 class BugInformationTypePortletView(InformationTypePortletMixin,
                                     LaunchpadView):

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2013-02-06 04:22:43 +0000
+++ lib/lp/bugs/interfaces/bug.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces related to bugs."""
@@ -313,7 +313,6 @@
     initial_message = Attribute(
         "The message that was specified when creating the bug")
     questions = Attribute("List of questions related to this bug.")
-    specifications = Attribute("List of related specifications.")
     tags = exported(List(
         title=_("Tags"),
         description=_("Space-separated keywords for classifying "
@@ -383,6 +382,9 @@
             value_type=Reference(schema=IMessage)),
         exported_as='messages'))
 
+    def getSpecifications(user):
+        """List of related specifications that the user can view."""
+
     def _indexed_messages(include_content=False, include_parents=False):
         """Low level query for getting bug messages.
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2013-02-06 04:56:33 +0000
+++ lib/lp/bugs/model/bug.py	2013-02-21 07:22:22 +0000
@@ -98,6 +98,11 @@
 from lp.app.interfaces.services import IService
 from lp.app.model.launchpad import InformationTypeMixin
 from lp.app.validators import LaunchpadValidationError
+from lp.blueprints.model.specification import Specification
+from lp.blueprints.model.specificationbug import SpecificationBug
+from lp.blueprints.model.specificationsearch import (
+    get_specification_privacy_filter,
+    )
 from lp.bugs.adapters.bug import convert_to_information_type
 from lp.bugs.adapters.bugchange import (
     BranchLinkedToBug,
@@ -360,11 +365,7 @@
     cves = SQLRelatedJoin('Cve', intermediateTable='BugCve',
         orderBy='sequence', joinColumn='bug', otherColumn='cve')
     cve_links = SQLMultipleJoin('BugCve', joinColumn='bug', orderBy='id')
-    duplicates = SQLMultipleJoin(
-        'Bug', joinColumn='duplicateof', orderBy='id')
-    specifications = SQLRelatedJoin('Specification', joinColumn='bug',
-        otherColumn='specification', intermediateTable='SpecificationBug',
-        orderBy='-datecreated')
+    duplicates = SQLMultipleJoin('Bug', joinColumn='duplicateof', orderBy='id')
     questions = SQLRelatedJoin('Question', joinColumn='bug',
         otherColumn='question', intermediateTable='QuestionBug',
         orderBy='-datecreated')
@@ -379,6 +380,14 @@
     heat_last_updated = UtcDateTimeCol(default=None)
     latest_patch_uploaded = UtcDateTimeCol(default=None)
 
+    def getSpecifications(self, user):
+        """See `IBug`."""
+        return IStore(SpecificationBug).find(
+            Specification,
+            SpecificationBug.bugID == self.id,
+            SpecificationBug.specificationID == Specification.id,
+            *get_specification_privacy_filter(user))
+
     @property
     def security_related(self):
         return self.information_type in SECURITY_INFORMATION_TYPES

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2012-10-18 14:43:24 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -528,7 +528,7 @@
         ])
 
 
-class TestBugTaskPrivacy(TestCase):
+class TestBugTaskPrivacy(TestCaseWithFactory):
     """Verify that the bug is either private or public.
 
     XXX: rharding 2012-05-14 bug=999298: These tests are ported from doctests
@@ -553,14 +553,15 @@
         ubuntu_team = getUtility(IPersonSet).getByEmail('[e-mail]')
         bug_upstream_firefox_crashes.bug.subscribe(ubuntu_team, ubuntu_team)
 
-        old_state = Snapshot(bug_upstream_firefox_crashes.bug,
-                             providing=IBug)
-        self.assertTrue(bug_upstream_firefox_crashes.bug.setPrivate(True,
-                                                                    foobar))
-
-        bug_set_private = ObjectModifiedEvent(bug_upstream_firefox_crashes.bug,
-                                              old_state,
-                                              ["id", "title", "private"])
+        old_state = Snapshot(
+            bug_upstream_firefox_crashes.bug, providing=IBug)
+        self.assertTrue(
+            bug_upstream_firefox_crashes.bug.setPrivate(True, foobar))
+
+        bug_set_private = ObjectModifiedEvent(
+            bug_upstream_firefox_crashes.bug, old_state,
+            ["id", "title", "private"])
+
         notify(bug_set_private)
         flush_database_updates()
 
@@ -671,6 +672,28 @@
         bug_upstream_firefox_crashes.transitionToStatus(
             BugTaskStatus.NEW, getUtility(ILaunchBag).user)
 
+    def _createBugAndSpecification(self):
+        bug = self.factory.makeBug()
+        spec = self.factory.makeSpecification(
+            information_type=InformationType.PROPRIETARY)
+        with person_logged_in(spec.product.owner):
+            spec.linkBug(bug)
+        return spec, bug     
+
+    def test_bug_specifications_is_filtered_for_anonymous(self):
+        spec, bug = self._createBugAndSpecification()
+        self.assertContentEqual([], bug.getSpecifications(None))
+
+    def test_bug_specifications_is_filtered_for_unknown_user(self):
+        spec, bug = self._createBugAndSpecification()
+        self.assertContentEqual(
+            [], bug.getSpecifications(self.factory.makePerson()))
+
+    def test_bug_specifications_for_authorised_user(self):
+        spec, bug = self._createBugAndSpecification()
+        self.assertContentEqual(
+            [spec], bug.getSpecifications(spec.product.owner))
+
 
 class TestBugTaskDelta(TestCaseWithFactory):
 

=== modified file 'lib/lp/bugs/templates/bug-portlet-specs.pt'
--- lib/lp/bugs/templates/bug-portlet-specs.pt	2009-10-29 21:39:12 +0000
+++ lib/lp/bugs/templates/bug-portlet-specs.pt	2013-02-21 07:22:22 +0000
@@ -3,10 +3,10 @@
     xmlns:metal="http://xml.zope.org/namespaces/metal"
     xmlns:i18n="http://xml.zope.org/namespaces/i18n"
     class="portlet vertical" id="portlet-blueprints"
-    tal:condition="context/specifications">
+    tal:condition="view/specifications">
   <h2>Related blueprints</h2>
   <ul>
-    <li tal:repeat="spec context/specifications"
+    <li tal:repeat="spec view/specifications"
         tal:content="structure spec/fmt:link" />
   </ul>
 </div>

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2012-12-12 04:59:52 +0000
+++ lib/lp/code/browser/branch.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Branch views."""
@@ -354,7 +354,7 @@
         return Link('+linkbug', text, icon='add')
 
     def link_blueprint(self):
-        if self.context.spec_links:
+        if list(self.context.getSpecificationLinks(self.user)):
             text = 'Link to another blueprint'
         else:
             text = 'Link to a blueprint'
@@ -689,6 +689,10 @@
             self.context.branch, IBranch['lifecycle_status'],
             header='Change status to', css_class_prefix='branchstatus')
 
+    @property
+    def spec_links(self):
+        return self.context.getSpecificationLinks(self.user)
+
 
 class BranchInProductView(BranchView):
 

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2013-01-07 02:40:55 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Views, navigation and actions for BranchMergeProposals."""
@@ -719,8 +719,11 @@
     def has_bug_or_spec(self):
         """Return whether or not the merge proposal has a linked bug or spec.
         """
-        branch = self.context.source_branch
-        return self.linked_bugtasks or branch.spec_links
+        return self.linked_bugtasks or self.spec_links
+
+    @property
+    def spec_links(self):
+        return self.context.source_branch.getSpecificationLinks(self.user)
 
     @cachedproperty
     def linked_bugtasks(self):

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2013-01-16 06:41:43 +0000
+++ lib/lp/code/interfaces/branch.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Branch interfaces."""
@@ -482,6 +482,9 @@
             value_type=Reference(Interface)),  # Really ISpecificationBranch
         as_of="beta")
 
+    def getSpecificationLinks(user):
+        """Fetch the `ISpecificationBranch`'s that the user can view."""
+
     @call_with(registrant=REQUEST_USER)
     @operation_parameters(
         spec=Reference(schema=Interface))  # Really ISpecification

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2013-02-14 05:36:37 +0000
+++ lib/lp/code/model/branch.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -66,6 +66,11 @@
     IPrivacy,
     )
 from lp.app.interfaces.services import IService
+from lp.blueprints.model.specification import Specification
+from lp.blueprints.model.specificationbranch import SpecificationBranch
+from lp.blueprints.model.specificationsearch import (
+    get_specification_privacy_filter,
+    )
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
 from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
@@ -438,9 +443,20 @@
         """See `IBranch`."""
         return bug.unlinkBranch(self, user)
 
-    spec_links = SQLMultipleJoin('SpecificationBranch',
-        joinColumn='branch',
-        orderBy='id')
+    spec_links = SQLMultipleJoin(
+        'SpecificationBranch', joinColumn='branch', orderBy='id')
+
+    def getSpecificationLinks(self, user):
+        """See `IBranch`."""
+        tables = [
+            SpecificationBranch,
+            Join(
+                Specification,
+                SpecificationBranch.specificationID == Specification.id)]
+        return Store.of(self).using(*tables).find(
+            SpecificationBranch,
+            SpecificationBranch.branchID == self.id,
+            *get_specification_privacy_filter(user))
 
     def linkSpecification(self, spec, registrant):
         """See `IBranch`."""
@@ -459,8 +475,7 @@
     @property
     def active_landing_targets(self):
         """Merge proposals not in final states where this branch is source."""
-        store = Store.of(self)
-        return store.find(
+        return Store.of(self).find(
             BranchMergeProposal, BranchMergeProposal.source_branch == self,
             Not(BranchMergeProposal.queue_status.is_in(
                 BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
@@ -615,8 +630,7 @@
 
     def getStackedBranches(self):
         """See `IBranch`."""
-        store = Store.of(self)
-        return store.find(Branch, Branch.stacked_on == self)
+        return Store.of(self).find(Branch, Branch.stacked_on == self)
 
     def getStackedOnBranches(self):
         """See `IBranch`."""

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2013-01-07 02:40:55 +0000
+++ lib/lp/code/model/tests/test_branch.py	2013-02-21 07:22:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for Branches."""
@@ -1573,10 +1573,10 @@
         spec2.linkBranch(self.branch, self.branch.owner)
         spec2_branch_id = self.branch.spec_links[1].id
         self.branch.destroySelf(break_references=True)
-        self.assertRaises(SQLObjectNotFound, SpecificationBranch.get,
-                          spec1_branch_id)
-        self.assertRaises(SQLObjectNotFound, SpecificationBranch.get,
-                          spec2_branch_id)
+        self.assertRaises(
+            SQLObjectNotFound, SpecificationBranch.get, spec1_branch_id)
+        self.assertRaises(
+            SQLObjectNotFound, SpecificationBranch.get, spec2_branch_id)
 
     def test_branchWithSeriesRequirements(self):
         """Deletion requirements for a series' branch are right."""

=== modified file 'lib/lp/code/templates/branch-macros.pt'
--- lib/lp/code/templates/branch-macros.pt	2012-08-07 07:07:02 +0000
+++ lib/lp/code/templates/branch-macros.pt	2013-02-21 07:22:22 +0000
@@ -76,7 +76,7 @@
           <metal:branch-link use-macro="branch/@@+macros/bug-branch-links"/>
         </tal:branch-link>
 
-        <div tal:repeat="spec_link mergeproposal/source_branch/spec_links">
+        <div tal:repeat="spec_link view/spec_links">
           <img src="/@@/blueprint"
                tal:replace="structure spec_link/specification/image:icon" />
           <a tal:attributes="href spec_link/specification/fmt:url:blueprints"
@@ -140,7 +140,7 @@
       show_edit - show the edit form
   </tal:comment>
 
-  <tal:spec-tasks repeat="spec_branch branch/spec_links">
+  <tal:spec-tasks repeat="spec_branch view/spec_links">
     <div tal:define="has_edit_permission spec_branch/required:launchpad.AnyPerson;
                      show_edit show_edit|nothing;
                      show_edit python: show_edit and has_edit_permission;