Merge lp:~lifeless/launchpad/malone into lp:launchpad

Proposed by Robert Collins
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11582
Proposed branch: lp:~lifeless/launchpad/malone
Merge into: lp:launchpad
Diff against target: 598 lines (+223/-101)
13 files modified
lib/canonical/launchpad/webapp/batching.py (+18/-0)
lib/lp/blueprints/browser/tests/test_specificationtarget.py (+6/-27)
lib/lp/bugs/browser/bugtracker.py (+29/-16)
lib/lp/bugs/browser/tests/test_bugtracker_views.py (+51/-17)
lib/lp/bugs/interfaces/bugtracker.py (+8/-0)
lib/lp/bugs/model/bugtracker.py (+15/-1)
lib/lp/bugs/stories/bugtracker/bugtrackers-index.txt (+6/-4)
lib/lp/bugs/templates/bugtrackers-index.pt (+6/-4)
lib/lp/bugs/tests/test_bugtracker.py (+28/-1)
lib/lp/registry/browser/person.py (+5/-19)
lib/lp/registry/browser/team.py (+0/-2)
lib/lp/registry/browser/tests/test_team.py (+4/-10)
lib/lp/testing/matchers.py (+47/-0)
To merge this branch: bzr merge lp:~lifeless/launchpad/malone
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+35941@code.launchpad.net

Commit message

Batch /bugs/bugtrackers

Description of the change

Batch the bug tracker lists - theres no point showing 1000 by default; the batching will also shrink the time massively; we can actually tune it later (this was sensible to do regardless and will give more headroom than just tuning it in the short term).

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :
Download full text (19.3 KiB)

The change is basically fine, and clearly a good idea. I have a number of code clarity comments though, see below.

> === modified file 'lib/canonical/launchpad/webapp/batching.py'
> --- lib/canonical/launchpad/webapp/batching.py 2010-09-06 18:52:45 +0000
> +++ lib/canonical/launchpad/webapp/batching.py 2010-09-19 22:21:05 +0000
> @@ -77,6 +77,24 @@
> return self.batch.total() > self.batch.size
>
>
> +class ActiveBatchNavigator(BatchNavigator):
> + """A paginator for active items.
> +
> + Used when a view needs to display more than one BatchNavigator of items.
> + """
> + start_variable_name = 'active_start'
> + batch_variable_name = 'active_batch'
> +
> +
> +class InactiveBatchNavigator(BatchNavigator):
> + """A paginator for inactive items.
> +
> + Used when a view needs to display more than one BatchNavigator of items.
> + """
> + start_variable_name = 'inactive_start'
> + batch_variable_name = 'inactive_batch'
> +
> +

These are fine and clear enough -- are they generic enough to go in a
generic enough location though? Later: oh, I see you moved them from
other code. It's a shame you have to subclass to change these.

> class TableBatchNavigator(BatchNavigator):
> """See canonical.launchpad.interfaces.ITableBatchNavigator."""
> implements(ITableBatchNavigator)

> === modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py'
> --- lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-17 14:04:29 +0000
> +++ lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-19 22:21:05 +0000
> @@ -3,11 +3,9 @@
>
> __metaclass__ = type
>
> -import unittest
>
> from zope.security.proxy import removeSecurityProxy
>
> -from canonical.launchpad.webapp.batching import BatchNavigator
> from canonical.launchpad.testing.pages import find_tag_by_id
> from canonical.testing.layers import DatabaseFunctionalLayer
> from lp.blueprints.interfaces.specificationtarget import (
> @@ -21,6 +19,7 @@
> login_person,
> TestCaseWithFactory,
> )
> +from lp.testing.matchers import IsConfiguredBatchNavigator
> from lp.testing.views import (
> create_view,
> create_initialized_view,
> @@ -108,22 +107,11 @@
> # subclass.
> person = self.factory.makePerson()
> view = create_initialized_view(person, name='+assignments')

Just up from here there's a comment:

        # Some pages turn up in very large contexts and patch. E.g.
        # Distro:+assignments which uses SpecificationAssignmentsView, a
        # subclass.

Is patch a typo for batch here?

> - self.assertIsInstance(view.specs_batched, BatchNavigator)
> -
> - def test_batch_headings(self):
> - person = self.factory.makePerson()
> - view = create_initialized_view(person, name='+assignments')
> - navigator = view.specs_batched
> - self.assertEqual('specification', navigator._singular_heading)
> - self.assertEqual('specifications', navigator._plural_heading)
> -
> - def test_batch_size(self):
> # Because +assignments is meant to provide an overview, we default to
> # 500 as the default batch size.
> - ...

review: Approve
Revision history for this message
Robert Collins (lifeless) wrote :

Thanks for the feedback will do most of it.

I have a few quirks though:
 - matchers inline - i agree, but the noddy style guide line wrapping
rules make it ugly. If you agree that inline is better even so, I'll
move it inline and wrap it the way I would in every other python
project.

 - the flavor comment is there because its a deprecated API. I think
the deprecation is a mistake until we actually don't need it.

Thanks,
Rob

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

For the first, I don't think:

        self.assertThat(
            view.specs_batched,
            IsConfiguredBatchNavigator(
                'specification', 'specifications', batch_size=500))

is too bad, but *shrug*, indent it however you like :)

