Merge lp:~thumper/launchpad/blueprint-linked-bug-tasks into lp:launchpad

[Tim Penhey]

[r=lifeless,wallyworld][bug=487337] Show the appropriate bug task for the bugs linked to the blueprints.

This branch primarily fixes bug 487337.

A perhaps slightly over-engineered fix, but it is something that
I've been meaning to do for a while. Choosing the correct bug
task to show for links when we link to bugs not bug tasks has been
a bit of a problem for a while (IMO).

This branch adds a new method in its own module (to avoid circular
dependencies) called filter_bugtasks_by_context. This function
aims to choose the most appropriate bug task for different contexts.
It does this by creating the appropriate weighting calculator for
the context as different contexts put different weights on the different
bug tasks. These are then sorted for any particular bug, and the
bug task with the best weighting is chosen.

This method now replaces the branch specific bugtask chooser.

In order to do this without database queries, the IDs of the various
context objects are used. This meant exposing the field IDs in a few
different interfaces.

The bug task search method is extended to check for a specific blueprint
being linked rather than just any blueprint. This is then used in
the blueprint method getLinkedBugTasks. The user is passed in, and
is used by the searching code to check for visibility, so we don't have
to post process to check for visibility.

Finally, the formatted bugtask is shown on the blueprint page along
with the current status.

[Ian Booth (wallyworld)]

This is great, especially the no sql bit, and it fits in with other work already done to optimise and improve the security around retrieving bug tasks.

I'm wondering how the order of looking for contexts was determined in getLinkedBugTasks():

67 + if self.distroseries is not None:
68 + context = self.distroseries
69 + elif self.distribution is not None:
70 + context = self.distribution
71 + elif self.productseries is not None:
72 + context = self.productseries
73 + else:
74 + context = self.product

distroseries and productseries are set as goals i think? So one of the other is set. And product and distribution as targets. What makes distroseries more important than productseries for example? Perhaps a comment of two to explain would be good. Since this is only used here, there's no need for a helper method unless it may be something useful to abstract out.

review: Approve (*code)

[Robert Collins (lifeless)]

On Thu, Mar 17, 2011 at 4:56 PM, Ian Booth <email address hidden> wrote:
> Review: Approve *code
> This is great, especially the no sql bit, and it fits in with other work already done to optimise and improve the security around retrieving bug tasks.
>
> I'm wondering how the order of looking for contexts was determined in getLinkedBugTasks():

its arbitrary and the order doesn't matter - see the check constraints
on bugtask.

[Tim Penhey (thumper)]

On Fri, 18 Mar 2011 08:17:54 Robert Collins wrote:
> On Thu, Mar 17, 2011 at 4:56 PM, Ian Booth <email address hidden> wrote:
> > Review: Approve *code
> > This is great, especially the no sql bit, and it fits in with other work
> > already done to optimise and improve the security around retrieving bug
> > tasks.
>
> > I'm wondering how the order of looking for contexts was determined in
getLinkedBugTasks():
> its arbitrary and the order doesn't matter - see the check constraints
> on bugtask.

It isn't entirely arbitrary. Blueprints use the associated productseries or
distroseries to indicate goals. If a particular bug had a bug task for the
goal series, it made more sense to return that one than to return the more
generic distro or product task.

[Robert Collins (lifeless)]

On Fri, Mar 18, 2011 at 9:39 AM, Tim Penhey <email address hidden> wrote:
> On Fri, 18 Mar 2011 08:17:54 Robert Collins wrote:
>> On Thu, Mar 17, 2011 at 4:56 PM, Ian Booth <email address hidden> wrote:
>> > Review: Approve *code
>> > This is great, especially the no sql bit, and it fits in with other work
>> > already done to optimise and improve the security around retrieving bug
>> > tasks.
>>
>> > I'm wondering how the order of looking for contexts was determined in
> getLinkedBugTasks():
>> its arbitrary and the order doesn't matter - see the check constraints
>> on bugtask.
>
> It isn't entirely arbitrary.  Blueprints use the associated productseries or
> distroseries to indicate goals.  If a particular bug had a bug task for the
> goal series, it made more sense to return that one than to return the more
> generic distro or product task.

Bugtasks are constrainted to have only one target: one of (product
series, product, etc etc). So the code that was changed which is
examining only one task, could be in any order and have the same
outcome.

[Robert Collins (lifeless)]

