launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06886
[Merge] lp:~linaro-infrastructure/launchpad/team-engineering-view into lp:launchpad
Guilherme Salgado has proposed merging lp:~linaro-infrastructure/launchpad/team-engineering-view into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~linaro-infrastructure/launchpad/team-engineering-view/+merge/99342
Those new methods will be used by a new view showing the upcoming work assigned to members of a team (https://dev.launchpad.net/Projects/WorkItems).
Both queries do some LeftJoins to bring in all the related data in a single query and avoid hitting the DB again for every returned item. The new filters to BugTaskSet.search() was a suggestion from Robert and we agreed to do the conjoined master filtering in python because the existing one in BugTaskSet.search() works only when you're filtering results for a single milestone.
Also, the plan is to not do batching on those pages as the page only includes stuff assigned to a team member *and* targeted to a milestone in the next few months. However, the new page will be guarded by a feature flag and we'll be conducting user testing to make sure we can avoid batching there.
--
https://code.launchpad.net/~linaro-infrastructure/launchpad/team-engineering-view/+merge/99342
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~linaro-infrastructure/launchpad/team-engineering-view into lp:launchpad.
=== modified file 'lib/lp/blueprints/model/specificationworkitem.py'
--- lib/lp/blueprints/model/specificationworkitem.py 2012-02-28 04:24:19 +0000
+++ lib/lp/blueprints/model/specificationworkitem.py 2012-03-26 14:53:00 +0000
@@ -47,8 +47,9 @@
def __repr__(self):
title = self.title.encode('ASCII', 'backslashreplace')
+ assignee = getattr(self.assignee, 'name', None)
return '<SpecificationWorkItem [%s] %s: %s of %s>' % (
- self.assignee, title, self.status, self.specification)
+ assignee, title, self.status.name, self.specification)
def __init__(self, title, status, specification, assignee, milestone,
sequence):
=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py 2012-03-06 21:13:36 +0000
+++ lib/lp/bugs/interfaces/bugtask.py 2012-03-26 14:53:00 +0000
@@ -1185,7 +1185,8 @@
linked_branches=None, linked_blueprints=None,
structural_subscriber=None, modified_since=None,
created_since=None, exclude_conjoined_tasks=False, cve=None,
- upstream_target=None):
+ upstream_target=None, milestone_dateexpected_before=None,
+ milestone_dateexpected_after=None):
self.bug = bug
self.searchtext = searchtext
@@ -1236,6 +1237,8 @@
self.exclude_conjoined_tasks = exclude_conjoined_tasks
self.cve = cve
self.upstream_target = upstream_target
+ self.milestone_dateexpected_before = milestone_dateexpected_before
+ self.milestone_dateexpected_after = milestone_dateexpected_after
def setProduct(self, product):
"""Set the upstream context on which to filter the search."""
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2012-03-25 23:33:53 +0000
+++ lib/lp/bugs/model/bug.py 2012-03-26 14:53:00 +0000
@@ -2129,7 +2129,7 @@
"""A set of known persons able to view this bug.
This method must return an empty set or bug searches will trigger late
- evaluation. Any 'should be set on load' propertis must be done by the
+ evaluation. Any 'should be set on load' properties must be done by the
bug search.
If you are tempted to change this method, don't. Instead see
=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py 2012-03-20 09:43:46 +0000
+++ lib/lp/bugs/model/bugtasksearch.py 2012-03-26 14:53:00 +0000
@@ -659,6 +659,18 @@
extra_clauses.append(nominated_for_clause)
clauseTables.append(BugNomination)
+ dateexpected_before = params.milestone_dateexpected_before
+ dateexpected_after = params.milestone_dateexpected_after
+ if dateexpected_after or dateexpected_before:
+ clauseTables.append(Milestone)
+ extra_clauses.append("BugTask.milestone = Milestone.id")
+ if dateexpected_after:
+ extra_clauses.append("Milestone.dateexpected >= %s"
+ % sqlvalues(dateexpected_after))
+ if dateexpected_before:
+ extra_clauses.append("Milestone.dateexpected <= %s"
+ % sqlvalues(dateexpected_before))
+
clause, decorator = _get_bug_privacy_filter_with_decorator(params.user)
if clause:
extra_clauses.append(clause)
=== modified file 'lib/lp/bugs/tests/test_bugtask_search.py'
--- lib/lp/bugs/tests/test_bugtask_search.py 2012-03-08 11:51:36 +0000
+++ lib/lp/bugs/tests/test_bugtask_search.py 2012-03-26 14:53:00 +0000
@@ -1572,6 +1572,36 @@
return [bugtask.bug.id for bugtask in expected_bugtasks]
+class TestMilestoneDueDateFiltering(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_milestone_date_filters(self):
+ today = datetime.today().date()
+ ten_days_ago = today - timedelta(days=10)
+ ten_days_from_now = today + timedelta(days=10)
+ current_milestone = self.factory.makeMilestone(dateexpected=today)
+ old_milestone = self.factory.makeMilestone(
+ dateexpected=ten_days_ago)
+ future_milestone = self.factory.makeMilestone(
+ dateexpected=ten_days_from_now)
+ current_milestone_bug = self.factory.makeBug(
+ milestone=current_milestone)
+ old_milestone_bug = self.factory.makeBug(milestone=old_milestone)
+ future_milestone_bug = self.factory.makeBug(
+ milestone=future_milestone)
+ # Search for bugs whose milestone.dateexpected is between yesterday
+ # and tomorrow. This will return only the one task targeted to
+ # current_milestone.
+ params = BugTaskSearchParams(
+ user=None,
+ milestone_dateexpected_after=today - timedelta(days=1),
+ milestone_dateexpected_before=today + timedelta(days=1))
+ result = getUtility(IBugTaskSet).search(params)
+ self.assertEqual(
+ current_milestone_bug.bugtasks, list(result))
+
+
def test_suite():
suite = unittest.TestSuite()
loader = unittest.TestLoader()
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2012-03-02 19:53:30 +0000
+++ lib/lp/registry/interfaces/person.py 2012-03-26 14:53:00 +0000
@@ -1518,6 +1518,19 @@
:return: a boolean.
"""
+ def getAssignedSpecificationWorkItemsDueBefore(date):
+ """Return SpecificationWorkItems assigned to this person (or members
+ of this team) and whose milestone is due between today and the given
+ date.
+ """
+
+ def getAssignedBugTasksDueBefore(date, user):
+ """Get all BugTasks assigned to this person (or members of this team)
+ and whose milestone is due between today and the given date.
+ """
+
+ participant_ids = List(
+ title=_("The DB IDs of this team's participants"), value_type=Int())
active_member_count = Attribute(
"The number of real people who are members of this team.")
# activemembers.value_type.schema will be set to IPerson once
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-03-25 23:33:53 +0000
+++ lib/lp/registry/model/person.py 2012-03-26 14:53:00 +0000
@@ -62,6 +62,7 @@
from storm.expr import (
Alias,
And,
+ Coalesce,
Desc,
Exists,
In,
@@ -127,6 +128,7 @@
SpecificationImplementationStatus,
SpecificationSort,
)
+from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
from lp.blueprints.model.specification import (
HasSpecificationsMixin,
Specification,
@@ -221,6 +223,7 @@
KarmaCategory,
KarmaTotalCache,
)
+from lp.registry.model.milestone import Milestone
from lp.registry.model.personlocation import PersonLocation
from lp.registry.model.pillar import PillarName
from lp.registry.model.sourcepackagename import SourcePackageName
@@ -290,6 +293,7 @@
REDEEMABLE_VOUCHER_STATUSES,
VOUCHER_STATUSES,
)
+from lp.services.searchbuilder import any
from lp.services.statistics.interfaces.statistic import ILaunchpadStatisticSet
from lp.services.verification.interfaces.authtoken import LoginTokenType
from lp.services.verification.interfaces.logintoken import ILoginTokenSet
@@ -1468,6 +1472,100 @@
"""See `IPerson`."""
self._inTeam_cache = {}
+ @cachedproperty
+ def participant_ids(self):
+ """See `IPerson`."""
+ return list(Store.of(self).find(
+ TeamParticipation.personID, TeamParticipation.teamID == self.id))
+
+ def getAssignedSpecificationWorkItemsDueBefore(self, date):
+ """See `IPerson`."""
+ from lp.registry.model.person import Person
+ from lp.registry.model.product import Product
+ from lp.registry.model.distribution import Distribution
+ store = Store.of(self)
+ WorkItem = SpecificationWorkItem
+ origin = [
+ WorkItem,
+ Join(Specification, WorkItem.specification == Specification.id),
+ LeftJoin(Product, Specification.product == Product.id),
+ LeftJoin(Distribution,
+ Specification.distribution == Distribution.id),
+ # WorkItems may not have a milestone and in that case they inherit
+ # the one from the spec.
+ Join(Milestone,
+ Coalesce(WorkItem.milestone_id,
+ Specification.milestoneID) == Milestone.id),
+ # WorkItems may not have an assignee and in that case they inherit
+ # the one from the spec.
+ Join(Person,
+ Coalesce(WorkItem.assignee_id,
+ Specification.assigneeID) == Person.id),
+ ]
+ today = datetime.today().date()
+ results = store.using(*origin).find(
+ (WorkItem, Milestone, Specification, Person, Product,
+ Distribution),
+ AND(Milestone.dateexpected <= date,
+ Milestone.dateexpected >= today,
+ Person.id.is_in(self.participant_ids))
+ )
+ for result in results:
+ yield result[0]
+
+ def getAssignedBugTasksDueBefore(self, date, user):
+ """See `IPerson`."""
+ from lp.bugs.model.bugtask import BugTask
+ from lp.registry.model.distribution import Distribution
+ from lp.registry.model.distroseries import DistroSeries
+ from lp.registry.model.productseries import ProductSeries
+ today = datetime.today().date()
+ search_params = BugTaskSearchParams(
+ user, assignee=any(*self.participant_ids),
+ milestone_dateexpected_before=date,
+ milestone_dateexpected_after=today)
+ # BugTaskSet.search() performs eager loading for
+ # Product/SourcePackageName, but we want that for the ones below as
+ # well, so we do it with prejoins.
+ prejoins = [
+ (ProductSeries, LeftJoin(
+ ProductSeries, BugTask.productseries == ProductSeries.id)),
+ (DistroSeries, LeftJoin(
+ DistroSeries, BugTask.distroseries == DistroSeries.id)),
+ (Distribution, LeftJoin(
+ Distribution, BugTask.distribution == Distribution.id)),
+ (Milestone, LeftJoin(
+ Milestone, BugTask.milestone == Milestone.id)),
+ (Person, LeftJoin(
+ Person, BugTask.assignee == Person.id)),
+ ]
+ results = getUtility(IBugTaskSet).search(
+ search_params, prejoins=prejoins)
+
+ for task in results:
+ # We skip masters (instead of slaves) from conjoined relationships
+ # because we can do that without hittind the DB, which would not
+ # be possible if we wanted to skip the slaves. The simple (but
+ # expensive) way to skip the slaves would be to skip any tasks
+ # that have a non-None .conjoined_master.
+ productseries = task.productseries
+ distroseries = task.distroseries
+ if productseries is not None and task.product is None:
+ dev_focus_id = productseries.product.development_focusID
+ if (productseries.id == dev_focus_id and
+ task.status not in BugTask._NON_CONJOINED_STATUSES):
+ continue
+ elif (distroseries is not None
+ and task.sourcepackagename is not None):
+ # Distribution.currentseries is expensive to run for every
+ # bugtask (as it goes through every series of that
+ # distribution), but it's a cached property and there's only
+ # one distribution with bugs in LP, so we can afford to do
+ # it here.
+ if distroseries.distribution.currentseries == distroseries:
+ continue
+ yield task
+
#
# ITeam methods
#
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2012-02-28 04:24:19 +0000
+++ lib/lp/registry/tests/test_person.py 2012-03-26 14:53:00 +0000
@@ -3,7 +3,10 @@
__metaclass__ = type
-from datetime import datetime
+from datetime import (
+ datetime,
+ timedelta,
+ )
from lazr.lifecycle.snapshot import Snapshot
from lazr.restful.utils import smartquote
@@ -1113,3 +1116,213 @@
self.assertContentEqual(
[team2.teamowner],
get_recipients(team2))
+
+
+class Test_getAssignedSpecificationWorkItemsDueBefore(TestCaseWithFactory):
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(Test_getAssignedSpecificationWorkItemsDueBefore, self).setUp()
+ self.team = self.factory.makeTeam()
+ today = datetime.today().date()
+ next_month = today + timedelta(days=30)
+ next_year = today + timedelta(days=366)
+ self.current_milestone = self.factory.makeMilestone(
+ dateexpected=next_month)
+ self.product = self.current_milestone.product
+ self.future_milestone = self.factory.makeMilestone(
+ dateexpected=next_year, product=self.product)
+
+ def test_basic(self):
+ assigned_spec = self.factory.makeSpecification(
+ assignee=self.team.teamowner, milestone=self.current_milestone,
+ product=self.product)
+ # Create a workitem with no explicit assignee/milestone. This way it
+ # will inherit the ones from the spec it belongs to.
+ workitem = self.factory.makeSpecificationWorkItem(
+ title=u'workitem 1', specification=assigned_spec)
+
+ # Create a workitem with somebody who's not a member of our team as
+ # the assignee. This workitem must not be in the list returned by
+ # getAssignedSpecificationWorkItemsDueBefore().
+ self.factory.makeSpecificationWorkItem(
+ title=u'workitem 2', specification=assigned_spec,
+ assignee=self.factory.makePerson())
+
+ # Create a workitem targeted to a milestone too far in the future.
+ # This workitem must not be in the list returned by
+ # getAssignedSpecificationWorkItemsDueBefore().
+ self.factory.makeSpecificationWorkItem(
+ title=u'workitem 3', specification=assigned_spec,
+ milestone=self.future_milestone)
+
+ workitems = self.team.getAssignedSpecificationWorkItemsDueBefore(
+ self.current_milestone.dateexpected)
+
+ self.assertEqual([workitem], list(workitems))
+
+ def test_skips_workitems_with_milestone_in_the_past(self):
+ today = datetime.today().date()
+ milestone = self.factory.makeMilestone(
+ dateexpected=today - timedelta(days=1))
+ spec = self.factory.makeSpecification(
+ assignee=self.team.teamowner, milestone=milestone,
+ product=milestone.product)
+ self.factory.makeSpecificationWorkItem(
+ title=u'workitem 1', specification=spec)
+
+ workitems = self.team.getAssignedSpecificationWorkItemsDueBefore(today)
+
+ self.assertEqual([], list(workitems))
+
+ def test_includes_workitems_from_future_spec(self):
+ assigned_spec = self.factory.makeSpecification(
+ assignee=self.team.teamowner, milestone=self.future_milestone,
+ product=self.product)
+ # This workitem inherits the spec's milestone and that's too far in
+ # the future so it won't be in the returned list.
+ self.factory.makeSpecificationWorkItem(
+ title=u'workitem 1', specification=assigned_spec)
+ # This one, on the other hand, is explicitly targeted to the current
+ # milestone, so it is included in the returned list even though its
+ # spec is targeted to the future milestone.
+ workitem = self.factory.makeSpecificationWorkItem(
+ title=u'workitem 2', specification=assigned_spec,
+ milestone=self.current_milestone)
+
+ workitems = self.team.getAssignedSpecificationWorkItemsDueBefore(
+ self.current_milestone.dateexpected)
+
+ self.assertEqual([workitem], list(workitems))
+
+ def test_includes_workitems_from_foreign_spec(self):
+ # This spec is assigned to a person who's not a member of our team, so
+ # only the workitems that are explicitly assigned to a member of our
+ # team will be in the returned list.
+ foreign_spec = self.factory.makeSpecification(
+ assignee=self.factory.makePerson(),
+ milestone=self.current_milestone, product=self.product)
+ # This one is not explicitly assigned to anyone, so it inherits the
+ # assignee of its spec and hence is not in the returned list.
+ self.factory.makeSpecificationWorkItem(
+ title=u'workitem 1', specification=foreign_spec)
+
+ # This one, on the other hand, is explicitly assigned to the a member
+ # of our team, so it is included in the returned list even though its
+ # spec is not assigned to a member of our team.
+ workitem = self.factory.makeSpecificationWorkItem(
+ title=u'workitem 2', specification=foreign_spec,
+ assignee=self.team.teamowner)
+
+ workitems = self.team.getAssignedSpecificationWorkItemsDueBefore(
+ self.current_milestone.dateexpected)
+
+ self.assertEqual([workitem], list(workitems))
+
+
+class Test_getAssignedBugTasksDueBefore(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(Test_getAssignedBugTasksDueBefore, self).setUp()
+ self.team = self.factory.makeTeam()
+ self.today = datetime.today().date()
+
+ def _assignBugTaskToTeamOwner(self, bugtask):
+ removeSecurityProxy(bugtask).assignee = self.team.teamowner
+
+ def test_basic(self):
+ milestone = self.factory.makeMilestone(dateexpected=self.today)
+ # This bug is assigned to a team member and targeted to a milestone
+ # whose due date is before the cutoff date we pass in, so it will be
+ # included in the return of getAssignedBugTasksDueBefore().
+ milestoned_bug = self.factory.makeBug(milestone=milestone)
+ self._assignBugTaskToTeamOwner(milestoned_bug.bugtasks[0])
+ # This one is assigned to a team member but not milestoned, so it is
+ # not included in the return of getAssignedBugTasksDueBefore().
+ non_milestoned_bug = self.factory.makeBug()
+ self._assignBugTaskToTeamOwner(non_milestoned_bug.bugtasks[0])
+ # This one is milestoned but not assigned to a team member, so it is
+ # not included in the return of getAssignedBugTasksDueBefore() either.
+ non_assigned_bug = self.factory.makeBug()
+ self._assignBugTaskToTeamOwner(non_assigned_bug.bugtasks[0])
+
+ bugtasks = list(self.team.getAssignedBugTasksDueBefore(
+ self.today + timedelta(days=1), user=None))
+
+ self.assertEqual(1, len(bugtasks))
+ self.assertEqual(milestoned_bug.bugtasks[0], bugtasks[0])
+
+ def test_skips_tasks_targeted_to_old_milestones(self):
+ past_milestone = self.factory.makeMilestone(
+ dateexpected=self.today - timedelta(days=1))
+ bug = self.factory.makeBug(milestone=past_milestone)
+ self._assignBugTaskToTeamOwner(bug.bugtasks[0])
+
+ bugtasks = list(self.team.getAssignedBugTasksDueBefore(
+ self.today + timedelta(days=1), user=None))
+
+ self.assertEqual(0, len(bugtasks))
+
+ def test_skips_private_bugs_the_user_is_not_allowed_to_see(self):
+ milestone = self.factory.makeMilestone(dateexpected=self.today)
+ private_bug = removeSecurityProxy(
+ self.factory.makeBug(milestone=milestone, private=True))
+ self._assignBugTaskToTeamOwner(private_bug.bugtasks[0])
+ private_bug2 = removeSecurityProxy(
+ self.factory.makeBug(milestone=milestone, private=True))
+ self._assignBugTaskToTeamOwner(private_bug2.bugtasks[0])
+
+ with person_logged_in(private_bug2.owner):
+ bugtasks = list(self.team.getAssignedBugTasksDueBefore(
+ self.today + timedelta(days=1),
+ removeSecurityProxy(private_bug2).owner))
+
+ self.assertEqual(private_bug2.bugtasks, bugtasks)
+
+ def test_skips_distroseries_task_that_is_a_conjoined_master(self):
+ distroseries = self.factory.makeDistroSeries()
+ sourcepackagename = self.factory.makeSourcePackageName()
+ milestone = self.factory.makeMilestone(
+ distroseries=distroseries, dateexpected=self.today)
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=distroseries, sourcepackagename=sourcepackagename)
+ bug = self.factory.makeBug(
+ milestone=milestone, sourcepackagename=sourcepackagename,
+ distribution=distroseries.distribution)
+ package = distroseries.getSourcePackage(sourcepackagename.name)
+ removeSecurityProxy(bug).addTask(bug.owner, package)
+ self.assertEqual(2, len(bug.bugtasks))
+ slave, master = bug.bugtasks
+ self._assignBugTaskToTeamOwner(master)
+ self.assertEqual(None, master.conjoined_master)
+ self.assertEqual(master, slave.conjoined_master)
+ self.assertEqual(slave.milestone, master.milestone)
+ self.assertEqual(slave.assignee, master.assignee)
+
+ bugtasks = list(self.team.getAssignedBugTasksDueBefore(
+ self.today + timedelta(days=1), user=None))
+
+ self.assertEqual([slave], bugtasks)
+
+ def test_skips_productseries_task_that_is_a_conjoined_master(self):
+ milestone = self.factory.makeMilestone(dateexpected=self.today)
+ removeSecurityProxy(milestone.product).development_focus = (
+ milestone.productseries)
+ bug = self.factory.makeBug(
+ series=milestone.productseries, milestone=milestone)
+ self.assertEqual(2, len(bug.bugtasks))
+ slave, master = bug.bugtasks
+
+ # This will cause the assignee to propagate to the other bugtask as
+ # well since they're conjoined.
+ self._assignBugTaskToTeamOwner(slave)
+ self.assertEqual(master, slave.conjoined_master)
+ self.assertEqual(slave.milestone, master.milestone)
+ self.assertEqual(slave.assignee, master.assignee)
+
+ bugtasks = list(self.team.getAssignedBugTasksDueBefore(
+ self.today + timedelta(days=1), user=None))
+
+ self.assertEqual([slave], bugtasks)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-03-26 07:05:16 +0000
+++ lib/lp/testing/factory.py 2012-03-26 14:53:00 +0000
@@ -844,23 +844,26 @@
return getUtility(ITranslatorSet).new(group, language, person)
def makeMilestone(self, product=None, distribution=None,
- productseries=None, name=None, active=True):
- if product is None and distribution is None and productseries is None:
+ productseries=None, name=None, active=True,
+ dateexpected=None, distroseries=None):
+ if (product is None and distribution is None and productseries is None
+ and distroseries is None):
product = self.makeProduct()
- if distribution is None:
+ if distribution is None and distroseries is None:
if productseries is not None:
product = productseries.product
else:
productseries = self.makeProductSeries(product=product)
- distroseries = None
+ elif distroseries is None:
+ distroseries = self.makeDistroSeries(distribution=distribution)
else:
- distroseries = self.makeDistroSeries(distribution=distribution)
+ distribution = distroseries.distribution
if name is None:
name = self.getUniqueString()
return ProxyFactory(
Milestone(product=product, distribution=distribution,
productseries=productseries, distroseries=distroseries,
- name=name, active=active))
+ name=name, active=active, dateexpected=dateexpected))
def makeProcessor(self, family=None, name=None, title=None,
description=None):
@@ -1656,7 +1659,7 @@
or distribution parameters, or the those parameters must be None.
:param series: If set, the series.product must match the product
parameter, or the series.distribution must match the distribution
- parameter, or the those parameters must be None.
+ parameter, or those parameters must be None.
:param tags: If set, the tags to be added with the bug.
:param distribution: If set, the sourcepackagename is used as the
default bug target.