For the second, fine.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/batching.py'
2--- lib/canonical/launchpad/webapp/batching.py 2010-09-06 18:52:45 +0000
3+++ lib/canonical/launchpad/webapp/batching.py 2010-09-20 05:15:57 +0000
4@@ -77,6 +77,24 @@
5 return self.batch.total() > self.batch.size
6
7
8+class ActiveBatchNavigator(BatchNavigator):
9+ """A paginator for active items.
10+
11+ Used when a view needs to display more than one BatchNavigator of items.
12+ """
13+ start_variable_name = 'active_start'
14+ batch_variable_name = 'active_batch'
15+
16+
17+class InactiveBatchNavigator(BatchNavigator):
18+ """A paginator for inactive items.
19+
20+ Used when a view needs to display more than one BatchNavigator of items.
21+ """
22+ start_variable_name = 'inactive_start'
23+ batch_variable_name = 'inactive_batch'
24+
25+
26 class TableBatchNavigator(BatchNavigator):
27 """See canonical.launchpad.interfaces.ITableBatchNavigator."""
28 implements(ITableBatchNavigator)
29
30=== modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py'
31--- lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-17 14:04:29 +0000
32+++ lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-20 05:15:57 +0000
33@@ -3,11 +3,9 @@
34
35 __metaclass__ = type
36
37-import unittest
38
39 from zope.security.proxy import removeSecurityProxy
40
41-from canonical.launchpad.webapp.batching import BatchNavigator
42 from canonical.launchpad.testing.pages import find_tag_by_id
43 from canonical.testing.layers import DatabaseFunctionalLayer
44 from lp.blueprints.interfaces.specificationtarget import (
45@@ -21,6 +19,7 @@
46 login_person,
47 TestCaseWithFactory,
48 )
49+from lp.testing.matchers import IsConfiguredBatchNavigator
50 from lp.testing.views import (
51 create_view,
52 create_initialized_view,
53@@ -103,27 +102,17 @@
54 '<div id="involvement" class="portlet involvement">' in view())
55
56 def test_specs_batch(self):
57- # Some pages turn up in very large contexts and patch. E.g.
58+ # Some pages turn up in very large contexts and so batch. E.g.
59 # Distro:+assignments which uses SpecificationAssignmentsView, a
60 # subclass.
61 person = self.factory.makePerson()
62 view = create_initialized_view(person, name='+assignments')
63- self.assertIsInstance(view.specs_batched, BatchNavigator)
64-
65- def test_batch_headings(self):
66- person = self.factory.makePerson()
67- view = create_initialized_view(person, name='+assignments')
68- navigator = view.specs_batched
69- self.assertEqual('specification', navigator._singular_heading)
70- self.assertEqual('specifications', navigator._plural_heading)
71-
72- def test_batch_size(self):
73 # Because +assignments is meant to provide an overview, we default to
74 # 500 as the default batch size.
75- person = self.factory.makePerson()
76- view = create_initialized_view(person, name='+assignments')
77- navigator = view.specs_batched
78- self.assertEqual(500, view.specs_batched.default_size)
79+ self.assertThat(
80+ view.specs_batched,
81+ IsConfiguredBatchNavigator(
82+ 'specification', 'specifications', batch_size=500))
83
84
85 class TestAssignments(TestCaseWithFactory):
86@@ -243,13 +232,3 @@
87 login_person(project_group.owner)
88 view = create_initialized_view(project_group, '+specs')
89 self.assertEqual(False, view.can_configure_blueprints)
90-
91-
92-def test_suite():
93- suite = unittest.TestSuite()
94- suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))
95- return suite
96-
97-
98-if __name__ == '__main__':
99- unittest.TextTestRunner().run(test_suite())
100
101=== modified file 'lib/lp/bugs/browser/bugtracker.py'
102--- lib/lp/bugs/browser/bugtracker.py 2010-09-03 15:02:39 +0000
103+++ lib/lp/bugs/browser/bugtracker.py 2010-09-20 05:15:57 +0000
104@@ -54,7 +54,11 @@
105 structured,
106 )
107 from canonical.launchpad.webapp.authorization import check_permission
108-from canonical.launchpad.webapp.batching import BatchNavigator
109+from canonical.launchpad.webapp.batching import (
110+ ActiveBatchNavigator,
111+ BatchNavigator,
112+ InactiveBatchNavigator,
113+ )
114 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
115 from canonical.launchpad.webapp.menu import NavigationMenu
116 from canonical.lazr.utils import smartquote
117@@ -154,21 +158,30 @@
118 pillar_limit = 3
119
120 def initialize(self):
121- # Sort the bug trackers into active and inactive lists so that
122- # we can display them separately.
123- all_bug_trackers = list(self.context)
124- self.active_bug_trackers = [
125- bug_tracker for bug_tracker in all_bug_trackers
126- if bug_tracker.active]
127- self.inactive_bug_trackers = [
128- bug_tracker for bug_tracker in all_bug_trackers
129- if not bug_tracker.active]
130-
131- bugtrackerset = getUtility(IBugTrackerSet)
132- # The caching of bugtracker pillars here avoids us hitting the
133- # database multiple times for each bugtracker.
134- self._pillar_cache = bugtrackerset.getPillarsForBugtrackers(
135- all_bug_trackers)
136+ # eager load related pillars. In future we should do this for
137+ # just the rendered trackers, and also use group by to get
138+ # bug watch counts per tracker. However the batching makes
139+ # the inefficiency tolerable for now. Robert Collins 20100919.
140+ self._pillar_cache = self.context.getPillarsForBugtrackers(
141+ list(self.context.trackers()))
142+
143+ @property
144+ def inactive_tracker_count(self):
145+ return self.inactive_trackers.currentBatch().listlength
146+
147+ @cachedproperty
148+ def active_trackers(self):
149+ results = self.context.trackers(active=True)
150+ navigator = ActiveBatchNavigator(results, self.request)
151+ navigator.setHeadings('tracker', 'trackers')
152+ return navigator
153+
154+ @cachedproperty
155+ def inactive_trackers(self):
156+ results = self.context.trackers(active=False)
157+ navigator = InactiveBatchNavigator(results, self.request)
158+ navigator.setHeadings('tracker', 'trackers')
159+ return navigator
160
161 def getPillarData(self, bugtracker):
162 """Return dict of pillars and booleans indicating ellipsis.
163
164=== modified file 'lib/lp/bugs/browser/tests/test_bugtracker_views.py'
165--- lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-08-26 20:08:43 +0000
166+++ lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-09-20 05:15:57 +0000
167@@ -3,27 +3,61 @@
168
169 """Tests for BugTracker views."""
170
171+from __future__ import with_statement
172+
173 __metaclass__ = type
174
175-import unittest
176-
177-from datetime import datetime, timedelta
178-from pytz import utc
179-
180 from zope.component import getUtility
181-from zope.security.interfaces import Unauthorized
182
183-from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
184+from canonical.launchpad.testing.pages import find_tag_by_id
185+from canonical.launchpad.webapp import canonical_url
186 from canonical.testing.layers import DatabaseFunctionalLayer
187-
188-from lp.bugs.browser.bugtracker import (
189- BugTrackerEditView)
190+from lp.bugs.interfaces.bugtracker import IBugTrackerSet
191 from lp.registry.interfaces.person import IPersonSet
192-from lp.testing import login, TestCaseWithFactory
193-from lp.testing.sampledata import ADMIN_EMAIL, NO_PRIVILEGE_EMAIL
194+from lp.testing import (
195+ login,
196+ TestCaseWithFactory,
197+ )
198 from lp.testing.views import create_initialized_view
199-
200-
201-
202-def test_suite():
203- return unittest.TestLoader().loadTestsFromName(__name__)
204+from lp.testing.matchers import IsConfiguredBatchNavigator
205+from lp.testing import (
206+ person_logged_in,
207+ TestCaseWithFactory,
208+ )
209+from lp.testing.sampledata import ADMIN_EMAIL
210+
211+
212+class TestBugTrackerSetView(TestCaseWithFactory):
213+
214+ layer = DatabaseFunctionalLayer
215+
216+ def test_trackers_are_batch_navigators(self):
217+ trackers = getUtility(IBugTrackerSet)
218+ view = create_initialized_view(trackers, name='+index')
219+ matcher = IsConfiguredBatchNavigator('tracker', 'trackers')
220+ self.assertThat(view.active_trackers, matcher)
221+ self.assertThat(view.inactive_trackers, matcher)
222+
223+ def test_page_is_batched(self):
224+ active_tracker1 = self.factory.makeBugTracker()
225+ active_tracker2 = self.factory.makeBugTracker()
226+ inactive_tracker1 = self.factory.makeBugTracker()
227+ inactive_tracker2 = self.factory.makeBugTracker()
228+ admin = getUtility(IPersonSet).find(ADMIN_EMAIL).any()
229+ with person_logged_in(admin):
230+ inactive_tracker1.active = False
231+ inactive_tracker2.active = False
232+ trackers = getUtility(IBugTrackerSet)
233+ url = (canonical_url(trackers) +
234+ "/+index?active_batch=1&inactive_batch=1")
235+ browser = self.getUserBrowser(url)
236+ content = browser.contents
237+ # XXX RobertCollns 20100919 bug=642504. The support for multiple batches
238+ # isn't complete and the id for the nav links gets duplicated.
239+ #self.assertEqual('next',
240+ # find_tag_by_id(content, 'upper-batch-nav-batchnav-next')['class'])
241+ #self.assertEqual('next',
242+ # find_tag_by_id(content, 'lower-batch-nav-batchnav-next')['class'])
243+ # Instead we check the string appears.
244+ self.assertTrue('upper-batch-nav-batchnav-next' in content)
245+
246
247=== modified file 'lib/lp/bugs/interfaces/bugtracker.py'
248--- lib/lp/bugs/interfaces/bugtracker.py 2010-08-26 20:08:43 +0000
249+++ lib/lp/bugs/interfaces/bugtracker.py 2010-09-20 05:15:57 +0000
250@@ -432,6 +432,14 @@
251 def getPillarsForBugtrackers(bug_trackers):
252 """Return dict mapping bugtrackers to lists of pillars."""
253
254+ def trackers(active=None):
255+ """Return a ResultSet of bugtrackers.
256+
257+ :param active: If True, only active trackers are returned, if False
258+ only inactive trackers are returned. All trackers are returned
259+ by default.
260+ """
261+
262
263 class IBugTrackerAlias(Interface):
264 """Another URL for a remote bug system.
265
266=== modified file 'lib/lp/bugs/model/bugtracker.py'
267--- lib/lp/bugs/model/bugtracker.py 2010-08-26 20:08:43 +0000
268+++ lib/lp/bugs/model/bugtracker.py 2010-09-20 05:15:57 +0000
269@@ -8,7 +8,8 @@
270 'BugTracker',
271 'BugTrackerAlias',
272 'BugTrackerAliasSet',
273- 'BugTrackerSet']
274+ 'BugTrackerSet',
275+ ]
276
277
278 from datetime import datetime
279@@ -66,6 +67,8 @@
280 IBugTrackerSet,
281 SINGLE_PRODUCT_BUGTRACKERTYPES,
282 )
283+from canonical.launchpad.webapp.interfaces import (
284+ DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
285 from lp.bugs.interfaces.bugtrackerperson import BugTrackerPersonAlreadyExists
286 from lp.bugs.model.bug import Bug
287 from lp.bugs.model.bugmessage import BugMessage
288@@ -577,6 +580,17 @@
289 """See `IBugTrackerSet`."""
290 return BugTracker.select()
291
292+ def trackers(self, active=None):
293+ # Without context, cannot tell what store flavour is desirable.
294+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
295+ if active is not None:
296+ clauses = [BugTracker.active==active]
297+ else:
298+ clauses = []
299+ results = store.find(BugTracker, *clauses)
300+ results.order_by(BugTracker.name)
301+ return results
302+
303 def ensureBugTracker(self, baseurl, owner, bugtrackertype,
304 title=None, summary=None, contactdetails=None, name=None):
305 """See `IBugTrackerSet`."""
306
307=== modified file 'lib/lp/bugs/stories/bugtracker/bugtrackers-index.txt'
308--- lib/lp/bugs/stories/bugtracker/bugtrackers-index.txt 2010-07-26 13:30:57 +0000
309+++ lib/lp/bugs/stories/bugtracker/bugtrackers-index.txt 2010-09-20 05:15:57 +0000
310@@ -39,11 +39,11 @@
311 Email Address
312 0
313 ------------------------
314- GNU Savannah Bug Tracker
315- http://savannah.gnu.org/
316- --> http://savannah.gnu.org/
317+ T'other Gnome GBugGTracker
318+ http://bugzilla.gnome.org/
319+ --> http://bugzilla.gnome.org/
320 &mdash;
321- Savane
322+ Bugzilla
323 0
324 ------------------------
325 ...
326@@ -68,6 +68,8 @@
327 &mdash;
328 Email Address
329 0
330+ ------------------------
331+ ...
332
333 The watch counts match the number of bugs listed, of course:
334
335
336=== modified file 'lib/lp/bugs/templates/bugtrackers-index.pt'
337--- lib/lp/bugs/templates/bugtrackers-index.pt 2009-09-01 16:24:52 +0000
338+++ lib/lp/bugs/templates/bugtrackers-index.pt 2010-09-20 05:15:57 +0000
339@@ -13,6 +13,7 @@
340 </tal:comment>
341 <metal:macros fill-slot="bogus">
342 <metal:macro define-macro="tracker-table">
343+ <tal:navigation content="structure trackers/@@+navigation-links-upper" />
344 <table class="sortable listing" tal:attributes="id id">
345 <thead>
346 <tr>
347@@ -24,7 +25,7 @@
348 </tr>
349 </thead>
350 <tbody>
351- <tr tal:repeat="tracker trackers">
352+ <tr tal:repeat="tracker trackers/currentBatch">
353 <td>
354 <a tal:replace="structure tracker/fmt:link" />
355 </td>
356@@ -53,6 +54,7 @@
357 </tr>
358 </tbody>
359 </table>
360+ <tal:navigation content="structure trackers/@@+navigation-links-lower" />
361 </metal:macro>
362 </metal:macros>
363
364@@ -70,7 +72,7 @@
365 the external status changes.
366 </p>
367 <tal:table define="id string:trackers;
368- trackers view/active_bug_trackers">
369+ trackers view/active_trackers">
370 <metal:table use-macro="template/macros/tracker-table" />
371 </tal:table>
372 <p style="margin-top: 1em"
373@@ -81,11 +83,11 @@
374 </p>
375 </div>
376 </div>
377- <div class="yui-u" tal:condition="view/inactive_bug_trackers">
378+ <div class="yui-u" tal:condition="view/inactive_tracker_count">
379 <div class="portlet">
380 <h2>Inactive bug trackers</h2>
381 <tal:table define="id string:inactive-trackers;
382- trackers view/inactive_bug_trackers">
383+ trackers view/inactive_trackers">
384 <metal:table use-macro="template/macros/tracker-table" />
385 </tal:table>
386 </div>
387
388=== modified file 'lib/lp/bugs/tests/test_bugtracker.py'
389--- lib/lp/bugs/tests/test_bugtracker.py 2010-08-26 20:08:43 +0000
390+++ lib/lp/bugs/tests/test_bugtracker.py 2010-09-20 05:15:57 +0000
391@@ -28,7 +28,10 @@
392
393 from canonical.launchpad.ftests import login_person
394 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
395-from canonical.testing import LaunchpadFunctionalLayer
396+from canonical.testing import (
397+ DatabaseFunctionalLayer,
398+ LaunchpadFunctionalLayer,
399+ )
400 from lp.bugs.externalbugtracker import (
401 BugTrackerConnectError,
402 Mantis,
403@@ -38,12 +41,36 @@
404 BugTrackerType,
405 IBugTracker,
406 )
407+from lp.bugs.model.bugtracker import BugTrackerSet
408 from lp.bugs.tests.externalbugtracker import Urlib2TransportTestHandler
409 from lp.registry.interfaces.person import IPersonSet
410 from lp.testing import login, login_person, TestCaseWithFactory
411 from lp.testing.sampledata import ADMIN_EMAIL
412
413
414+class TestBugTrackerSet(TestCaseWithFactory):
415+
416+ layer = DatabaseFunctionalLayer
417+
418+ def test_trackers(self):
419+ tracker = self.factory.makeBugTracker()
420+ trackers = BugTrackerSet()
421+ # Active trackers are in all trackers,
422+ self.assertTrue(tracker in trackers.trackers())
423+ # and active,
424+ self.assertTrue(tracker in trackers.trackers(active=True))
425+ # But not inactive.
426+ self.assertFalse(tracker in trackers.trackers(active=False))
427+ login(ADMIN_EMAIL)
428+ tracker.active = False
429+ # Inactive trackers are in all trackers
430+ self.assertTrue(tracker in trackers.trackers())
431+ # and inactive,
432+ self.assertTrue(tracker in trackers.trackers(active=False))
433+ # but not in active.
434+ self.assertFalse(tracker in trackers.trackers(active=True))
435+
436+
437 class BugTrackerTestCase(TestCaseWithFactory):
438 """Unit tests for the `BugTracker` class."""
439
440
441=== modified file 'lib/lp/registry/browser/person.py'
442--- lib/lp/registry/browser/person.py 2010-09-15 16:44:41 +0000
443+++ lib/lp/registry/browser/person.py 2010-09-20 05:15:57 +0000
444@@ -194,7 +194,11 @@
445 structured,
446 )
447 from canonical.launchpad.webapp.authorization import check_permission
448-from canonical.launchpad.webapp.batching import BatchNavigator
449+from canonical.launchpad.webapp.batching import (
450+ ActiveBatchNavigator,
451+ BatchNavigator,
452+ InactiveBatchNavigator,
453+ )
454 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
455 from canonical.launchpad.webapp.interfaces import (
456 ILaunchBag,
457@@ -1385,24 +1389,6 @@
458 links = ['profile', 'polls', 'members', 'ppas']
459
460
461-class ActiveBatchNavigator(BatchNavigator):
462- """A paginator for active items.
463-
464- Used when a view needs to display more than one BatchNavigator of items.
465- """
466- start_variable_name = 'active_start'
467- batch_variable_name = 'active_batch'
468-
469-
470-class InactiveBatchNavigator(BatchNavigator):
471- """A paginator for inactive items.
472-
473- Used when a view needs to display more than one BatchNavigator of items.
474- """
475- start_variable_name = 'inactive_start'
476- batch_variable_name = 'inactive_batch'
477-
478-
479 class TeamMembershipView(LaunchpadView):
480 """The view behind ITeam/+members."""
481
482
483=== modified file 'lib/lp/registry/browser/team.py'
484--- lib/lp/registry/browser/team.py 2010-09-17 18:32:23 +0000
485+++ lib/lp/registry/browser/team.py 2010-09-20 05:15:57 +0000
486@@ -829,8 +829,6 @@
487 """
488 results = self.mailing_list.getReviewableMessages()
489 navigator = BatchNavigator(results, self.request)
490- # Subclasses often set the singular and plural headings,
491- # but we can use the generic class too.
492 navigator.setHeadings('message', 'messages')
493 return navigator
494
495
496=== modified file 'lib/lp/registry/browser/tests/test_team.py'
497--- lib/lp/registry/browser/tests/test_team.py 2010-09-14 00:21:04 +0000
498+++ lib/lp/registry/browser/tests/test_team.py 2010-09-20 05:15:57 +0000
499@@ -3,10 +3,10 @@
500
501 __metaclass__ = type
502
503-from canonical.launchpad.webapp.batching import BatchNavigator
504 from canonical.testing import DatabaseFunctionalLayer
505 from lp.registry.browser.person import TeamOverviewMenu
506 from lp.testing import TestCaseWithFactory
507+from lp.testing.matchers import IsConfiguredBatchNavigator
508 from lp.testing.menu import check_menu_links
509 from lp.testing.views import create_initialized_view
510
511@@ -47,12 +47,6 @@
512 team = self.factory.makeTeam()
513 self.factory.makeMailingList(team, team.teamowner)
514 view = create_initialized_view(team, name='+mailinglist-moderate')
515- self.assertIsInstance(view.held_messages, BatchNavigator)
516-
517- def test_held_message_headings(self):
518- team = self.factory.makeTeam()
519- self.factory.makeMailingList(team, team.teamowner)
520- view = create_initialized_view(team, name='+mailinglist-moderate')
521- navigator = view.held_messages
522- self.assertEqual('message', navigator._singular_heading)
523- self.assertEqual('messages', navigator._plural_heading)
524+ self.assertThat(
525+ view.held_messages,
526+ IsConfiguredBatchNavigator('message', 'messages'))
527
528=== modified file 'lib/lp/testing/matchers.py'
529--- lib/lp/testing/matchers.py 2010-08-25 20:04:40 +0000
530+++ lib/lp/testing/matchers.py 2010-09-20 05:15:57 +0000
531@@ -17,9 +17,12 @@
532 from testtools.content import Content
533 from testtools.content_type import UTF8_TEXT
534 from testtools.matchers import (
535+ Equals,
536 Matcher,
537 Mismatch,
538+ MismatchesAll,
539 )
540+from testtools import matchers
541 from zope.interface.exceptions import (
542 BrokenImplementation,
543 BrokenMethodImplementation,
544@@ -31,6 +34,8 @@
545 Proxy,
546 )
547
548+from canonical.launchpad.webapp.batching import BatchNavigator
549+
550
551 class DoesNotProvide(Mismatch):
552 """An object does not provide an interface."""
553@@ -220,3 +225,45 @@
554 if not matchee.startswith(self.expected):
555 return DoesNotStartWith(matchee, self.expected)
556 return None
557+
558+
559+class IsConfiguredBatchNavigator(Matcher):
560+ """Check that an object is a batch navigator."""
561+
562+ def __init__(self, singular, plural, batch_size=None):
563+ """Create a ConfiguredBatchNavigator.
564+
565+ :param singular: The singular header the batch should be using.
566+ :param plural: The plural header the batch should be using.
567+ :param batch_size: The batch size that should be configured by default.
568+ """
569+ self._single = Equals(singular)
570+ self._plural = Equals(plural)
571+ self._batch = None
572+ if batch_size:
573+ self._batch = Equals(batch_size)
574+ self.matchers = dict(
575+ _singular_heading=self._single, _plural_heading=self._plural)
576+ if self._batch:
577+ self.matchers['default_size'] = self._batch
578+
579+ def __str__(self):
580+ if self._batch:
581+ batch = ", %r" % self._batch.expected
582+ else:
583+ batch = ''
584+ return "ConfiguredBatchNavigator(%r, %r%s)" % (
585+ self._single.expected, self._plural.expected, batch)
586+
587+ def match(self, matchee):
588+ if not isinstance(matchee, BatchNavigator):
589+ # Testtools doesn't have an IsInstanceMismatch yet.
590+ return matchers._BinaryMismatch(
591+ BatchNavigator, 'isinstance', matchee)
592+ mismatches = []
593+ for attrname, matcher in self.matchers.items():
594+ mismatch = matcher.match(getattr(matchee, attrname))
595+ if mismatch is not None:
596+ mismatches.append(mismatch)
597+ if mismatches:
598+ return MismatchesAll(mismatches)