Merge lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part2 into lp:launchpad

[Edwin Grubbs]

[Edwin Grubbs (edwin-grubbs)]

Summary
-------

This branch is dependent on
lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1

I made the following changes:
 * Added a windmill test
 * Changed the return value from IPerson.addMember() so
   the UI knows what status it decided to use.
 * If the user is allowed to invite a member because they can admin
   the owning team and if the user is allowed to accept the invitation
   because they can admin the member team, addMember() now just fully
   approves the membership.

Over 50 tests broke because I changed the return value for
addMember() and setStatus(). I only fixed one of them as a sample.
The rest will have to wait for a followup branch, since I'm interested
in getting feedback on the changes I made already.

Implementation details
----------------------

lib/canonical/launchpad/icing/style-3-0.css
lib/canonical/launchpad/icing/style.css
    Moved the unseen style to style-3-0.css since style.css
    will be going away eventually.

lib/canonical/launchpad/javascript/code/codereview.js
    Added bug number to XXX.

lib/canonical/launchpad/javascript/registry/team.js
    Added handling of teams as new members.

lib/lp/registry/browser/person.py
    Added code for a list of recently invited members in
    the membership portlet.
    Using a css class to hide the lists instead of setting
    the element's style attribute.
    Modified the view for +add-my-teams so that it matches
    the behavior of the ajax "Add member" link.

lib/lp/registry/browser/tests/mailinglist-views.txt
    Fixed test for new return values.

lib/lp/registry/interfaces/person.py
    Documented new return value and organized docs by param.

lib/lp/registry/model/person.py
    Changed return value.

lib/lp/registry/templates/team-index.pt
    Fixed mixing space between left and right portlets.

lib/lp/registry/templates/team-members.pt
    Hide former members list from non-admins, since some users
    complain when it appears that they used to be a member of a team
    that they were added to on accident.

lib/lp/registry/templates/team-portlet-membership.pt
    Added recently invited member list.

lib/lp/registry/windmill/tests/test_team_index.py
    Added windmill test.

Tests
-----

./bin/test -vv -t mailinglist-views.txt
./bin/test -vv --layer=RegistryWindmillTest -t test_team_index

Demo and Q/A
------------

* Open http://launchpad.dev/~testing-spanish-team
  * Log in as <email address hidden>.
  * Click on the green "Add member" link in the portlet.
  * Search for "rosetta" and click on the rosetta admins team.
  * That team should be added to the "Recently approved" list
    since carlos admins both of them.
* Click on the green "Add member" link in the portlet.
  * Search for "simple" and click on the Simple Team.
  * That team should be added to the "Recently invited" list
    since carlos does not admin the Simple Team.
* Log out.
  * The "Add member" link should be gone.

[Guilherme Salgado (salgado)]

Hi Edwin,

This is a nice branch but I think we're missing some test coverage for
some changes you introduced.

 review needsfixing

On Fri, 2009-12-18 at 06:36 +0000, Edwin Grubbs wrote:
> Summary
> -------
>
> This branch is dependent on
> lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1
>
> I made the following changes:
> * Added a windmill test
> * Changed the return value from IPerson.addMember() so
> the UI knows what status it decided to use.
> * If the user is allowed to invite a member because they can admin
> the owning team and if the user is allowed to accept the invitation
> because they can admin the member team, addMember() now just fully
> approves the membership.
>
> Over 50 tests broke because I changed the return value for
> addMember() and setStatus(). I only fixed one of them as a sample.
> The rest will have to wait for a followup branch, since I'm interested
> in getting feedback on the changes I made already.

I think most of these tests are probably not interested in the return
value of addMember(), and in that case I suggest you change these ones
to assign the return value of addMember() to a dummy variable so that
they don't break again when we need to change the return value once
again.

