launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/add-member-648476 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #648476 Team owner should always see "(+) Add Member" link on team overview
  https://bugs.launchpad.net/bugs/648476


Summary
=======

The +members page doesn't show the +Add member button when there are no members of the team; since there can still be a teamowner and that teamowner should still have control (even if his membership is deactivated) the button should still be present, as it is in the overview page.

Proposed fix
============

Change the display conditions of the Add member link so it's always present for the team owner.

Preimplementation talk
======================

Spoke with Curtis Hovey.

Implementation details
======================

The conditions aren't actually the problem--the link is inside a block that displays when there are members. This branch simply adds it to the block that displays when there aren't members as well.

Tests
=====

bin/test -vvct team-views

Q&A
===

Create a team in launchpad.dev. Deactivate your membership (as the owner) and go to +members for the team. You should see the message saying there are no members, and below that an add member link.

Lint
====

make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/tests/team-views.txt
  lib/lp/registry/templates/team-members.pt

-- 
https://code.launchpad.net/~jcsackett/launchpad/add-member-648476/+merge/38887
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/add-member-648476 into lp:launchpad/devel.

=== modified file 'lib/lp/registry/browser/tests/team-views.txt'
--- lib/lp/registry/browser/tests/team-views.txt	2010-10-03 15:30:06 +0000
+++ lib/lp/registry/browser/tests/team-views.txt	2010-10-19 20:46:32 +0000
@@ -1,6 +1,8 @@
-= Team pages =
+Team pages
+==========
 
-== Home page ==
+Home page
+---------
 
     >>> from canonical.launchpad.interfaces import (
     ...     IPersonSet, ITeamMembershipSet)
@@ -65,7 +67,8 @@
     form fields.
 
 
-== +map-portlet ==
++map-portlet
+------------
 
 The team profile page contain the location portlet that shows a map.
 
@@ -90,7 +93,8 @@
     >>> team_view = create_initialized_view(
     ...     ubuntu_team, '+index', cookie=cookie)
 
-== +map ==
++map
+----
 
 A team's +map page will show a map with markers on the location of all
 members of that team.  That page also lists the local times of members.
@@ -192,7 +196,8 @@
     min_lng: -46...
 
 
-== Contacting the team ==
+Contacting the team
+-------------------
 
 Logged in users can contact any other Launchpad user through a 'Contact this
 user' feature (see person-pages.txt).  Similarly, a logged in user can contact
@@ -213,7 +218,8 @@
     Contact this team's owner
 
 
-== Mugshots ==
+Mugshots
+--------
 
 The mugshots for all members of a team can be seen on the +mugshots
 page.  The display of mugshots is batched.
@@ -240,7 +246,8 @@
     <Person at ... mark (Mark Shuttleworth)>
 
 
-== Privacy and visibility ==
+Privacy and visibility
+----------------------
 
 Team visibility is used on the page to indicate privacy.  GuadaMen is a public
 team.
@@ -266,7 +273,8 @@
     >>> print view.visibility_portlet_class
     portlet private
 
-== +add-my-teams ==
++add-my-teams
+-------------
 
 This page lists the teams that you administer and can add as a member
 to the current team. Private teams cannot be added as members to another

=== modified file 'lib/lp/registry/templates/team-members.pt'
--- lib/lp/registry/templates/team-members.pt	2010-06-24 20:07:30 +0000
+++ lib/lp/registry/templates/team-members.pt	2010-10-19 20:46:32 +0000
@@ -102,6 +102,11 @@
       </h2>
 
       <p>This team doesn't have any members yet.</p>
+
+      <div tal:define="link view/menu:overview/add_member"
+           tal:condition="link/enabled">
+        <a tal:replace="structure link/fmt:link" />
+      </div>
     </li>
     <br />