Launchpad itself

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

  1. malone
  2. Merge into devel
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
=== 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-20 05:15:57 +0000
@@ -77,6 +77,24 @@
77 return self.batch.total() > self.batch.size77 return self.batch.total() > self.batch.size
7878
7979
80class ActiveBatchNavigator(BatchNavigator):
81 """A paginator for active items.
82
83 Used when a view needs to display more than one BatchNavigator of items.
84 """
85 start_variable_name = 'active_start'
86 batch_variable_name = 'active_batch'
87
88
89class InactiveBatchNavigator(BatchNavigator):
90 """A paginator for inactive items.
91
92 Used when a view needs to display more than one BatchNavigator of items.
93 """
94 start_variable_name = 'inactive_start'
95 batch_variable_name = 'inactive_batch'
96
97
80class TableBatchNavigator(BatchNavigator):98class TableBatchNavigator(BatchNavigator):
81 """See canonical.launchpad.interfaces.ITableBatchNavigator."""99 """See canonical.launchpad.interfaces.ITableBatchNavigator."""
82 implements(ITableBatchNavigator)100 implements(ITableBatchNavigator)
83101
=== 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-20 05:15:57 +0000
@@ -3,11 +3,9 @@
33
4__metaclass__ = type4__metaclass__ = type
55
6import unittest
76
8from zope.security.proxy import removeSecurityProxy7from zope.security.proxy import removeSecurityProxy
98
10from canonical.launchpad.webapp.batching import BatchNavigator
11from canonical.launchpad.testing.pages import find_tag_by_id9from canonical.launchpad.testing.pages import find_tag_by_id
12from canonical.testing.layers import DatabaseFunctionalLayer10from canonical.testing.layers import DatabaseFunctionalLayer
13from lp.blueprints.interfaces.specificationtarget import (11from lp.blueprints.interfaces.specificationtarget import (
@@ -21,6 +19,7 @@
21 login_person,19 login_person,
22 TestCaseWithFactory,20 TestCaseWithFactory,
23 )21 )
22from lp.testing.matchers import IsConfiguredBatchNavigator
24from lp.testing.views import (23from lp.testing.views import (
25 create_view,24 create_view,
26 create_initialized_view,25 create_initialized_view,
@@ -103,27 +102,17 @@
103 '<div id="involvement" class="portlet involvement">' in view())102 '<div id="involvement" class="portlet involvement">' in view())
104103
105 def test_specs_batch(self):104 def test_specs_batch(self):
106 # Some pages turn up in very large contexts and patch. E.g.105 # Some pages turn up in very large contexts and so batch. E.g.
107 # Distro:+assignments which uses SpecificationAssignmentsView, a106 # Distro:+assignments which uses SpecificationAssignmentsView, a
108 # subclass.107 # subclass.
109 person = self.factory.makePerson()108 person = self.factory.makePerson()
110 view = create_initialized_view(person, name='+assignments')109 view = create_initialized_view(person, name='+assignments')
111 self.assertIsInstance(view.specs_batched, BatchNavigator)
112
113 def test_batch_headings(self):
114 person = self.factory.makePerson()
115 view = create_initialized_view(person, name='+assignments')
116 navigator = view.specs_batched
117 self.assertEqual('specification', navigator._singular_heading)
118 self.assertEqual('specifications', navigator._plural_heading)
119
120 def test_batch_size(self):
121 # Because +assignments is meant to provide an overview, we default to110 # Because +assignments is meant to provide an overview, we default to
122 # 500 as the default batch size.111 # 500 as the default batch size.
123 person = self.factory.makePerson()112 self.assertThat(
124 view = create_initialized_view(person, name='+assignments')113 view.specs_batched,
125 navigator = view.specs_batched114 IsConfiguredBatchNavigator(
126 self.assertEqual(500, view.specs_batched.default_size)115 'specification', 'specifications', batch_size=500))
127116
128117
129class TestAssignments(TestCaseWithFactory):118class TestAssignments(TestCaseWithFactory):
@@ -243,13 +232,3 @@
243 login_person(project_group.owner)232 login_person(project_group.owner)
244 view = create_initialized_view(project_group, '+specs')233 view = create_initialized_view(project_group, '+specs')
245 self.assertEqual(False, view.can_configure_blueprints)234 self.assertEqual(False, view.can_configure_blueprints)
246
247
248def test_suite():
249 suite = unittest.TestSuite()
250 suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))
251 return suite
252
253
254if __name__ == '__main__':
255 unittest.TextTestRunner().run(test_suite())
256235
=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py 2010-09-03 15:02:39 +0000
+++ lib/lp/bugs/browser/bugtracker.py 2010-09-20 05:15:57 +0000
@@ -54,7 +54,11 @@
54 structured,54 structured,
55 )55 )
56from canonical.launchpad.webapp.authorization import check_permission56from canonical.launchpad.webapp.authorization import check_permission
57from canonical.launchpad.webapp.batching import BatchNavigator57from canonical.launchpad.webapp.batching import (
58 ActiveBatchNavigator,
59 BatchNavigator,
60 InactiveBatchNavigator,
61 )
58from canonical.launchpad.webapp.breadcrumb import Breadcrumb62from canonical.launchpad.webapp.breadcrumb import Breadcrumb
59from canonical.launchpad.webapp.menu import NavigationMenu63from canonical.launchpad.webapp.menu import NavigationMenu
60from canonical.lazr.utils import smartquote64from canonical.lazr.utils import smartquote
@@ -154,21 +158,30 @@
154 pillar_limit = 3158 pillar_limit = 3
155159
156 def initialize(self):160 def initialize(self):
157 # Sort the bug trackers into active and inactive lists so that161 # eager load related pillars. In future we should do this for
158 # we can display them separately.162 # just the rendered trackers, and also use group by to get
159 all_bug_trackers = list(self.context)163 # bug watch counts per tracker. However the batching makes
160 self.active_bug_trackers = [164 # the inefficiency tolerable for now. Robert Collins 20100919.
161 bug_tracker for bug_tracker in all_bug_trackers165 self._pillar_cache = self.context.getPillarsForBugtrackers(
162 if bug_tracker.active]166 list(self.context.trackers()))
163 self.inactive_bug_trackers = [167
164 bug_tracker for bug_tracker in all_bug_trackers168 @property
165 if not bug_tracker.active]169 def inactive_tracker_count(self):
166170 return self.inactive_trackers.currentBatch().listlength
167 bugtrackerset = getUtility(IBugTrackerSet)171
168 # The caching of bugtracker pillars here avoids us hitting the172 @cachedproperty
169 # database multiple times for each bugtracker.173 def active_trackers(self):
170 self._pillar_cache = bugtrackerset.getPillarsForBugtrackers(174 results = self.context.trackers(active=True)
171 all_bug_trackers)175 navigator = ActiveBatchNavigator(results, self.request)
176 navigator.setHeadings('tracker', 'trackers')
177 return navigator
178
179 @cachedproperty
180 def inactive_trackers(self):
181 results = self.context.trackers(active=False)
182 navigator = InactiveBatchNavigator(results, self.request)
183 navigator.setHeadings('tracker', 'trackers')
184 return navigator
172185
173 def getPillarData(self, bugtracker):186 def getPillarData(self, bugtracker):
174 """Return dict of pillars and booleans indicating ellipsis.187 """Return dict of pillars and booleans indicating ellipsis.
175188
=== modified file 'lib/lp/bugs/browser/tests/test_bugtracker_views.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-08-26 20:08:43 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_views.py 2010-09-20 05:15:57 +0000
@@ -3,27 +3,61 @@
33
4"""Tests for BugTracker views."""4"""Tests for BugTracker views."""
55
6from __future__ import with_statement
7
6__metaclass__ = type8__metaclass__ = type
79
8import unittest
9
10from datetime import datetime, timedelta
11from pytz import utc
12
13from zope.component import getUtility10from zope.component import getUtility
14from zope.security.interfaces import Unauthorized
1511
16from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities12from canonical.launchpad.testing.pages import find_tag_by_id
13from canonical.launchpad.webapp import canonical_url
17from canonical.testing.layers import DatabaseFunctionalLayer14from canonical.testing.layers import DatabaseFunctionalLayer
1815from lp.bugs.interfaces.bugtracker import IBugTrackerSet
19from lp.bugs.browser.bugtracker import (
20 BugTrackerEditView)
21from lp.registry.interfaces.person import IPersonSet16from lp.registry.interfaces.person import IPersonSet
22from lp.testing import login, TestCaseWithFactory17from lp.testing import (
23from lp.testing.sampledata import ADMIN_EMAIL, NO_PRIVILEGE_EMAIL18 login,
19 TestCaseWithFactory,
20 )
24from lp.testing.views import create_initialized_view21from lp.testing.views import create_initialized_view
2522from lp.testing.matchers import IsConfiguredBatchNavigator
2623from lp.testing import (
2724 person_logged_in,
28def test_suite():25 TestCaseWithFactory,
29 return unittest.TestLoader().loadTestsFromName(__name__)26 )
27from lp.testing.sampledata import ADMIN_EMAIL
28
29
30class TestBugTrackerSetView(TestCaseWithFactory):
31
32 layer = DatabaseFunctionalLayer
33
34 def test_trackers_are_batch_navigators(self):
35 trackers = getUtility(IBugTrackerSet)
36 view = create_initialized_view(trackers, name='+index')
37 matcher = IsConfiguredBatchNavigator('tracker', 'trackers')
38 self.assertThat(view.active_trackers, matcher)
39 self.assertThat(view.inactive_trackers, matcher)
40
41 def test_page_is_batched(self):
42 active_tracker1 = self.factory.makeBugTracker()
43 active_tracker2 = self.factory.makeBugTracker()
44 inactive_tracker1 = self.factory.makeBugTracker()
45 inactive_tracker2 = self.factory.makeBugTracker()
46 admin = getUtility(IPersonSet).find(ADMIN_EMAIL).any()
47 with person_logged_in(admin):
48 inactive_tracker1.active = False
49 inactive_tracker2.active = False
50 trackers = getUtility(IBugTrackerSet)
51 url = (canonical_url(trackers) +
52 "/+index?active_batch=1&inactive_batch=1")
53 browser = self.getUserBrowser(url)
54 content = browser.contents
55 # XXX RobertCollns 20100919 bug=642504. The support for multiple batches
56 # isn't complete and the id for the nav links gets duplicated.
57 #self.assertEqual('next',
58 # find_tag_by_id(content, 'upper-batch-nav-batchnav-next')['class'])
59 #self.assertEqual('next',
60 # find_tag_by_id(content, 'lower-batch-nav-batchnav-next')['class'])
61 # Instead we check the string appears.
62 self.assertTrue('upper-batch-nav-batchnav-next' in content)
63
3064
=== modified file 'lib/lp/bugs/interfaces/bugtracker.py'
--- lib/lp/bugs/interfaces/bugtracker.py 2010-08-26 20:08:43 +0000
+++ lib/lp/bugs/interfaces/bugtracker.py 2010-09-20 05:15:57 +0000
@@ -432,6 +432,14 @@
432 def getPillarsForBugtrackers(bug_trackers):432 def getPillarsForBugtrackers(bug_trackers):
433 """Return dict mapping bugtrackers to lists of pillars."""433 """Return dict mapping bugtrackers to lists of pillars."""
434434
435 def trackers(active=None):
436 """Return a ResultSet of bugtrackers.
437
438 :param active: If True, only active trackers are returned, if False
439 only inactive trackers are returned. All trackers are returned
440 by default.
441 """
442
435443
436class IBugTrackerAlias(Interface):444class IBugTrackerAlias(Interface):
437 """Another URL for a remote bug system.445 """Another URL for a remote bug system.
438446
=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py 2010-08-26 20:08:43 +0000
+++ lib/lp/bugs/model/bugtracker.py 2010-09-20 05:15:57 +0000
@@ -8,7 +8,8 @@
8 'BugTracker',8 'BugTracker',
9 'BugTrackerAlias',9 'BugTrackerAlias',
10 'BugTrackerAliasSet',10 'BugTrackerAliasSet',
11 'BugTrackerSet']11 'BugTrackerSet',
12 ]
1213
1314
14from datetime import datetime15from datetime import datetime
@@ -66,6 +67,8 @@
66 IBugTrackerSet,67 IBugTrackerSet,
67 SINGLE_PRODUCT_BUGTRACKERTYPES,68 SINGLE_PRODUCT_BUGTRACKERTYPES,
68 )69 )
70from canonical.launchpad.webapp.interfaces import (
71 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
69from lp.bugs.interfaces.bugtrackerperson import BugTrackerPersonAlreadyExists72from lp.bugs.interfaces.bugtrackerperson import BugTrackerPersonAlreadyExists
70from lp.bugs.model.bug import Bug73from lp.bugs.model.bug import Bug
71from lp.bugs.model.bugmessage import BugMessage74from lp.bugs.model.bugmessage import BugMessage
@@ -577,6 +580,17 @@
577 """See `IBugTrackerSet`."""580 """See `IBugTrackerSet`."""
578 return BugTracker.select()581 return BugTracker.select()
579582
583 def trackers(self, active=None):
584 # Without context, cannot tell what store flavour is desirable.
585 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
586 if active is not None:
587 clauses = [BugTracker.active==active]
588 else:
589 clauses = []
590 results = store.find(BugTracker, *clauses)
591 results.order_by(BugTracker.name)
592 return results
593
580 def ensureBugTracker(self, baseurl, owner, bugtrackertype,594 def ensureBugTracker(self, baseurl, owner, bugtrackertype,
581 title=None, summary=None, contactdetails=None, name=None):595 title=None, summary=None, contactdetails=None, name=None):
582 """See `IBugTrackerSet`."""596 """See `IBugTrackerSet`."""
583597
=== modified file 'lib/lp/bugs/stories/bugtracker/bugtrackers-index.txt'
--- lib/lp/bugs/stories/bugtracker/bugtrackers-index.txt 2010-07-26 13:30:57 +0000
+++ lib/lp/bugs/stories/bugtracker/bugtrackers-index.txt 2010-09-20 05:15:57 +0000
@@ -39,11 +39,11 @@
39 Email Address39 Email Address
40 040 0
41 ------------------------41 ------------------------
42 GNU Savannah Bug Tracker42 T'other Gnome GBugGTracker
43 http://savannah.gnu.org/43 http://bugzilla.gnome.org/
44 --> http://savannah.gnu.org/44 --> http://bugzilla.gnome.org/
45 &mdash;45 &mdash;
46 Savane46 Bugzilla
47 047 0
48 ------------------------48 ------------------------
49 ...49 ...
@@ -68,6 +68,8 @@
68 &mdash;68 &mdash;
69 Email Address69 Email Address
70 070 0
71 ------------------------
72 ...
7173
72The watch counts match the number of bugs listed, of course:74The watch counts match the number of bugs listed, of course:
7375
7476
=== modified file 'lib/lp/bugs/templates/bugtrackers-index.pt'
--- lib/lp/bugs/templates/bugtrackers-index.pt 2009-09-01 16:24:52 +0000
+++ lib/lp/bugs/templates/bugtrackers-index.pt 2010-09-20 05:15:57 +0000
@@ -13,6 +13,7 @@
13 </tal:comment>13 </tal:comment>
14 <metal:macros fill-slot="bogus">14 <metal:macros fill-slot="bogus">
15 <metal:macro define-macro="tracker-table">15 <metal:macro define-macro="tracker-table">
16 <tal:navigation content="structure trackers/@@+navigation-links-upper" />
16 <table class="sortable listing" tal:attributes="id id">17 <table class="sortable listing" tal:attributes="id id">
17 <thead>18 <thead>
18 <tr>19 <tr>
@@ -24,7 +25,7 @@
24 </tr>25 </tr>
25 </thead>26 </thead>
26 <tbody>27 <tbody>
27 <tr tal:repeat="tracker trackers">28 <tr tal:repeat="tracker trackers/currentBatch">
28 <td>29 <td>
29 <a tal:replace="structure tracker/fmt:link" />30 <a tal:replace="structure tracker/fmt:link" />
30 </td>31 </td>
@@ -53,6 +54,7 @@
53 </tr>54 </tr>
54 </tbody>55 </tbody>
55 </table>56 </table>
57 <tal:navigation content="structure trackers/@@+navigation-links-lower" />
56 </metal:macro>58 </metal:macro>
57 </metal:macros>59 </metal:macros>
5860
@@ -70,7 +72,7 @@
70 the external status changes.72 the external status changes.
71 </p>73 </p>
72 <tal:table define="id string:trackers;74 <tal:table define="id string:trackers;
73 trackers view/active_bug_trackers">75 trackers view/active_trackers">
74 <metal:table use-macro="template/macros/tracker-table" />76 <metal:table use-macro="template/macros/tracker-table" />
75 </tal:table>77 </tal:table>
76 <p style="margin-top: 1em"78 <p style="margin-top: 1em"
@@ -81,11 +83,11 @@
81 </p>83 </p>
82 </div>84 </div>
83 </div>85 </div>
84 <div class="yui-u" tal:condition="view/inactive_bug_trackers">86 <div class="yui-u" tal:condition="view/inactive_tracker_count">
85 <div class="portlet">87 <div class="portlet">
86 <h2>Inactive bug trackers</h2>88 <h2>Inactive bug trackers</h2>
87 <tal:table define="id string:inactive-trackers;89 <tal:table define="id string:inactive-trackers;
88 trackers view/inactive_bug_trackers">90 trackers view/inactive_trackers">
89 <metal:table use-macro="template/macros/tracker-table" />91 <metal:table use-macro="template/macros/tracker-table" />
90 </tal:table>92 </tal:table>
91 </div>93 </div>
9294
=== modified file 'lib/lp/bugs/tests/test_bugtracker.py'
--- lib/lp/bugs/tests/test_bugtracker.py 2010-08-26 20:08:43 +0000
+++ lib/lp/bugs/tests/test_bugtracker.py 2010-09-20 05:15:57 +0000
@@ -28,7 +28,10 @@
2828
29from canonical.launchpad.ftests import login_person29from canonical.launchpad.ftests import login_person
30from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities30from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
31from canonical.testing import LaunchpadFunctionalLayer31from canonical.testing import (
32 DatabaseFunctionalLayer,
33 LaunchpadFunctionalLayer,
34 )
32from lp.bugs.externalbugtracker import (35from lp.bugs.externalbugtracker import (
33 BugTrackerConnectError,36 BugTrackerConnectError,
34 Mantis,37 Mantis,
@@ -38,12 +41,36 @@
38 BugTrackerType,41 BugTrackerType,
39 IBugTracker,42 IBugTracker,
40 )43 )
44from lp.bugs.model.bugtracker import BugTrackerSet
41from lp.bugs.tests.externalbugtracker import Urlib2TransportTestHandler45from lp.bugs.tests.externalbugtracker import Urlib2TransportTestHandler
42from lp.registry.interfaces.person import IPersonSet46from lp.registry.interfaces.person import IPersonSet
43from lp.testing import login, login_person, TestCaseWithFactory47from lp.testing import login, login_person, TestCaseWithFactory
44from lp.testing.sampledata import ADMIN_EMAIL48from lp.testing.sampledata import ADMIN_EMAIL
4549
4650
51class TestBugTrackerSet(TestCaseWithFactory):
52
53 layer = DatabaseFunctionalLayer
54
55 def test_trackers(self):
56 tracker = self.factory.makeBugTracker()
57 trackers = BugTrackerSet()
58 # Active trackers are in all trackers,
59 self.assertTrue(tracker in trackers.trackers())
60 # and active,
61 self.assertTrue(tracker in trackers.trackers(active=True))
62 # But not inactive.
63 self.assertFalse(tracker in trackers.trackers(active=False))
64 login(ADMIN_EMAIL)
65 tracker.active = False
66 # Inactive trackers are in all trackers
67 self.assertTrue(tracker in trackers.trackers())
68 # and inactive,
69 self.assertTrue(tracker in trackers.trackers(active=False))
70 # but not in active.
71 self.assertFalse(tracker in trackers.trackers(active=True))
72
73
47class BugTrackerTestCase(TestCaseWithFactory):74class BugTrackerTestCase(TestCaseWithFactory):
48 """Unit tests for the `BugTracker` class."""75 """Unit tests for the `BugTracker` class."""
4976
5077
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-09-15 16:44:41 +0000
+++ lib/lp/registry/browser/person.py 2010-09-20 05:15:57 +0000
@@ -194,7 +194,11 @@
194 structured,194 structured,
195 )195 )
196from canonical.launchpad.webapp.authorization import check_permission196from canonical.launchpad.webapp.authorization import check_permission
197from canonical.launchpad.webapp.batching import BatchNavigator197from canonical.launchpad.webapp.batching import (
198 ActiveBatchNavigator,
199 BatchNavigator,
200 InactiveBatchNavigator,
201 )
198from canonical.launchpad.webapp.breadcrumb import Breadcrumb202from canonical.launchpad.webapp.breadcrumb import Breadcrumb
199from canonical.launchpad.webapp.interfaces import (203from canonical.launchpad.webapp.interfaces import (
200 ILaunchBag,204 ILaunchBag,
@@ -1385,24 +1389,6 @@
1385 links = ['profile', 'polls', 'members', 'ppas']1389 links = ['profile', 'polls', 'members', 'ppas']
13861390
13871391
1388class ActiveBatchNavigator(BatchNavigator):
1389 """A paginator for active items.
1390
1391 Used when a view needs to display more than one BatchNavigator of items.
1392 """
1393 start_variable_name = 'active_start'
1394 batch_variable_name = 'active_batch'
1395
1396
1397class InactiveBatchNavigator(BatchNavigator):
1398 """A paginator for inactive items.
1399
1400 Used when a view needs to display more than one BatchNavigator of items.
1401 """
1402 start_variable_name = 'inactive_start'
1403 batch_variable_name = 'inactive_batch'
1404
1405
1406class TeamMembershipView(LaunchpadView):1392class TeamMembershipView(LaunchpadView):
1407 """The view behind ITeam/+members."""1393 """The view behind ITeam/+members."""
14081394
14091395
=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py 2010-09-17 18:32:23 +0000
+++ lib/lp/registry/browser/team.py 2010-09-20 05:15:57 +0000
@@ -829,8 +829,6 @@
829 """829 """
830 results = self.mailing_list.getReviewableMessages()830 results = self.mailing_list.getReviewableMessages()
831 navigator = BatchNavigator(results, self.request)831 navigator = BatchNavigator(results, self.request)
832 # Subclasses often set the singular and plural headings,
833 # but we can use the generic class too.
834 navigator.setHeadings('message', 'messages')832 navigator.setHeadings('message', 'messages')
835 return navigator833 return navigator
836834
837835
=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py 2010-09-14 00:21:04 +0000
+++ lib/lp/registry/browser/tests/test_team.py 2010-09-20 05:15:57 +0000
@@ -3,10 +3,10 @@
33
4__metaclass__ = type4__metaclass__ = type
55
6from canonical.launchpad.webapp.batching import BatchNavigator
7from canonical.testing import DatabaseFunctionalLayer6from canonical.testing import DatabaseFunctionalLayer
8from lp.registry.browser.person import TeamOverviewMenu7from lp.registry.browser.person import TeamOverviewMenu
9from lp.testing import TestCaseWithFactory8from lp.testing import TestCaseWithFactory
9from lp.testing.matchers import IsConfiguredBatchNavigator
10from lp.testing.menu import check_menu_links10from lp.testing.menu import check_menu_links
11from lp.testing.views import create_initialized_view11from lp.testing.views import create_initialized_view
1212
@@ -47,12 +47,6 @@
47 team = self.factory.makeTeam()47 team = self.factory.makeTeam()
48 self.factory.makeMailingList(team, team.teamowner)48 self.factory.makeMailingList(team, team.teamowner)
49 view = create_initialized_view(team, name='+mailinglist-moderate')49 view = create_initialized_view(team, name='+mailinglist-moderate')
50 self.assertIsInstance(view.held_messages, BatchNavigator)50 self.assertThat(
5151 view.held_messages,
52 def test_held_message_headings(self):52 IsConfiguredBatchNavigator('message', 'messages'))
53 team = self.factory.makeTeam()
54 self.factory.makeMailingList(team, team.teamowner)
55 view = create_initialized_view(team, name='+mailinglist-moderate')
56 navigator = view.held_messages
57 self.assertEqual('message', navigator._singular_heading)
58 self.assertEqual('messages', navigator._plural_heading)
5953
=== modified file 'lib/lp/testing/matchers.py'
--- lib/lp/testing/matchers.py 2010-08-25 20:04:40 +0000
+++ lib/lp/testing/matchers.py 2010-09-20 05:15:57 +0000
@@ -17,9 +17,12 @@
17from testtools.content import Content17from testtools.content import Content
18from testtools.content_type import UTF8_TEXT18from testtools.content_type import UTF8_TEXT
19from testtools.matchers import (19from testtools.matchers import (
20 Equals,
20 Matcher,21 Matcher,
21 Mismatch,22 Mismatch,
23 MismatchesAll,
22 )24 )
25from testtools import matchers
23from zope.interface.exceptions import (26from zope.interface.exceptions import (
24 BrokenImplementation,27 BrokenImplementation,
25 BrokenMethodImplementation,28 BrokenMethodImplementation,
@@ -31,6 +34,8 @@
31 Proxy,34 Proxy,
32 )35 )
3336
37from canonical.launchpad.webapp.batching import BatchNavigator
38
3439
35class DoesNotProvide(Mismatch):40class DoesNotProvide(Mismatch):
36 """An object does not provide an interface."""41 """An object does not provide an interface."""
@@ -220,3 +225,45 @@
220 if not matchee.startswith(self.expected):225 if not matchee.startswith(self.expected):
221 return DoesNotStartWith(matchee, self.expected)226 return DoesNotStartWith(matchee, self.expected)
222 return None227 return None
228
229
230class IsConfiguredBatchNavigator(Matcher):
231 """Check that an object is a batch navigator."""
232
233 def __init__(self, singular, plural, batch_size=None):
234 """Create a ConfiguredBatchNavigator.
235
236 :param singular: The singular header the batch should be using.
237 :param plural: The plural header the batch should be using.
238 :param batch_size: The batch size that should be configured by default.
239 """
240 self._single = Equals(singular)
241 self._plural = Equals(plural)
242 self._batch = None
243 if batch_size:
244 self._batch = Equals(batch_size)
245 self.matchers = dict(
246 _singular_heading=self._single, _plural_heading=self._plural)
247 if self._batch:
248 self.matchers['default_size'] = self._batch
249
250 def __str__(self):
251 if self._batch:
252 batch = ", %r" % self._batch.expected
253 else:
254 batch = ''
255 return "ConfiguredBatchNavigator(%r, %r%s)" % (
256 self._single.expected, self._plural.expected, batch)
257
258 def match(self, matchee):
259 if not isinstance(matchee, BatchNavigator):
260 # Testtools doesn't have an IsInstanceMismatch yet.
261 return matchers._BinaryMismatch(
262 BatchNavigator, 'isinstance', matchee)
263 mismatches = []
264 for attrname, matcher in self.matchers.items():
265 mismatch = matcher.match(getattr(matchee, attrname))
266 if mismatch is not None:
267 mismatches.append(mismatch)
268 if mismatches:
269 return MismatchesAll(mismatches)