> === modified file 'lib/canonical/launchpad/javascript/registry/team.js'
> --- lib/canonical/launchpad/javascript/registry/team.js 2009-12-18 06:36:27 +0000
> +++ lib/canonical/launchpad/javascript/registry/team.js 2009-12-18 06:36:29 +0000
> @@ -15,87 +15,122 @@
> * @method setup_add_member_handler
> */
> module.setup_add_member_handler = function() {
> + if (Y.UA.ie) {
> + return;
> + }
> +
> var config = {
> header: 'Add a member',
> step_title: 'Search',
> picker_activator: '.menu-link-add_member'
> };
>
> - var picker = Y.lp.picker.create(
> - 'ValidTeamMember',
> - function(result) { _add_member(result); },
> - config);
> + Y.lp.picker.create('ValidTeamMember', _add_member, config);
> };
>
> -var _add_member = function(result) {
> - var spinner = Y.one('#add-member-spinner');
> - var addmember_link = Y.one('.menu-link-add_member');
> - addmember_link.setStyle('display', 'none');
> - spinner.setStyle('display', 'inline');
> - function disable_spinner() {
> - addmember_link.setStyle('display', 'inline');
> - spinner.setStyle('display', 'none');
> - }
> +var _add_member = function(selected_person) {
> + var box = Y.one('#membership');
> + var spinner = box.query('#add-member-spinner');
> + var addmember_link = box.query('.menu-link-add_member');
> + addmember_link.addClass('unseen');
> + spinner.removeClass('unseen');
> + var disable_spinner = function() {
> + addmember_link.removeClass('unseen');
> + spinner.addClass('unseen');
> + };
> lp_client = new LP.client.Launchpad();
>
> var error_handler = new LP.client.ErrorHandler();
> error_handler.clearProgressUI = disable_spinner;
> error_handler.showError = function(error_msg) {
> - Y.lp.display_error(Y.one('.menu-link-add_member'), error_msg);
> + Y.lp.displ...

review: Needs Fixing

[Edwin Grubbs (edwin-grubbs)]

Here are screenshots of this feature that show how adding a person or a team as a member updates the appropriate list and updates the counts at the top of the portlet. If the user adds a team that he does not admin, then the team is invited. If the user does admin the team being added, it skips the invitation status and goes straight to approved.

https://chinstrap.canonical.com/~egrubbs/addmember/

[Martin Albisetti (beuno)]

Hi Edwin,

The branch in general looks great, you solved the bulk of the challenges already.
As per our conversation on IRC, the actual fix for the "user is already approved" message is not getting in the situation in the first place, so filing a bug about is a good start.

If we're going to keep the dialog around for a while, I would do one of two things:

1) Add buttons to dismiss the dialog or go back to the results (OK and BACK?)
2) Show the message instead of the results (ie, keep the search fields visible to re-search)

I would also re-phrase the message to something like "$user is already part of the team"

review: Needs Fixing (ui)

[Edwin Grubbs (edwin-grubbs)]

Hi Salgado,

Thanks for the review. Comments below.

> Hi Edwin,
>
> This is a nice branch but I think we're missing some test coverage for
> some changes you introduced.
>
> review needsfixing
>
> On Fri, 2009-12-18 at 06:36 +0000, Edwin Grubbs wrote:
> > Summary
> > -------
> >
> > This branch is dependent on
> > lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1
> >
> > I made the following changes:
> > * Added a windmill test
> > * Changed the return value from IPerson.addMember() so
> > the UI knows what status it decided to use.
> > * If the user is allowed to invite a member because they can admin
> > the owning team and if the user is allowed to accept the invitation
> > because they can admin the member team, addMember() now just fully
> > approves the membership.
> >
> > Over 50 tests broke because I changed the return value for
> > addMember() and setStatus(). I only fixed one of them as a sample.
> > The rest will have to wait for a followup branch, since I'm interested
> > in getting feedback on the changes I made already.
>
> I think most of these tests are probably not interested in the return
> value of addMember(), and in that case I suggest you change these ones
> to assign the return value of addMember() to a dummy variable so that
> they don't break again when we need to change the return value once
> again.

I added the dummy variable in the mailinglist-views.txt test, and I will
do that for the others.

> > === modified file 'lib/canonical/launchpad/javascript/registry/team.js'
> > --- lib/canonical/launchpad/javascript/registry/team.js 2009-12-18 06:36:27
> +0000
> > +++ lib/canonical/launchpad/javascript/registry/team.js 2009-12-18 06:36:29
> +0000
> > @@ -15,87 +15,122 @@
> > * @method setup_add_member_handler
> > */
> > module.setup_add_member_handler = function() {
> > + if (Y.UA.ie) {
> > + return;
> > + }
> > +
> > var config = {
> > header: 'Add a member',
> > step_title: 'Search',
> > picker_activator: '.menu-link-add_member'
> > };
> >
> > - var picker = Y.lp.picker.create(
> > - 'ValidTeamMember',
> > - function(result) { _add_member(result); },
> > - config);
> > + Y.lp.picker.create('ValidTeamMember', _add_member, config);
> > };
> >
> > -var _add_member = function(result) {
> > - var spinner = Y.one('#add-member-spinner');
> > - var addmember_link = Y.one('.menu-link-add_member');
> > - addmember_link.setStyle('display', 'none');
> > - spinner.setStyle('display', 'inline');
> > - function disable_spinner() {
> > - addmember_link.setStyle('display', 'inline');
> > - spinner.setStyle('display', 'none');
> > - }
> > +var _add_member = function(selected_person) {
> > + var box = Y.one('#membership');
> > + var spinner = box.query('#add-member-spinner');
> > + var addmember_link = box.query('.menu-link-add_member');
> > + addmember_link.addClass('unseen');
> > + spinner.removeClass('unseen');
> > + var disable_spinner = function() {
> > + addmember_link.removeClass('unseen');
> > + spinner.addClass('unseen');
> > + };
> > ...

[Edwin Grubbs (edwin-grubbs)]

Here is the incremental diff that addresses both Salgado's and Martin's comments.

{{{
=== modified file 'lib/canonical/launchpad/javascript/lp/errors.js'
--- lib/canonical/launchpad/javascript/lp/errors.js 2009-11-23 19:29:02 +0000
+++ lib/canonical/launchpad/javascript/lp/errors.js 2009-12-21 20:33:38 +0000
@@ -69,7 +69,7 @@
  * @param flash_node {Node} The node to red flash.
  * @param msg {String} The message to display.
 */
-var display_error = function(flash_node, msg) {
+Y.lp.display_error = function(flash_node, msg) {
     create_error_overlay();
     maybe_red_flash(flash_node, function(){
         error_overlay.showError(msg);
@@ -77,5 +77,36 @@
     });
 };

-Y.lp.display_error = display_error;
-}, "0.1", {"requires":["lazr.formoverlay"]});
+
+var info_overlay;
+/*
+ * Display the form overlay for non-error informational messages.
+ *
+ * @method display_info
+ * @param msg {String} The message to display.
+*/
+Y.lp.display_info = function(msg) {
+ if (info_overlay === undefined) {
+ info_overlay = new Y.lazr.PrettyOverlay({
+ //headerContent: '<h2><img src="/@@/info-large"/>Notice</h2>',
+ centered: true,
+ visible: false
+ });
+ info_overlay.render();
+ }
+ var content = Y.Node.create(
+ '<div style="background: url(/@@/info-large) no-repeat; ' +
+ 'min-height: 32px; padding-left: 40px; padding-top: 16px"/></div>');
+ content.appendChild(Y.Node.create(msg));
+ var button_div = Y.Node.create('<div style="text-align: right"></div>');
+ var ok_button = Y.Node.create('<button>OK</button>');
+ ok_button.on('click', function(e) {
+ info_overlay.fire('cancel');
+ });
+ button_div.appendChild(ok_button);
+ content.appendChild(button_div);
+ info_overlay.set('bodyContent', content);
+ info_overlay.show();
+};
+
+}, "0.1", {"requires":["lazr.formoverlay", "lazr.overlay"]});

=== modified file 'lib/canonical/launchpad/javascript/registry/team.js'
--- lib/canonical/launchpad/javascript/registry/team.js 2009-12-18 06:06:30 +0000
+++ lib/canonical/launchpad/javascript/registry/team.js 2009-12-21 20:35:57 +0000
@@ -54,13 +54,14 @@
                 var members_section, members_ul, count_elem;
                 if (did_status_change === false) {
                     disable_spinner();
- Y.lp.display_error(
- addmember_link,
+ Y.lp.display_info(
                         selected_person.title + ' is already ' +
- current_status + '.');
+ current_status.toLowerCase() +
+ ' as a member of the team.');
                     return;
                 }
                 var is_team = false;
+ current_status = 'foo';
                 if (current_status == 'Invited') {
                     is_team = true;
                     members_section = box.query('#recently-invited');
@@ -79,6 +80,7 @@
                     Y.lp.display_error(
                         addmember_link,
                         'Unexpected status: ' + current_status);
+ return;
                 }
                 var firs...

[Edwin Grubbs (edwin-grubbs)]

Hi Martin,

Thanks for the review. For some reason, your comment didn't get added to the merge proposal, so I pasted it here again.

On Mon, Dec 21, 2009 at 1:35 PM, Martin Albisetti <email address hidden> wrote:
> Review: Needs Fixing ui
> Hi Edwin,
>
> The branch in general looks great, you solved the bulk of the challenges already.
> As per our conversation on IRC, the actual fix for the "user is already approved" message is not getting in the situation in the first place, so filing a bug about is a good start.

Opened bug 499210 for this.

> If we're going to keep the dialog around for a while, I would do one of two things:
>
> 1) Add buttons to dismiss the dialog or go back to the results (OK and BACK?)
> 2) Show the message instead of the results (ie, keep the search fields visible to re-search)

I added an "OK" button to dismiss the dialog.

> I would also re-phrase the message to something like "$user is already part of the team"

There actually are two potential errors, since a team could already be in an invited status, so it would be unclear to say that they are already part of the team. Here are screenshots of both error messages.

https://chinstrap.canonical.com/~egrubbs/addmember/new_approved_error_message.png
https://chinstrap.canonical.com/~egrubbs/addmember/new_invited_error_message.png

[Curtis Hovey (sinzui)]

Hi Edwin.

This is a nice addition. I have no remarks about new test. I have a suggestion about the team story

> === modified file 'lib/lp/registry/stories/team/xx-team-home.txt'
> --- lib/lp/registry/stories/team/xx-team-home.txt 2009-11-22 15:43:16 +0000
> +++ lib/lp/registry/stories/team/xx-team-home.txt 2009-12-21 19:53:58 +0000

...

> @@ -88,14 +93,14 @@
> Show received invitations
>
> If the team does not have any recently approved or proposed members,
> -the recent members sections are not displayed:
> +the recent members sections are hidden using the "unseen" css class:
>
> >>> browser.open('http://launchpad.dev/~launchpad')
> >>> print find_tag_by_id(browser.contents, 'recently-approved')
> - None
> + <td id="recently-approved" ... class="unseen">...
>
> - >>> print find_tag_by_id(browser.contents, 'recently-applied')
> - None
> + >>> print find_tag_by_id(browser.contents, 'recently-proposed')
> + <td id="recently-proposed" class="unseen">...

These tests will break if we reimplement this as a list. We should not print
markup in user stories because they do no read the source. The test is parsing
the tree many times. I think this can be expressed like this so that it runs
faster and is robust.

    >>> browser.open('http://launchpad.dev/~launchpad')
    >>> content = find_main_content(browser.contents)
    >>> tag = find_tag_by_id(content, 'recently-approved')
    >>> print tag['class']
    unseen

    >>> tag = find_tag_by_id(content, 'recently-proposed')
    >>> print tag['class']
    unseen

.

review: Approve (code and js)

[Martin Albisetti (beuno)]

Thanks Edwin. The dialog now looks better, minus an alignment issue between the text and the icon. If you could top-align the text with the icon, it would be super great.

The only remaining issue for me is the fact that the persons' name doesn't appear on the page grayed out while it's being saved, like everywhere else.

Could you factor in that behavior?

[Edwin Grubbs (edwin-grubbs)]

> Thanks Edwin. The dialog now looks better, minus an alignment issue between
> the text and the icon. If you could top-align the text with the icon, it would
> be super great.
>
> The only remaining issue for me is the fact that the persons' name doesn't
> appear on the page grayed out while it's being saved, like everywhere else.
>
> Could you factor in that behavior?

Martin, I've run into a snag adding the grayed out team/person with a spinner. A team may be added to the "Latest members" or the "Latest invited" list depending on whether the user is also an admin on the team being added as a member. Also, the "Latest members" and "Latest invited" labels may need to be unhidden and presumably re-hidden if they are empty at the end of the process. It seems like a bad idea to make another REST request to gather the information just to put the spinner in the right spot.

[Guilherme Salgado (salgado)]

On Mon, 2009-12-21 at 20:54 +0000, Edwin Grubbs wrote:
> Here is the incremental diff that addresses both Salgado's and
> Martin's comments.

Looks good to me, but there's some code that was apparently written to
test corner cases and left behind accidentally. See below

>
> {{{
> === modified file 'lib/canonical/launchpad/javascript/lp/errors.js'
> --- lib/canonical/launchpad/javascript/lp/errors.js 2009-11-23 19:29:02 +0000
> +++ lib/canonical/launchpad/javascript/lp/errors.js 2009-12-21 20:33:38 +0000
> @@ -69,7 +69,7 @@
> * @param flash_node {Node} The node to red flash.
> * @param msg {String} The message to display.
> */
> -var display_error = function(flash_node, msg) {
> +Y.lp.display_error = function(flash_node, msg) {
> create_error_overlay();
> maybe_red_flash(flash_node, function(){
> error_overlay.showError(msg);
> @@ -77,5 +77,36 @@
> });
> };
>
> -Y.lp.display_error = display_error;
> -}, "0.1", {"requires":["lazr.formoverlay"]});
> +
> +var info_overlay;
> +/*
> + * Display the form overlay for non-error informational messages.
> + *
> + * @method display_info
> + * @param msg {String} The message to display.
> +*/
> +Y.lp.display_info = function(msg) {
> + if (info_overlay === undefined) {
> + info_overlay = new Y.lazr.PrettyOverlay({
> + //headerContent: '<h2><img src="/@@/info-large"/>Notice</h2>',

Why is this commented out?

> + centered: true,
> + visible: false
> + });
> + info_overlay.render();
> + }
> + var content = Y.Node.create(
> + '<div style="background: url(/@@/info-large) no-repeat; ' +
> + 'min-height: 32px; padding-left: 40px; padding-top: 16px"/></div>');
> + content.appendChild(Y.Node.create(msg));
> + var button_div = Y.Node.create('<div style="text-align: right"></div>');
> + var ok_button = Y.Node.create('<button>OK</button>');
> + ok_button.on('click', function(e) {
> + info_overlay.fire('cancel');
> + });
> + button_div.appendChild(ok_button);
> + content.appendChild(button_div);
> + info_overlay.set('bodyContent', content);
> + info_overlay.show();
> +};
> +
> +}, "0.1", {"requires":["lazr.formoverlay", "lazr.overlay"]});
>
> === modified file 'lib/canonical/launchpad/javascript/registry/team.js'
> --- lib/canonical/launchpad/javascript/registry/team.js 2009-12-18 06:06:30 +0000
> +++ lib/canonical/launchpad/javascript/registry/team.js 2009-12-21 20:35:57 +0000
> @@ -54,13 +54,14 @@
> var members_section, members_ul, count_elem;
> if (did_status_change === false) {
> disable_spinner();
> - Y.lp.display_error(
> - addmember_link,
> + Y.lp.display_info(
> selected_person.title + ' is already ' +
> - current_status + '.');
> + current_status.toLowerCase() +
> + ' as a member of the team.');
> return;
> }
> var is_team = false;
> + current_status = 'foo';

!?

I really hope your windmill te...

[Martin Albisetti (beuno)]

This is a pattern we will need to solve many times, so lets land this now, and start a discussion on how to fix it in a more general manner.

Thanks Edwin, this seems good to land for me.

review: Approve (ui)

[Edwin Grubbs (edwin-grubbs)]

I removed the commented out headerContent, which was unnecessary. I also removed the debugging code that set current_status to 'foo'.

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2009-12-21 15:44:34 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2010-01-04 18:54:21 +0000
@@ -215,6 +215,9 @@
     font-weight: bold;
     }
 