Ok, so I like what this achieves.

I think the code could be a little leaner: two specific suggestions.

Rather than classes with __call__, use closures.

Secondly, rather than type inspection, extend the contract for IHasBugs to include getBugTaskWeightFunction - that way you don't need to extend this if block if we add a new IHasBugs, the implementor of a new IHasBugs will just implement that method.

review: Approve

=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py	2011-03-10 01:25:13 +0000
+++ lib/lp/blueprints/browser/specification.py	2011-03-28 00:03:43 +0000
@@ -557,8 +557,7 @@
 
     @cachedproperty
     def bug_links(self):
-        return [bug_link for bug_link in self.context.bug_links
-                if check_permission('launchpad.View', bug_link.bug)]
+        return self.context.getLinkedBugTasks(self.user)
 
 
 class SpecificationView(SpecificationSimpleView):

=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py	2011-03-24 12:54:40 +0000
+++ lib/lp/blueprints/interfaces/specification.py	2011-03-28 00:03:43 +0000
@@ -510,6 +510,16 @@
     def getBranchLink(branch):
         """Return the SpecificationBranch link for the branch, or None."""
 
+    def getLinkedBugTasks(user):
+        """Return the bug tasks that are relevant to this blueprint.
+
+        When multiple tasks are on a bug, if one of the tasks is for the
+        target, then only that task is returned. Otherwise the default
+        bug task is returned.
+
+        :param user: The user doing the search.
+        """
+
 
 class ISpecificationEditRestricted(Interface):
     """Specification's attributes and methods protected with launchpad.Edit.

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2011-03-14 22:14:13 +0000
+++ lib/lp/blueprints/model/specification.py	2011-03-28 00:03:43 +0000
@@ -30,6 +30,7 @@
     SQL,
     )
 from storm.store import Store
+from zope.component import getUtility
 from zope.event import notify
 from zope.interface import implements
 
@@ -77,6 +78,11 @@
     SpecificationSubscription,
     )
 from lp.bugs.interfaces.buglink import IBugLinkTarget
+from lp.bugs.interfaces.bugtask import (
+    BugTaskSearchParams,
+    IBugTaskSet,
+    )
+from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
 from lp.bugs.model.buglinktarget import BugLinkTargetMixin
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distroseries import IDistroSeries
@@ -669,11 +675,26 @@
         spec_branch = self.getBranchLink(branch)
         spec_branch.destroySelf()
 
+    def getLinkedBugTasks(self, user):
+        """See `ISpecification`."""
+        params = BugTaskSearchParams(user=user, linked_blueprints=self.id)
+        tasks = getUtility(IBugTaskSet).search(params)
+        if self.distroseries is not None:
+            context = self.distroseries
+        elif self.distribution is not None:
+            context = self.distribution
+        elif self.productseries is not None:
+            context = self.productseries
+        else:
+            context = self.product
+        return filter_bugtasks_by_context(context, tasks)
+
     def __repr__(self):
         return '<Specification %s %r for %r>' % (
             self.id, self.name, self.target.name)
 
 
+
 class HasSpecificationsMixin:
     """A mixin class that implements many of the common shortcut properties
     for other classes that have specifications.

=== modified file 'lib/lp/blueprints/stories/blueprints/xx-buglinks.txt'
--- lib/lp/blueprints/stories/blueprints/xx-buglinks.txt	2010-08-26 02:30:06 +0000
+++ lib/lp/blueprints/stories/blueprints/xx-buglinks.txt	2011-03-28 00:03:43 +0000
@@ -13,7 +13,7 @@
     ...     'http://launchpad.dev/firefox/+spec/svg-support')
     >>> print extract_text(find_tag_by_id(anon_browser.contents, 'bug_links'))
     Related bugs
-    Bug #1: Firefox does not support SVG
+    Bug #1: Firefox does not support SVG   New
 
 
 == Adding Links ==

=== modified file 'lib/lp/blueprints/templates/specification-index.pt'
--- lib/lp/blueprints/templates/specification-index.pt	2011-03-10 01:44:17 +0000
+++ lib/lp/blueprints/templates/specification-index.pt	2011-03-28 00:03:43 +0000
@@ -208,11 +208,18 @@
           <div id="bug_links">
             <h3>Related bugs</h3>
 
