Launchpad itself

Merge lp:~henninge/launchpad/bug-461756 into lp:launchpad

  1. bug-461756
  2. Merge into devel
Proposed by Henning Eggers
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~henninge/launchpad/bug-461756
Merge into: lp:launchpad
Diff against target: 159 lines
6 files modified
lib/lp/translations/browser/distroseries.py (+2/-1)
lib/lp/translations/browser/productseries.py (+2/-1)
lib/lp/translations/interfaces/potemplate.py (+2/-1)
lib/lp/translations/model/potemplate.py (+10/-3)
lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt (+4/-4)
lib/lp/translations/stories/productseries/xx-productseries-templates.txt (+16/-1)
To merge this branch: bzr merge lp:~henninge/launchpad/bug-461756
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Review via email: mp+14211@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 461756 =

The template ordering on a productseries or distroseries +templates page
seemed to be rather arbitrary. In reality, they were ordered by database
ids, which is not very useful if you want to find a specific template on
the page.

This ordering is taken from the POTemplateSubset ordering where the
template are retrieved from the database.

== Proposed fix ==

Add an optional ordering flag to POTemplateSubset to apply an ordering
by names to the query. The view code can then set that flag.

Ordering will be by POTemplate.name for a productseries and by
(SourcePackageName.name, POTemplate.name) for a distroseries.

== Implementation details ==

The flag 'ordered_by_names' defaults to 'False' to retain the old
behaviour on which other code (and tests) might depend.

== Tests ==

bin/test -vvct distroseries-templates -t productseries-templates

== Demo and Q/A ==

(rev 8619 on staging or rev 9804 on edge for bug 461818 to be fixed.)

Go to this url and verify that the templates are listed alphabetically
by the names of the source packages and the names of the templates.
https://translations.staging.launchpad.net/ubuntu/karmic/+templates

Here is one for a product, should be ordered by the names of the templates:
https://translations.staging.launchpad.net/ocportal/4.2.0/+templates

Note that the latter appears sorted, even without this fix, but that is
not true. Some templates are out of order. They were probably imported
in alphabetical order.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/translations/interfaces/potemplate.py
  lib/lp/translations/browser/distroseries.py
  lib/lp/translations/stories/productseries/xx-productseries-templates.txt
  lib/lp/translations/browser/productseries.py
  lib/lp/translations/model/potemplate.py
  lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt

== Pylint notices ==

lib/lp/translations/interfaces/potemplate.py
    9: [F0401] Unable to import 'lazr.enum' (No module named enum)

Revision history for this message
Данило Шеган (danilo) wrote :

From IRC:

<danilos> henninge, just one question about yours: is there a reason not to default to ordered_by_names=True?
<henninge> danilos: the risk of breaking other code/tests. That is the only reason.
<henninge> backward compatibility
<danilos> henninge, right, it'd be nice to see how much stuff breaks, but not that important right now

So, while it'd be nice to run at least a full lp.translations test suite and see exactly what breaks if we default to True (especially if it ends up not being much, so we can fix it), it's not a requirement for landing this branch.

