Merge lp://staging/~adiroiban/launchpad/bug-121520-merged into lp://staging/launchpad

Proposed by Adi Roiban
Status: Merged
Approved by: Graham Binns
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://staging/~adiroiban/launchpad/bug-121520-merged
Merge into: lp://staging/launchpad
Diff against target: 110 lines (+62/-8)
3 files modified
lib/lp/translations/stories/standalone/xx-pofile-details.txt (+51/-1)
lib/lp/translations/templates/pofile-details.pt (+6/-4)
lib/lp/translations/templates/pofile-translate-contributors.pt (+5/-3)
To merge this branch: bzr merge lp://staging/~adiroiban/launchpad/bug-121520-merged
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+16183@code.staging.launchpad.net

Commit message

Don't show merged account in the list of contributors for a PO file.

To post a comment you must log in.
Revision history for this message
Adi Roiban (adiroiban) wrote :

= Bug 121520 =
On https://translations.launchpad.net/+languages/st we can see that Leonardo Gregianin appears twice, one with this current account (leogregianin) and an old merged one, (leogregianin-merged.

We should not show merged accounts as they lead to „dead” pages.

== Proposed fix ==
Add a condition in the template to skip account with merged=True

== Tests ==
lp-test -t pofile-details

== Demo and Q/A ==
As I don't know how to create a merged account, and I assume it is not that trivial (no hints in the Factory) you can test it on staging
https://translations.staging.launchpad.net/+languages/st

The current page test is check that the valid/active contributors are displayed.

= 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/templates/pofile-details.pt
  lib/lp/translations/templates/pofile-translate-contributors.pt

Revision history for this message
Graham Binns (gmb) wrote :

This branch needs a test before it can land.

As discussed on IRC, the way to create a merged person is to create two people and then set the `merged` property of one of them to point to the other.

review: Needs Fixing
Revision history for this message
Adi Roiban (adiroiban) wrote :

I added a page test for checking this behavior.

Revision history for this message
Graham Binns (gmb) wrote :

Hi Adi,

This looks good. My only suggestion is that you change "A new account and a merged account is created for showing this behavior." to "We'll create two new accounts to demonstrate this." since that scans a little better.

r=me with that change; let me know when you've made it and I'll push this branch through EC2 for you.

review: Approve (code)
Revision history for this message
Adi Roiban (adiroiban) wrote :

Hi Graham,
Thanks for the suggestion!

It should be pushed.

Cheers

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-details.txt'
--- lib/lp/translations/stories/standalone/xx-pofile-details.txt 2009-09-18 15:42:19 +0000
+++ lib/lp/translations/stories/standalone/xx-pofile-details.txt 2009-12-15 14:14:15 +0000
@@ -1,3 +1,6 @@
1POFile Details Page
2===================
3
1Each translation file has an overview page which shows the list of4Each translation file has an overview page which shows the list of
2contributors.5contributors.
36
@@ -103,7 +106,9 @@
103 >>> print anon_browser.title106 >>> print anon_browser.title
104 Browsing Spanish translation...107 Browsing Spanish translation...
105108
106== Invalid input ==109
110Invalid input
111-------------
107112
108Manually filtering by non-existent user warns the user of the problem.113Manually filtering by non-existent user warns the user of the problem.
109114
@@ -121,3 +126,48 @@
121 [u'No person to filter by specified.',126 [u'No person to filter by specified.',
122 u'This person has made no contributions to this file.']127 u'This person has made no contributions to this file.']
123128
129
130Merged accounts
131---------------
132
133On the overview page of each translation pofile, users will not see merged
134accounts.
135
136We'll create two new accounts to demonstrate this.
137
138 >>> from zope.component import getUtility
139 >>> from canonical.launchpad.interfaces.launchpad import (
140 ... ILaunchpadCelebrities)
141
142 >>> login("admin@canonical.com")
143 >>> ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
144 >>> hoary = ubuntu.getSeries('hoary')
145 >>> translator = factory.makePerson(displayname="Poly Glot")
146 >>> merged_translator = factory.makePerson(displayname="Mere Pere")
147 >>> package = factory.makeSourcePackage(distroseries=hoary)
148 >>> template = factory.makePOTemplate(
149 ... distroseries=hoary,
150 ... sourcepackagename=package.sourcepackagename, name='first')
151 >>> language_code = 'es'
152 >>> pofile = factory.makePOFile(language_code, potemplate=template)
153 >>> potmsgset = factory.makePOTMsgSet(template)
154 >>> potmsgset.setSequence(template, 1)
155 >>> translation = factory.makeTranslationMessage(pofile=pofile,
156 ... translator=merged_translator, potmsgset=potmsgset)
157 >>> potmsgset.setSequence(template, 2)
158 >>> translation = factory.makeTranslationMessage(pofile=pofile,
159 ... translator=translator, potmsgset=potmsgset)
160 >>> merged_translator.merged = translator
161 >>> logout()
162
163 >>> browser.open(
164 ... ("http://translations.launchpad.dev/"
165 ... "ubuntu/hoary/+source/%s/+pots/%s/%s/+details") % (
166 ... package.name, template.name, language_code))
167 >>> print extract_text(find_main_content(browser.contents))
168 Details for ...
169 Contributors to this translation
170 The following people have made some contribution to this specific
171 translation:
172 Poly Glot (filter)
173
124174
=== modified file 'lib/lp/translations/templates/pofile-details.pt'
--- lib/lp/translations/templates/pofile-details.pt 2009-09-16 15:27:39 +0000
+++ lib/lp/translations/templates/pofile-details.pt 2009-12-15 14:14:15 +0000
@@ -42,10 +42,12 @@
4242
43 <ul tal:condition="view/contributors">43 <ul tal:condition="view/contributors">
44 <li tal:repeat="contributor view/contributors">44 <li tal:repeat="contributor view/contributors">
45 <a tal:replace="structure contributor/fmt:link" />45 <tal:not-merged condition="not: contributor/merged">
46 (<a tal:attributes="46 <a tal:replace="structure contributor/fmt:link" />
47 href string:${context/fmt:url}/+filter?person=${contributor/name}"47 (<a tal:attributes="
48 >filter</a>)48 href string:${context/fmt:url}/+filter?person=${contributor/name}"
49 >filter</a>)
50 </tal:not-merged>
49 </li>51 </li>
50 </ul>52 </ul>
5153
5254
=== modified file 'lib/lp/translations/templates/pofile-translate-contributors.pt'
--- lib/lp/translations/templates/pofile-translate-contributors.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/translations/templates/pofile-translate-contributors.pt 2009-12-15 14:14:15 +0000
@@ -9,9 +9,11 @@
9 <p tal:condition="view/contributors">9 <p tal:condition="view/contributors">
10 Contributors to this translation:10 Contributors to this translation:
11 <tal:contributors repeat="contributor view/contributors">11 <tal:contributors repeat="contributor view/contributors">
12 <tal:contributor replace="structure contributor/fmt:link" /><tal:comma12 <tal:not-merged condition="not: contributor/merged">
13 condition="not:repeat/contributor/end">,</tal:comma><tal:period13 <tal:contributor replace="structure contributor/fmt:link" /><tal:comma
14 condition="repeat/contributor/end">.</tal:period>14 condition="not:repeat/contributor/end">,</tal:comma><tal:period
15 condition="repeat/contributor/end">.</tal:period>
16 </tal:not-merged>
15 </tal:contributors>17 </tal:contributors>
16 </p>18 </p>
17</tal:root>19</tal:root>