+.unseen {display: none;}
+
+
 /* Common presentations */
 div.see-all {
     font-size: 123.1%; /* Same as a h2 */

=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css	2009-12-19 19:00:37 +0000
+++ lib/canonical/launchpad/icing/style.css	2010-01-04 18:54:21 +0000
@@ -428,8 +428,6 @@
 fieldset .expanded {display: block;}
 fieldset.collapsible legend {font-weight: normal;}
 
-.unseen {display: none;}
-
 form.primary.search {
   margin-bottom: 2em;
 }

=== modified file 'lib/canonical/launchpad/javascript/code/codereview.js'
--- lib/canonical/launchpad/javascript/code/codereview.js	2010-01-04 18:54:20 +0000
+++ lib/canonical/launchpad/javascript/code/codereview.js	2010-01-04 18:54:21 +0000
@@ -22,12 +22,13 @@
     var link = Y.one('#request-review');
     if (link !== null) {
         link.addClass('js-action');
-        /* XXX: salgado, 2009-11-11: This will cause the picker to be
-         * recreated every time the user clicks on the link.  Although that
-         * makes it unnecessary to have the widget cleared, it makes it
-         * impossible to persist the state of the picker between clicks on the
-         * link.  We should probably have a policy to enforce that we
-         * just hide/show widgets when a link is clicked more than once,
+        /* XXX: salgado 2009-11-11 bug=497603
+         * This will cause the picker to be recreated every time the
+         * user clicks on the link.  Although that makes it unnecessary
+         * to have the widget cleared, it makes it impossible to persist
+         * the state of the picker between clicks on the link.  We
+         * should probably have a policy to enforce that we just
+         * hide/show widgets when a link is clicked more than once,
          * instead of recreating the widgets every time.
          */
         link.on('click', show_request_review_form);

=== modified file 'lib/canonical/launchpad/javascript/lp/errors.js'
--- lib/canonical/launchpad/javascript/lp/errors.js	2009-11-23 19:29:02 +0000
+++ lib/canonical/launchpad/javascript/lp/errors.js	2010-01-04 18:54:21 +0000
@@ -69,7 +69,7 @@
  * @param flash_node {Node} The node to red flash.
  * @param msg {String} The message to display.
 */
-var display_error = function(flash_node, msg) {
+Y.lp.display_error = function(flash_node, msg) {
     create_error_overlay();
     maybe_red_flash(flash_node, function(){
         error_overlay.showError(msg);
@@ -77,5 +77,35 @@
     });
 };
 
-Y.lp.display_error = display_error;
-}, "0.1", {"requires":["lazr.formoverlay"]});
+
+var info_overlay;
+/*
+ * Display the form overlay for non-error informational messages.
+ *
+ * @method display_info
+ * @param msg {String} The message to display.
+*/
+Y.lp.display_info = function(msg) {
+    if (info_overlay === undefined) {
+        info_overlay = new Y.lazr.PrettyOverlay({
+            centered: true,
+            visible: false
+        });
+        info_overlay.render();
+    }
+    var content = Y.Node.create(
+      '<div style="background: url(/@@/info-large) no-repeat; ' +
+      'min-height: 32px; padding-left: 40px; padding-top: 16px"/></div>');
+    content.appendChild(Y.Node.create(msg));
+    var button_div = Y.Node.create('<div style="text-align: right"></div>');
+    var ok_button = Y.Node.create('<button>OK</button>');
+    ok_button.on('click', function(e) {
+        info_overlay.fire('cancel');
+    });
+    button_div.appendChild(ok_button);
+    content.appendChild(button_div);
+    info_overlay.set('bodyContent', content);
+    info_overlay.show();
+};
+
+}, "0.1", {"requires":["lazr.formoverlay", "lazr.overlay"]});

