Code review comment for lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part2

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 first_node = members_ul.get('firstChild');

=== modified file 'lib/lp/registry/browser/tests/mailinglist-views.txt'
--- lib/lp/registry/browser/tests/mailinglist-views.txt 2009-12-18 06:04:44 +0000
+++ lib/lp/registry/browser/tests/mailinglist-views.txt 2009-12-21 19:55:45 +0000
@@ -33,8 +33,7 @@
 No Privileges Person is added as a member of the Aardvarks.

     >>> no_priv = person_set.getByEmail('<email address hidden>')
- >>> aardvarks.addMember(no_priv, sample_person)
- (True, <DBItem TeamMembershipStatus.APPROVED, (2) Approved>)
+ >>> ignored = aardvarks.addMember(no_priv, sample_person)

 But regular members can't access the +mailinglist page.

@@ -54,8 +53,8 @@
     ... no_priv, aardvarks)
     >>> team_membership.status
     <DBItem TeamMembershipStatus.APPROVED...
- >>> team_membership.setStatus(TeamMembershipStatus.ADMIN, sample_person)
- True
+ >>> ignored = team_membership.setStatus(
+ ... TeamMembershipStatus.ADMIN, sample_person)

 Now No Privileges Person has permission to request mailing lists.

@@ -210,8 +209,7 @@

 A non-owner member can see information about the mailing list.

- >>> pmt.addMember(sample_person, owner)
- (True, <DBItem TeamMembershipStatus.APPROVED...
+ >>> ignored = pmt.addMember(sample_person, owner)
     >>> login('<email address hidden>')
     >>> view = create_initialized_view(pmt, '+index')
     >>> print view.archive_url

=== 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
@@ -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
@@ -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">...

 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 +192,7 @@
     >>> owner_browser = setupBrowser(auth="Basic <email address hidden>: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 2009-12-21 20:08:16 +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
 =======================

}}}