-            <ul tal:condition="view/bug_links">
-              <li tal:repeat="link view/bug_links">
-                <tal:link replace="structure link/bug/fmt:link" />
-              </li>
-            </ul>
+            <table tal:condition="view/bug_links">
+              <tr tal:repeat="bugtask view/bug_links">
+                <td>
+                  <tal:link replace="structure bugtask/fmt:link" />
+                </td>
+                <td>
+                  <span tal:content="bugtask/status/title"
+                        tal:attributes="class string:status${bugtask/status/name}"
+                        >Triaged</span>
+                </td>
+              </tr>
+            </table>
 
             <ul class="horizontal">
               <li tal:define="link context_menu/linkbug"

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-03-24 14:13:45 +0000
+++ lib/lp/bugs/configure.zcml	2011-03-28 00:03:43 +0000
@@ -168,6 +168,10 @@
     <facet
         facet="bugs">
 
+      <class class="lp.bugs.interfaces.bugtaskfilter.OrderedBugTask">
+        <allow attributes="rank id task"/>
+      </class>
+
         <!-- IBugTask -->
 
         <class
@@ -190,8 +194,12 @@
                     date_fix_released
                     date_left_closed
                     date_closed
+                    distributionID
+                    distroseriesID
                     milestoneID
+                    productID
                     productseriesID
+                    sourcepackagenameID
                     task_age
                     bug_subscribers
                     is_complete

=== modified file 'lib/lp/bugs/interfaces/bugtarget.py'
--- lib/lp/bugs/interfaces/bugtarget.py	2011-03-23 13:51:39 +0000
+++ lib/lp/bugs/interfaces/bugtarget.py	2011-03-28 00:03:43 +0000
@@ -20,6 +20,7 @@
     'IOfficialBugTagTargetRestricted',
     ]
 
+
 from lazr.enum import DBEnumeratedType
 from lazr.restful.declarations import (
     call_with,
@@ -279,6 +280,16 @@
                        None, all statuses will be included.
         """
 
+    def getBugTaskWeightFunction():
+        """Return a function that is used to weight the bug tasks.
+
+        The function should take a bug task as a parameter and return
+        an OrderedBugTask.
+
+        The ordered bug tasks are used to choose the most relevant bug task
+        for any particular context.
+        """
+
 
 class IBugTarget(IHasBugs):
     """An entity on which a bug can be reported.

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2011-03-28 00:03:43 +0000
@@ -462,17 +462,21 @@
         BugField(title=_("Bug"), readonly=True))
     product = Choice(
         title=_('Project'), required=False, vocabulary='Product')
+    productID = Attribute('The product ID')
     productseries = Choice(
         title=_('Series'), required=False, vocabulary='ProductSeries')
     productseriesID = Attribute('The product series ID')
     sourcepackagename = Choice(
         title=_("Package"), required=False,
         vocabulary='SourcePackageName')
+    sourcepackagenameID = Attribute('The sourcepackagename ID')
     distribution = Choice(
         title=_("Distribution"), required=False, vocabulary='Distribution')
+    distributionID = Attribute('The distribution ID')
     distroseries = Choice(
         title=_("Series"), required=False,
         vocabulary='DistroSeries')