=== modified file 'lib/canonical/launchpad/javascript/registry/team.js'
--- lib/canonical/launchpad/javascript/registry/team.js	2010-01-04 18:54:20 +0000
+++ lib/canonical/launchpad/javascript/registry/team.js	2010-01-04 18:54:21 +0000
@@ -15,87 +15,121 @@
  * @method setup_add_member_handler
  */
 module.setup_add_member_handler = function() {
+    if (Y.UA.ie) {
+        return;
+    }
+
     var config = {
         header: 'Add a member',
         step_title: 'Search',
         picker_activator: '.menu-link-add_member'
     };
 
-    var picker = Y.lp.picker.create(
-        'ValidTeamMember',
-        function(result) { _add_member(result); },
-        config);
+    Y.lp.picker.create('ValidTeamMember', _add_member, config);
 };
 
-var _add_member = function(result) {
-    var spinner = Y.one('#add-member-spinner');
-    var addmember_link = Y.one('.menu-link-add_member');
-    addmember_link.setStyle('display', 'none');
-    spinner.setStyle('display', 'inline');
-    function disable_spinner() {
-        addmember_link.setStyle('display', 'inline');
-        spinner.setStyle('display', 'none');
-    }
+var _add_member = function(selected_person) {
+    var box = Y.one('#membership');
+    var spinner = box.query('#add-member-spinner');
+    var addmember_link = box.query('.menu-link-add_member');
+    addmember_link.addClass('unseen');
+    spinner.removeClass('unseen');
+    var disable_spinner = function() {
+        addmember_link.removeClass('unseen');
+        spinner.addClass('unseen');
+    };
     lp_client = new LP.client.Launchpad();
 
     var error_handler = new LP.client.ErrorHandler();
     error_handler.clearProgressUI = disable_spinner;
     error_handler.showError = function(error_msg) {
-        Y.lp.display_error(Y.one('.menu-link-add_member'), error_msg);
+        Y.lp.display_error(addmember_link, error_msg);
     };
 
-    config = {
+    addmember_config = {
         on: {
-            success: function(member_added) {
-                if (!member_added) {
-                    disable_spinner();
-                    alert('Already a member.');
-                    return;
-                }
-                if (result.css.match("team")) {
-                    disable_spinner();
-                    alert('This is a team');
-                    return;
-                }
-                var members_section = Y.one('#recently-approved');
-                var members_ul = Y.one('#recently-approved-ul');
+            success: function(change_and_status) {
+                var did_status_change = change_and_status[0];
+                var current_status = change_and_status[1];
+                var members_section, members_ul, count_elem;
+                if (did_status_change === false) {
+                    disable_spinner();
+                    Y.lp.display_info(
+                        selected_person.title + ' is already ' +
+                        current_status.toLowerCase() +
+                        ' as a member of the team.');
+                    return;
+                }
+
+                if (current_status == 'Invited') {
+                    members_section = box.query('#recently-invited');
+                    members_ul = box.query('#recently-invited-ul');
+                    count_elem = box.query('#invited-member-count');
+                } else if (current_status == 'Proposed') {
+                    members_section = box.query('#recently-proposed');
+                    members_ul = box.query('#recently-proposed-ul');
+                    count_elem = box.query('#proposed-member-count');
+                } else if (current_status == 'Approved') {
+                    members_section = box.query('#recently-approved');
+                    members_ul = box.query('#recently-approved-ul');
+                    count_elem = box.query('#approved-member-count');
+                } else {
+                    Y.lp.display_error(
+                        addmember_link,
+                        'Unexpected status: ' + current_status);
+                    return;
+                }
                 var first_node = members_ul.get('firstChild');
-                config = {
+
+                var xhtml_person_handler = function(person_html) {
+                    if (count_elem === null && current_status == 'Invited') {
+                        count_elem = Y.Node.create(
+                            '<strong id="invited-member-count">' +
+                            '1</strong>');
+                        var count_box = Y.one(
+                            '#membership #membership-counts');
+                        count_box.append(Y.Node.create(
+                            '<span>, </span>'));
+                        count_box.append(count_elem);
+                        count_box.append(Y.Node.create(
+                            '<span> <a href="/+members#invited">' +
+                            'invited members</a></span>'));
+                    } else {
+                        var count = count_elem.get('innerHTML');
+                        count = parseInt(count, 10) + 1;
+                        count_elem.set('innerHTML', count);
+                    }
+                    person_repr = Y.Node.create(
+                        '<li>' + person_html + '</li>');
+                    members_section.removeClass('unseen');
+                    members_ul.insertBefore(person_repr, first_node);
+                    anim = Y.lazr.anim.green_flash({node: person_repr});
+                    anim.run();
+                    disable_spinner();
+                };
+
+                xhtml_person_config = {
                     on: {
-                        success: function(person_html) {
-                            var total_members = Y.one(
-                                '#member-count').get('innerHTML');
-                            total_members = parseInt(total_members, 10) + 1;
-                            Y.one('#member-count').set(
-                                'innerHTML', total_members);
-                            person_repr = Y.Node.create(
-                                '<li>' + person_html + '</li>');
-                            members_section.setStyle('visibility', 'visible');
-                            members_ul.insertBefore(
-                                person_repr, first_node);
-                            anim = Y.lazr.anim.green_flash(
-                                {node: person_repr});
-                            anim.run();
-                            disable_spinner();
-                        },
+                        success: xhtml_person_handler,
                         failure: error_handler.getFailureHandler()
                     },
                     accept: LP.client.XHTML
                 };
-                lp_client.get(result.api_uri, config);
+                lp_client.get(selected_person.api_uri, xhtml_person_config);
             },
             failure: error_handler.getFailureHandler()
         },
         parameters: {
-            // XXX: Why do I always have to get absolute URIs out of the URIs
+            // XXX: EdwinGrubbs 2009-12-16 bug=497602
+            // Why do I always have to get absolute URIs out of the URIs
             // in the picker's result/client.links?
             reviewer: LP.client.get_absolute_uri(LP.client.links.me),
-            person: LP.client.get_absolute_uri(result.api_uri)
+            person: LP.client.get_absolute_uri(selected_person.api_uri)
         }
     };
 
     lp_client.named_post(
-        LP.client.cache.context.self_link, 'addMember', config);
+        LP.client.cache.context.self_link, 'addMember', addmember_config);
 };
 
 }, '0.1', {requires: [

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2010-01-04 18:54:20 +0000
+++ lib/lp/registry/browser/person.py	2010-01-04 18:54:21 +0000
@@ -95,6 +95,7 @@
 from itertools import chain
 from operator import attrgetter, itemgetter
 from textwrap import dedent
+from storm.zope.interfaces import IResultSet
 
 from zope.error.interfaces import IErrorReportingUtility
 from zope.app.form.browser import TextAreaWidget, TextWidget
@@ -227,13 +228,8 @@
     logoutPerson, allowUnauthenticatedSession)
 from canonical.launchpad.webapp.menu import get_current_view
 from canonical.launchpad.webapp.publisher import LaunchpadView
-<<<<<<< TREE
-from canonical.launchpad.webapp.tales import (
-    DateTimeFormatterAPI, FormattersAPI)
-=======
-from canonical.launchpad.webapp.tales import (
-    DateTimeFormatterAPI, PersonFormatterAPI)
->>>>>>> MERGE-SOURCE
+from canonical.launchpad.webapp.tales import (
+    DateTimeFormatterAPI, FormattersAPI, PersonFormatterAPI)
 from lazr.uri import URI, InvalidURIError
 
 from canonical.launchpad import _
@@ -2489,11 +2485,7 @@
 
     @property
     def next_url(self):
-<<<<<<< TREE
         """Redirect to the +languages page if request originated there."""
-=======
-        """Redirect back to the originating page."""
->>>>>>> MERGE-SOURCE
         redirection_url = self.request.get('redirection_url')
         if redirection_url:
             return redirection_url
@@ -2501,11 +2493,7 @@
 
     @property
     def cancel_url(self):
-<<<<<<< TREE
         """Redirect to the +languages page if request originated there."""
-=======
-        """Redirect back to the originating page."""
->>>>>>> MERGE-SOURCE
         redirection_url = self.getRedirectionURL()
         if redirection_url:
             return redirection_url
@@ -2745,6 +2733,13 @@
             orderBy='-TeamMembership.date_proposed')
         return members[:5]
 
+    @cachedproperty
+    def recently_invited_members(self):
+        members = self.context.getMembersByStatus(
+            TeamMembershipStatus.INVITED,
+            orderBy='-TeamMembership.date_proposed')
+        return members[:5]
+
     @property
     def recently_approved_hidden(self):
         """Optionally hide the div.
@@ -2752,8 +2747,8 @@
         The AJAX on the page needs the elements to be present
         but hidden in case it adds a member to the list.
         """
-        if self.recently_approved_members.count() == 0:
-            return 'visibility: collapse'
+        if IResultSet(self.recently_approved_members).is_empty():
+            return 'unseen'
         else:
             return ''
 
@@ -2764,8 +2759,20 @@
         The AJAX on the page needs the elements to be present
         but hidden in case it adds a member to the list.
         """
-        if self.recently_proposed_members.count() == 0:
-            return 'visibility: collapse'
+        if IResultSet(self.recently_proposed_members).is_empty():
+            return 'unseen'
+        else:
+            return ''
+
+    @property
+    def recently_invited_hidden(self):
+        """Optionally hide the div.
+
+        The AJAX on the page needs the elements to be present
+        but hidden in case it adds a member to the list.
+        """
+        if IResultSet(self.recently_invited_members).is_empty():
+            return 'unseen'
         else:
             return ''
 
@@ -4241,9 +4248,14 @@
     def continue_action(self, action, data):
         """Make the selected teams join this team."""
         context = self.context
+        is_admin = check_permission('launchpad.Admin', context)
         for team in data['teams']:
-            team.join(context, requester=self.user)
-        if context.subscriptionpolicy == TeamSubscriptionPolicy.MODERATED:
+            if is_admin:
+                context.addMember(team, reviewer=self.user)
+            else:
+                team.join(context, requester=self.user)
+        if (context.subscriptionpolicy == TeamSubscriptionPolicy.MODERATED
+            and not is_admin):
             msg = 'proposed to this team.'
         else:
             msg = 'added to this team.'

=== modified file 'lib/lp/registry/browser/tests/mailinglist-views.txt'
--- lib/lp/registry/browser/tests/mailinglist-views.txt	2009-12-03 19:12:02 +0000
+++ lib/lp/registry/browser/tests/mailinglist-views.txt	2010-01-04 18:54:21 +0000
@@ -33,7 +33,7 @@
 No Privileges Person is added as a member of the Aardvarks.
 
     >>> no_priv = person_set.getByEmail('[e-mail]')
-    >>> aardvarks.addMember(no_priv, sample_person)
+    >>> ignored = aardvarks.addMember(no_priv, sample_person)
 
 But regular members can't access the +mailinglist page.
 
@@ -52,8 +52,9 @@
     >>> team_membership = getUtility(ITeamMembershipSet).getByPersonAndTeam(
     ...     no_priv, aardvarks)
     >>> team_membership.status
-    <DBItem TeamMembershipStatus.APPROVED, (2) Approved>
-    >>> team_membership.setStatus(TeamMembershipStatus.ADMIN, sample_person)
+    <DBItem TeamMembershipStatus.APPROVED...
+    >>> ignored = team_membership.setStatus(
+    ...     TeamMembershipStatus.ADMIN, sample_person)
 
 Now No Privileges Person has permission to request mailing lists.
 
@@ -208,7 +209,7 @@
 
 A non-owner member can see information about the mailing list.
 
-    >>> pmt.addMember(sample_person, owner)
+    >>> ignored = pmt.addMember(sample_person, owner)
     >>> login('[e-mail]')
     >>> view = create_initialized_view(pmt, '+index')
     >>> print view.archive_url

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2009-12-09 23:11:31 +0000
+++ lib/lp/registry/interfaces/person.py	2010-01-04 18:54:21 +0000
@@ -1406,23 +1406,33 @@
                   may_subscribe_to_list=True):
         """Add the given person as a member of this team.
 
-        If the given person is already a member of this team we'll simply
-        change its membership status. Otherwise a new TeamMembership is
-        created with the given status.
-
-        If the person is actually a team and force_team_add is False, the
-        team will actually be invited to join this one. Otherwise the team
-        is added as if it were a person.
-
-        If the person is not a team, and may_subscribe_to_list
-        is True, then the person may be subscribed to the team's
-        mailing list, depending on the list status and the person's
-        auto-subscribe settings.
-
-        The given status must be either Approved, Proposed or Admin.
-
-        The reviewer is the user who made the given person a member of this
-        team.
+        :param person: If the given person is already a member of this
+            team we'll simply change its membership status. Otherwise a new
+            TeamMembership is created with the given status.
+
+        :param reviewer: The user who made the given person a member of this
+            team.
+
+        :param comment: String that will be assigned to the
+            proponent_comment, reviwer_comment, or acknowledger comment.
+
+        :param status: `TeamMembershipStatus` value must be either
+            Approved, Proposed or Admin.
+
+        :param force_team_add: If the person is actually a team and
+            force_team_add is False, the team will actually be invited to
+            join this one. Otherwise the team is added as if it were a
+            person.
+
+        :param may_subscribe_to_list: If the person is not a team, and
+            may_subscribe_to_list is True, then the person may be subscribed
+            to the team's mailing list, depending on the list status and the
+            person's auto-subscribe settings.
+
+        :return: A tuple containing a boolean indicating when the
+            membership status changed and the current `TeamMembershipStatus`.
+            This depends on the desired status passed as an argument, the
+            subscription policy and the user's priveleges.
         """
 
     def deactivateAllMembers(comment, reviewer):

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-01-04 18:54:20 +0000
+++ lib/lp/registry/model/person.py	2010-01-04 18:54:21 +0000
@@ -1231,12 +1231,15 @@
             # By default, teams can only be invited as members, meaning that
             # one of the team's admins will have to accept the invitation
             # before the team is made a member. If force_team_add is True,
-            # though, then we'll add a team as if it was a person.
-            if not force_team_add:
+            # or the user is also an admin of the proposed member, then
+            # we'll add a team as if it was a person.
+            is_reviewer_admin_of_new_member = (
+                person in reviewer.getAdministratedTeams())
+            if not force_team_add and not is_reviewer_admin_of_new_member:
                 status = TeamMembershipStatus.INVITED
                 event = TeamInvitationEvent
 
-        member_added = True
+        status_changed = True
         expires = self.defaultexpirationdate
         tm = TeamMembership.selectOneBy(person=person, team=self)
         if tm is None:
@@ -1251,12 +1254,12 @@
             # We can't use tm.setExpirationDate() here because the reviewer
             # here will be the member themselves when they join an OPEN team.
             tm.dateexpires = expires
-            member_added = tm.setStatus(status, reviewer, comment)
+            status_changed = tm.setStatus(status, reviewer, comment)
 
         if not person.is_team and may_subscribe_to_list:
             person.autoSubscribeToMailingList(self.mailing_list,
                                               requester=reviewer)
-        return member_added
+        return (status_changed, tm.status)
 
     # The three methods below are not in the IPerson interface because we want
     # to protect them with a launchpad.Edit permission. We could do that by

=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py	2010-01-04 18:54:20 +0000
+++ lib/lp/registry/model/teammembership.py	2010-01-04 18:54:21 +0000
@@ -318,7 +318,6 @@
                 "%(team)s is a member of %(person)s."
                 % dict(person=self.person.name, team=self.team.name))
 
-
         old_status = self.status
         self.status = status
 
@@ -367,17 +366,10 @@
         # When a member proposes himself, a more detailed notification is
         # sent to the team admins by a subscriber of JoinTeamEvent; that's
         # why we don't send anything here.
-<<<<<<< TREE
-        if self.person == self.last_changed_by and self.status == proposed:
-            return
-
-        if not silent:
-            self._sendStatusChangeNotification(old_status)
-=======
-        if self.person != self.last_changed_by or self.status != proposed:
+        if ((self.person != self.last_changed_by or self.status != proposed)
+            and not silent):
             self._sendStatusChangeNotification(old_status)
         return True
->>>>>>> MERGE-SOURCE
 
     def _sendStatusChangeNotification(self, old_status):
         """Send a status change notification to all team admins and the

=== modified file 'lib/lp/registry/stories/team/xx-team-home.txt'
--- lib/lp/registry/stories/team/xx-team-home.txt	2009-11-22 15:43:16 +0000
+++ lib/lp/registry/stories/team/xx-team-home.txt	2010-01-04 18:54:21 +0000
@@ -13,19 +13,24 @@
 
     >>> print extract_text(
     ...     find_tag_by_id(browser.contents, 'recently-approved'))
-    Recently approved
+    Latest members
     Warty Gnome Team
     Daniel Silverstone
     Celso Providelo
     Steve Alexander...
 
     >>> print extract_text(
-    ...     find_tag_by_id(browser.contents, 'recently-applied'))
+    ...     find_tag_by_id(browser.contents, 'recently-proposed'))
     Pending approval
     Sample Person
     Andrew Bennetts
 
     >>> print extract_text(
+    ...     find_tag_by_id(browser.contents, 'recently-invited'))
+    Latest invited
+    Warty Security Team
+
+    >>> print extract_text(
     ...     find_tag_by_id(browser.contents, 'team-owner'))
     Owner:
     Mark Shuttleworth
@@ -87,15 +92,21 @@
     itself is not a member of any other team.
     Show received invitations
 
-If the team does not have any recently approved or proposed members,
-the recent members sections are not displayed:
+If the team does not have any recently approved, proposed, or invited
+members, the empty lists are hidden using the "unseen" css class:
 
     >>> browser.open('http://launchpad.dev/~launchpad')
-    >>> print find_tag_by_id(browser.contents, 'recently-approved')
-    None
-
-    >>> print find_tag_by_id(browser.contents, 'recently-applied')
-    None
+    >>> tag = find_tag_by_id(browser.contents, 'recently-approved')
+    >>> print tag['class']
+    unseen
+
+    >>> tag = find_tag_by_id(browser.contents, 'recently-proposed')
+    >>> print tag['class']
+    unseen
+
+    >>> tag = find_tag_by_id(browser.contents, 'recently-invited')
+    >>> print tag['class']
+    unseen
 
 In the above case there's no user logged in, so it doesn't actually show
 what's the user's involvement with the team. If the user logs in, he'll see
@@ -187,7 +198,7 @@
     >>> owner_browser = setupBrowser(auth="Basic [e-mail]:test")
     >>> owner_browser.open('http://launchpad.dev/~ubuntu-team')
     >>> print extract_text(
-    ...     find_tag_by_id(owner_browser.contents, 'recently-applied'))
+    ...     find_tag_by_id(owner_browser.contents, 'recently-proposed'))
     Pending approval
     Sample Person
     Andrew Bennetts

