Launchpad itself

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

  1. bug-121520-merged
  2. Merge into devel
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
1=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-details.txt'
2--- lib/lp/translations/stories/standalone/xx-pofile-details.txt 2009-09-18 15:42:19 +0000
3+++ lib/lp/translations/stories/standalone/xx-pofile-details.txt 2009-12-15 14:14:15 +0000
4@@ -1,3 +1,6 @@
5+POFile Details Page
6+===================
7+
8 Each translation file has an overview page which shows the list of
9 contributors.
10
11@@ -103,7 +106,9 @@
12 >>> print anon_browser.title
13 Browsing Spanish translation...
14
15-== Invalid input ==
16+
17+Invalid input
18+-------------
19
20 Manually filtering by non-existent user warns the user of the problem.
21
22@@ -121,3 +126,48 @@
23 [u'No person to filter by specified.',
24 u'This person has made no contributions to this file.']
25
26+
27+Merged accounts
28+---------------
29+
30+On the overview page of each translation pofile, users will not see merged
31+accounts.
32+
33+We'll create two new accounts to demonstrate this.
34+
35+ >>> from zope.component import getUtility
36+ >>> from canonical.launchpad.interfaces.launchpad import (
37+ ... ILaunchpadCelebrities)
38+
39+ >>> login("admin@canonical.com")
40+ >>> ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
41+ >>> hoary = ubuntu.getSeries('hoary')
42+ >>> translator = factory.makePerson(displayname="Poly Glot")
43+ >>> merged_translator = factory.makePerson(displayname="Mere Pere")
44+ >>> package = factory.makeSourcePackage(distroseries=hoary)
45+ >>> template = factory.makePOTemplate(
46+ ... distroseries=hoary,
47+ ... sourcepackagename=package.sourcepackagename, name='first')
48+ >>> language_code = 'es'
49+ >>> pofile = factory.makePOFile(language_code, potemplate=template)
50+ >>> potmsgset = factory.makePOTMsgSet(template)
51+ >>> potmsgset.setSequence(template, 1)
52+ >>> translation = factory.makeTranslationMessage(pofile=pofile,
53+ ... translator=merged_translator, potmsgset=potmsgset)
54+ >>> potmsgset.setSequence(template, 2)
55+ >>> translation = factory.makeTranslationMessage(pofile=pofile,
56+ ... translator=translator, potmsgset=potmsgset)
57+ >>> merged_translator.merged = translator
58+ >>> logout()
59+
60+ >>> browser.open(
61+ ... ("http://translations.launchpad.dev/"
62+ ... "ubuntu/hoary/+source/%s/+pots/%s/%s/+details") % (
63+ ... package.name, template.name, language_code))
64+ >>> print extract_text(find_main_content(browser.contents))
65+ Details for ...
66+ Contributors to this translation
67+ The following people have made some contribution to this specific
68+ translation:
69+ Poly Glot (filter)
70+
71
72=== modified file 'lib/lp/translations/templates/pofile-details.pt'
73--- lib/lp/translations/templates/pofile-details.pt 2009-09-16 15:27:39 +0000
74+++ lib/lp/translations/templates/pofile-details.pt 2009-12-15 14:14:15 +0000
75@@ -42,10 +42,12 @@
76
77 <ul tal:condition="view/contributors">
78 <li tal:repeat="contributor view/contributors">
79- <a tal:replace="structure contributor/fmt:link" />
80- (<a tal:attributes="
81- href string:${context/fmt:url}/+filter?person=${contributor/name}"
82- >filter</a>)
83+ <tal:not-merged condition="not: contributor/merged">
84+ <a tal:replace="structure contributor/fmt:link" />
85+ (<a tal:attributes="
86+ href string:${context/fmt:url}/+filter?person=${contributor/name}"
87+ >filter</a>)
88+ </tal:not-merged>
89 </li>
90 </ul>
91
92
93=== modified file 'lib/lp/translations/templates/pofile-translate-contributors.pt'
94--- lib/lp/translations/templates/pofile-translate-contributors.pt 2009-07-17 17:59:07 +0000
95+++ lib/lp/translations/templates/pofile-translate-contributors.pt 2009-12-15 14:14:15 +0000
96@@ -9,9 +9,11 @@
97 <p tal:condition="view/contributors">
98 Contributors to this translation:
99 <tal:contributors repeat="contributor view/contributors">
100- <tal:contributor replace="structure contributor/fmt:link" /><tal:comma
101- condition="not:repeat/contributor/end">,</tal:comma><tal:period
102- condition="repeat/contributor/end">.</tal:period>
103+ <tal:not-merged condition="not: contributor/merged">
104+ <tal:contributor replace="structure contributor/fmt:link" /><tal:comma
105+ condition="not:repeat/contributor/end">,</tal:comma><tal:period
106+ condition="repeat/contributor/end">.</tal:period>
107+ </tal:not-merged>
108 </tal:contributors>
109 </p>
110 </tal:root>