Launchpad itself

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

  1. registry
  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: 11544
Proposed branch: lp:~lifeless/launchpad/registry
Merge into: lp:launchpad
Diff against target: 194 lines (+67/-15)
5 files modified
lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt (+8/-8)
lib/lp/registry/browser/team.py (+13/-5)
lib/lp/registry/browser/tests/test_team.py (+21/-0)
lib/lp/registry/stories/mailinglists/moderation.txt (+15/-0)
lib/lp/registry/templates/team-mailinglist-moderate.pt (+10/-2)
To merge this branch: bzr merge lp:~lifeless/launchpad/registry
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+35354@code.launchpad.net

Commit message

Help timeouts in mailing list pages by batching.

Description of the change

Address timeouts in mailing list pages by batching (the batch size may need tweaking, but we'll want to see if the default is sufficient).

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Oh, this also includes a small fix from curtis which was needed to test this change properly; he swears it won't break the world.

Revision history for this message
Tim Penhey (thumper) wrote :

Looks fine. Are the id changes in the generic template going to cause other issues?

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt'
2--- lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2010-02-25 00:47:17 +0000
3+++ lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2010-09-14 00:24:54 +0000
4@@ -27,9 +27,9 @@
5 class="batch-navigation-links">
6 <a
7 tal:condition="first_page_url"
8- tal:attributes="href first_page_url"
9+ tal:attributes="href first_page_url;
10+ id string:${view/css_class}-batchnav-first"
11 class="first"
12- id="batchnav_first"
13 rel="first"
14 >First</a>
15 <span tal:condition="not:first_page_url" class="first inactive"
16@@ -37,9 +37,9 @@
17 &#149;
18 <a
19 tal:condition="prev_page_url"
20- tal:attributes="href prev_page_url"
21+ tal:attributes="href prev_page_url;
22+ id string:${view/css_class}-batchnav-previous"
23 class="previous"
24- id="batchnav_previous"
25 rel="previous"
26 >Previous</a>
27 <span tal:condition="not:prev_page_url" class="previous inactive"
28@@ -47,8 +47,8 @@
29 &#149;
30 <a
31 tal:condition="next_page_url"
32- tal:attributes="href next_page_url"
33- id="batchnav_next"
34+ tal:attributes="href next_page_url;
35+ id string:${view/css_class}-batchnav-next"
36 class="next"
37 rel="next"
38 ><strong>Next</strong></a>
39@@ -58,9 +58,9 @@
40 &#149;
41 <a
42 tal:condition="last_page_url"
43- tal:attributes="href last_page_url"
44+ tal:attributes="href last_page_url;
45+ id string:${view/css_class}-batchnav-last"
46 class="last"
47- id="batchnav_last"
48 rel="last"
49 >Last</a>
50 <span tal:condition="not:last_page_url" class="last inactive"
51
52=== modified file 'lib/lp/registry/browser/team.py'
53--- lib/lp/registry/browser/team.py 2010-09-08 13:26:50 +0000
54+++ lib/lp/registry/browser/team.py 2010-09-14 00:24:54 +0000
55@@ -807,21 +807,29 @@
56 assert(self.mailing_list is not None), (
57 'No mailing list: %s' % self.context.name)
58
59- @property
60+ @cachedproperty
61 def hold_count(self):
62 """The number of message being held for moderator approval.
63
64 :return: Number of message being held for moderator approval.
65 """
66- return self.mailing_list.getReviewableMessages().count()
67+ ## return self.mailing_list.getReviewableMessages().count()
68+ # This looks like it would be more efficient, but it raises
69+ # LocationError.
70+ return self.held_messages.currentBatch().listlength
71
72- @property
73+ @cachedproperty
74 def held_messages(self):
75 """All the messages being held for moderator approval.
76
77 :return: Sequence of held messages.
78 """
79- return self.mailing_list.getReviewableMessages()
80+ results = self.mailing_list.getReviewableMessages()
81+ navigator = BatchNavigator(results, self.request)
82+ # Subclasses often set the singular and plural headings,
83+ # but we can use the generic class too.
84+ navigator.setHeadings('message', 'messages')
85+ return navigator
86
87 @action('Moderate', name='moderate')
88 def moderate_action(self, action, data):
89@@ -830,7 +838,7 @@
90 # won't be in data. Instead, get it out of the request.
91 reviewable = self.hold_count
92 disposed_count = 0
93- for message in self.held_messages:
94+ for message in self.held_messages.currentBatch():
95 action_name = self.request.form_ng.getOne(
96 'field.' + quote(message.message_id))
97 # This essentially acts like a switch statement or if/elifs. It
98
99=== modified file 'lib/lp/registry/browser/tests/test_team.py'
100--- lib/lp/registry/browser/tests/test_team.py 2010-03-05 13:56:38 +0000
101+++ lib/lp/registry/browser/tests/test_team.py 2010-09-14 00:24:54 +0000
102@@ -3,10 +3,12 @@
103
104 __metaclass__ = type
105
106+from canonical.launchpad.webapp.batching import BatchNavigator
107 from canonical.testing import DatabaseFunctionalLayer
108 from lp.registry.browser.person import TeamOverviewMenu
109 from lp.testing import TestCaseWithFactory
110 from lp.testing.menu import check_menu_links
111+from lp.testing.views import create_initialized_view
112
113
114 class TestTeamMenu(TestCaseWithFactory):
115@@ -35,3 +37,22 @@
116 self.assertEqual(True, check_menu_links(menu))
117 link = menu.configure_mailing_list()
118 self.assertEqual('Configure mailing list', link.text)
119+
120+
121+class TestModeration(TestCaseWithFactory):
122+
123+ layer = DatabaseFunctionalLayer
124+
125+ def test_held_messages_is_batch_navigator(self):
126+ team = self.factory.makeTeam()
127+ self.factory.makeMailingList(team, team.teamowner)
128+ view = create_initialized_view(team, name='+mailinglist-moderate')
129+ self.assertIsInstance(view.held_messages, BatchNavigator)
130+
131+ def test_held_message_headings(self):
132+ team = self.factory.makeTeam()
133+ self.factory.makeMailingList(team, team.teamowner)
134+ view = create_initialized_view(team, name='+mailinglist-moderate')
135+ navigator = view.held_messages
136+ self.assertEqual('message', navigator._singular_heading)
137+ self.assertEqual('messages', navigator._plural_heading)
138
139=== modified file 'lib/lp/registry/stories/mailinglists/moderation.txt'
140--- lib/lp/registry/stories/mailinglists/moderation.txt 2009-12-03 20:28:54 +0000
141+++ lib/lp/registry/stories/mailinglists/moderation.txt 2010-09-14 00:24:54 +0000
142@@ -100,6 +100,21 @@
143 2 messages have been posted to your mailing list...
144 ...
145
146+If there are more messages than the batch size, they get batched.
147+
148+ >>> admin_browser.open(
149+ ... 'http://launchpad.dev/~guadamen/+mailinglist-moderate?batch=1')
150+ >>> find_tag_by_id(admin_browser.contents, 'upper-batch-nav-batchnav-next')['class']
151+ u'next'
152+ >>> find_tag_by_id(admin_browser.contents, 'lower-batch-nav-batchnav-next')['class']
153+ u'next'
154+
155+To test easily, we use the default batch size below.
156+
157+ >>> admin_browser.open(
158+ ... 'http://launchpad.dev/~guadamen/+mailinglist-moderate')
159+
160+
161 Each held message displays some details about what's being held.
162
163 >>> print extract_text(find_tag_by_id(
164
165=== modified file 'lib/lp/registry/templates/team-mailinglist-moderate.pt'
166--- lib/lp/registry/templates/team-mailinglist-moderate.pt 2009-09-14 01:28:41 +0000
167+++ lib/lp/registry/templates/team-mailinglist-moderate.pt 2010-09-14 00:24:54 +0000
168@@ -35,16 +35,24 @@
169 action for obvious spam.</li>
170 <li><strong>Hold</strong> - Continue to hold the message, deferring
171 your decision until later.</li>
172+ <li>Toss</li>
173 </ul>
174 </div>
175- <table class="listing" metal:fill-slot="widgets">
176+ <div metal:fill-slot="widgets">
177+ <tal:navigation
178+ replace="structure view/held_messages/@@+navigation-links-upper" />
179+
180+ <table class="listing">
181 <thead><tr>
182 <th>Message details</th>
183 <th>Approve</th><th>Decline</th><th>Discard</th><th>Hold</th>
184 </tr></thead>
185- <span tal:repeat="message view/held_messages"
186+ <span tal:repeat="message view/held_messages/currentBatch"
187 tal:content="structure message/@@+moderation" />
188 </table>
189+ <tal:navigation
190+ replace="structure view/held_messages/@@+navigation-links-lower" />
191+ </div>
192 </metal:form>
193 </div>
194 <span tal:condition="not: view/hold_count" id="legend">