=== modified file 'lib/lp/registry/stories/team/xx-team-membership.txt'
--- lib/lp/registry/stories/team/xx-team-membership.txt	2009-11-22 15:43:16 +0000
+++ lib/lp/registry/stories/team/xx-team-membership.txt	2010-01-04 18:54:21 +0000
@@ -174,6 +174,19 @@
     >>> message in second_browser.contents
     True
 
+An admin can see the former members of the team.
+
+    >>> browser.open('http://launchpad.dev/~name18/+members')
+    >>> print extract_text(
+    ...     find_tag_by_id(browser.contents, 'inactivemembers'))
+    Name    Joined in   Status...
+
+Other users cannot see the former members of the team.
+
+    >>> user_browser.open('http://launchpad.dev/~name18/+members')
+    >>> print find_tag_by_id(user_browser.contents, 'inactivemembers')
+    None
+
 
 Team Participation page
 =======================

=== modified file 'lib/lp/registry/templates/team-index.pt'
--- lib/lp/registry/templates/team-index.pt	2010-01-04 18:54:20 +0000
+++ lib/lp/registry/templates/team-index.pt	2010-01-04 18:54:21 +0000
@@ -86,7 +86,7 @@
     <div class="first yui-u">
       <metal:details use-macro="context/@@+person-macros/team-details" />
     </div>
