launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03792
[Merge] lp:~sinzui/launchpad/display-name-with-id-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/display-name-with-id-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #789147 in Launchpad itself: "show user display name and Lp Id in comments and logged-in-as"
https://bugs.launchpad.net/launchpad/+bug/789147
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/display-name-with-id-0/+merge/63006
Show Launchpad Id next to display name in comments and logged-status.
Launchpad bug: https://bugs.launchpad.net/bugs/789147
Pre-implementation: teal squad
The login-status area of every page, and user comments must show the user's
display name and Launchpad Id. The format is
Jonathan Lange (jml)
Users will see who they are logged in as and equate that to the users
displayed in comments. A tales formatter is need to ensure consistency, maybe
<user>/fmt:link-display-name-id
This presentation should be behind a feature flag.
--------------------------------------------------------------------
RULES
* Add a new formatter rule for IPerson to show
Jonathan Lange (jml)
* Use the format in the launchpad-loginstatus template of every page.
* Hide it with a feature flag.
* Removed PersonFormatterAPI.nameLink() that is only used by
launchpad-loginstatus
* Use the format in the bug comment header.
* Hide it with a feature flag.
* Use the format in the merge proposal comment header.
* Hide it with a feature flag.
* Use the format in the answers comment header.
* Hide it with a feature flag.
QA
* Login to https://qastaging.launchpad.net/
* Verify the page say who you are logged in as in the header:
display name (Launchpad-Id)
* View any bug with comments.
* Verify that you can see the display name and Launchpad-id for each
user who commented
* View any merge proposal with comments.
* Verify that you can see the display name and Launchpad-id for each
user who commented
* View any question with comments.
* Verify that you can see the display name and Launchpad-id for each
user who commented
LINT
lib/canonical/launchpad/webapp/tests/test_login.py
lib/lp/answers/stories/question-workflow.txt
lib/lp/answers/templates/questionmessage-display.pt
lib/lp/app/browser/tales.py
lib/lp/app/doc/tales.txt
lib/lp/app/templates/launchpad-loginstatus.pt
lib/lp/bugs/stories/bugs/xx-bug-activity.txt
lib/lp/bugs/stories/bugs/xx-numbered-comments.txt
lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt
lib/lp/bugs/templates/bugcomment-box.pt
lib/lp/bugs/templates/bugcomment-macros.pt
lib/lp/code/stories/branches/xx-code-review-comments.txt
lib/lp/code/templates/codereviewcomment-header.pt
lib/lp/services/comments/templates/comment-header.pt
^ lint hates the stories. I can fix the line length, white space and header
formats before I land.
TEST
./bin/test -vv \
-t tales.txt -t test_login \
-t xx-bug-activity -t xx-numbered-comments -t xx-remote-bug-comments \
-t question-workflow -t xx-code-review-comments
IMPLEMENTATION
Replaced nameLink with link_display_name_id.
lib/lp/app/browser/tales.py
lib/lp/app/doc/tales.txt
Updated the only use of nameLink to link_display_name_id.
lib/canonical/launchpad/templates/launchpad-loginstatus.pt
lib/canonical/launchpad/webapp/tests/test_login.py
Update bug comments to use link_display_name_id. There are three use cases:
The owner of the comment, the owner of the remove comment, the user in the
bug activity.
lib/lp/bugs/stories/bugs/xx-bug-activity.txt
lib/lp/bugs/stories/bugs/xx-numbered-comments.txt
lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt
lib/lp/bugs/templates/bugcomment-box.pt
lib/lp/bugs/templates/bugcomment-macros.pt
Update question comments to use link_display_name_id. One place to change and
only one test.
lib/lp/answers/stories/question-workflow.txt
lib/lp/answers/templates/questionmessage-display.pt
Updated service comments and code comments to use link_display_name_id. Note
that link_display_name_id always goes to the mainsite so the explicit rootsite
path argument is not needed.
lib/lp/code/stories/branches/xx-code-review-comments.txt
lib/lp/code/templates/codereviewcomment-header.pt
lib/lp/services/comments/templates/comment-header.pt
--
https://code.launchpad.net/~sinzui/launchpad/display-name-with-id-0/+merge/63006
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/display-name-with-id-0 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/webapp/tests/test_login.py'
--- lib/canonical/launchpad/webapp/tests/test_login.py 2011-04-21 10:02:26 +0000
+++ lib/canonical/launchpad/webapp/tests/test_login.py 2011-05-31 15:50:40 +0000
@@ -561,6 +561,13 @@
layer = AppServerLayer
def test_replay_attacks_do_not_succeed(self):
+ # Enable the picker_enhancements feature to test the commenter name.
+ from lp.services.features.testing import FeatureFixture
+ feature_flag = {'disclosure.picker_enhancements.enabled': 'on'}
+ flags = FeatureFixture(feature_flag)
+ flags.setUp()
+ self.addCleanup(flags.cleanUp)
+
browser = Browser(mech_browser=MyMechanizeBrowser())
browser.open('%s/+login' % self.layer.appserver_root_url())
# On a JS-enabled browser this page would've been auto-submitted
@@ -572,7 +579,7 @@
fill_login_form_and_submit(browser, 'test@xxxxxxxxxxxxx', 'test')
login_status = extract_text(
find_tag_by_id(browser.contents, 'logincontrol'))
- self.assertIn('name12', login_status)
+ self.assertIn('Sample Person (name12)', login_status)
# Now we look up (in urls_redirected_to) the +openid-callback URL that
# was used to complete the authentication and open it on a different
=== modified file 'lib/lp/answers/stories/question-workflow.txt'
--- lib/lp/answers/stories/question-workflow.txt 2011-05-16 02:23:17 +0000
+++ lib/lp/answers/stories/question-workflow.txt 2011-05-31 15:50:40 +0000
@@ -1,5 +1,11 @@
= Question Workflow =
+ >>> # Enable the picker_enhancements feature to test the commenter name.
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.picker_enhancements.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
The status of a question changes based on the action done by users on
it. To demonstrate the workflow, we will use the existing question #2 on
the Firefox product which was filed by 'Sample Person'.
@@ -331,7 +337,7 @@
<img src="/@@/favourite-yes" ... title="Marked as best answer" />
>>> answerer = bestAnswer.parent.find('a')
>>> print extract_text(answerer)
- No Privileges Person
+ No Privileges Person (no-priv)
>>> message = bestAnswer.parent.parent.find(
... 'div', 'boardCommentBody highlighted')
>>> print extract_text(message)
@@ -441,3 +447,7 @@
... owner_browser.contents, 'ask-your-own-question')
>>> content is None
True
+
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
=== modified file 'lib/lp/answers/templates/questionmessage-display.pt'
--- lib/lp/answers/templates/questionmessage-display.pt 2011-05-16 18:07:09 +0000
+++ lib/lp/answers/templates/questionmessage-display.pt 2011-05-31 15:50:40 +0000
@@ -12,7 +12,7 @@
title="Marked as best answer" />
</tal:bestanswer>
<tal:comment_has_owner>
- <tal:comment_owner replace="structure context/owner/fmt:link" />
+ <tal:comment_owner replace="structure context/owner/fmt:link-display-name-id" />
</tal:comment_has_owner>
said
<span
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py 2011-05-26 00:21:17 +0000
+++ lib/lp/app/browser/tales.py 2011-05-31 15:50:40 +0000
@@ -1147,7 +1147,7 @@
'icon': 'icon',
'displayname': 'displayname',
'unique_displayname': 'unique_displayname',
- 'name_link': 'nameLink',
+ 'link-display-name-id': 'link_display_name_id',
}
final_traversable_names = {'local-time': 'local_time'}
@@ -1209,9 +1209,18 @@
else:
return '<img src="%s" width="14" height="14" />' % custom_icon
- def nameLink(self, view_name):
- """Return the Launchpad id of the person, linked to their profile."""
- return self._makeLink(view_name, 'mainsite', self._context.name)
+ def link_display_name_id(self, view_name):
+ """Return a link to the user's profile page.
+
+ The link text uses both the display name and Launchpad id to clearly
+ indicate which user profile is linked.
+ """
+ from lp.services.features import getFeatureFlag
+ if bool(getFeatureFlag('disclosure.picker_enhancements.enabled')):
+ text = '%s (%s)' % (self._context.displayname, self._context.name)
+ else:
+ text = self._context.displayname
+ return self._makeLink(view_name, 'mainsite', text)
class TeamFormatterAPI(PersonFormatterAPI):
=== modified file 'lib/lp/app/doc/tales.txt'
--- lib/lp/app/doc/tales.txt 2011-05-29 23:34:26 +0000
+++ lib/lp/app/doc/tales.txt 2011-05-31 15:50:40 +0000
@@ -433,8 +433,17 @@
>>> print test_tales("person/fmt:link", person=mark)
<a ...http://launchpad.dev/~mark...
- >>> print test_tales("person/fmt:name_link", person=mark)
- <a ...http://launchpad.dev/~mark...
+ >>> # Enable the picker_enhancements feature to test the commenter name.
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.picker_enhancements.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
+ >>> print test_tales("person/fmt:link-display-name-id", person=mark)
+ <a ...http://launchpad.dev/~mark...>Mark Shuttleworth (mark)</a>
+
+ >>> # Clean up the feature flag.
+ >>> flags.cleanUp()
>>> print test_tales("team/fmt:link", team=ubuntu_team)
<a ...http://launchpad.dev/~ubuntu-team...
=== modified file 'lib/lp/app/templates/launchpad-loginstatus.pt'
--- lib/lp/app/templates/launchpad-loginstatus.pt 2011-05-27 20:26:32 +0000
+++ lib/lp/app/templates/launchpad-loginstatus.pt 2011-05-31 15:50:40 +0000
@@ -16,7 +16,7 @@
<li class="no-events">No AJAX events detected</li>
</ul>
</div>
- <a tal:replace="structure request/lp:person/fmt:name_link" /> •
+ <a tal:replace="structure request/lp:person/fmt:link-display-name-id" /> •
<input type="submit" name="logout" value="Log Out" />
</form>
</div>
=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-activity.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2011-05-11 14:54:59 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2011-05-31 15:50:40 +0000
@@ -2,6 +2,12 @@
The bug activity page is where you find the "changelog" of a bug.
+ >>> # Enable the picker_enhancements feature to test the commenter name.
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.picker_enhancements.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
>>> anon_browser.open(
... 'http://bugs.launchpad.dev/debian/+source/'
... 'mozilla-firefox/+bug/3/+activity')
@@ -62,13 +68,13 @@
>>> user_browser.getControl('Post Comment').click()
>>> print_comments(user_browser.contents, slice(None))
In...
- Bug Watch Updater
+ Bug Watch Updater (bug-watch-updater)
on 2007-12-18
Changed in thunderbird:
status:
Unknown => New
--------
- No Privileges Person
+ No Privileges Person (no-priv)
...
#7
Here's a comment for testing, like.
@@ -89,7 +95,7 @@
Bug
...
--------
- Foo Bar ... ago
+ Foo Bar (name16) ... ago
summary:
- Nonsensical bugs are useless
+ A new title for this bug
@@ -127,7 +133,7 @@
>>> admin_browser.open('http://launchpad.dev/bugs/15')
>>> print_comments(admin_browser.contents)
- Foo Bar
+ Foo Bar (name16)
... ago
summary:
...
@@ -185,7 +191,7 @@
>>> admin_browser.open('http://launchpad.dev/bugs/15')
>>> print_comments(admin_browser.contents)
- Foo Bar
+ Foo Bar (name16)
... ago
summary:
...
@@ -219,7 +225,7 @@
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- Foo Bar
+ Foo Bar (name16)
... ago
summary:
...
@@ -243,7 +249,7 @@
>>> admin_browser.getControl('Status').value = ['Confirmed']
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- Foo Bar
+ Foo Bar (name16)
... ago
Changed in mozilla-firefox (Ubuntu):
status:
@@ -262,7 +268,7 @@
>>> admin_browser.getControl('Comment').value = "Lookit, a change!"
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- Foo Bar
+ Foo Bar (name16)
wrote
... ago:
#2
@@ -286,7 +292,7 @@
... 'Package').value = 'linux-source-2.6.15'
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
- Foo Bar
+ Foo Bar (name16)
wrote
... ago:
#2
@@ -294,3 +300,7 @@
affects:
mozilla-firefox (Ubuntu) => linux-source-2.6.15 (Ubuntu)
--------
+
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
=== modified file 'lib/lp/bugs/stories/bugs/xx-numbered-comments.txt'
--- lib/lp/bugs/stories/bugs/xx-numbered-comments.txt 2009-06-18 19:02:30 +0000
+++ lib/lp/bugs/stories/bugs/xx-numbered-comments.txt 2011-05-31 15:50:40 +0000
@@ -1,5 +1,11 @@
= Comment numbering =
+ >>> # Enable the picker_enhancements feature to test the commenter name.
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.picker_enhancements.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
Bug respondents frequently refer to other comments by number,
eg. "applied patch in comment #34". This number is stable, and part of
the permalink URL. To help users find the relevant comments more
@@ -17,15 +23,19 @@
... extract_text(number_node),
... extract_text(person_node),
... extract_text(comment_node)[:50])
- #1: Valentina Commissari
+ #1: Valentina Commissari (tsukimi)
The solution to this is to make Jokosher use autoa
- #2: Diogo Matsubara
+ #2: Diogo Matsubara (matsubara)
I'm not sure that autoaudiosink is in fact the bes
- #3: Karl Tilbury
+ #3: Karl Tilbury (karl)
Unfortunately, the lead developer of autoaudiosink
- #4: Daniel Henrique Debonzi
+ #4: Daniel Henrique Debonzi (debonzi)
The strangest thing should be happening here. My s
- #5: Edgar Bursic
+ #5: Edgar Bursic (edgar)
And so it should be displayed again.
- #6: Dave Miller
+ #6: Dave Miller (justdave)
This title, however, is the same as the bug title
+
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
=== modified file 'lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt'
--- lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt 2011-05-31 15:50:40 +0000
@@ -1,5 +1,11 @@
= Remote bug comments =
+ >>> # Enable the picker_enhancements feature to test the commenter name.
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.picker_enhancements.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
Comments imported into Launchpad from external bug trackers are
formatted for display in a way that distinguishes them from comments
entered within Launchpad itself.
@@ -11,7 +17,7 @@
>>> print extract_text(details)
In
Debian Bug tracker #308994,
- josh
+ josh (jbuhl-nospam)
wrote
...
gnome-volume-manager: dvd+rw unreadable when automounted ...
@@ -111,3 +117,7 @@
>>> footer = remote_bug_comment.find(attrs={'class': 'boardCommentFooter'})
>>> 'Reply' in extract_text(footer)
False
+
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
=== modified file 'lib/lp/bugs/templates/bugcomment-box.pt'
--- lib/lp/bugs/templates/bugcomment-box.pt 2011-05-16 15:29:15 +0000
+++ lib/lp/bugs/templates/bugcomment-box.pt 2011-05-31 15:50:40 +0000
@@ -17,7 +17,7 @@
<tr>
<td tal:condition="not: comment/bugwatch">
<tal:comment_owner
- replace="structure comment/owner/fmt:link" />
+ replace="structure comment/owner/fmt:link-display-name-id" />
wrote
<span tal:attributes="title comment/datecreated/fmt:datetime"
tal:content="comment/datecreated/fmt:displaydate">
@@ -34,7 +34,7 @@
<a tal:attributes="href comment/bugwatch/url"
tal:content="comment/bugwatch/title" />,
<tal:comment_owner
- replace="structure comment/owner/fmt:link" />
+ replace="structure comment/owner/fmt:link-display-name-id" />
wrote
<span tal:attributes="title comment/datecreated/fmt:datetime"
tal:content="comment/datecreated/fmt:displaydate">
=== modified file 'lib/lp/bugs/templates/bugcomment-macros.pt'
--- lib/lp/bugs/templates/bugcomment-macros.pt 2011-02-15 08:54:52 +0000
+++ lib/lp/bugs/templates/bugcomment-macros.pt 2011-05-31 15:50:40 +0000
@@ -32,7 +32,7 @@
tal:attributes="class string:boardComment">
<div class="boardCommentDetails">
<tal:activity_has_person>
- <tal:activity_person replace="structure activity_person/fmt:link" />
+ <tal:activity_person replace="structure activity_person/fmt:link-display-name-id" />
</tal:activity_has_person>
<span
tal:attributes="title activity_date/fmt:datetime"
=== modified file 'lib/lp/code/stories/branches/xx-code-review-comments.txt'
--- lib/lp/code/stories/branches/xx-code-review-comments.txt 2011-01-30 20:40:37 +0000
+++ lib/lp/code/stories/branches/xx-code-review-comments.txt 2011-05-31 15:50:40 +0000
@@ -12,6 +12,11 @@
>>> merge_proposal_url = canonical_url(merge_proposal)
>>> merge_proposal_path = canonical_url(
... merge_proposal, force_local_path=True)
+ >>> # Enable the picker_enhancements feature to test the commenter name.
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.picker_enhancements.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
>>> logout()
>>> eric_browser = setupBrowser(auth="Basic eric@xxxxxxxxxxx:test")
@@ -38,7 +43,7 @@
... print extract_text(tag)
>>> print_comments('boardCommentDetails')
- Eric the Viking wrote ...
+ Eric the Viking (eric) wrote ...
>>> print_comments('boardCommentBody')
This is a very long comment about what things should be done to the
source branch to land it. When this comment is replied to, it should
@@ -110,7 +115,7 @@
source branch to land it. When this comment is replied to, it should
wrap the line properly.
>>> print_comments('boardCommentDetails', anon_browser, index=0)
- Eric the Viking wrote ... ago
+ Eric the Viking (eric) wrote ... ago
Posted in a previous version of this proposal
>>> details = find_tags_by_class(
... anon_browser.contents, 'boardCommentDetails')[0]
@@ -188,3 +193,6 @@
4. By ... on 2009-09-12
and it works!
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
=== modified file 'lib/lp/code/templates/codereviewcomment-header.pt'
--- lib/lp/code/templates/codereviewcomment-header.pt 2010-09-20 10:48:52 +0000
+++ lib/lp/code/templates/codereviewcomment-header.pt 2011-05-31 15:50:40 +0000
@@ -3,7 +3,7 @@
xmlns:metal="http://xml.zope.org/namespaces/metal";
omit-tag="">
- <tal:author replace="structure context/comment_author/fmt:link:mainsite"/>
+ <tal:author replace="structure context/comment_author/fmt:link-display-name-id"/>
<tal:has-body condition="context/has_body">wrote</tal:has-body>
<tal:date replace="context/comment_date/fmt:displaydate" />
<span tal:condition="context/from_superseded" class="sprite warning-icon"
=== modified file 'lib/lp/services/comments/templates/comment-header.pt'
--- lib/lp/services/comments/templates/comment-header.pt 2010-09-17 10:50:51 +0000
+++ lib/lp/services/comments/templates/comment-header.pt 2011-05-31 15:50:40 +0000
@@ -3,7 +3,7 @@
xmlns:metal="http://xml.zope.org/namespaces/metal";
omit-tag="">
- <tal:author replace="structure context/comment_author/fmt:link:mainsite"/>
+ <tal:author replace="structure context/comment_author/fmt:link-display-name-id"/>
<tal:has-body condition="context/has_body">wrote</tal:has-body>
<tal:date replace="context/comment_date/fmt:displaydate" />
</tal:root>