Great work!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/browser/distroseries.py'
2--- lib/lp/translations/browser/distroseries.py 2009-10-22 11:49:30 +0000
3+++ lib/lp/translations/browser/distroseries.py 2009-10-30 11:20:32 +0000
4@@ -167,7 +167,8 @@
5
6 def iter_templates(self):
7 potemplateset = getUtility(IPOTemplateSet)
8- return potemplateset.getSubset(distroseries=self.context)
9+ return potemplateset.getSubset(distroseries=self.context,
10+ ordered_by_names=True)
11
12
13 class DistroSeriesView(LaunchpadView, TranslationsMixin):
14
15=== modified file 'lib/lp/translations/browser/productseries.py'
16--- lib/lp/translations/browser/productseries.py 2009-10-22 13:00:21 +0000
17+++ lib/lp/translations/browser/productseries.py 2009-10-30 11:20:32 +0000
18@@ -486,7 +486,8 @@
19 def iter_templates(self):
20 """Return an iterator of all `IPOTemplates` for the series."""
21 potemplateset = getUtility(IPOTemplateSet)
22- return potemplateset.getSubset(productseries=self.context)
23+ return potemplateset.getSubset(productseries=self.context,
24+ ordered_by_names=True)
25
26
27 class LinkTranslationsBranchView(LaunchpadEditFormView):
28
29=== modified file 'lib/lp/translations/interfaces/potemplate.py'
30--- lib/lp/translations/interfaces/potemplate.py 2009-10-27 10:40:46 +0000
31+++ lib/lp/translations/interfaces/potemplate.py 2009-10-30 11:20:32 +0000
32@@ -634,7 +634,8 @@
33 """Return an iterator over all POTemplate sorted by modification."""
34
35 def getSubset(distroseries=None, sourcepackagename=None,
36- productseries=None, iscurrent=None):
37+ productseries=None, iscurrent=None,
38+ ordered_by_names=False):
39 """Return a POTemplateSubset object depending on the given arguments.
40 """
41
42
43=== modified file 'lib/lp/translations/model/potemplate.py'
44--- lib/lp/translations/model/potemplate.py 2009-10-29 17:46:00 +0000
45+++ lib/lp/translations/model/potemplate.py 2009-10-30 11:20:32 +0000
46@@ -999,7 +999,8 @@
47 implements(IPOTemplateSubset)
48
49 def __init__(self, sourcepackagename=None, from_sourcepackagename=None,
50- distroseries=None, productseries=None, iscurrent=None):
51+ distroseries=None, productseries=None, iscurrent=None,
52+ ordered_by_names=False):
53 """Create a new `POTemplateSubset` object.
54
55 The set of POTemplate depends on the arguments you pass to this
56@@ -1024,9 +1025,13 @@
57 if productseries is not None:
58 self.clauses.append(
59 POTemplate.productseriesID == productseries.id)
60+ if ordered_by_names:
61+ self.orderby = [POTemplate.name]
62 else:
63 self.clauses.append(
64 POTemplate.distroseriesID == distroseries.id)
65+ if ordered_by_names:
66+ self.orderby = [SourcePackageName.name, POTemplate.name]
67 if from_sourcepackagename is not None:
68 self.clauses.append(
69 POTemplate.from_sourcepackagenameID ==
70@@ -1237,13 +1242,15 @@
71 return POTemplate.select(orderBy=['-date_last_updated'])
72
73 def getSubset(self, distroseries=None, sourcepackagename=None,
74- productseries=None, iscurrent=None):
75+ productseries=None, iscurrent=None,
76+ ordered_by_names=False):
77 """See `IPOTemplateSet`."""
78 return POTemplateSubset(
79 distroseries=distroseries,
80 sourcepackagename=sourcepackagename,
81 productseries=productseries,
82- iscurrent=iscurrent)
83+ iscurrent=iscurrent,
84+ ordered_by_names=ordered_by_names)
85
86 def getSubsetFromImporterSourcePackageName(self, distroseries,
87 sourcepackagename, iscurrent=None):
88
89=== modified file 'lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt'
90--- lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt 2009-10-22 11:49:30 +0000
91+++ lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt 2009-10-30 11:20:32 +0000
92@@ -29,12 +29,12 @@
93 >>> table = find_tag_by_id(admin_browser.contents, 'templates_table')
94 >>> print extract_text(table)
95 Source package Template name Actions
96- pmount pmount Edit Upload Download Administer
97+ evolution disabled-template Edit Upload Download Administer
98 evolution evolution-2.2 Edit Upload Download Administer
99+ evolution man Edit Upload Download Administer
100 mozilla pkgconf-mozilla Edit Upload Download Administer
101- evolution man Edit Upload Download Administer
102 pmount man Edit Upload Download Administer
103- evolution disabled-template Edit Upload Download Administer
104+ pmount pmount Edit Upload Download Administer
105
106
107 == Links to the templates ==
108@@ -53,4 +53,4 @@
109 ... 'http://translations.launchpad.dev/ubuntu/hoary/+templates')
110 >>> admin_browser.getLink('Edit').click()
111 >>> print admin_browser.url
112- http://translations.launchpad.dev/ubuntu/hoary/+source/pmount/+pots/pmount/+edit
113+ http://translations.../evolution/+pots/disabled-template/+edit
114
115=== modified file 'lib/lp/translations/stories/productseries/xx-productseries-templates.txt'
116--- lib/lp/translations/stories/productseries/xx-productseries-templates.txt 2009-10-22 11:49:30 +0000
117+++ lib/lp/translations/stories/productseries/xx-productseries-templates.txt 2009-10-30 11:20:32 +0000
118@@ -4,6 +4,19 @@
119 templates in this series and provides easy access to the various subpages of
120 each template.
121
122+== Preparation ==
123+
124+To test the ordering of templates in the listing, we need another template
125+that is new but must appear at the top of the list.
126+
127+ >>> login('foo.bar@canonical.com')
128+ >>> from zope.component import getUtility
129+ >>> from lp.registry.interfaces.product import IProductSet
130+ >>> evolution = getUtility(IProductSet).getByName('evolution')
131+ >>> evolution_trunk = evolution.getSeries('trunk')
132+ >>> template = factory.makePOTemplate(productseries=evolution_trunk,
133+ ... name='at-the-top')
134+ >>> logout()
135
136 == Getting there ==
137
138@@ -24,6 +37,7 @@
139 >>> table = find_tag_by_id(user_browser.contents, 'templates_table')
140 >>> print extract_text(table)
141 Template name Actions
142+ at-the-top Download
143 evolution-2.2 Download
144 evolution-2.2-test Download
145
146@@ -35,6 +49,7 @@
147 >>> table = find_tag_by_id(admin_browser.contents, 'templates_table')
148 >>> print extract_text(table)
149 Template name Actions
150+ at-the-top Edit Upload Download Administer
151 evolution-2.2 Edit Upload Download Administer
152 evolution-2.2-test Edit Upload Download Administer
153
154@@ -55,4 +70,4 @@
155 ... 'http://translations.launchpad.dev/evolution/trunk/+templates')
156 >>> admin_browser.getLink('Edit').click()
157 >>> print admin_browser.url
158- http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/+edit
159+ http://translations.launchpad.dev/evolution/trunk/+pots/at-the-top/+edit