-    <div class="first yui-u">
+    <div class="yui-u">
       <div tal:content="structure context/@@+portlet-membership" />
     </div>
   </div>

=== modified file 'lib/lp/registry/templates/team-members.pt'
--- lib/lp/registry/templates/team-members.pt	2009-10-26 14:23:12 +0000
+++ lib/lp/registry/templates/team-members.pt	2010-01-04 18:54:21 +0000
@@ -207,57 +207,59 @@
 
     <li tal:condition="view/inactive_memberships/batch/total"
         tal:define="inactive_batch nocall:view/inactive_memberships/currentBatch">
-      <a name="former"></a>
-      <h2>Former members</h2>
-
-      <p>
-        These are the members whose subscriptions have expired, or were
-        deactivated by themselves or by a team administrator.
-      </p>
-
-      <div class="lesser" id="inactive-top-navigation">
-        <tal:navigation
-          content="structure view/inactive_memberships/@@+navigation-links-upper" />
-      </div>
-      <table class="listing sortable" id="inactivemembers">
-        <thead>
-          <tr>
-            <th>Name</th>
-            <th>Joined in</th>
-            <th>Status</th>
-            <th>&nbsp;</th>
-          </tr>
-        </thead>
-        <tbody>
-          <tr tal:repeat="membership inactive_batch">
-            <tal:block define="member membership/person">
-            <td>
-              <a tal:attributes="href member/fmt:url"
-                 tal:content="member/displayname" />
-            </td>
-            <td
-              tal:content="membership/datejoined/fmt:date"
-            >2005-04-01</td>
-
-            <td tal:condition="membership/isExpired">
-                Expired on
-                <span tal:replace="membership/dateexpires/fmt:date" />
-            </td>
-            <td
-              tal:condition="not: membership/isExpired"
-              tal:content="membership/status/title" />
-            <td tal:condition="user_can_edit_memberships">
-              <a tal:attributes="href membership/fmt:url"
-                ><img src="/@@/edit"
-                      title="Change membership duration or status" /></a>
-            </td>
-            </tal:block>
-          </tr>
-        </tbody>
-      </table>
-      <div class="lesser">
-        <tal:navigation
-          content="structure view/inactive_memberships/@@+navigation-links-lower" />
+      <div tal:condition="user_can_edit_memberships">
+        <a name="former"></a>
+        <h2>Former members</h2>
+
+        <p>
+          These are the members whose subscriptions have expired, or were
+          deactivated by themselves or by a team administrator.
+        </p>
+
+        <div class="lesser" id="inactive-top-navigation">
+          <tal:navigation
+            content="structure view/inactive_memberships/@@+navigation-links-upper" />
+        </div>
+        <table class="listing sortable" id="inactivemembers">
+          <thead>
+            <tr>
+              <th>Name</th>
+              <th>Joined in</th>
+              <th>Status</th>
+              <th>&nbsp;</th>
+            </tr>
+          </thead>
+          <tbody>
+            <tr tal:repeat="membership inactive_batch">
+              <tal:block define="member membership/person">
+              <td>
+                <a tal:attributes="href member/fmt:url"
+                  tal:content="member/displayname" />
+              </td>
+              <td
+                tal:content="membership/datejoined/fmt:date"
+              >2005-04-01</td>
+
+              <td tal:condition="membership/isExpired">
+                  Expired on
+                  <span tal:replace="membership/dateexpires/fmt:date" />
+              </td>
+              <td
+                tal:condition="not: membership/isExpired"
+                tal:content="membership/status/title" />
+              <td tal:condition="user_can_edit_memberships">
+                <a tal:attributes="href membership/fmt:url"
+                  ><img src="/@@/edit"
+                        title="Change membership duration or status" /></a>
+              </td>
+              </tal:block>
+            </tr>
+          </tbody>
+        </table>
+        <div class="lesser">
+          <tal:navigation
+            content="structure view/inactive_memberships/@@+navigation-links-lower" />
+        </div>
       </div>
     </li>
   </ul>

