Launchpad itself

Merge lp:~adeuring/launchpad/bug-675595-2 into lp:launchpad

  1. bug-675595-2
  2. Merge into devel
Proposed by Abel Deuring
Status: Merged
Approved by: Māris Fogels
Approved revision: no longer in the source branch.
Merged at revision: 11973
Proposed branch: lp:~adeuring/launchpad/bug-675595-2
Merge into: lp:launchpad
Prerequisite: lp:~adeuring/launchpad/bug-675595
Diff against target: 565 lines (+200/-119)
9 files modified
lib/canonical/launchpad/systemhomes.py (+7/-4)
lib/lp/bugs/browser/bugtask.py (+2/-2)
lib/lp/bugs/browser/tests/test_buglisting.py (+26/-0)
lib/lp/bugs/doc/milestones-from-bugtask-search.txt (+0/-54)
lib/lp/bugs/interfaces/bugtask.py (+0/-7)
lib/lp/bugs/interfaces/malone.py (+1/-1)
lib/lp/bugs/model/bugtask.py (+0/-38)
lib/lp/registry/browser/person.py (+26/-12)
lib/lp/registry/browser/tests/test_person_view.py (+138/-1)
To merge this branch: bzr merge lp:~adeuring/launchpad/bug-675595-2
Reviewer Review Type Date Requested Status
Māris Fogels (community) Approve
Review via email: mp+41604@code.launchpad.net

Commit message

[r=mars][ui=none] [r=mars, gmb][ui=none][bug=675595] new optional parameter prejoins for BugTaskSearchListingView.searchUnbatched() and for searchUnbatched() of several person related view classes; BugTaskSet.getAssignedMilestonesFromSearch() removed; RelevantMilestonesMixin.getMilestoneWidgetValues() retrieves Milestone instances directly via searchUnbatched(); allow to prejoin arbitrary tables in the SQL query in BugTaskSet.search()

Description of the change

This branch fixes bug 675595: "BugTaskSet.getAssignedMilestonesFromSearch()
can call BugTaskSet._search() directly and retrieve the milestones with
one SQL query instead of two"

Working on the branch, I realized that the method
BugTaskSet.getAssignedMilestonesFromSearch() is in fact unnecessary:

The only callsite was RelevantMilestonesMixin.getMilestoneWidgetValues();
since we can now retrieve milestones by pre-joining the table in
BugTaskSet.searchTasks(), I simply removed getAssignedMilestonesFromSearch().

getMilestoneWidgetValues() calls searchTasks() via the method
BugTaskSearchListingView.searchUnbatched(), so I added the parameter
"prejoins" to this method.

tests:

./bin/test -vvt test_buglisting -t test_person_view

To post a comment you must log in.
Revision history for this message
Māris Fogels (mars) wrote :

Hi Abel,

This code looks good! r=mars, with two comments:

First, on line 72 of the diff, I think you should include a comment that says "If the table prejoin failed, then this will issue two additional SQL queries".

Second, the SQL prejoins in lp.registry.browser.person look odd - it looks like ORM code that should live in the model has been placed in the view. Is there a better place to put these operations?

I recognize that a refactoring of the TaskSearchListingView is not trivial, so please consider the refactoring as optional. If you do want to try the refactoring, it should be in a follow-up branch.

With the above taken into consideration, I think you can land this.

Maris

review: Approve
Revision history for this message
Abel Deuring (adeuring) wrote :

Hi Māris,

thanks for the review!

On 23.11.2010 16:16, Māris Fogels wrote:
> Review: Approve
> Hi Abel,
>
> This code looks good! r=mars, with two comments:
>
> First, on line 72 of the diff, I think you should include a comment that says "If the table prejoin failed, then this will issue two additional SQL queries".

Good idea, added.

>
> Second, the SQL prejoins in lp.registry.browser.person look odd - it looks like ORM code that should live in the model has been placed in the view. Is there a better place to put these operations?

I am not if it is worth the effort: Basically, we can now pre-join more
or less arbitrary tables in calls of BugTaskSet.searchTasks(). This
method is used quite frequently, and for each callsite we might need to
pre-join other tables. There might even be conflicts: We can at present
join the same table only once, and for one callsite it might be
reasonable to pre-join for exmaple Person in order to retrieve a bug
reporter, while another callsite would benefit from pre-joining a
bugtask assignee.

