launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00090
[Merge] lp:~adiroiban/launchpad/bug-475435 into lp:launchpad/devel
Adi Roiban has proposed merging lp:~adiroiban/launchpad/bug-475435 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#475435 More timeouts on +templates page
https://bugs.launchpad.net/bugs/475435
= Bug 475435 =
+template page times out on pages with more than 100 templates
== Proposed fix ==
* reduce the amount of SQL queries
* reduce the usage of URL formatters
== Pre-implementation notes ==
I have discussed with Danilo at LP Epic and he agreed with using "hardcoded" link instead of URL formatters for admin links.
== Implementation details ==
Since the metal:fill-slot="head_epilogue" is going to be placed in <head> tag, I have changed it from <div> to a generic tal tag. DIVs are not allowed in <head>.
I have removed the language_count, since they are not really that important and are adding more queries. If this branch solve the timeout problem, we might consider adding the language count again.
To easy the review process I plan to fix the lint errors after the review is done, since those warnings were not introduces by the changes from this branch.
== Tests ==
./bin/test -t xx-series-templates.txt
== Demo and Q/A ==
Testing is a bit difficult since the problem appears only when there are many templates.
On lp.dev you should add more than 1000 templates to a distribution series and then access
https://translations.launchpad.dev/ubuntu/hoary/+templates
The page should load in less than 10 seconds.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/browser/potemplate.py
lib/lp/translations/stories/standalone/xx-series-templates.txt
lib/lp/translations/templates/object-templates.pt
./lib/lp/translations/browser/potemplate.py
833: W291 trailing whitespace
833: Line has trailing whitespace.
./lib/lp/translations/stories/standalone/xx-series-templates.txt
71: want exceeds 78 characters.
73: want exceeds 78 characters.
87: source exceeds 78 characters.
89: want exceeds 78 characters.
93: source exceeds 78 characters.
95: want exceeds 78 characters.
102: source exceeds 78 characters.
104: want exceeds 78 characters.
108: source exceeds 78 characters.
110: want exceeds 78 characters.
190: want exceeds 78 characters.
191: want exceeds 78 characters.
192: want exceeds 78 characters.
--
https://code.launchpad.net/~adiroiban/launchpad/bug-475435/+merge/29889
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adiroiban/launchpad/bug-475435 into lp:launchpad/devel.
=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py 2010-06-03 16:08:33 +0000
+++ lib/lp/translations/browser/potemplate.py 2010-07-14 15:11:11 +0000
@@ -806,6 +806,7 @@
productseries = None
label = "Translation templates"
page_title = "All templates"
+ template_edit_permission = None
def initialize(self, series, is_distroseries=True):
self.is_distroseries = is_distroseries
@@ -828,8 +829,16 @@
return "inactive-template"
def isVisible(self, template):
- if (template.iscurrent or
- check_permission('launchpad.Edit', template)):
+ '''Returns True if the template should be display to users.
+
+ Since all templates from a distroseries have the same permissions,
+ security checking is cached and the condition is based on `iscurrent`
+ attribute.
+ '''
+ if self.template_edit_permission is None:
+ self.template_edit_permission = (
+ check_permission('launchpad.Edit', template))
+ if (template.iscurrent or self.template_edit_permission):
return True
else:
return False
=== modified file 'lib/lp/translations/stories/standalone/xx-series-templates.txt'
--- lib/lp/translations/stories/standalone/xx-series-templates.txt 2010-02-16 13:26:21 +0000
+++ lib/lp/translations/stories/standalone/xx-series-templates.txt 2010-07-14 15:11:11 +0000
@@ -38,9 +38,9 @@
>>> table = find_tag_by_id(anon_browser.contents, 'templates_table')
>>> print extract_text(table)
- Priority Source package Template name Length Languages Updated
- 100 evolution evolution-2.2 22 2 2005-05-06
- 0 evolution man 1 1 2006-08-14 ...
+ Priority Source package Template name Length Updated
+ 100 evolution evolution-2.2 22 2005-05-06
+ 0 evolution man 1 2006-08-14 ...
Logged-in users see a link to all the active translation templates
on a distribution series translation page.
@@ -174,10 +174,10 @@
>>> table = find_tag_by_id(user_browser.contents, 'templates_table')
>>> print extract_text(table)
- Priority Template name Length Languages Updated Actions
- 0 at-the-top 0 0 ... Download
- 0 evolution-2.2 22 1 2005-08-25 Download
- 0 evolutio... 8 1 2006-12-13 Download
+ Priority Template name Length Updated Actions
+ 0 at-the-top 0 ... Download
+ 0 evolution-2.2 22 2005-08-25 Download
+ 0 evolutio... 8 2006-12-13 Download
If an administrator views this page, links to the templates admin page are
shown, too.
=== modified file 'lib/lp/translations/templates/object-templates.pt'
--- lib/lp/translations/templates/object-templates.pt 2010-02-15 16:00:01 +0000
+++ lib/lp/translations/templates/object-templates.pt 2010-07-14 15:11:11 +0000
@@ -5,7 +5,7 @@
metal:use-macro="view/macro:page/main_only"
>
<body>
- <div metal:fill-slot="head_epilogue">
+ <tal:head_epilogue metal:fill-slot="head_epilogue">
<style type="text/css">
.inactive_links a{
background: none;
@@ -53,7 +53,7 @@
});
});
</script>
- </div>
+ </tal:head_epilogue>
<div metal:fill-slot="main">
<div class="translation-help-links">
<a href="https://help.launchpad.net/Translations";
@@ -79,7 +79,6 @@
class="sourcepackage_column">Source package</th>
<th class="template_column">Template name</th>
<th class="length_column">Length</th>
- <th class="languages_column">Languages</th>
<th class="lastupdate_column">Updated</th>
<th class="actions_column"
tal:condition="context/required:launchpad.AnyPerson">
@@ -89,7 +88,8 @@
<tbody>
<tal:templates repeat="template view/iter_templates">
<tr tal:define="
- inactive_css_class python:view.rowCSSClass(template)"
+ inactive_css_class python:view.rowCSSClass(template);
+ template_url template/fmt:url"
tal:condition="python: view.isVisible(template)"
tal:attributes="
class string: template_row ${inactive_css_class}">
@@ -100,7 +100,7 @@
class="sourcepackage_column">Source package
</td>
<td class="template_column">
- <a tal:attributes="href template/fmt:url">
+ <a tal:attributes="href template_url">
<span tal:content="template/name">Template name</span>
</a>
<tal:inactive condition="not: template/iscurrent">
@@ -108,9 +108,7 @@
</tal:inactive>
</td>
<td class="length_column"
- tal:content="template/getPOTMsgSetsCount">1777</td>
- <td class="languages_column"
- tal:content="template/language_count">777</td>
+ tal:content="template/messagecount">1777</td>
<td class="lastupdate_column">
<span class="sortkey"
tal:condition="template/date_last_updated"
@@ -130,18 +128,14 @@
<td class="actions_column"
tal:condition="context/required:launchpad.AnyPerson">
<div class="template_links">
- <a tal:replace="
- structure template/menu:translations/edit/fmt:link
- " />
- <a tal:replace="
- structure template/menu:translations/upload/fmt:link
- " />
- <a tal:replace="
- structure template/menu:translations/download/fmt:link
- " />
- <a tal:replace="
- structure template/menu:translations/administer/fmt:link
- " />
+ <a class="sprite edit" tal:attributes="
+ href string: ${template_url}/+edit">Edit</a>
+ <a class="sprite add" tal:attributes="
+ href string: ${template_url}/+upload">Upload</a>
+ <a class="sprite download" tal:attributes="
+ href string: ${template_url}/+export">Download</a>
+ <a class="sprite edit" tal:attributes="
+ href string: ${template_url}/+admin">Administer</a>
</div>
</td>
</tr>