=== modified file 'lib/lp/registry/templates/team-portlet-membership.pt'
--- lib/lp/registry/templates/team-portlet-membership.pt	2010-01-04 18:54:20 +0000
+++ lib/lp/registry/templates/team-portlet-membership.pt	2010-01-04 18:54:21 +0000
@@ -15,24 +15,24 @@
 
   <div id="membership-summary" class="portletBody portletContent"
     style="margin-bottom: 1.5em;">
-    <div id="membership-summary">
-      <div>
+    <div>
+      <div id="membership-counts">
         <img src="/@@/team" alt="team" />
-        <strong id="member-count">
+        <strong id="approved-member-count">
           <tal:active content="context/all_member_count" />
         </strong>
         <a tal:attributes="href string:${context/fmt:url/+members}#active"
           >active members</a><tal:invited
             define="invited_member_count context/invited_member_count"
             condition="invited_member_count">,
-          <strong>
+          <strong id="invited-member-count">
             <tal:invited_count content="context/invited_member_count" />
           </strong>
           <a tal:attributes="href string:${context/fmt:url/+members}#invited"
             >invited members</a></tal:invited><tal:proposed
               define="proposed_member_count context/proposed_member_count"
               condition="proposed_member_count">,
-          <strong>
+          <strong id="proposed-member-count">
             <tal:proposed_count content="proposed_member_count" />
           </strong>
           <a tal:attributes="href string:${context/fmt:url/+members}#proposed"