+    distroseriesID = Attribute('The distroseries ID')
     milestone = exported(ReferenceChoice(
         title=_('Milestone'),
         required=False,

=== added file 'lib/lp/bugs/interfaces/bugtaskfilter.py'
--- lib/lp/bugs/interfaces/bugtaskfilter.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/interfaces/bugtaskfilter.py	2011-03-28 00:03:43 +0000
@@ -0,0 +1,68 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Fiter bugtasks based on context."""
+
+__metaclass__ = type
+__all__ = [
+    'filter_bugtasks_by_context',
+    'OrderedBugTask',
+    'simple_weight_calculator',
+    ]
+
+
+from collections import defaultdict, namedtuple
+from operator import attrgetter
+
+from lp.bugs.interfaces.bugtarget import IHasBugs
+
+
+OrderedBugTask = namedtuple('OrderedBugTask', 'rank id task')
+
+
+def simple_weight_calculator(bugtask):
+    """All tasks have the same weighting."""
+    return OrderedBugTask(1, bugtask.id, bugtask)
+
+
+def filter_bugtasks_by_context(context, bugtasks):
+    """Return the bugtasks filtered so there is only one bug task per bug.
+
+    The context is used to return the most relevent bugtask for that context.
+
+    An initial constraint is to not require any database queries from this
+    method.
+
+    Current contexts that impact selection:
+      IProduct
+      IProductSeries
+      IDistribution
+      IDistroSeries
+      ISourcePackage
+    Others:
+      get the first bugtask for any particular bug
+
+    If the context is a Product, then return the product bug task if there is
+    one.  If the context is a ProductSeries, then return the productseries
+    task if there is one, and if there isn't, look for the product task.  A
+    similar approach is taked for Distribution and distroseries.
+
+    For source packages, we look for the source package task, followed by the
+    distro source package, then the distroseries task, and lastly the distro
+    task.
+
+    If there is no specific matching task, we return the first task (the one
+    with the smallest database id).
+    """
+    has_bugs = IHasBugs(context, None)
+    if has_bugs is None:
+        weight_calculator = simple_weight_calculator
+    else:
+        weight_calculator = has_bugs.getBugTaskWeightFunction()
+
+    bug_mapping = defaultdict(list)
+    for task in bugtasks:
+        bug_mapping[task.bugID].append(weight_calculator(task))
+
+    filtered = [sorted(tasks)[0].task for tasks in bug_mapping.itervalues()]
+    return sorted(filtered, key=attrgetter('bugID'))

=== modified file 'lib/lp/bugs/model/bugtarget.py'
--- lib/lp/bugs/model/bugtarget.py	2011-02-17 04:00:06 +0000
+++ lib/lp/bugs/model/bugtarget.py	2011-03-28 00:03:43 +0000
@@ -51,6 +51,7 @@
     RESOLVED_BUGTASK_STATUSES,
     UNRESOLVED_BUGTASK_STATUSES,
     )
+from lp.bugs.interfaces.bugtaskfilter import simple_weight_calculator
 from lp.bugs.model.bugtask import (
     BugTaskSet,
     get_bug_privacy_filter,
@@ -233,6 +234,10 @@
         counts = cur.fetchone()
         return dict(zip(statuses, counts))
 
+    def getBugTaskWeightFunction(self):
+        """Default weight function is the simple one."""
+        return simple_weight_calculator
+
 
 class BugTargetBase(HasBugsBase):
     """Standard functionality for IBugTargets.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-03-28 00:03:41 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-03-28 00:03:43 +0000
@@ -2323,16 +2323,25 @@
     def _buildBlueprintRelatedClause(self, params):
         """Find bugs related to Blueprints, or not."""
         linked_blueprints = params.linked_blueprints
-        if linked_blueprints == BugBlueprintSearch.BUGS_WITH_BLUEPRINTS:
-            return "EXISTS (%s)" % (
-                "SELECT 1 FROM SpecificationBug"
-                " WHERE SpecificationBug.bug = Bug.id")
-        elif linked_blueprints == BugBlueprintSearch.BUGS_WITHOUT_BLUEPRINTS:
-            return "NOT EXISTS (%s)" % (
-                "SELECT 1 FROM SpecificationBug"
-                " WHERE SpecificationBug.bug = Bug.id")
+        if linked_blueprints is None:
+            return None
+        elif zope_isinstance(linked_blueprints, BaseItem):
+            if linked_blueprints == BugBlueprintSearch.BUGS_WITH_BLUEPRINTS:
+                return "EXISTS (%s)" % (
+                    "SELECT 1 FROM SpecificationBug"
+                    " WHERE SpecificationBug.bug = Bug.id")
+            elif (linked_blueprints ==
+                  BugBlueprintSearch.BUGS_WITHOUT_BLUEPRINTS):
+                return "NOT EXISTS (%s)" % (
+                    "SELECT 1 FROM SpecificationBug"
+                    " WHERE SpecificationBug.bug = Bug.id")
         else:
-            return None
+            # A specific search term has been supplied.
+            return """EXISTS (
+                    SELECT TRUE FROM SpecificationBug
+                    WHERE SpecificationBug.bug=Bug.id AND
+                    SpecificationBug.specification %s)
+                """ % search_value_to_where_condition(linked_blueprints)
 
     def buildOrigin(self, join_tables, prejoin_tables, clauseTables):
         """Build the parameter list for Store.using().

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-03-22 01:33:16 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-03-28 00:03:43 +0000
@@ -58,6 +58,7 @@
     login_person,
     logout,
     normalize_whitespace,
+    person_logged_in,
     StormStatementRecorder,
     TestCase,
     TestCaseWithFactory,
@@ -1010,6 +1011,27 @@
         self.assertEqual([task2], list(result))
 
 
+class BugTaskSetSearchTest(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_explicit_blueprint_specified(self):
+        # If the linked_blueprints is an integer id, then only bugtasks for
+        # bugs that are linked to that blueprint are returned.
+        bug1 = self.factory.makeBug()
+        blueprint1 = self.factory.makeBlueprint()
+        with person_logged_in(blueprint1.owner):
+            blueprint1.linkBug(bug1)
+        bug2 = self.factory.makeBug()
+        blueprint2 = self.factory.makeBlueprint()
+        with person_logged_in(blueprint2.owner):
+            blueprint2.linkBug(bug2)
+        self.factory.makeBug()
+        params = BugTaskSearchParams(user=None, linked_blueprints=blueprint1.id)
+        tasks = set(getUtility(IBugTaskSet).search(params))
+        self.assertThat(set(bug1.bugtasks), Equals(tasks))
+
+
 class BugTaskSearchBugsElsewhereTest(unittest.TestCase):
     """Tests for searching bugs filtering on related bug tasks.
 

=== added file 'lib/lp/bugs/tests/test_bugtaskfilter.py'
--- lib/lp/bugs/tests/test_bugtaskfilter.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_bugtaskfilter.py	2011-03-28 00:03:43 +0000
@@ -0,0 +1,196 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for lp.bugs.interfaces.bugtaskfilter."""
+
+__metaclass__ = type
+
+from testtools.matchers import Equals
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
+from lp.testing import (
+    StormStatementRecorder,
+    TestCaseWithFactory,
+    )
+from lp.testing.matchers import HasQueryCount
+
+
+class TestFilterBugTasksByContext(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_simple_case(self):
+        bug = self.factory.makeBug()
+        tasks = list(bug.bugtasks)
+        self.assertThat(
+            filter_bugtasks_by_context(None, tasks),
+            Equals(tasks))
+
+    def test_multiple_bugs(self):
+        bug1 = self.factory.makeBug()
+        bug2 = self.factory.makeBug()
+        bug3 = self.factory.makeBug()
+        tasks = list(bug1.bugtasks)
+        tasks.extend(bug2.bugtasks)
+        tasks.extend(bug3.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(None, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(len(filtered), Equals(3))
+        self.assertThat(filtered, Equals(tasks))
+
+    def test_two_product_tasks_case_no_context(self):
+        widget = self.factory.makeProduct()
+        bug = self.factory.makeBug(product=widget)
+        cogs = self.factory.makeProduct()
+        self.factory.makeBugTask(bug=bug, target=cogs)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(None, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([bug.getBugTask(widget)]))
+
+    def test_two_product_tasks_case(self):
+        widget = self.factory.makeProduct()
+        bug = self.factory.makeBug(product=widget)
+        cogs = self.factory.makeProduct()
+        task = self.factory.makeBugTask(bug=bug, target=cogs)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(cogs, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_product_context_with_series_task(self):
+        bug = self.factory.makeBug()
+        widget = self.factory.makeProduct()
+        task = self.factory.makeBugTask(bug=bug, target=widget)
+        self.factory.makeBugTask(bug=bug, target=widget.development_focus)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(widget, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_productseries_context_with_series_task(self):
+        bug = self.factory.makeBug()
+        widget = self.factory.makeProduct()
+        self.factory.makeBugTask(bug=bug, target=widget)
+        series = widget.development_focus
+        task = self.factory.makeBugTask(bug=bug, target=series)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(series, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_productseries_context_with_only_product_task(self):
+        bug = self.factory.makeBug()
+        widget = self.factory.makeProduct()
+        task = self.factory.makeBugTask(bug=bug, target=widget)
+        series = widget.development_focus
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(series, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_distro_context(self):
+        bug = self.factory.makeBug()
+        mint = self.factory.makeDistribution()
+        task = self.factory.makeBugTask(bug=bug, target=mint)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(mint, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_distro_context_with_series_task(self):
+        bug = self.factory.makeBug()
+        mint = self.factory.makeDistribution()
+        task = self.factory.makeBugTask(bug=bug, target=mint)
+        devel = self.factory.makeDistroSeries(mint)
+        self.factory.makeBugTask(bug=bug, target=devel)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(mint, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_distroseries_context_with_series_task(self):
+        bug = self.factory.makeBug()
+        mint = self.factory.makeDistribution()
+        self.factory.makeBugTask(bug=bug, target=mint)
+        devel = self.factory.makeDistroSeries(mint)
+        task = self.factory.makeBugTask(bug=bug, target=devel)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(devel, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_distroseries_context_with_no_series_task(self):
+        bug = self.factory.makeBug()
+        mint = self.factory.makeDistribution()
+        task = self.factory.makeBugTask(bug=bug, target=mint)
+        devel = self.factory.makeDistroSeries(mint)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(devel, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_sourcepackage_context_with_sourcepackage_task(self):
+        bug = self.factory.makeBug()
+        sp = self.factory.makeSourcePackage()
+        task = self.factory.makeBugTask(bug=bug, target=sp)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(sp, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_sourcepackage_context_with_distrosourcepackage_task(self):
+        bug = self.factory.makeBug()
+        sp = self.factory.makeSourcePackage()
+        dsp = sp.distribution_sourcepackage
+        task = self.factory.makeBugTask(bug=bug, target=dsp)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(sp, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_sourcepackage_context_series_task(self):
+        bug = self.factory.makeBug()
+        sp = self.factory.makeSourcePackage()
+        task = self.factory.makeBugTask(bug=bug, target=sp.distroseries)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(sp, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_sourcepackage_context_distro_task(self):
+        bug = self.factory.makeBug()
+        sp = self.factory.makeSourcePackage()
+        task = self.factory.makeBugTask(bug=bug, target=sp.distribution)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(sp, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
+    def test_sourcepackage_context_distro_task_with_other_distro_package(self):
+        bug = self.factory.makeBug()
+        sp = self.factory.makeSourcePackage()
+        task = self.factory.makeBugTask(bug=bug, target=sp.distribution)
+        other_sp = self.factory.makeSourcePackage(
+            sourcepackagename=sp.sourcepackagename)
+        self.factory.makeBugTask(bug=bug, target=other_sp)
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(sp, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2011-03-24 12:03:02 +0000
+++ lib/lp/code/interfaces/branch.py	2011-03-28 00:03:43 +0000
@@ -418,7 +418,7 @@
         When multiple tasks are on a bug, if one of the tasks is for the
         branch.target, then only that task is returned. Otherwise the default
         bug task is returned.
-        
+
         :param user: The user doing the search.
         :param status_filter: Passed onto the bug search as a constraint.
         """

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2011-03-23 16:28:51 +0000
+++ lib/lp/code/model/branch.py	2011-03-28 00:03:43 +0000
@@ -7,7 +7,6 @@
 __all__ = [
     'Branch',
     'BranchSet',
-    'filter_one_task_per_bug',
     ]
 
 from datetime import datetime
@@ -75,6 +74,7 @@
     BugTaskSearchParams,
     IBugTaskSet,
     )
+from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.code.bzr import (
     BranchFormat,
@@ -316,7 +316,7 @@
         tasks = shortlist(getUtility(IBugTaskSet).search(params), 1000)
         # Post process to discard irrelevant tasks: we only return one task per
         # bug, and cannot easily express this in sql (yet).
-        return filter_one_task_per_bug(self, tasks)
+        return filter_bugtasks_by_context(self.target.context, tasks)
 
     def linkBug(self, bug, registrant):
         """See `IBranch`."""
@@ -1390,27 +1390,3 @@
     """
     update_trigger_modified_fields(branch)
     send_branch_modified_notifications(branch, event)
-
-
-def filter_one_task_per_bug(branch, tasks):
-    """Given bug tasks for a branch, discard irrelevant ones.
-
-    Cannot easily be expressed in SQL yet, so we need this helper method.
-    """
-    order = {}
-    bugtarget = branch.target.context
-    # First pass calculates the order and selects the bugtasks that match
-    # our target.
-    # Second pass selects the earliest bugtask where the bug has no task on
-    # our target.
-    for pos, task in enumerate(tasks):
-        bug = task.bug
-        if bug not in order:
-            order[bug] = [pos, None]
-        if task.target == bugtarget:
-            order[bug][1] = task
-    for task in tasks:
-        index = order[task.bug]
-        if index[1] is None:
-            index[1] = task
-    return [task for pos, task in sorted(order.values())]

=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py	2011-03-26 19:33:54 +0000
+++ lib/lp/code/model/branchcollection.py	2011-03-28 00:03:43 +0000
@@ -41,6 +41,7 @@
     IBugTaskSet,
     BugTaskSearchParams,
     )
+from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
 from lp.bugs.model.bugbranch import BugBranch
 from lp.bugs.model.bugtask import BugTask
 from lp.code.interfaces.branch import user_has_special_branch_access
@@ -54,10 +55,7 @@
 from lp.code.enums import BranchMergeProposalStatus
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES
-from lp.code.model.branch import (
-    Branch,
-    filter_one_task_per_bug,
-    )
+from lp.code.model.branch import Branch
 from lp.code.model.branchmergeproposal import BranchMergeProposal
 from lp.code.model.branchsubscription import BranchSubscription
 from lp.code.model.codeimport import CodeImport
@@ -342,7 +340,7 @@
             # Now filter those down to one bugtask per branch
             for branch, tasks in bugtasks_for_branch.iteritems():
                 linked_bugtasks[branch.id].extend(
-                    filter_one_task_per_bug(branch, tasks))
+                    filter_bugtasks_by_context(branch.target.context, tasks))
 
         return [make_rev_info(
                 rev, merge_proposal_revs, linked_bugtasks)

=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py	2011-03-10 14:05:51 +0000
+++ lib/lp/registry/interfaces/distroseries.py	2011-03-28 00:03:43 +0000
@@ -223,6 +223,7 @@
             Interface, # Really IDistribution, see circular import fix below.
             title=_("Distribution"), required=True,
             description=_("The distribution for which this is a series.")))
+    distributionID = Attribute('The distribution ID.')
     named_version = Attribute('The combined display name and version.')
     parent = Attribute('The structural parent of this series - the distro')
     components = Attribute("The series components.")

=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py	2011-03-24 14:13:45 +0000
+++ lib/lp/registry/interfaces/productseries.py	2011-03-28 00:03:43 +0000
@@ -137,6 +137,7 @@
         ReferenceChoice(title=_('Project'), required=True,
             vocabulary='Product', schema=Interface), # really IProduct
         exported_as='project')
+    productID = Attribute('The product ID.')
 
     status = exported(
         Choice(

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2011-03-24 11:21:19 +0000
+++ lib/lp/registry/model/distribution.py	2011-03-28 00:03:43 +0000
@@ -107,6 +107,7 @@
     BugTaskStatus,
     UNRESOLVED_BUGTASK_STATUSES,
     )
+from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
 from lp.bugs.model.bug import (
     BugSet,
     get_bug_tags,
@@ -1840,6 +1841,22 @@
         productseries = sourcepackage.productseries
         return productseries.product.invitesTranslationEdits(person, language)
 
+    def getBugTaskWeightFunction(self):
+        """Provide a weight function to determine optimal bug task.
+
+        Full weight is given to tasks for this distribution.
+
+        Given that there must be a distribution task for a series of that
+        distribution to have a task, we give no more weighting to a
+        distroseries task than any other.
+        """
+        distributionID = self.id
+        def weight_function(bugtask):
+            if bugtask.distributionID == distributionID:
+                return OrderedBugTask(1, bugtask.id, bugtask)
+            return OrderedBugTask(2, bugtask.id, bugtask)
+        return weight_function
+
 
 class DistributionSet:
     """This class is to deal with Distribution related stuff"""

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2011-03-24 11:46:45 +0000
+++ lib/lp/registry/model/distroseries.py	2011-03-28 00:03:43 +0000
@@ -83,6 +83,7 @@
     Specification,
     )
 from lp.bugs.interfaces.bugtarget import IHasBugHeat
+from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
 from lp.bugs.model.bug import (
     get_bug_tags,
     get_bug_tags_open_count,
@@ -1982,6 +1983,25 @@
         return Store.of(self).find(
             DistroSeries, DistroSeries.parent_series == self)
 
+    def getBugTaskWeightFunction(self):
+        """Provide a weight function to determine optimal bug task.
+
+        Full weight is given to tasks for this distro series.
+
+        If the series isn't found, the distribution task is better than
+        others.
+        """
+        seriesID = self.id
+        distributionID = self.distributionID
+        def weight_function(bugtask):
+            if bugtask.distroseriesID == seriesID:
+                return OrderedBugTask(1, bugtask.id, bugtask)
+            elif bugtask.distributionID == distributionID:
+                return OrderedBugTask(2, bugtask.id, bugtask)
+            else:
+                return OrderedBugTask(3, bugtask.id, bugtask)
+        return weight_function
+
 
 class DistroSeriesSet:
     implements(IDistroSeriesSet)

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2011-03-24 20:48:35 +0000
+++ lib/lp/registry/model/product.py	2011-03-28 00:03:43 +0000
@@ -113,6 +113,7 @@
 from lp.blueprints.model.sprint import HasSprintsMixin
 from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
 from lp.bugs.interfaces.bugtarget import IHasBugHeat
+from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
 from lp.bugs.model.bug import (
     BugSet,
     get_bug_tags,
@@ -1331,6 +1332,22 @@
             SourcePackageRecipeData.base_branch == Branch.id,
             Branch.product == self)
 
+    def getBugTaskWeightFunction(self):
+        """Provide a weight function to determine optimal bug task.
+
+        Full weight is given to tasks for this product.
+
+        Given that there must be a product task for a series of that product
+        to have a task, we give no more weighting to a productseries task than
+        any other.
+        """
+        productID = self.id
+        def weight_function(bugtask):
+            if bugtask.productID == productID:
+                return OrderedBugTask(1, bugtask.id, bugtask)
+            return OrderedBugTask(2, bugtask.id, bugtask)
+        return weight_function
+
 
 class ProductSet:
     implements(IProductSet)

=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py	2011-03-24 11:01:45 +0000
+++ lib/lp/registry/model/productseries.py	2011-03-28 00:03:43 +0000
@@ -59,6 +59,7 @@
     Specification,
     )
 from lp.bugs.interfaces.bugtarget import IHasBugHeat
+from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
 from lp.bugs.model.bug import (
     get_bug_tags,
     get_bug_tags_open_count,
@@ -661,6 +662,24 @@
             landmarks=landmarks,
             product=self.product)
 
+    def getBugTaskWeightFunction(self):
+        """Provide a weight function to determine optimal bug task.
+
+        Full weight is given to tasks for this product series.
+
+        If the series isn't found, the product task is better than others.
+        """
+        seriesID = self.id
+        productID = self.productID
+        def weight_function(bugtask):
+            if bugtask.productseriesID == seriesID:
+                return OrderedBugTask(1, bugtask.id, bugtask)
+            elif bugtask.productID == productID:
+                return OrderedBugTask(2, bugtask.id, bugtask)
+            else:
+                return OrderedBugTask(3, bugtask.id, bugtask)
+        return weight_function
+
 
 class TimelineProductSeries:
     """See `ITimelineProductSeries`."""

=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2011-03-24 11:01:45 +0000
+++ lib/lp/registry/model/sourcepackage.py	2011-03-28 00:03:43 +0000
@@ -41,6 +41,7 @@
     QuestionTargetSearch,
     )
 from lp.bugs.interfaces.bugtarget import IHasBugHeat
+from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
 from lp.bugs.model.bug import get_bug_tags_open_count
 from lp.bugs.model.bugtarget import (
     BugTargetBase,
@@ -756,3 +757,27 @@
     def linkedBranches(self):
         """See `ISourcePackage`."""
         return dict((p.name, b) for (p, b) in self.linked_branches)
+
+    def getBugTaskWeightFunction(self):
+        """Provide a weight function to determine optimal bug task.
+
+        We look for the source package task, followed by the distro source
+        package, then the distroseries task, and lastly the distro task.
+        """
+        sourcepackagenameID = self.sourcepackagename.id
+        seriesID = self.distroseries.id
+        distributionID = self.distroseries.distributionID
+        def weight_function(bugtask):
+            if bugtask.sourcepackagenameID == sourcepackagenameID:
+                if bugtask.distroseriesID == seriesID:
+                    return OrderedBugTask(1, bugtask.id, bugtask)
+                elif bugtask.distributionID == distributionID:
+                    return OrderedBugTask(2, bugtask.id, bugtask)
+            elif bugtask.distroseriesID == seriesID:
+                return OrderedBugTask(3, bugtask.id, bugtask)
+            elif bugtask.distributionID == distributionID:
+                return OrderedBugTask(4, bugtask.id, bugtask)
+            # Catch the default case, and where there is a task for the same
+            # sourcepackage on a different distro.
+            return OrderedBugTask(5, bugtask.id, bugtask)
+        return weight_function