So I think that the callsites of searchTasks() should decide what to
pre-join -- and these callsite live quite often in browser code. But
perhaps I am too much focused on the way how I implemented the branch
and I'm completely missing some better way to tell searchTasks() what
pre-join...

Abel

>
> I recognize that a refactoring of the TaskSearchListingView is not trivial, so please consider the refactoring as optional. If you do want to try the refactoring, it should be in a follow-up branch.
>
> With the above taken into consideration, I think you can land this.
>
> Maris

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/systemhomes.py'
--- lib/canonical/launchpad/systemhomes.py 2010-11-08 13:11:30 +0000
+++ lib/canonical/launchpad/systemhomes.py 2010-11-24 10:13:32 +0000
@@ -127,9 +127,10 @@
127 def __init__(self):127 def __init__(self):
128 self.title = 'Malone: the Launchpad bug tracker'128 self.title = 'Malone: the Launchpad bug tracker'
129129
130 def searchTasks(self, search_params):130 def searchTasks(self, search_params, prejoins=[]):
131 """See `IMaloneApplication`."""131 """See `IMaloneApplication`."""
132 return getUtility(IBugTaskSet).search(search_params)132 return getUtility(IBugTaskSet).search(
133 search_params, prejoins=prejoins)
133134
134 def createBug(self, owner, title, description, target,135 def createBug(self, owner, title, description, target,
135 security_related=False, private=False, tags=None):136 security_related=False, private=False, tags=None):
@@ -235,8 +236,10 @@
235 """See `IRosettaApplication`."""236 """See `IRosettaApplication`."""
236 projects = getUtility(ITranslationsOverview)237 projects = getUtility(ITranslationsOverview)
237 for project in projects.getMostTranslatedPillars():238 for project in projects.getMostTranslatedPillars():
238 yield { 'pillar' : project['pillar'],239 yield {
239 'font_size' : project['weight'] * 10 }240 'pillar': project['pillar'],
241 'font_size': project['weight'] * 10,
242 }
240243
241 def translatable_distroseriess(self):244 def translatable_distroseriess(self):
242 """See `IRosettaApplication`."""245 """See `IRosettaApplication`."""
243246
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2010-11-24 08:05:42 +0000
+++ lib/lp/bugs/browser/bugtask.py 2010-11-24 10:13:32 +0000
@@ -2679,7 +2679,7 @@
2679 return self._getBatchNavigator(unbatchedTasks)2679 return self._getBatchNavigator(unbatchedTasks)
26802680
2681 def searchUnbatched(self, searchtext=None, context=None,2681 def searchUnbatched(self, searchtext=None, context=None,
2682 extra_params=None):2682 extra_params=None, prejoins=[]):
2683 """Return a `SelectResults` object for the GET search criteria.2683 """Return a `SelectResults` object for the GET search criteria.
26842684
2685 :param searchtext: Text that must occur in the bug report. If2685 :param searchtext: Text that must occur in the bug report. If
@@ -2697,7 +2697,7 @@
2697 search_params = self.buildSearchParams(2697 search_params = self.buildSearchParams(
2698 searchtext=searchtext, extra_params=extra_params)2698 searchtext=searchtext, extra_params=extra_params)
2699 search_params.user = self.user2699 search_params.user = self.user
2700 tasks = context.searchTasks(search_params)2700 tasks = context.searchTasks(search_params, prejoins=prejoins)
2701 return tasks2701 return tasks
27022702
2703 def getWidgetValues(2703 def getWidgetValues(
27042704
=== modified file 'lib/lp/bugs/browser/tests/test_buglisting.py'
--- lib/lp/bugs/browser/tests/test_buglisting.py 2010-11-21 17:36:32 +0000
+++ lib/lp/bugs/browser/tests/test_buglisting.py 2010-11-24 10:13:32 +0000
@@ -3,6 +3,9 @@
33
4__metaclass__ = type4__metaclass__ = type
55
6from storm.expr import LeftJoin
7from storm.store import Store
8from testtools.matchers import Equals
6from zope.component import getUtility9from zope.component import getUtility
710
8from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities11from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
@@ -13,12 +16,17 @@
13 )16 )
14from canonical.launchpad.webapp.publisher import canonical_url17from canonical.launchpad.webapp.publisher import canonical_url
15from canonical.testing.layers import DatabaseFunctionalLayer18from canonical.testing.layers import DatabaseFunctionalLayer
19from lp.bugs.model.bugtask import BugTask
20from lp.registry.model.person import Person
16from lp.testing import (21from lp.testing import (
17 BrowserTestCase,22 BrowserTestCase,
18 login_person,23 login_person,
19 person_logged_in,24 person_logged_in,
25 StormStatementRecorder,
20 TestCaseWithFactory,26 TestCaseWithFactory,
21 )27 )
28
29from lp.testing.matchers import HasQueryCount
22from lp.testing.views import create_initialized_view30from lp.testing.views import create_initialized_view
2331
2432
@@ -130,6 +138,24 @@
130 find_tag_by_id(browser.contents, 'portlet-tags'),138 find_tag_by_id(browser.contents, 'portlet-tags'),
131 "portlet-tags should not be shown.")139 "portlet-tags should not be shown.")
132140
141 def test_searchUnbatched_can_preload_objects(self):
142 # BugTaskSearchListingView.searchUnbatched() can optionally
143 # preload objects while retureving the bugtasks.
144 product = self.factory.makeProduct()
145 bugtask_1 = self.factory.makeBug(product=product).default_bugtask
146 bugtask_2 = self.factory.makeBug(product=product).default_bugtask
147 view = create_initialized_view(product, '+bugs')
148 Store.of(product).invalidate()
149 with StormStatementRecorder() as recorder:
150 prejoins=[(Person, LeftJoin(Person, BugTask.owner==Person.id))]
151 bugtasks = list(view.searchUnbatched(prejoins=prejoins))
152 self.assertEqual(
153 [bugtask_1, bugtask_2], bugtasks)
154 # If the table prejoin failed, then this will issue two
155 # additional SQL queries
156 [bugtask.owner for bugtask in bugtasks]
157 self.assertThat(recorder, HasQueryCount(Equals(1)))
158
133159
134class BugTargetTestCase(TestCaseWithFactory):160class BugTargetTestCase(TestCaseWithFactory):
135 """Test helpers for setting up `IBugTarget` tests."""161 """Test helpers for setting up `IBugTarget` tests."""
136162
=== removed file 'lib/lp/bugs/doc/milestones-from-bugtask-search.txt'
--- lib/lp/bugs/doc/milestones-from-bugtask-search.txt 2010-09-14 15:32:53 +0000
+++ lib/lp/bugs/doc/milestones-from-bugtask-search.txt 1970-01-01 00:00:00 +0000
@@ -1,54 +0,0 @@
1= Discover the assigned milestones from a BugTask search =
2
3The IBugTaskSet method getAssignedMilestonesFromSearch() accepts a
4result set that yields BugTasks. It attempt to efficiently find all
5the distinct milestones assigned to those BugTasks. Typically this
6should be called with the results from calling IHasBugs.searchTasks()
7or IBugTaskSet.search().
8
9 >>> from lp.bugs.interfaces.bugtask import (
10 ... BugTaskSearchParams, IBugTaskSet)
11
12 >>> person = factory.makePerson()
13 >>> login(person.preferredemail.email)
14
15 >>> milestone = factory.makeMilestone()
16
17 >>> bugtask = factory.makeBugTask(target=milestone.product)
18 >>> bugtask.milestone = milestone
19 >>> bugtask.subscribe(person, person)
20 <lp.bugs.model.bugsubscription.BugSubscription ...>
21 >>> transaction.commit()
22
23The results of a search for all bug tasks related to a person can be
24passed to getAssignedMilestonesFromSearch() to discover all the
25milestones used.
26
27 >>> bugtask_set = getUtility(IBugTaskSet)
28
29 >>> bugtasks = bugtask_set.search(
30 ... BugTaskSearchParams(person, assignee=person),
31 ... BugTaskSearchParams(person, subscriber=person),
32 ... BugTaskSearchParams(person, owner=person, bug_reporter=person),
33 ... BugTaskSearchParams(person, bug_commenter=person))
34
35 >>> milestones = bugtask_set.getAssignedMilestonesFromSearch(
36 ... bugtasks)
37
38 >>> milestones
39 [<Milestone ...>]
40 >>> len(milestones)
41 1
42 >>> milestone in milestones
43 True
44
45Because get_assigned_milestones_from_bugtasks() attempts to be
46efficient, by customising the result set passed in, it rejects
47attempts to pass in lists of bug tasks.
48
49 >>> milestones = bugtask_set.getAssignedMilestonesFromSearch(
50 ... list(bugtasks))
51 Traceback (most recent call last):
52 ...
53 AssertionError: search_results must provide
54 IResultSet or ISQLObjectResultSet
550
=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py 2010-11-24 10:13:31 +0000
+++ lib/lp/bugs/interfaces/bugtask.py 2010-11-24 10:13:32 +0000
@@ -1414,13 +1414,6 @@
1414 :param params: the BugTaskSearchParams to search on.1414 :param params: the BugTaskSearchParams to search on.
1415 """1415 """
14161416
1417 def getAssignedMilestonesFromSearch(search_results):
1418 """Returns distinct milestones for the given tasks.
1419
1420 :param search_results: A result set yielding BugTask objects,
1421 typically the result of calling `BugTaskSet.search()`.
1422 """
1423
1424 def getStatusCountsForProductSeries(user, product_series):1417 def getStatusCountsForProductSeries(user, product_series):
1425 """Returns status counts for a product series' bugs.1418 """Returns status counts for a product series' bugs.
14261419
14271420
=== modified file 'lib/lp/bugs/interfaces/malone.py'
--- lib/lp/bugs/interfaces/malone.py 2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/interfaces/malone.py 2010-11-24 10:13:32 +0000
@@ -32,7 +32,7 @@
32 """Application root for malone."""32 """Application root for malone."""
33 export_as_webservice_collection(IBug)33 export_as_webservice_collection(IBug)
3434
35 def searchTasks(search_params):35 def searchTasks(search_params, prejoins=[]):
36 """Search IBugTasks with the given search parameters."""36 """Search IBugTasks with the given search parameters."""
3737
38 bug_count = Attribute("The number of bugs recorded in Launchpad")38 bug_count = Attribute("The number of bugs recorded in Launchpad")
3939
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2010-11-24 10:13:31 +0000
+++ lib/lp/bugs/model/bugtask.py 2010-11-24 10:13:32 +0000
@@ -2309,44 +2309,6 @@
2309 """See `IBugTaskSet`."""2309 """See `IBugTaskSet`."""
2310 return self._search(BugTask.bugID, [], params).result_set2310 return self._search(BugTask.bugID, [], params).result_set
23112311
2312 def getAssignedMilestonesFromSearch(self, search_results):
2313 """See `IBugTaskSet`."""
2314 # XXX: Gavin Panella 2009-03-05 bug=338184: There is currently
2315 # no clean way to get the underlying Storm ResultSet from an
2316 # SQLObjectResultSet, so we must remove the security proxy for
2317 # a moment.
2318 if ISQLObjectResultSet.providedBy(search_results):
2319 search_results = removeSecurityProxy(search_results)._result_set
2320 # Check that we have a Storm result set before we start doing
2321 # things with it.
2322 assert IResultSet.providedBy(search_results), (
2323 "search_results must provide IResultSet or ISQLObjectResultSet")
2324 # Remove ordering and make distinct.
2325 search_results = search_results.order_by().config(distinct=True)
2326 # Get milestone IDs.
2327 milestone_ids = [
2328 milestone_id for milestone_id in (
2329 search_results.values(BugTask.milestoneID))
2330 if milestone_id is not None]
2331 # Query for milestones.
2332 if len(milestone_ids) == 0:
2333 return []
2334 else:
2335 # Import here because of cyclic references.
2336 from lp.registry.model.milestone import (
2337 Milestone, milestone_sort_key)
2338 # We need the store that was used, we have no objects to key off
2339 # of other than the search result, and Store.of() doesn't
2340 # currently work on result sets. Additionally it may be a
2341 # DecoratedResultSet.
2342 if zope_isinstance(search_results, DecoratedResultSet):
2343 store = removeSecurityProxy(search_results).result_set._store
2344 else:
2345 store = search_results._store
2346 milestones = store.find(
2347 Milestone, Milestone.id.is_in(milestone_ids))
2348 return sorted(milestones, key=milestone_sort_key, reverse=True)
2349
2350 def createTask(self, bug, owner, product=None, productseries=None,2312 def createTask(self, bug, owner, product=None, productseries=None,
2351 distribution=None, distroseries=None,2313 distribution=None, distroseries=None,
2352 sourcepackagename=None,2314 sourcepackagename=None,
23532315
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-11-24 08:05:42 +0000
+++ lib/lp/registry/browser/person.py 2010-11-24 10:13:32 +0000
@@ -105,6 +105,7 @@
105 URI,105 URI,
106 )106 )
107import pytz107import pytz
108from storm.expr import Join
108from storm.zope.interfaces import IResultSet109from storm.zope.interfaces import IResultSet
109from z3c.ptcompat import ViewPageTemplateFile110from z3c.ptcompat import ViewPageTemplateFile
110from zope.app.form.browser import (111from zope.app.form.browser import (
@@ -242,6 +243,7 @@
242 IBugTaskSet,243 IBugTaskSet,
243 UNRESOLVED_BUGTASK_STATUSES,244 UNRESOLVED_BUGTASK_STATUSES,
244 )245 )
246from lp.bugs.model.bugtask import BugTask
245from lp.buildmaster.enums import BuildStatus247from lp.buildmaster.enums import BuildStatus
246from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin248from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
247from lp.code.errors import InvalidNamespace249from lp.code.errors import InvalidNamespace
@@ -304,6 +306,10 @@
304 IWikiName,306 IWikiName,
305 IWikiNameSet,307 IWikiNameSet,
306 )308 )
309from lp.registry.model.milestone import (
310 Milestone,
311 milestone_sort_key,
312 )
307from lp.services.fields import LocationField313from lp.services.fields import LocationField
308from lp.services.geoip.interfaces import IRequestPreferredLanguages314from lp.services.geoip.interfaces import IRequestPreferredLanguages
309from lp.services.openid.adapters.openid import CurrentOpenIDEndPoint315from lp.services.openid.adapters.openid import CurrentOpenIDEndPoint
@@ -2133,8 +2139,12 @@
21332139
2134 def getMilestoneWidgetValues(self):2140 def getMilestoneWidgetValues(self):
2135 """Return data used to render the milestone checkboxes."""2141 """Return data used to render the milestone checkboxes."""
2136 milestones = getUtility(IBugTaskSet).getAssignedMilestonesFromSearch(2142 prejoins = [
2137 self.searchUnbatched())2143 (Milestone, Join(Milestone, BugTask.milestone == Milestone.id))]
2144 milestones = [
2145 bugtask.milestone
2146 for bugtask in self.searchUnbatched(prejoins=prejoins)]
2147 milestones = sorted(milestones, key=milestone_sort_key, reverse=True)
2138 return [2148 return [
2139 dict(title=milestone.title, value=milestone.id, checked=False)2149 dict(title=milestone.title, value=milestone.id, checked=False)
2140 for milestone in milestones]2150 for milestone in milestones]
@@ -2150,7 +2160,7 @@
2150 page_title = 'Related bugs'2160 page_title = 'Related bugs'
21512161
2152 def searchUnbatched(self, searchtext=None, context=None,2162 def searchUnbatched(self, searchtext=None, context=None,
2153 extra_params=None):2163 extra_params=None, prejoins=[]):
2154 """Return the open bugs related to a person.2164 """Return the open bugs related to a person.
21552165
2156 :param extra_params: A dict that provides search params added to2166 :param extra_params: A dict that provides search params added to
@@ -2181,7 +2191,7 @@
21812191
2182 return context.searchTasks(2192 return context.searchTasks(
2183 assignee_params, subscriber_params, owner_params,2193 assignee_params, subscriber_params, owner_params,
2184 commenter_params)2194 commenter_params, prejoins=prejoins)
21852195
2186 def getSearchPageHeading(self):2196 def getSearchPageHeading(self):
2187 return "Bugs related to %s" % self.context.displayname2197 return "Bugs related to %s" % self.context.displayname
@@ -2210,7 +2220,7 @@
2210 page_title = 'Assigned bugs'2220 page_title = 'Assigned bugs'
22112221
2212 def searchUnbatched(self, searchtext=None, context=None,2222 def searchUnbatched(self, searchtext=None, context=None,
2213 extra_params=None):2223 extra_params=None, prejoins=[]):
2214 """Return the open bugs assigned to a person."""2224 """Return the open bugs assigned to a person."""
2215 if context is None:2225 if context is None:
2216 context = self.context2226 context = self.context
@@ -2222,7 +2232,8 @@
2222 extra_params['assignee'] = context2232 extra_params['assignee'] = context
22232233
2224 sup = super(PersonAssignedBugTaskSearchListingView, self)2234 sup = super(PersonAssignedBugTaskSearchListingView, self)
2225 return sup.searchUnbatched(searchtext, context, extra_params)2235 return sup.searchUnbatched(
2236 searchtext, context, extra_params, prejoins)
22262237
2227 def shouldShowAssigneeWidget(self):2238 def shouldShowAssigneeWidget(self):
2228 """Should the assignee widget be shown on the advanced search page?"""2239 """Should the assignee widget be shown on the advanced search page?"""
@@ -2267,7 +2278,7 @@
2267 page_title = 'Commented bugs'2278 page_title = 'Commented bugs'
22682279
2269 def searchUnbatched(self, searchtext=None, context=None,2280 def searchUnbatched(self, searchtext=None, context=None,
2270 extra_params=None):2281 extra_params=None, prejoins=[]):
2271 """Return the open bugs commented on by a person."""2282 """Return the open bugs commented on by a person."""
2272 if context is None:2283 if context is None:
2273 context = self.context2284 context = self.context
@@ -2279,7 +2290,8 @@
2279 extra_params['bug_commenter'] = context2290 extra_params['bug_commenter'] = context
22802291
2281 sup = super(PersonCommentedBugTaskSearchListingView, self)2292 sup = super(PersonCommentedBugTaskSearchListingView, self)
2282 return sup.searchUnbatched(searchtext, context, extra_params)2293 return sup.searchUnbatched(
2294 searchtext, context, extra_params, prejoins)
22832295
2284 def getSearchPageHeading(self):2296 def getSearchPageHeading(self):
2285 """The header for the search page."""2297 """The header for the search page."""
@@ -2312,7 +2324,7 @@
2312 page_title = 'Reported bugs'2324 page_title = 'Reported bugs'
23132325
2314 def searchUnbatched(self, searchtext=None, context=None,2326 def searchUnbatched(self, searchtext=None, context=None,
2315 extra_params=None):2327 extra_params=None, prejoins=[]):
2316 """Return the bugs reported by a person."""2328 """Return the bugs reported by a person."""
2317 if context is None:2329 if context is None:
2318 context = self.context2330 context = self.context
@@ -2327,7 +2339,8 @@
2327 extra_params['bug_reporter'] = context2339 extra_params['bug_reporter'] = context
23282340
2329 sup = super(PersonReportedBugTaskSearchListingView, self)2341 sup = super(PersonReportedBugTaskSearchListingView, self)
2330 return sup.searchUnbatched(searchtext, context, extra_params)2342 return sup.searchUnbatched(
2343 searchtext, context, extra_params, prejoins)
23312344
2332 def getSearchPageHeading(self):2345 def getSearchPageHeading(self):
2333 """The header for the search page."""2346 """The header for the search page."""
@@ -2368,7 +2381,7 @@
2368 page_title = 'Subscribed bugs'2381 page_title = 'Subscribed bugs'
23692382
2370 def searchUnbatched(self, searchtext=None, context=None,2383 def searchUnbatched(self, searchtext=None, context=None,
2371 extra_params=None):2384 extra_params=None, prejoins=[]):
2372 """Return the bugs subscribed to by a person."""2385 """Return the bugs subscribed to by a person."""
2373 if context is None:2386 if context is None:
2374 context = self.context2387 context = self.context
@@ -2380,7 +2393,8 @@
2380 extra_params['subscriber'] = context2393 extra_params['subscriber'] = context
23812394
2382 sup = super(PersonSubscribedBugTaskSearchListingView, self)2395 sup = super(PersonSubscribedBugTaskSearchListingView, self)
2383 return sup.searchUnbatched(searchtext, context, extra_params)2396 return sup.searchUnbatched(
2397 searchtext, context, extra_params, prejoins)
23842398
2385 def getSearchPageHeading(self):2399 def getSearchPageHeading(self):
2386 """The header for the search page."""2400 """The header for the search page."""
23872401
=== modified file 'lib/lp/registry/browser/tests/test_person_view.py'
--- lib/lp/registry/browser/tests/test_person_view.py 2010-10-06 08:16:37 +0000
+++ lib/lp/registry/browser/tests/test_person_view.py 2010-11-24 10:13:32 +0000
@@ -4,8 +4,10 @@
4__metaclass__ = type4__metaclass__ = type
55
6import transaction6import transaction
7from storm.expr import LeftJoin
8from storm.store import Store
9from testtools.matchers import Equals
7from zope.component import getUtility10from zope.component import getUtility
8from zope.security.proxy import removeSecurityProxy
911
10from canonical.config import config12from canonical.config import config
11from canonical.launchpad.ftests import (13from canonical.launchpad.ftests import (
@@ -25,6 +27,7 @@
25 )27 )
2628
27from lp.app.errors import NotFoundError29from lp.app.errors import NotFoundError
30from lp.bugs.model.bugtask import BugTask
28from lp.buildmaster.enums import BuildStatus31from lp.buildmaster.enums import BuildStatus
29from lp.registry.browser.person import (32from lp.registry.browser.person import (
30 PersonEditView,33 PersonEditView,
@@ -43,15 +46,20 @@
43 )46 )
4447
45from lp.registry.model.karma import KarmaCategory48from lp.registry.model.karma import KarmaCategory
49from lp.registry.model.milestone import milestone_sort_key
46from lp.soyuz.enums import (50from lp.soyuz.enums import (
47 ArchiveStatus,51 ArchiveStatus,
48 PackagePublishingStatus,52 PackagePublishingStatus,
49 )53 )
54from lp.registry.model.person import Person
50from lp.soyuz.tests.test_publishing import SoyuzTestPublisher55from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
51from lp.testing import (56from lp.testing import (
52 login_person,57 login_person,
58 person_logged_in,
59 StormStatementRecorder,
53 TestCaseWithFactory,60 TestCaseWithFactory,
54 )61 )
62from lp.testing.matchers import HasQueryCount
55from lp.testing.views import (63from lp.testing.views import (
56 create_initialized_view,64 create_initialized_view,
57 create_view,65 create_view,
@@ -809,3 +817,132 @@
809 login_person(self.person)817 login_person(self.person)
810 view = create_initialized_view(self.team, '+subscriptions')818 view = create_initialized_view(self.team, '+subscriptions')
811 self.assertTrue(view.canUnsubscribeFromBugTasks())819 self.assertTrue(view.canUnsubscribeFromBugTasks())
820
821
822class BugTaskViewsTestBase:
823 """A base class for bugtask search related tests."""
824
825 layer = DatabaseFunctionalLayer
826
827 def setUp(self):
828 super(BugTaskViewsTestBase, self).setUp()
829 self.person = self.factory.makePerson()
830 with person_logged_in(self.person):
831 self.subscribed_bug = self.factory.makeBug()
832 self.subscribed_bug.subscribe(
833 self.person, subscribed_by=self.person)
834 self.assigned_bug = self.factory.makeBug()
835 self.assigned_bug.default_bugtask.transitionToAssignee(
836 self.person)
837 self.owned_bug = self.factory.makeBug(owner=self.person)
838 self.commented_bug = self.factory.makeBug()
839 self.commented_bug.newMessage(owner=self.person)
840
841 for bug in (self.subscribed_bug, self.assigned_bug, self.owned_bug,
842 self.commented_bug):
843 with person_logged_in(bug.default_bugtask.product.owner):
844 milestone = self.factory.makeMilestone(
845 product=bug.default_bugtask.product)
846 bug.default_bugtask.transitionToMilestone(
847 milestone, bug.default_bugtask.product.owner)
848
849 def test_searchUnbatched(self):
850 view = create_initialized_view(self.person, self.view_name)
851 self.assertEqual(
852 self.expected_for_search_unbatched, list(view.searchUnbatched()))
853
854 def test_searchUnbatched_with_prejoins(self):
855 view = create_initialized_view(self.person, self.view_name)
856 Store.of(self.subscribed_bug).invalidate()
857 with StormStatementRecorder() as recorder:
858 prejoins=[(Person, LeftJoin(Person, BugTask.owner==Person.id))]
859 bugtasks = view.searchUnbatched(prejoins=prejoins)
860 [bugtask.owner for bugtask in bugtasks]
861 self.assertThat(recorder, HasQueryCount(Equals(1)))
862
863 def test_getMilestoneWidgetValues(self):
864 view = create_initialized_view(self.person, self.view_name)
865 milestones = [
866 bugtask.milestone
867 for bugtask in self.expected_for_search_unbatched]
868 milestones = sorted(milestones, key=milestone_sort_key, reverse=True)
869 expected = [
870 {
871 'title': milestone.title,
872 'value': milestone.id,
873 'checked': False,
874 }
875 for milestone in milestones]
876 Store.of(milestones[0]).invalidate()
877 with StormStatementRecorder() as recorder:
878 self.assertEqual(expected, view.getMilestoneWidgetValues())
879 self.assertThat(recorder, HasQueryCount(Equals(1)))
880
881
882class TestPersonRelatedBugTaskSearchListingView(
883 BugTaskViewsTestBase, TestCaseWithFactory):
884 """Tests for PersonRelatedBugTaskSearchListingView."""
885
886 view_name = '+bugs'
887
888 def setUp(self):
889 super(TestPersonRelatedBugTaskSearchListingView, self).setUp()
890 self.expected_for_search_unbatched = [
891 self.subscribed_bug.default_bugtask,
892 self.assigned_bug.default_bugtask,
893 self.owned_bug.default_bugtask,
894 self.commented_bug.default_bugtask,
895 ]
896
897
898class TestPersonAssignedBugTaskSearchListingView(
899 BugTaskViewsTestBase, TestCaseWithFactory):
900 """Tests for PersonAssignedBugTaskSearchListingView."""
901
902 view_name = '+assignedbugs'
903
904 def setUp(self):
905 super(TestPersonAssignedBugTaskSearchListingView, self).setUp()
906 self.expected_for_search_unbatched = [
907 self.assigned_bug.default_bugtask,
908 ]
909
910
911class TestPersonCommentedBugTaskSearchListingView(
912 BugTaskViewsTestBase, TestCaseWithFactory):
913 """Tests for PersonAssignedBugTaskSearchListingView."""
914
915 view_name = '+commentedbugs'
916
917 def setUp(self):
918 super(TestPersonCommentedBugTaskSearchListingView, self).setUp()
919 self.expected_for_search_unbatched = [
920 self.commented_bug.default_bugtask,
921 ]
922
923
924class TestPersonReportedBugTaskSearchListingView(
925 BugTaskViewsTestBase, TestCaseWithFactory):
926 """Tests for PersonAssignedBugTaskSearchListingView."""
927
928 view_name = '+reportedbugs'
929
930 def setUp(self):
931 super(TestPersonReportedBugTaskSearchListingView, self).setUp()
932 self.expected_for_search_unbatched = [
933 self.owned_bug.default_bugtask,
934 ]
935
936
937class TestPersonSubscribedBugTaskSearchListingView(
938 BugTaskViewsTestBase, TestCaseWithFactory):
939 """Tests for PersonAssignedBugTaskSearchListingView."""
940
941 view_name = '+subscribedbugs'
942
943 def setUp(self):
944 super(TestPersonSubscribedBugTaskSearchListingView, self).setUp()
945 self.expected_for_search_unbatched = [
946 self.subscribed_bug.default_bugtask,
947 self.owned_bug.default_bugtask,
948 ]