@@ -95,37 +95,40 @@
   <table style="margin: 0px 0px .5em 0px;">
     <tr>
       <td style="padding: 0px 1em 1em 0px;"
-          tal:define="link context/menu:overview/add_member">
-        <span id="add-member-spinner" class="update-in-progress-message"
-              style="display: none">
+          tal:define="link context/menu:overview/add_member"
+          tal:condition="link/enabled">
+        <span id="add-member-spinner"
+              class="unseen update-in-progress-message">
           Saving...
         </span>
         <tal:add-member replace="structure link/fmt:link-icon" />
       </td>
-      <td style="padding: 0px 0px 1em 0px;"
+      <td colspan="2"
+          style="padding: 0px 0px 1em 0px;"
           tal:define="link context/menu:overview/mugshots">
         <tal:mugshots replace="structure link/fmt:link-icon" />
       </td>
     </tr>
+  </table>
+  <table>
     <tr>
-      <td style="padding: 3px 3em 0px 0px;">
-        <div id="recently-approved"
-             tal:attributes="style view/recently_approved_hidden">
-          <h3 style="color:black; font-weight:bold; margin: 0px">
-            Latest members
-          </h3>
-          <ul id="recently-approved-ul">
-            <li tal:repeat="person view/recently_approved_members"
-                tal:content="structure person/fmt:link" />
-          </ul>
-        </div>
+      <td id="recently-approved"
+          style="padding-right: 1em"
+          tal:attributes="class view/recently_approved_hidden">
+        <h3 style="color:black; font-weight:bold; margin: 0px">
+          Latest members
+        </h3>
+        <ul id="recently-approved-ul">
+          <li tal:repeat="person view/recently_approved_members"
+              tal:content="structure person/fmt:link" />
+        </ul>
       </td>
-      <td style="padding: 0px;" id="recently-applied"
-        tal:condition="view/recently_proposed_members">
+      <td id="recently-proposed"
+          tal:attributes="class view/recently_proposed_hidden">
         <h3 style="color:black; font-weight:bold; margin: 0px">
           Pending approval
         </h3>
-        <ul>
+        <ul id="recently-proposed-ul">
           <li tal:repeat="person view/recently_proposed_members"
               tal:content="structure person/fmt:link" />
         </ul>
@@ -136,6 +139,16 @@
       </td>
     </tr>
   </table>
+  <div id="recently-invited"
+        tal:attributes="class view/recently_invited_hidden">
+    <h3 style="color:black; font-weight:bold; margin: 0px">
+      Latest invited
+    </h3>
+    <ul id="recently-invited-ul">
+      <li tal:repeat="person view/recently_invited_members"
+          tal:content="structure person/fmt:link" />
+    </ul>
+  </div>
   </tal:can-view>
 </div>
 </tal:root>

=== added file 'lib/lp/registry/windmill/tests/test_team_index.py'
--- lib/lp/registry/windmill/tests/test_team_index.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/windmill/tests/test_team_index.py	2010-01-04 18:54:21 +0000
@@ -0,0 +1,74 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test team index page."""
+
+__metaclass__ = type
+__all__ = []
+
+import unittest
+
+from windmill.authoring import WindmillTestClient
+
+from canonical.launchpad.windmill.testing import lpuser
+from canonical.launchpad.windmill.testing.widgets import (
+    search_and_select_picker_widget)
+
+from lp.registry.windmill.testing import RegistryWindmillLayer
+from lp.testing import TestCaseWithFactory
+
+
+class TestTeamIndex(TestCaseWithFactory):
+    """Test team index page."""
+
+    layer = RegistryWindmillLayer
+
+    def setUp(self):
+        self.client = WindmillTestClient(__name__)
+
+    def test_addmember(self):
+        self.client.open(
+            url=u'http://launchpad.dev:8085/~testing-spanish-team')
+
+        lpuser.TRANSLATIONS_ADMIN.ensure_login(self.client)
+
+        addmember_xpath = (
+            '//*[@id="membership"]' +
+            '//a[text()="Add member" ' +
+            'and contains(@class, "js-action")]')
+        # Add rosetta-admins team as a member.
+        approved_count_xpath = '//*[@id="approved-member-count"]'
+        code = "lookupNode({xpath: '%s'}).textContent" % approved_count_xpath
+        result = self.client.commands.execJS(code=code)
+        old_approved_count = int(result['result'])
+
+        self.client.waits.forElement(xpath=addmember_xpath)
+        self.client.click(xpath=addmember_xpath)
+        # Xpath is 1-indexed.
+        search_and_select_picker_widget(self.client, 'rosetta', 1)
+
+        self.client.waits.forElement(
+            xpath='//ul[@id="recently-approved-ul"]/li[1]/a',
+            option='href|~rosetta-admins')
+
+        self.client.asserts.assertText(
+            xpath=approved_count_xpath,
+            validator=str(old_approved_count+1))
+
+        # Add another team as a member.
+        invited_count_xpath = '//*[@id="invited-member-count"]'
+        self.client.asserts.assertNotNode(xpath=invited_count_xpath)
+        self.client.click(xpath=addmember_xpath)
+        search_and_select_picker_widget(self.client, 'simple', 1)
+
+        self.client.waits.forElement(
+            xpath='//ul[@id="recently-invited-ul"]/li[1]/a',
+            option='href|~simple-team')
+
+        self.client.asserts.assertText(
+            xpath=invited_count_xpath,
+            validator="1")
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)