Launchpad itself

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

  1. bug-482176-add-team-member-ajax-part2
  2. Merge into devel
Proposed by Edwin Grubbs
Status: Merged
Approved by: Guilherme Salgado
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part2
Merge into: lp:launchpad
Prerequisite: lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1
Diff against target: 962 lines (+399/-202)
16 files modified
lib/canonical/launchpad/icing/style-3-0.css (+3/-0)
lib/canonical/launchpad/icing/style.css (+0/-2)
lib/canonical/launchpad/javascript/code/codereview.js (+7/-6)
lib/canonical/launchpad/javascript/lp/errors.js (+33/-3)
lib/canonical/launchpad/javascript/registry/team.js (+83/-49)
lib/lp/registry/browser/person.py (+33/-21)
lib/lp/registry/browser/tests/mailinglist-views.txt (+5/-4)
lib/lp/registry/interfaces/person.py (+27/-17)
lib/lp/registry/model/person.py (+8/-5)
lib/lp/registry/model/teammembership.py (+2/-10)
lib/lp/registry/stories/team/xx-team-home.txt (+21/-10)
lib/lp/registry/stories/team/xx-team-membership.txt (+13/-0)
lib/lp/registry/templates/team-index.pt (+1/-1)
lib/lp/registry/templates/team-members.pt (+53/-51)
lib/lp/registry/templates/team-portlet-membership.pt (+36/-23)
lib/lp/registry/windmill/tests/test_team_index.py (+74/-0)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part2
Reviewer Review Type Date Requested Status
Martin Albisetti (community) ui Approve
Curtis Hovey (community) code and js Approve
Guilherme Salgado (community) Needs Fixing
Canonical Launchpad Engineering code ui Pending
Review via email: mp+16325@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Edwin Grubbs (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.

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.

Revision history for this message
Guilherme Salgado (salgado) wrote :
Download full text (12.2 KiB)

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
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

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/

Revision history for this message
Martin Albisetti (beuno) wrote :

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)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (13.1 KiB)

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');
> > + };
> > ...

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (7.4 KiB)

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...

Read more...

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

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

Revision history for this message
Curtis Hovey (sinzui) wrote :

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)
Revision history for this message
Martin Albisetti (beuno) wrote :

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?

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> 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.

Revision history for this message
Guilherme Salgado (salgado) wrote :
Download full text (3.7 KiB)

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...

Read more...

