launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06409
[Merge] lp:~sinzui/launchpad/usless-team-links into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/usless-team-links into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #808802 in Launchpad itself: "Team members page (+members) mentions "former members" and "pending members" but doesn't show them unless the viewer is an administrator"
https://bugs.launchpad.net/launchpad/+bug/808802
Bug #933912 in Launchpad itself: "+reviewaccount link shows up on team pages, generating oopses"
https://bugs.launchpad.net/launchpad/+bug/933912
Bug #934309 in Launchpad itself: "3 of the 7 team bugs navigation links are impossible"
https://bugs.launchpad.net/launchpad/+bug/934309
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/usless-team-links/+merge/93616
Remove useless links shown on team pages.
Pre-implementation: no one
Bug # 933912 +reviewaccount link shows up on team pages, generating oopses
Teams do not have account...the link should not exist.
Bug #934309 3 of the 7 team bugs navigation link are impossible
As noted in bug #244558, 3 of the 7 navigation links in the side bar
are impossible. I cannot log in as a team to report a bug, comment
on a bug, or be affected by a bug. The valid links are
All related bugs
Assigned bugs
Subscribed bugs
Subscribed packages
Bug #808802 +members mentions "former members" and "pending members"
Only admins see the links to pending and former members, but the
page always shows the headings
--------------------------------------------------------------------
RULES
Bug # 933912 +reviewaccount link shows up on team pages, generating oopses
The link to review the account was a hack added to the review page
when account was separated from person. The menu is smart enough to
not to show the link, but the template is an idiot. We are getting
rid of account, so this link will not be needed in the future. The
link can be removed from the template since the menu has the link.
Bug #934309 3 of the 7 team bugs navigation link are impossible
Set link.enabled to false when the context is a team.
Bug #808802 +members mentions "former members" and "pending members"
As mpt suggests, the sentence can be removed because it is
redundant or contradictory to the information shown.
ADDENDUM
The page title/breadcrumb is messed up, redundant, and truncated.
The view does not define the required simple terse page_title attr
so the label is reused; it is shown twice in the page title and
breadcrumbs.
QA
Bug # 933912 +reviewaccount link shows up on team pages, generating oopses
* Visit https://qastaging.launchpad.net/~launchpad
* Verify there is not a link to Administer account.
* Verify there is a link to Administer the team
* Follow the link to
https://qastaging.launchpad.net/~launchpad/+review
* Verify there is not a link to
Review the user's account information.
Bug #934309 3 of the 7 team bugs navigation link are impossible
* Visit https://bugs.qastaging.launchpad.net/~launchpad
* Verify these links are listed:
All related bugs, Assigned bugs, Subscribed bugs, Subscribed packages
* Verify these links are not present:
Affecting Bugs, Commented bugs, Reported Bugs
* Visit https://bugs.qastaging.launchpad.net/~sinzui
* Verify these links are listed:
All related bugs, Assigned bugs, Subscribed bugs, Subscribed packages
Affecting Bugs, Commented bugs, Reported Bugs
Bug #808802 +members mentions "former members" and "pending members"
* Visit https://launchpad.net/~launchpad/+members
* Verify the page does not show
Active, pending and former members of this team.
above the Active members heading.
ADDENDUM
* Verify the breadcrumbs are
“Canonical Launchpad Engineering” team >> Members
* Verify the page title is
Members : “Canonical Launchpad Engineering” team
LINT
lib/lp/bugs/browser/tests/test_person_bugs.py
lib/lp/registry/browser/person.py
lib/lp/registry/browser/team.py
lib/lp/registry/browser/tests/test_team.py
lib/lp/registry/stories/person/xx-admin-person-review.txt
lib/lp/registry/templates/person-review.pt
lib/lp/registry/templates/team-members.pt
TEST
./bin/test -vvc -t person-admin-views -t xx-admin-person-review lp.registry
./bin/test -vvc lp.bugs.browser.tests.test_person_bugs
./bin/test -vvc -t TeamMembershipView lp.registry.browser.tests.test_team
IMPLEMENTATION
Removed inline link to the user account form. Removed story tests that
duplicated the tests in lib/lp/registry/browser/tests/person-admin-views.txt
lib/lp/registry/stories/person/xx-admin-person-review.txt
lib/lp/registry/templates/person-review.pt
Updated the link to pass the enabled arg when the context is a team.
lib/lp/bugs/browser/tests/test_person_bugs.py
lib/lp/registry/browser/person.py
Removed the redundant/contradictory sentence.
lib/lp/registry/templates/team-members.pt
Add a page_title to the view to fix the breadcrumbs and page title.
lib/lp/registry/browser/team.py
lib/lp/registry/browser/tests/test_team.py
--
https://code.launchpad.net/~sinzui/launchpad/usless-team-links/+merge/93616
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/usless-team-links into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/tests/test_person_bugs.py'
--- lib/lp/bugs/browser/tests/test_person_bugs.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/browser/tests/test_person_bugs.py 2012-02-17 17:18:19 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
from lp.app.errors import UnexpectedFormData
+from lp.app.browser.tales import MenuAPI
from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.registry.browser import person
from lp.testing import (
@@ -17,6 +18,34 @@
from lp.testing.views import create_initialized_view
+class PersonBugsMenuTestCase(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_user(self):
+ user = self.factory.makePerson()
+ menu_api = MenuAPI(user)
+ menu_api._selectedfacetname = 'bugs'
+ enabled_links = sorted(
+ link.name for link in menu_api.navigation.values()
+ if link.enabled)
+ expected_links = [
+ 'affectingbugs', 'assignedbugs', 'commentedbugs',
+ 'relatedbugs', 'reportedbugs', 'softwarebugs', 'subscribedbugs']
+ self.assertEqual(expected_links, enabled_links)
+
+ def test_team(self):
+ team = self.factory.makeTeam()
+ menu_api = MenuAPI(team)
+ menu_api._selectedfacetname = 'bugs'
+ enabled_links = sorted(
+ link.name for link in menu_api.navigation.values()
+ if link.enabled)
+ expected_links = [
+ 'assignedbugs', 'relatedbugs', 'softwarebugs', 'subscribedbugs']
+ self.assertEqual(expected_links, enabled_links)
+
+
class TestBugSubscriberPackageBugsSearchListingView(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2012-02-14 15:32:31 +0000
+++ lib/lp/registry/browser/person.py 2012-02-17 17:18:19 +0000
@@ -639,7 +639,10 @@
def reportedbugs(self):
text = 'Reported bugs'
summary = 'Bugs reported by %s.' % self.context.displayname
- return Link('+reportedbugs', text, site='bugs', summary=summary)
+ enabled = not self.context.is_team
+ return Link(
+ '+reportedbugs', text, site='bugs', summary=summary,
+ enabled=enabled)
def subscribedbugs(self):
text = 'Subscribed bugs'
@@ -651,12 +654,18 @@
text = 'Commented bugs'
summary = ('Bug reports on which %s has commented.'
% self.context.displayname)
- return Link('+commentedbugs', text, site='bugs', summary=summary)
+ enabled = not self.context.is_team
+ return Link(
+ '+commentedbugs', text, site='bugs', summary=summary,
+ enabled=enabled)
def affectingbugs(self):
text = 'Affecting bugs'
summary = ('Bugs affecting %s.' % self.context.displayname)
- return Link('+affectingbugs', text, site='bugs', summary=summary)
+ enabled = not self.context.is_team
+ return Link(
+ '+affectingbugs', text, site='bugs', summary=summary,
+ enabled=enabled)
class PersonSpecsMenu(NavigationMenu):
=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py 2012-02-16 03:12:17 +0000
+++ lib/lp/registry/browser/team.py 2012-02-17 17:18:19 +0000
@@ -1752,6 +1752,8 @@
class TeamMembershipView(LaunchpadView):
"""The view behind ITeam/+members."""
+ page_title = 'Members'
+
@cachedproperty
def label(self):
return smartquote('Members of "%s"' % self.context.displayname)
=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py 2012-02-16 03:12:17 +0000
+++ lib/lp/registry/browser/tests/test_team.py 2012-02-17 17:18:19 +0000
@@ -535,7 +535,7 @@
}
with person_logged_in(team.teamowner):
with FeatureFixture(self.feature_flag):
- view = create_initialized_view(
+ create_initialized_view(
personset, name=self.view_name, principal=team.teamowner,
form=form)
team = personset.getByName(team_name)
@@ -571,7 +571,7 @@
self.assertEqual(
['PRIVATE'],
browser.getControl(name="field.visibility").value)
-
+
class TestTeamMenu(TestCaseWithFactory):
@@ -778,6 +778,17 @@
view.add_action.success(data={'newmember': member_team})
+class TeamMembershipViewTestCase(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_init(self):
+ team = self.factory.makeTeam(name='pting')
+ view = create_initialized_view(team, name='+members')
+ self.assertEqual('Members', view.page_title)
+ self.assertEqual(u'Members of \u201cPting\u201d', view.label)
+
+
class TestTeamIndexView(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/registry/stories/person/xx-admin-person-review.txt'
--- lib/lp/registry/stories/person/xx-admin-person-review.txt 2012-01-15 13:32:27 +0000
+++ lib/lp/registry/stories/person/xx-admin-person-review.txt 2012-02-17 17:18:19 +0000
@@ -25,12 +25,8 @@
The review page has a link to the review account page.
- >>> admin_browser.getLink('Administer').click()
- >>> link = admin_browser.getLink(url='+reviewaccount')
- >>> print link.text
- edit[IMG] Review the user's account information
-
- >>> link.click()
+ >>> admin_browser.open('http://launchpad.dev/~salgado')
+ >>> admin_browser.getLink('Administer Account').click()
>>> print admin_browser.title
Review person's account...
@@ -51,45 +47,6 @@
>>> print link.text
edit[IMG] Review the user's Launchpad information
-The admin can update the user's account. Suspending an account will give a
-feedback message.
-
- >>> admin_browser.getControl(
- ... 'The status of this account').value = ['SUSPENDED']
- >>> admin_browser.getControl(
- ... name='field.status_comment').value = 'Bad boy.'
- >>> admin_browser.getControl('Change').click()
-
- >>> print admin_browser.title
- The one and only Salgado does not use Launchpad
- >>> print get_feedback_messages(admin_browser.contents)[0]
- The account "Guilherme Salgado" has been suspended.
-
-The admin can see the account information of a user that does not use
-Launchpad, and can change the account too. Note that all pages that belong
-to a suspended user have a 410 status code to ensure search engines remove
-them from their index.
-
- >>> admin_browser.getLink('Administer').click()
- >>> admin_browser.getLink(url='+reviewaccount').click()
- >>> print admin_browser.title
- Review person's account...
-
- >>> control = admin_browser.getControl(name='field.status_comment')
- >>> print control.value
- Bad boy.
-
- >>> control.value = 'Reinstated after he apologised'
- >>> admin_browser.getControl(
- ... 'The status of this account').value = ['ACTIVE']
- >>> admin_browser.getControl('Change').click()
- >>> print admin_browser.title
- The one and only Salgado does not use Launchpad
-
- >>> for message in get_feedback_messages(admin_browser.contents):
- ... print message
- The user is reactivated. He must use the "forgot password" to log in.
-
Registry experts
----------------
=== modified file 'lib/lp/registry/templates/person-review.pt'
--- lib/lp/registry/templates/person-review.pt 2010-10-10 21:54:16 +0000
+++ lib/lp/registry/templates/person-review.pt 2012-02-17 17:18:19 +0000
@@ -25,12 +25,6 @@
may cause problems with relying parties. PPA and mailing lists
will be broken too.
</p>
- <p>
- <a tal:attributes="
- href string:${view/context/fmt:url}/+reviewaccount"><img
- tal:attributes="alt string:edit" src="/@@/edit" />
- Review the user's account information</a>.
- </p>
</tal:review-person>
<tal:review-account
=== modified file 'lib/lp/registry/templates/team-members.pt'
--- lib/lp/registry/templates/team-members.pt 2010-10-19 19:42:44 +0000
+++ lib/lp/registry/templates/team-members.pt 2012-02-17 17:18:19 +0000
@@ -13,8 +13,6 @@
tal:define="user_can_edit_memberships context/required:launchpad.Edit;
active_member_count context/active_member_count">
- <p>Active, pending and former members of this team.</p>
-
<ul>
<li tal:condition="active_member_count"
tal:define="membership_batch nocall:view/active_memberships/currentBatch">
