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
=== modified file 'lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt'
--- lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2010-02-25 00:47:17 +0000
+++ lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2010-09-14 00:24:54 +0000
@@ -27,9 +27,9 @@
27 class="batch-navigation-links">27 class="batch-navigation-links">
28 <a28 <a
29 tal:condition="first_page_url"29 tal:condition="first_page_url"
30 tal:attributes="href first_page_url"30 tal:attributes="href first_page_url;
31 id string:${view/css_class}-batchnav-first"
31 class="first"32 class="first"
32 id="batchnav_first"
33 rel="first"33 rel="first"
34 >First</a>34 >First</a>
35 <span tal:condition="not:first_page_url" class="first inactive"35 <span tal:condition="not:first_page_url" class="first inactive"
@@ -37,9 +37,9 @@
37 &#149;37 &#149;
38 <a38 <a
39 tal:condition="prev_page_url"39 tal:condition="prev_page_url"
40 tal:attributes="href prev_page_url"40 tal:attributes="href prev_page_url;
41 id string:${view/css_class}-batchnav-previous"
41 class="previous"42 class="previous"
42 id="batchnav_previous"
43 rel="previous"43 rel="previous"
44 >Previous</a>44 >Previous</a>
45 <span tal:condition="not:prev_page_url" class="previous inactive"45 <span tal:condition="not:prev_page_url" class="previous inactive"
@@ -47,8 +47,8 @@
47 &#149;47 &#149;
48 <a48 <a
49 tal:condition="next_page_url"49 tal:condition="next_page_url"
50 tal:attributes="href next_page_url"50 tal:attributes="href next_page_url;
51 id="batchnav_next"51 id string:${view/css_class}-batchnav-next"
52 class="next"52 class="next"
53 rel="next"53 rel="next"
54 ><strong>Next</strong></a>54 ><strong>Next</strong></a>
@@ -58,9 +58,9 @@
58 &#149;58 &#149;
59 <a59 <a
60 tal:condition="last_page_url"60 tal:condition="last_page_url"
61 tal:attributes="href last_page_url"61 tal:attributes="href last_page_url;
62 id string:${view/css_class}-batchnav-last"
62 class="last"63 class="last"
63 id="batchnav_last"
64 rel="last"64 rel="last"
65 >Last</a>65 >Last</a>
66 <span tal:condition="not:last_page_url" class="last inactive"66 <span tal:condition="not:last_page_url" class="last inactive"
6767
=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py 2010-09-08 13:26:50 +0000
+++ lib/lp/registry/browser/team.py 2010-09-14 00:24:54 +0000
@@ -807,21 +807,29 @@
807 assert(self.mailing_list is not None), (807 assert(self.mailing_list is not None), (
808 'No mailing list: %s' % self.context.name)808 'No mailing list: %s' % self.context.name)
809809
810 @property810 @cachedproperty
811 def hold_count(self):811 def hold_count(self):
812 """The number of message being held for moderator approval.812 """The number of message being held for moderator approval.
813813
814 :return: Number of message being held for moderator approval.814 :return: Number of message being held for moderator approval.
815 """815 """
816 return self.mailing_list.getReviewableMessages().count()816 ## return self.mailing_list.getReviewableMessages().count()
817 # This looks like it would be more efficient, but it raises
818 # LocationError.
819 return self.held_messages.currentBatch().listlength
817820
818 @property821 @cachedproperty
819 def held_messages(self):822 def held_messages(self):
820 """All the messages being held for moderator approval.823 """All the messages being held for moderator approval.
821824
822 :return: Sequence of held messages.825 :return: Sequence of held messages.
823 """826 """
824 return self.mailing_list.getReviewableMessages()827 results = self.mailing_list.getReviewableMessages()
828 navigator = BatchNavigator(results, self.request)
829 # Subclasses often set the singular and plural headings,
830 # but we can use the generic class too.
831 navigator.setHeadings('message', 'messages')
832 return navigator
825833
826 @action('Moderate', name='moderate')834 @action('Moderate', name='moderate')
827 def moderate_action(self, action, data):835 def moderate_action(self, action, data):
@@ -830,7 +838,7 @@
830 # won't be in data. Instead, get it out of the request.838 # won't be in data. Instead, get it out of the request.
831 reviewable = self.hold_count839 reviewable = self.hold_count
832 disposed_count = 0840 disposed_count = 0
833 for message in self.held_messages:841 for message in self.held_messages.currentBatch():
834 action_name = self.request.form_ng.getOne(842 action_name = self.request.form_ng.getOne(
835 'field.' + quote(message.message_id))843 'field.' + quote(message.message_id))
836 # This essentially acts like a switch statement or if/elifs. It844 # This essentially acts like a switch statement or if/elifs. It
837845
=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py 2010-03-05 13:56:38 +0000
+++ lib/lp/registry/browser/tests/test_team.py 2010-09-14 00:24:54 +0000
@@ -3,10 +3,12 @@
33
4__metaclass__ = type4__metaclass__ = type
55
6from canonical.launchpad.webapp.batching import BatchNavigator
6from canonical.testing import DatabaseFunctionalLayer7from canonical.testing import DatabaseFunctionalLayer
7from lp.registry.browser.person import TeamOverviewMenu8from lp.registry.browser.person import TeamOverviewMenu
8from lp.testing import TestCaseWithFactory9from lp.testing import TestCaseWithFactory
9from lp.testing.menu import check_menu_links10from lp.testing.menu import check_menu_links
11from lp.testing.views import create_initialized_view
1012
1113
12class TestTeamMenu(TestCaseWithFactory):14class TestTeamMenu(TestCaseWithFactory):
@@ -35,3 +37,22 @@
35 self.assertEqual(True, check_menu_links(menu))37 self.assertEqual(True, check_menu_links(menu))
36 link = menu.configure_mailing_list()38 link = menu.configure_mailing_list()
37 self.assertEqual('Configure mailing list', link.text)39 self.assertEqual('Configure mailing list', link.text)
40
41
42class TestModeration(TestCaseWithFactory):
43
44 layer = DatabaseFunctionalLayer
45
46 def test_held_messages_is_batch_navigator(self):
47 team = self.factory.makeTeam()
48 self.factory.makeMailingList(team, team.teamowner)
49 view = create_initialized_view(team, name='+mailinglist-moderate')
50 self.assertIsInstance(view.held_messages, BatchNavigator)
51
52 def test_held_message_headings(self):
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)
3859
=== modified file 'lib/lp/registry/stories/mailinglists/moderation.txt'
--- lib/lp/registry/stories/mailinglists/moderation.txt 2009-12-03 20:28:54 +0000
+++ lib/lp/registry/stories/mailinglists/moderation.txt 2010-09-14 00:24:54 +0000
@@ -100,6 +100,21 @@
100 2 messages have been posted to your mailing list...100 2 messages have been posted to your mailing list...
101 ...101 ...
102102
103If there are more messages than the batch size, they get batched.
104
105 >>> admin_browser.open(
106 ... 'http://launchpad.dev/~guadamen/+mailinglist-moderate?batch=1')
107 >>> find_tag_by_id(admin_browser.contents, 'upper-batch-nav-batchnav-next')['class']
108 u'next'
109 >>> find_tag_by_id(admin_browser.contents, 'lower-batch-nav-batchnav-next')['class']
110 u'next'
111
112To test easily, we use the default batch size below.
113
114 >>> admin_browser.open(
115 ... 'http://launchpad.dev/~guadamen/+mailinglist-moderate')
116
117
103Each held message displays some details about what's being held.118Each held message displays some details about what's being held.
104119
105 >>> print extract_text(find_tag_by_id(120 >>> print extract_text(find_tag_by_id(
106121
=== modified file 'lib/lp/registry/templates/team-mailinglist-moderate.pt'
--- lib/lp/registry/templates/team-mailinglist-moderate.pt 2009-09-14 01:28:41 +0000
+++ lib/lp/registry/templates/team-mailinglist-moderate.pt 2010-09-14 00:24:54 +0000
@@ -35,16 +35,24 @@
35 action for obvious spam.</li>35 action for obvious spam.</li>
36 <li><strong>Hold</strong> - Continue to hold the message, deferring36 <li><strong>Hold</strong> - Continue to hold the message, deferring
37 your decision until later.</li>37 your decision until later.</li>
38 <li>Toss</li>
38 </ul>39 </ul>
39 </div>40 </div>
40 <table class="listing" metal:fill-slot="widgets">41 <div metal:fill-slot="widgets">
42 <tal:navigation
43 replace="structure view/held_messages/@@+navigation-links-upper" />
44
45 <table class="listing">
41 <thead><tr>46 <thead><tr>
42 <th>Message details</th>47 <th>Message details</th>
43 <th>Approve</th><th>Decline</th><th>Discard</th><th>Hold</th>48 <th>Approve</th><th>Decline</th><th>Discard</th><th>Hold</th>
44 </tr></thead>49 </tr></thead>
45 <span tal:repeat="message view/held_messages"50 <span tal:repeat="message view/held_messages/currentBatch"
46 tal:content="structure message/@@+moderation" />51 tal:content="structure message/@@+moderation" />
47 </table>52 </table>
53 <tal:navigation
54 replace="structure view/held_messages/@@+navigation-links-lower" />
55 </div>
48 </metal:form>56 </metal:form>
49 </div>57 </div>
50 <span tal:condition="not: view/hold_count" id="legend">58 <span tal:condition="not: view/hold_count" id="legend">