Revision history for this message
Martin Albisetti (beuno) wrote :

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)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
215 font-weight: bold;215 font-weight: bold;
216 }216 }
217217
218.unseen {display: none;}
219
220
218/* Common presentations */221/* Common presentations */
219div.see-all {222div.see-all {
220 font-size: 123.1%; /* Same as a h2 */223 font-size: 123.1%; /* Same as a h2 */
221224
=== 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 @@
428fieldset .expanded {display: block;}428fieldset .expanded {display: block;}
429fieldset.collapsible legend {font-weight: normal;}429fieldset.collapsible legend {font-weight: normal;}
430430
431.unseen {display: none;}
432
433form.primary.search {431form.primary.search {
434 margin-bottom: 2em;432 margin-bottom: 2em;
435}433}
436434
=== 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 @@
22 var link = Y.one('#request-review');22 var link = Y.one('#request-review');
23 if (link !== null) {23 if (link !== null) {
24 link.addClass('js-action');24 link.addClass('js-action');
25 /* XXX: salgado, 2009-11-11: This will cause the picker to be25 /* XXX: salgado 2009-11-11 bug=497603
26 * recreated every time the user clicks on the link. Although that26 * This will cause the picker to be recreated every time the
27 * makes it unnecessary to have the widget cleared, it makes it27 * user clicks on the link. Although that makes it unnecessary
28 * impossible to persist the state of the picker between clicks on the28 * to have the widget cleared, it makes it impossible to persist
29 * link. We should probably have a policy to enforce that we29 * the state of the picker between clicks on the link. We
30 * just hide/show widgets when a link is clicked more than once,30 * should probably have a policy to enforce that we just
31 * hide/show widgets when a link is clicked more than once,
31 * instead of recreating the widgets every time.32 * instead of recreating the widgets every time.
32 */33 */
33 link.on('click', show_request_review_form);34 link.on('click', show_request_review_form);
3435
=== 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 @@
69 * @param flash_node {Node} The node to red flash.69 * @param flash_node {Node} The node to red flash.
70 * @param msg {String} The message to display.70 * @param msg {String} The message to display.
71*/71*/
72var display_error = function(flash_node, msg) {72Y.lp.display_error = function(flash_node, msg) {
73 create_error_overlay();73 create_error_overlay();
74 maybe_red_flash(flash_node, function(){74 maybe_red_flash(flash_node, function(){
75 error_overlay.showError(msg);75 error_overlay.showError(msg);
@@ -77,5 +77,35 @@
77 });77 });
78};78};
7979
80Y.lp.display_error = display_error;80
81}, "0.1", {"requires":["lazr.formoverlay"]});81var info_overlay;
82/*
83 * Display the form overlay for non-error informational messages.
84 *
85 * @method display_info
86 * @param msg {String} The message to display.
87*/
88Y.lp.display_info = function(msg) {
89 if (info_overlay === undefined) {
90 info_overlay = new Y.lazr.PrettyOverlay({
91 centered: true,
92 visible: false
93 });
94 info_overlay.render();
95 }
96 var content = Y.Node.create(
97 '<div style="background: url(/@@/info-large) no-repeat; ' +
98 'min-height: 32px; padding-left: 40px; padding-top: 16px"/></div>');
99 content.appendChild(Y.Node.create(msg));
100 var button_div = Y.Node.create('<div style="text-align: right"></div>');
101 var ok_button = Y.Node.create('<button>OK</button>');
102 ok_button.on('click', function(e) {
103 info_overlay.fire('cancel');
104 });
105 button_div.appendChild(ok_button);
106 content.appendChild(button_div);
107 info_overlay.set('bodyContent', content);
108 info_overlay.show();
109};
110
111}, "0.1", {"requires":["lazr.formoverlay", "lazr.overlay"]});
82112
=== 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 @@
15 * @method setup_add_member_handler15 * @method setup_add_member_handler
16 */16 */
17module.setup_add_member_handler = function() {17module.setup_add_member_handler = function() {
18 if (Y.UA.ie) {
19 return;
20 }
21
18 var config = {22 var config = {
19 header: 'Add a member',23 header: 'Add a member',
20 step_title: 'Search',24 step_title: 'Search',
21 picker_activator: '.menu-link-add_member'25 picker_activator: '.menu-link-add_member'
22 };26 };
2327
24 var picker = Y.lp.picker.create(28 Y.lp.picker.create('ValidTeamMember', _add_member, config);
25 'ValidTeamMember',
26 function(result) { _add_member(result); },
27 config);
28};29};
2930
30var _add_member = function(result) {31var _add_member = function(selected_person) {
31 var spinner = Y.one('#add-member-spinner');32 var box = Y.one('#membership');
32 var addmember_link = Y.one('.menu-link-add_member');33 var spinner = box.query('#add-member-spinner');
33 addmember_link.setStyle('display', 'none');34 var addmember_link = box.query('.menu-link-add_member');
34 spinner.setStyle('display', 'inline');35 addmember_link.addClass('unseen');
35 function disable_spinner() {36 spinner.removeClass('unseen');
36 addmember_link.setStyle('display', 'inline');37 var disable_spinner = function() {
37 spinner.setStyle('display', 'none');38 addmember_link.removeClass('unseen');
38 }39 spinner.addClass('unseen');
40 };
39 lp_client = new LP.client.Launchpad();41 lp_client = new LP.client.Launchpad();
4042
41 var error_handler = new LP.client.ErrorHandler();43 var error_handler = new LP.client.ErrorHandler();
42 error_handler.clearProgressUI = disable_spinner;44 error_handler.clearProgressUI = disable_spinner;
43 error_handler.showError = function(error_msg) {45 error_handler.showError = function(error_msg) {
44 Y.lp.display_error(Y.one('.menu-link-add_member'), error_msg);46 Y.lp.display_error(addmember_link, error_msg);
45 };47 };
4648
47 config = {49 addmember_config = {
48 on: {50 on: {
49 success: function(member_added) {51 success: function(change_and_status) {
50 if (!member_added) {52 var did_status_change = change_and_status[0];
51 disable_spinner();53 var current_status = change_and_status[1];
52 alert('Already a member.');54 var members_section, members_ul, count_elem;
53 return;55 if (did_status_change === false) {
54 }56 disable_spinner();
55 if (result.css.match("team")) {57 Y.lp.display_info(
56 disable_spinner();58 selected_person.title + ' is already ' +
57 alert('This is a team');59 current_status.toLowerCase() +
58 return;60 ' as a member of the team.');
59 }61 return;
60 var members_section = Y.one('#recently-approved');62 }
61 var members_ul = Y.one('#recently-approved-ul');63
64 if (current_status == 'Invited') {
65 members_section = box.query('#recently-invited');
66 members_ul = box.query('#recently-invited-ul');
67 count_elem = box.query('#invited-member-count');
68 } else if (current_status == 'Proposed') {
69 members_section = box.query('#recently-proposed');
70 members_ul = box.query('#recently-proposed-ul');
71 count_elem = box.query('#proposed-member-count');
72 } else if (current_status == 'Approved') {
73 members_section = box.query('#recently-approved');
74 members_ul = box.query('#recently-approved-ul');
75 count_elem = box.query('#approved-member-count');
76 } else {
77 Y.lp.display_error(
78 addmember_link,
79 'Unexpected status: ' + current_status);
80 return;
81 }
62 var first_node = members_ul.get('firstChild');82 var first_node = members_ul.get('firstChild');
63 config = {83
84 var xhtml_person_handler = function(person_html) {
85 if (count_elem === null && current_status == 'Invited') {
86 count_elem = Y.Node.create(
87 '<strong id="invited-member-count">' +
88 '1</strong>');
89 var count_box = Y.one(
90 '#membership #membership-counts');
91 count_box.append(Y.Node.create(
92 '<span>, </span>'));
93 count_box.append(count_elem);
94 count_box.append(Y.Node.create(
95 '<span> <a href="/+members#invited">' +
96 'invited members</a></span>'));
97 } else {
98 var count = count_elem.get('innerHTML');
99 count = parseInt(count, 10) + 1;
100 count_elem.set('innerHTML', count);
101 }
102 person_repr = Y.Node.create(
103 '<li>' + person_html + '</li>');
104 members_section.removeClass('unseen');
105 members_ul.insertBefore(person_repr, first_node);
106 anim = Y.lazr.anim.green_flash({node: person_repr});
107 anim.run();
108 disable_spinner();
109 };
110
111 xhtml_person_config = {
64 on: {112 on: {
65 success: function(person_html) {113 success: xhtml_person_handler,
66 var total_members = Y.one(
67 '#member-count').get('innerHTML');
68 total_members = parseInt(total_members, 10) + 1;
69 Y.one('#member-count').set(
70 'innerHTML', total_members);
71 person_repr = Y.Node.create(
72 '<li>' + person_html + '</li>');
73 members_section.setStyle('visibility', 'visible');
74 members_ul.insertBefore(
75 person_repr, first_node);
76 anim = Y.lazr.anim.green_flash(
77 {node: person_repr});
78 anim.run();
79 disable_spinner();
80 },
81 failure: error_handler.getFailureHandler()114 failure: error_handler.getFailureHandler()
82 },115 },
83 accept: LP.client.XHTML116 accept: LP.client.XHTML
84 };117 };
85 lp_client.get(result.api_uri, config);118 lp_client.get(selected_person.api_uri, xhtml_person_config);
86 },119 },
87 failure: error_handler.getFailureHandler()120 failure: error_handler.getFailureHandler()
88 },121 },
89 parameters: {122 parameters: {
90 // XXX: Why do I always have to get absolute URIs out of the URIs123 // XXX: EdwinGrubbs 2009-12-16 bug=497602
124 // Why do I always have to get absolute URIs out of the URIs
91 // in the picker's result/client.links?125 // in the picker's result/client.links?
92 reviewer: LP.client.get_absolute_uri(LP.client.links.me),126 reviewer: LP.client.get_absolute_uri(LP.client.links.me),
93 person: LP.client.get_absolute_uri(result.api_uri)127 person: LP.client.get_absolute_uri(selected_person.api_uri)
94 }128 }
95 };129 };
96130
97 lp_client.named_post(131 lp_client.named_post(
98 LP.client.cache.context.self_link, 'addMember', config);132 LP.client.cache.context.self_link, 'addMember', addmember_config);
99};133};
100134
101}, '0.1', {requires: [135}, '0.1', {requires: [
102136
=== 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 @@
95from itertools import chain95from itertools import chain
96from operator import attrgetter, itemgetter96from operator import attrgetter, itemgetter
97from textwrap import dedent97from textwrap import dedent
98from storm.zope.interfaces import IResultSet
9899
99from zope.error.interfaces import IErrorReportingUtility100from zope.error.interfaces import IErrorReportingUtility
100from zope.app.form.browser import TextAreaWidget, TextWidget101from zope.app.form.browser import TextAreaWidget, TextWidget
@@ -227,13 +228,8 @@
227 logoutPerson, allowUnauthenticatedSession)228 logoutPerson, allowUnauthenticatedSession)
228from canonical.launchpad.webapp.menu import get_current_view229from canonical.launchpad.webapp.menu import get_current_view
229from canonical.launchpad.webapp.publisher import LaunchpadView230from canonical.launchpad.webapp.publisher import LaunchpadView
230<<<<<<< TREE231from canonical.launchpad.webapp.tales import (
231from canonical.launchpad.webapp.tales import (232 DateTimeFormatterAPI, FormattersAPI, PersonFormatterAPI)
232 DateTimeFormatterAPI, FormattersAPI)
233=======
234from canonical.launchpad.webapp.tales import (
235 DateTimeFormatterAPI, PersonFormatterAPI)
236>>>>>>> MERGE-SOURCE
237from lazr.uri import URI, InvalidURIError233from lazr.uri import URI, InvalidURIError
238234
239from canonical.launchpad import _235from canonical.launchpad import _
@@ -2489,11 +2485,7 @@
24892485
2490 @property2486 @property
2491 def next_url(self):2487 def next_url(self):
2492<<<<<<< TREE
2493 """Redirect to the +languages page if request originated there."""2488 """Redirect to the +languages page if request originated there."""
2494=======
2495 """Redirect back to the originating page."""
2496>>>>>>> MERGE-SOURCE
2497 redirection_url = self.request.get('redirection_url')2489 redirection_url = self.request.get('redirection_url')
2498 if redirection_url:2490 if redirection_url:
2499 return redirection_url2491 return redirection_url
@@ -2501,11 +2493,7 @@
25012493
2502 @property2494 @property
2503 def cancel_url(self):2495 def cancel_url(self):
2504<<<<<<< TREE
2505 """Redirect to the +languages page if request originated there."""2496 """Redirect to the +languages page if request originated there."""
2506=======
2507 """Redirect back to the originating page."""
2508>>>>>>> MERGE-SOURCE
2509 redirection_url = self.getRedirectionURL()2497 redirection_url = self.getRedirectionURL()
2510 if redirection_url:2498 if redirection_url:
2511 return redirection_url2499 return redirection_url
@@ -2745,6 +2733,13 @@
2745 orderBy='-TeamMembership.date_proposed')2733 orderBy='-TeamMembership.date_proposed')
2746 return members[:5]2734 return members[:5]
27472735
2736 @cachedproperty
2737 def recently_invited_members(self):
2738 members = self.context.getMembersByStatus(
2739 TeamMembershipStatus.INVITED,
2740 orderBy='-TeamMembership.date_proposed')
2741 return members[:5]
2742
2748 @property2743 @property
2749 def recently_approved_hidden(self):2744 def recently_approved_hidden(self):
2750 """Optionally hide the div.2745 """Optionally hide the div.
@@ -2752,8 +2747,8 @@
2752 The AJAX on the page needs the elements to be present2747 The AJAX on the page needs the elements to be present
2753 but hidden in case it adds a member to the list.2748 but hidden in case it adds a member to the list.
2754 """2749 """
2755 if self.recently_approved_members.count() == 0:2750 if IResultSet(self.recently_approved_members).is_empty():
2756 return 'visibility: collapse'2751 return 'unseen'
2757 else:2752 else:
2758 return ''2753 return ''
27592754
@@ -2764,8 +2759,20 @@
2764 The AJAX on the page needs the elements to be present2759 The AJAX on the page needs the elements to be present
2765 but hidden in case it adds a member to the list.2760 but hidden in case it adds a member to the list.
2766 """2761 """
2767 if self.recently_proposed_members.count() == 0:2762 if IResultSet(self.recently_proposed_members).is_empty():
2768 return 'visibility: collapse'2763 return 'unseen'
2764 else:
2765 return ''
2766
2767 @property
2768 def recently_invited_hidden(self):
2769 """Optionally hide the div.
2770
2771 The AJAX on the page needs the elements to be present
2772 but hidden in case it adds a member to the list.
2773 """
2774 if IResultSet(self.recently_invited_members).is_empty():
2775 return 'unseen'
2769 else:2776 else:
2770 return ''2777 return ''
27712778
@@ -4241,9 +4248,14 @@
4241 def continue_action(self, action, data):4248 def continue_action(self, action, data):
4242 """Make the selected teams join this team."""4249 """Make the selected teams join this team."""
4243 context = self.context4250 context = self.context
4251 is_admin = check_permission('launchpad.Admin', context)
4244 for team in data['teams']:4252 for team in data['teams']:
4245 team.join(context, requester=self.user)4253 if is_admin:
4246 if context.subscriptionpolicy == TeamSubscriptionPolicy.MODERATED:4254 context.addMember(team, reviewer=self.user)
4255 else:
4256 team.join(context, requester=self.user)
4257 if (context.subscriptionpolicy == TeamSubscriptionPolicy.MODERATED
4258 and not is_admin):
4247 msg = 'proposed to this team.'4259 msg = 'proposed to this team.'
4248 else:4260 else:
4249 msg = 'added to this team.'4261 msg = 'added to this team.'
42504262
=== 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 @@
33No Privileges Person is added as a member of the Aardvarks.33No Privileges Person is added as a member of the Aardvarks.
3434
35 >>> no_priv = person_set.getByEmail('no-priv@canonical.com')35 >>> no_priv = person_set.getByEmail('no-priv@canonical.com')
36 >>> aardvarks.addMember(no_priv, sample_person)36 >>> ignored = aardvarks.addMember(no_priv, sample_person)
3737
38But regular members can't access the +mailinglist page.38But regular members can't access the +mailinglist page.
3939
@@ -52,8 +52,9 @@
52 >>> team_membership = getUtility(ITeamMembershipSet).getByPersonAndTeam(52 >>> team_membership = getUtility(ITeamMembershipSet).getByPersonAndTeam(
53 ... no_priv, aardvarks)53 ... no_priv, aardvarks)
54 >>> team_membership.status54 >>> team_membership.status
55 <DBItem TeamMembershipStatus.APPROVED, (2) Approved>55 <DBItem TeamMembershipStatus.APPROVED...
56 >>> team_membership.setStatus(TeamMembershipStatus.ADMIN, sample_person)56 >>> ignored = team_membership.setStatus(
57 ... TeamMembershipStatus.ADMIN, sample_person)
5758
58Now No Privileges Person has permission to request mailing lists.59Now No Privileges Person has permission to request mailing lists.
5960
@@ -208,7 +209,7 @@
208209
209A non-owner member can see information about the mailing list.210A non-owner member can see information about the mailing list.
210211
211 >>> pmt.addMember(sample_person, owner)212 >>> ignored = pmt.addMember(sample_person, owner)
212 >>> login('test@canonical.com')213 >>> login('test@canonical.com')
213 >>> view = create_initialized_view(pmt, '+index')214 >>> view = create_initialized_view(pmt, '+index')
214 >>> print view.archive_url215 >>> print view.archive_url
215216
=== 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 @@
1406 may_subscribe_to_list=True):1406 may_subscribe_to_list=True):
1407 """Add the given person as a member of this team.1407 """Add the given person as a member of this team.
14081408
1409 If the given person is already a member of this team we'll simply1409 :param person: If the given person is already a member of this
1410 change its membership status. Otherwise a new TeamMembership is1410 team we'll simply change its membership status. Otherwise a new
1411 created with the given status.1411 TeamMembership is created with the given status.
14121412
1413 If the person is actually a team and force_team_add is False, the1413 :param reviewer: The user who made the given person a member of this
1414 team will actually be invited to join this one. Otherwise the team1414 team.
1415 is added as if it were a person.1415
14161416 :param comment: String that will be assigned to the
1417 If the person is not a team, and may_subscribe_to_list1417 proponent_comment, reviwer_comment, or acknowledger comment.
1418 is True, then the person may be subscribed to the team's1418
1419 mailing list, depending on the list status and the person's1419 :param status: `TeamMembershipStatus` value must be either
1420 auto-subscribe settings.1420 Approved, Proposed or Admin.
14211421
1422 The given status must be either Approved, Proposed or Admin.1422 :param force_team_add: If the person is actually a team and
14231423 force_team_add is False, the team will actually be invited to
1424 The reviewer is the user who made the given person a member of this1424 join this one. Otherwise the team is added as if it were a
1425 team.1425 person.
1426
1427 :param may_subscribe_to_list: If the person is not a team, and
1428 may_subscribe_to_list is True, then the person may be subscribed
1429 to the team's mailing list, depending on the list status and the
1430 person's auto-subscribe settings.
1431
1432 :return: A tuple containing a boolean indicating when the
1433 membership status changed and the current `TeamMembershipStatus`.
1434 This depends on the desired status passed as an argument, the
1435 subscription policy and the user's priveleges.
1426 """1436 """
14271437
1428 def deactivateAllMembers(comment, reviewer):1438 def deactivateAllMembers(comment, reviewer):
14291439
=== 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 @@
1231 # By default, teams can only be invited as members, meaning that1231 # By default, teams can only be invited as members, meaning that
1232 # one of the team's admins will have to accept the invitation1232 # one of the team's admins will have to accept the invitation
1233 # before the team is made a member. If force_team_add is True,1233 # before the team is made a member. If force_team_add is True,
1234 # though, then we'll add a team as if it was a person.1234 # or the user is also an admin of the proposed member, then
1235 if not force_team_add:1235 # we'll add a team as if it was a person.
1236 is_reviewer_admin_of_new_member = (
1237 person in reviewer.getAdministratedTeams())
1238 if not force_team_add and not is_reviewer_admin_of_new_member:
1236 status = TeamMembershipStatus.INVITED1239 status = TeamMembershipStatus.INVITED
1237 event = TeamInvitationEvent1240 event = TeamInvitationEvent
12381241
1239 member_added = True1242 status_changed = True
1240 expires = self.defaultexpirationdate1243 expires = self.defaultexpirationdate
1241 tm = TeamMembership.selectOneBy(person=person, team=self)1244 tm = TeamMembership.selectOneBy(person=person, team=self)
1242 if tm is None:1245 if tm is None:
@@ -1251,12 +1254,12 @@
1251 # We can't use tm.setExpirationDate() here because the reviewer1254 # We can't use tm.setExpirationDate() here because the reviewer
1252 # here will be the member themselves when they join an OPEN team.1255 # here will be the member themselves when they join an OPEN team.
1253 tm.dateexpires = expires1256 tm.dateexpires = expires
1254 member_added = tm.setStatus(status, reviewer, comment)1257 status_changed = tm.setStatus(status, reviewer, comment)
12551258
1256 if not person.is_team and may_subscribe_to_list:1259 if not person.is_team and may_subscribe_to_list:
1257 person.autoSubscribeToMailingList(self.mailing_list,1260 person.autoSubscribeToMailingList(self.mailing_list,
1258 requester=reviewer)1261 requester=reviewer)
1259 return member_added1262 return (status_changed, tm.status)
12601263
1261 # The three methods below are not in the IPerson interface because we want1264 # The three methods below are not in the IPerson interface because we want
1262 # to protect them with a launchpad.Edit permission. We could do that by1265 # to protect them with a launchpad.Edit permission. We could do that by
12631266
=== 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 @@
318 "%(team)s is a member of %(person)s."318 "%(team)s is a member of %(person)s."
319 % dict(person=self.person.name, team=self.team.name))319 % dict(person=self.person.name, team=self.team.name))
320320
321
322 old_status = self.status321 old_status = self.status
323 self.status = status322 self.status = status
324323
@@ -367,17 +366,10 @@
367 # When a member proposes himself, a more detailed notification is366 # When a member proposes himself, a more detailed notification is
368 # sent to the team admins by a subscriber of JoinTeamEvent; that's367 # sent to the team admins by a subscriber of JoinTeamEvent; that's
369 # why we don't send anything here.368 # why we don't send anything here.
370<<<<<<< TREE369 if ((self.person != self.last_changed_by or self.status != proposed)
371 if self.person == self.last_changed_by and self.status == proposed:370 and not silent):
372 return
373
374 if not silent:
375 self._sendStatusChangeNotification(old_status)
376=======
377 if self.person != self.last_changed_by or self.status != proposed:
378 self._sendStatusChangeNotification(old_status)371 self._sendStatusChangeNotification(old_status)
379 return True372 return True
380>>>>>>> MERGE-SOURCE
381373
382 def _sendStatusChangeNotification(self, old_status):374 def _sendStatusChangeNotification(self, old_status):
383 """Send a status change notification to all team admins and the375 """Send a status change notification to all team admins and the
384376
=== 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 @@
1313
14 >>> print extract_text(14 >>> print extract_text(
15 ... find_tag_by_id(browser.contents, 'recently-approved'))15 ... find_tag_by_id(browser.contents, 'recently-approved'))
16 Recently approved16 Latest members
17 Warty Gnome Team17 Warty Gnome Team
18 Daniel Silverstone18 Daniel Silverstone
19 Celso Providelo19 Celso Providelo
20 Steve Alexander...20 Steve Alexander...
2121
22 >>> print extract_text(22 >>> print extract_text(
23 ... find_tag_by_id(browser.contents, 'recently-applied'))23 ... find_tag_by_id(browser.contents, 'recently-proposed'))
24 Pending approval24 Pending approval
25 Sample Person25 Sample Person
26 Andrew Bennetts26 Andrew Bennetts
2727
28 >>> print extract_text(28 >>> print extract_text(
29 ... find_tag_by_id(browser.contents, 'recently-invited'))
30 Latest invited
31 Warty Security Team
32
33 >>> print extract_text(
29 ... find_tag_by_id(browser.contents, 'team-owner'))34 ... find_tag_by_id(browser.contents, 'team-owner'))
30 Owner:35 Owner:
31 Mark Shuttleworth36 Mark Shuttleworth
@@ -87,15 +92,21 @@
87 itself is not a member of any other team.92 itself is not a member of any other team.
88 Show received invitations93 Show received invitations
8994
90If the team does not have any recently approved or proposed members,95If the team does not have any recently approved, proposed, or invited
91the recent members sections are not displayed:96members, the empty lists are hidden using the "unseen" css class:
9297
93 >>> browser.open('http://launchpad.dev/~launchpad')98 >>> browser.open('http://launchpad.dev/~launchpad')
94 >>> print find_tag_by_id(browser.contents, 'recently-approved')99 >>> tag = find_tag_by_id(browser.contents, 'recently-approved')
95 None100 >>> print tag['class']
96101 unseen
97 >>> print find_tag_by_id(browser.contents, 'recently-applied')102
98 None103 >>> tag = find_tag_by_id(browser.contents, 'recently-proposed')
104 >>> print tag['class']
105 unseen
106
107 >>> tag = find_tag_by_id(browser.contents, 'recently-invited')
108 >>> print tag['class']
109 unseen
99110
100In the above case there's no user logged in, so it doesn't actually show111In the above case there's no user logged in, so it doesn't actually show
101what's the user's involvement with the team. If the user logs in, he'll see112what's the user's involvement with the team. If the user logs in, he'll see
@@ -187,7 +198,7 @@
187 >>> owner_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test")198 >>> owner_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test")
188 >>> owner_browser.open('http://launchpad.dev/~ubuntu-team')199 >>> owner_browser.open('http://launchpad.dev/~ubuntu-team')
189 >>> print extract_text(200 >>> print extract_text(
190 ... find_tag_by_id(owner_browser.contents, 'recently-applied'))201 ... find_tag_by_id(owner_browser.contents, 'recently-proposed'))
191 Pending approval202 Pending approval
192 Sample Person203 Sample Person
193 Andrew Bennetts204 Andrew Bennetts
194205
=== 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 @@
174 >>> message in second_browser.contents174 >>> message in second_browser.contents
175 True175 True
176176
177An admin can see the former members of the team.
178
179 >>> browser.open('http://launchpad.dev/~name18/+members')
180 >>> print extract_text(
181 ... find_tag_by_id(browser.contents, 'inactivemembers'))
182 Name Joined in Status...
183
184Other users cannot see the former members of the team.
185
186 >>> user_browser.open('http://launchpad.dev/~name18/+members')
187 >>> print find_tag_by_id(user_browser.contents, 'inactivemembers')
188 None
189
177190
178Team Participation page191Team Participation page
179=======================192=======================
180193
=== 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 @@
86 <div class="first yui-u">86 <div class="first yui-u">
87 <metal:details use-macro="context/@@+person-macros/team-details" />87 <metal:details use-macro="context/@@+person-macros/team-details" />
88 </div>88 </div>
89 <div class="first yui-u">89 <div class="yui-u">
90 <div tal:content="structure context/@@+portlet-membership" />90 <div tal:content="structure context/@@+portlet-membership" />
91 </div>91 </div>
92 </div>92 </div>
9393
=== 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 @@
207207
208 <li tal:condition="view/inactive_memberships/batch/total"208 <li tal:condition="view/inactive_memberships/batch/total"
209 tal:define="inactive_batch nocall:view/inactive_memberships/currentBatch">209 tal:define="inactive_batch nocall:view/inactive_memberships/currentBatch">
210 <a name="former"></a>210 <div tal:condition="user_can_edit_memberships">
211 <h2>Former members</h2>211 <a name="former"></a>
212212 <h2>Former members</h2>
213 <p>213
214 These are the members whose subscriptions have expired, or were214 <p>
215 deactivated by themselves or by a team administrator.215 These are the members whose subscriptions have expired, or were
216 </p>216 deactivated by themselves or by a team administrator.
217217 </p>
218 <div class="lesser" id="inactive-top-navigation">218
219 <tal:navigation219 <div class="lesser" id="inactive-top-navigation">
220 content="structure view/inactive_memberships/@@+navigation-links-upper" />220 <tal:navigation
221 </div>221 content="structure view/inactive_memberships/@@+navigation-links-upper" />
222 <table class="listing sortable" id="inactivemembers">222 </div>
223 <thead>223 <table class="listing sortable" id="inactivemembers">
224 <tr>224 <thead>
225 <th>Name</th>225 <tr>
226 <th>Joined in</th>226 <th>Name</th>
227 <th>Status</th>227 <th>Joined in</th>
228 <th>&nbsp;</th>228 <th>Status</th>
229 </tr>229 <th>&nbsp;</th>
230 </thead>230 </tr>
231 <tbody>231 </thead>
232 <tr tal:repeat="membership inactive_batch">232 <tbody>
233 <tal:block define="member membership/person">233 <tr tal:repeat="membership inactive_batch">
234 <td>234 <tal:block define="member membership/person">
235 <a tal:attributes="href member/fmt:url"235 <td>
236 tal:content="member/displayname" />236 <a tal:attributes="href member/fmt:url"
237 </td>237 tal:content="member/displayname" />
238 <td238 </td>
239 tal:content="membership/datejoined/fmt:date"239 <td
240 >2005-04-01</td>240 tal:content="membership/datejoined/fmt:date"
241241 >2005-04-01</td>
242 <td tal:condition="membership/isExpired">242
243 Expired on243 <td tal:condition="membership/isExpired">
244 <span tal:replace="membership/dateexpires/fmt:date" />244 Expired on
245 </td>245 <span tal:replace="membership/dateexpires/fmt:date" />
246 <td246 </td>
247 tal:condition="not: membership/isExpired"247 <td
248 tal:content="membership/status/title" />248 tal:condition="not: membership/isExpired"
249 <td tal:condition="user_can_edit_memberships">249 tal:content="membership/status/title" />
250 <a tal:attributes="href membership/fmt:url"250 <td tal:condition="user_can_edit_memberships">
251 ><img src="/@@/edit"251 <a tal:attributes="href membership/fmt:url"
252 title="Change membership duration or status" /></a>252 ><img src="/@@/edit"
253 </td>253 title="Change membership duration or status" /></a>
254 </tal:block>254 </td>
255 </tr>255 </tal:block>
256 </tbody>256 </tr>
257 </table>257 </tbody>
258 <div class="lesser">258 </table>
259 <tal:navigation259 <div class="lesser">
260 content="structure view/inactive_memberships/@@+navigation-links-lower" />260 <tal:navigation
261 content="structure view/inactive_memberships/@@+navigation-links-lower" />
262 </div>
261 </div>263 </div>
262 </li>264 </li>
263 </ul>265 </ul>
264266
=== 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 @@
1515
16 <div id="membership-summary" class="portletBody portletContent"16 <div id="membership-summary" class="portletBody portletContent"
17 style="margin-bottom: 1.5em;">17 style="margin-bottom: 1.5em;">
18 <div id="membership-summary">18 <div>
19 <div>19 <div id="membership-counts">
20 <img src="/@@/team" alt="team" />20 <img src="/@@/team" alt="team" />
21 <strong id="member-count">21 <strong id="approved-member-count">
22 <tal:active content="context/all_member_count" />22 <tal:active content="context/all_member_count" />
23 </strong>23 </strong>
24 <a tal:attributes="href string:${context/fmt:url/+members}#active"24 <a tal:attributes="href string:${context/fmt:url/+members}#active"
25 >active members</a><tal:invited25 >active members</a><tal:invited
26 define="invited_member_count context/invited_member_count"26 define="invited_member_count context/invited_member_count"
27 condition="invited_member_count">,27 condition="invited_member_count">,
28 <strong>28 <strong id="invited-member-count">
29 <tal:invited_count content="context/invited_member_count" />29 <tal:invited_count content="context/invited_member_count" />
30 </strong>30 </strong>
31 <a tal:attributes="href string:${context/fmt:url/+members}#invited"31 <a tal:attributes="href string:${context/fmt:url/+members}#invited"
32 >invited members</a></tal:invited><tal:proposed32 >invited members</a></tal:invited><tal:proposed
33 define="proposed_member_count context/proposed_member_count"33 define="proposed_member_count context/proposed_member_count"
34 condition="proposed_member_count">,34 condition="proposed_member_count">,
35 <strong>35 <strong id="proposed-member-count">
36 <tal:proposed_count content="proposed_member_count" />36 <tal:proposed_count content="proposed_member_count" />
37 </strong>37 </strong>
38 <a tal:attributes="href string:${context/fmt:url/+members}#proposed"38 <a tal:attributes="href string:${context/fmt:url/+members}#proposed"
@@ -95,37 +95,40 @@
95 <table style="margin: 0px 0px .5em 0px;">95 <table style="margin: 0px 0px .5em 0px;">
96 <tr>96 <tr>
97 <td style="padding: 0px 1em 1em 0px;"97 <td style="padding: 0px 1em 1em 0px;"
98 tal:define="link context/menu:overview/add_member">98 tal:define="link context/menu:overview/add_member"
99 <span id="add-member-spinner" class="update-in-progress-message"99 tal:condition="link/enabled">
100 style="display: none">100 <span id="add-member-spinner"
101 class="unseen update-in-progress-message">
101 Saving...102 Saving...
102 </span>103 </span>
103 <tal:add-member replace="structure link/fmt:link-icon" />104 <tal:add-member replace="structure link/fmt:link-icon" />
104 </td>105 </td>
105 <td style="padding: 0px 0px 1em 0px;"106 <td colspan="2"
107 style="padding: 0px 0px 1em 0px;"
106 tal:define="link context/menu:overview/mugshots">108 tal:define="link context/menu:overview/mugshots">
107 <tal:mugshots replace="structure link/fmt:link-icon" />109 <tal:mugshots replace="structure link/fmt:link-icon" />
108 </td>110 </td>
109 </tr>111 </tr>
112 </table>
113 <table>
110 <tr>114 <tr>
111 <td style="padding: 3px 3em 0px 0px;">115 <td id="recently-approved"
112 <div id="recently-approved"116 style="padding-right: 1em"
113 tal:attributes="style view/recently_approved_hidden">117 tal:attributes="class view/recently_approved_hidden">
114 <h3 style="color:black; font-weight:bold; margin: 0px">118 <h3 style="color:black; font-weight:bold; margin: 0px">
115 Latest members119 Latest members
116 </h3>120 </h3>
117 <ul id="recently-approved-ul">121 <ul id="recently-approved-ul">
118 <li tal:repeat="person view/recently_approved_members"122 <li tal:repeat="person view/recently_approved_members"
119 tal:content="structure person/fmt:link" />123 tal:content="structure person/fmt:link" />
120 </ul>124 </ul>
121 </div>
122 </td>125 </td>
123 <td style="padding: 0px;" id="recently-applied"126 <td id="recently-proposed"
124 tal:condition="view/recently_proposed_members">127 tal:attributes="class view/recently_proposed_hidden">
125 <h3 style="color:black; font-weight:bold; margin: 0px">128 <h3 style="color:black; font-weight:bold; margin: 0px">
126 Pending approval129 Pending approval
127 </h3>130 </h3>
128 <ul>131 <ul id="recently-proposed-ul">
129 <li tal:repeat="person view/recently_proposed_members"132 <li tal:repeat="person view/recently_proposed_members"
130 tal:content="structure person/fmt:link" />133 tal:content="structure person/fmt:link" />
131 </ul>134 </ul>
@@ -136,6 +139,16 @@
136 </td>139 </td>
137 </tr>140 </tr>
138 </table>141 </table>
142 <div id="recently-invited"
143 tal:attributes="class view/recently_invited_hidden">
144 <h3 style="color:black; font-weight:bold; margin: 0px">
145 Latest invited
146 </h3>
147 <ul id="recently-invited-ul">
148 <li tal:repeat="person view/recently_invited_members"
149 tal:content="structure person/fmt:link" />
150 </ul>
151 </div>
139 </tal:can-view>152 </tal:can-view>
140</div>153</div>
141</tal:root>154</tal:root>
142155
=== 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 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Test team index page."""
5
6__metaclass__ = type
7__all__ = []
8
9import unittest
10
11from windmill.authoring import WindmillTestClient
12
13from canonical.launchpad.windmill.testing import lpuser
14from canonical.launchpad.windmill.testing.widgets import (
15 search_and_select_picker_widget)
16
17from lp.registry.windmill.testing import RegistryWindmillLayer
18from lp.testing import TestCaseWithFactory
19
20
21class TestTeamIndex(TestCaseWithFactory):
22 """Test team index page."""
23
24 layer = RegistryWindmillLayer
25
26 def setUp(self):
27 self.client = WindmillTestClient(__name__)
28
29 def test_addmember(self):
30 self.client.open(
31 url=u'http://launchpad.dev:8085/~testing-spanish-team')
32
33 lpuser.TRANSLATIONS_ADMIN.ensure_login(self.client)
34
35 addmember_xpath = (
36 '//*[@id="membership"]' +
37 '//a[text()="Add member" ' +
38 'and contains(@class, "js-action")]')
39 # Add rosetta-admins team as a member.
40 approved_count_xpath = '//*[@id="approved-member-count"]'
41 code = "lookupNode({xpath: '%s'}).textContent" % approved_count_xpath
42 result = self.client.commands.execJS(code=code)
43 old_approved_count = int(result['result'])
44
45 self.client.waits.forElement(xpath=addmember_xpath)
46 self.client.click(xpath=addmember_xpath)
47 # Xpath is 1-indexed.
48 search_and_select_picker_widget(self.client, 'rosetta', 1)
49
50 self.client.waits.forElement(
51 xpath='//ul[@id="recently-approved-ul"]/li[1]/a',
52 option='href|~rosetta-admins')
53
54 self.client.asserts.assertText(
55 xpath=approved_count_xpath,
56 validator=str(old_approved_count+1))
57
58 # Add another team as a member.
59 invited_count_xpath = '//*[@id="invited-member-count"]'
60 self.client.asserts.assertNotNode(xpath=invited_count_xpath)
61 self.client.click(xpath=addmember_xpath)
62 search_and_select_picker_widget(self.client, 'simple', 1)
63
64 self.client.waits.forElement(
65 xpath='//ul[@id="recently-invited-ul"]/li[1]/a',
66 option='href|~simple-team')
67
68 self.client.asserts.assertText(
69 xpath=invited_count_xpath,
70 validator="1")
71
72
73def test_suite():
74 return unittest.TestLoader().loadTestsFromName(__name__)