Merge lp://staging/~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1 into lp://staging/launchpad
- bug-482176-add-team-member-ajax-part1
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | not available |
Proposed branch: | lp://staging/~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1 |
Merge into: | lp://staging/launchpad |
Prerequisite: | lp://staging/launchpad/db-devel |
Diff against target: |
830 lines (+294/-82) 17 files modified
lib/canonical/launchpad/javascript/bugs/bugtask-index.js (+4/-37) lib/canonical/launchpad/javascript/code/codereview.js (+9/-1) lib/canonical/launchpad/javascript/lp/picker.js (+5/-2) lib/canonical/launchpad/javascript/registry/team.js (+102/-0) lib/lp/app/templates/base-layout-macros.pt (+2/-0) lib/lp/bugs/tests/test_bugs_webservice.py (+0/-10) lib/lp/registry/browser/configure.zcml (+3/-0) lib/lp/registry/browser/person.py (+43/-12) lib/lp/registry/browser/tests/teammembership-views.txt (+1/-0) lib/lp/registry/browser/tests/test_person_webservice.py (+38/-0) lib/lp/registry/doc/teammembership-email-notification.txt (+6/-0) lib/lp/registry/doc/teammembership.txt (+39/-2) lib/lp/registry/interfaces/teammembership.py (+2/-0) lib/lp/registry/model/person.py (+3/-1) lib/lp/registry/model/teammembership.py (+4/-5) lib/lp/registry/templates/team-index.pt (+10/-0) lib/lp/registry/templates/team-portlet-membership.pt (+23/-12) |
To merge this branch: | bzr merge lp://staging/~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part1 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | code | Approve | |
Review via email: mp+16226@code.staging.launchpad.net |
Commit message
Description of the change
Edwin Grubbs (edwin-grubbs) wrote : | # |
Gavin Panella (allenap) wrote : | # |
Hi Edwin,
I've put a lot of comments in the diff below, but none of them are
critical, and a lot of them are suggestions. This is a nice new
feature that works well :)
review approve code
merge approve
Gavin.
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -239,21 +239,6 @@
>
>
> /*
> - * Clear the subscribe someone else picker.
> - *
> - * @method clear_picker
> - * @param e {Object} The event object.
> - */
> -function clear_picker(e) {
> - var input = Y.one('
> - input.set('value', '');
> - this.set('error', '');
> - this.set('results', [{}]);
> - this._results_
> - this.set('batches', []);
> -}
> -
> -/*
> * Initialize click handler for the subscribe someone else link.
> *
> * @method setup_subscribe
> @@ -262,23 +247,14 @@
> function setup_subscribe
> var config = {
> header: 'Subscribe someone else',
> - step_title: 'Search'
> + step_title: 'Search',
> + picker_activator: '.menu-
> };
>
> var picker = Y.lp.picker.create(
> 'ValidPersonOrT
> function(result) { subscribe_
> config);
> - // Clear results and search terms on cancel or save.
> - picker.on('save', clear_picker, picker);
> - picker.on('cancel', clear_picker, picker);
> -
> - var subscription_
> - subscription_
> - e.halt();
> - picker.show();
> - });
> - subscription_
> }
>
> /*
> @@ -626,21 +602,12 @@
> if (Y.Lang.
> var config = {
> header: 'Link a related branch',
> - step_title: 'Search'
> + step_title: 'Search',
> + picker_activator: '.menu-
> };
>
> var picker = Y.lp.picker.create(
> 'Branch', get_branch_
> -
> - // Clear results and search terms on cancel or save.
> - picker.on('save', clear_picker, picker);
> - picker.on('cancel', clear_picker, picker);
> -
> - link_branch_
> - e.halt();
> - picker.show();
> - });
> - link_branch_
> }
> }
Thanks for fixing this up.
>
>
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -22,6 +22,14 @@
> var link = Y.one('
> if (link !== null) {
> link.addClass(
> + /* XXX: salgado, 2009-11-11: This will cause the picker to be
> + * rec...
Preview Diff
1 | === modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js' |
2 | --- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-10 20:59:49 +0000 |
3 | +++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-12-16 02:39:23 +0000 |
4 | @@ -239,21 +239,6 @@ |
5 | |
6 | |
7 | /* |
8 | - * Clear the subscribe someone else picker. |
9 | - * |
10 | - * @method clear_picker |
11 | - * @param e {Object} The event object. |
12 | - */ |
13 | -function clear_picker(e) { |
14 | - var input = Y.one('.yui-picker-search-box input'); |
15 | - input.set('value', ''); |
16 | - this.set('error', ''); |
17 | - this.set('results', [{}]); |
18 | - this._results_box.set('innerHTML', ''); |
19 | - this.set('batches', []); |
20 | -} |
21 | - |
22 | -/* |
23 | * Initialize click handler for the subscribe someone else link. |
24 | * |
25 | * @method setup_subscribe_someone_else_handler |
26 | @@ -262,23 +247,14 @@ |
27 | function setup_subscribe_someone_else_handler(subscription) { |
28 | var config = { |
29 | header: 'Subscribe someone else', |
30 | - step_title: 'Search' |
31 | + step_title: 'Search', |
32 | + picker_activator: '.menu-link-addsubscriber' |
33 | }; |
34 | |
35 | var picker = Y.lp.picker.create( |
36 | 'ValidPersonOrTeam', |
37 | function(result) { subscribe_someone_else(result, subscription); }, |
38 | config); |
39 | - // Clear results and search terms on cancel or save. |
40 | - picker.on('save', clear_picker, picker); |
41 | - picker.on('cancel', clear_picker, picker); |
42 | - |
43 | - var subscription_link_someone_else = Y.one('.menu-link-addsubscriber'); |
44 | - subscription_link_someone_else.on('click', function(e) { |
45 | - e.halt(); |
46 | - picker.show(); |
47 | - }); |
48 | - subscription_link_someone_else.addClass('js-action'); |
49 | } |
50 | |
51 | /* |
52 | @@ -626,21 +602,12 @@ |
53 | if (Y.Lang.isValue(link_branch_link)) { |
54 | var config = { |
55 | header: 'Link a related branch', |
56 | - step_title: 'Search' |
57 | + step_title: 'Search', |
58 | + picker_activator: '.menu-link-addbranch' |
59 | }; |
60 | |
61 | var picker = Y.lp.picker.create( |
62 | 'Branch', get_branch_and_link_to_bug, config); |
63 | - |
64 | - // Clear results and search terms on cancel or save. |
65 | - picker.on('save', clear_picker, picker); |
66 | - picker.on('cancel', clear_picker, picker); |
67 | - |
68 | - link_branch_link.on('click', function(e) { |
69 | - e.halt(); |
70 | - picker.show(); |
71 | - }); |
72 | - link_branch_link.addClass('js-action'); |
73 | } |
74 | } |
75 | |
76 | |
77 | === modified file 'lib/canonical/launchpad/javascript/code/codereview.js' |
78 | --- lib/canonical/launchpad/javascript/code/codereview.js 2009-11-30 23:51:34 +0000 |
79 | +++ lib/canonical/launchpad/javascript/code/codereview.js 2009-12-16 02:39:23 +0000 |
80 | @@ -22,6 +22,14 @@ |
81 | var link = Y.one('#request-review'); |
82 | if (link !== null) { |
83 | link.addClass('js-action'); |
84 | + /* XXX: salgado, 2009-11-11: This will cause the picker to be |
85 | + * recreated every time the user clicks on the link. Although that |
86 | + * makes it unnecessary to have the widget cleared, it makes it |
87 | + * impossible to persist the state of the picker between clicks on the |
88 | + * link. We should probably have a policy to enforce that we |
89 | + * just hide/show widgets when a link is clicked more than once, |
90 | + * instead of recreating the widgets every time. |
91 | + */ |
92 | link.on('click', show_request_review_form); |
93 | } |
94 | link = Y.one('.menu-link-set_commit_message'); |
95 | @@ -51,7 +59,7 @@ |
96 | */ |
97 | function commit_message_listener(message, saved) |
98 | { |
99 | - if (message == '') { |
100 | + if (message === '') { |
101 | // Hide the multiline editor |
102 | Y.one('#edit-commit-message').addClass('unseen'); |
103 | // Show the link again |
104 | |
105 | === modified file 'lib/canonical/launchpad/javascript/lp/picker.js' |
106 | --- lib/canonical/launchpad/javascript/lp/picker.js 2009-12-01 15:11:51 +0000 |
107 | +++ lib/canonical/launchpad/javascript/lp/picker.js 2009-12-16 02:39:23 +0000 |
108 | @@ -15,6 +15,8 @@ |
109 | * @param {String} resource_uri The object being modified. |
110 | * @param {String} attribute_name The attribute on the resource being |
111 | * modified. |
112 | + * @param {Bool} show_remove_button Should the remove button be shown? |
113 | + * @param {Bool} show_assign_me_button Should the 'assign me' button be shown? |
114 | * @param {String} content_box_id |
115 | * @param {Object} config Object literal of config name/value pairs. |
116 | * config.header is a line of text at the top of |
117 | @@ -219,10 +221,10 @@ |
118 | "string: " + vocabulary); |
119 | } |
120 | |
121 | - var picker = new Y.Picker({ |
122 | + var new_config = Y.merge(config, { |
123 | align: { |
124 | points: [Y.WidgetPositionExt.CC, |
125 | - Y.WidgetPositionExt.CC] |
126 | + Y.WidgetPositionExt.CC] |
127 | }, |
128 | progressbar: true, |
129 | progress: 100, |
130 | @@ -231,6 +233,7 @@ |
131 | zIndex: 1000, |
132 | visible: false |
133 | }); |
134 | + var picker = new Y.Picker(new_config); |
135 | |
136 | picker.subscribe('save', function (e) { |
137 | Y.log('Got save event.'); |
138 | |
139 | === added file 'lib/canonical/launchpad/javascript/registry/team.js' |
140 | --- lib/canonical/launchpad/javascript/registry/team.js 1970-01-01 00:00:00 +0000 |
141 | +++ lib/canonical/launchpad/javascript/registry/team.js 2009-12-16 02:39:23 +0000 |
142 | @@ -0,0 +1,102 @@ |
143 | +/** Copyright (c) 2009, Canonical Ltd. All rights reserved. |
144 | + * |
145 | + * Objects for subscription handling. |
146 | + * |
147 | + * @module lp.subscriber |
148 | + */ |
149 | + |
150 | +YUI.add('registry.team', function(Y) { |
151 | + |
152 | +var module = Y.namespace('registry.team'); |
153 | + |
154 | +/* |
155 | + * Initialize click handler for the add member link |
156 | + * |
157 | + * @method setup_add_member_handler |
158 | + */ |
159 | +module.setup_add_member_handler = function() { |
160 | + var config = { |
161 | + header: 'Add a member', |
162 | + step_title: 'Search', |
163 | + picker_activator: '.menu-link-add_member' |
164 | + }; |
165 | + |
166 | + var picker = Y.lp.picker.create( |
167 | + 'ValidTeamMember', |
168 | + function(result) { _add_member(result); }, |
169 | + config); |
170 | +}; |
171 | + |
172 | +var _add_member = function(result) { |
173 | + var spinner = Y.one('#add-member-spinner'); |
174 | + var addmember_link = Y.one('.menu-link-add_member'); |
175 | + addmember_link.setStyle('display', 'none'); |
176 | + spinner.setStyle('display', 'inline'); |
177 | + function disable_spinner() { |
178 | + addmember_link.setStyle('display', 'inline'); |
179 | + spinner.setStyle('display', 'none'); |
180 | + } |
181 | + lp_client = new LP.client.Launchpad(); |
182 | + |
183 | + var error_handler = new LP.client.ErrorHandler(); |
184 | + error_handler.clearProgressUI = disable_spinner; |
185 | + error_handler.showError = function(error_msg) { |
186 | + Y.lp.display_error(Y.one('.menu-link-add_member'), error_msg); |
187 | + }; |
188 | + |
189 | + config = { |
190 | + on: { |
191 | + success: function(member_added) { |
192 | + if (!member_added) { |
193 | + disable_spinner(); |
194 | + alert('Already a member.'); |
195 | + return; |
196 | + } |
197 | + if (result.css.match("team")) { |
198 | + disable_spinner(); |
199 | + alert('This is a team'); |
200 | + return; |
201 | + } |
202 | + var members_section = Y.one('#recently-approved'); |
203 | + var members_ul = Y.one('#recently-approved-ul'); |
204 | + var first_node = members_ul.get('firstChild'); |
205 | + config = { |
206 | + on: { |
207 | + success: function(person_html) { |
208 | + var total_members = Y.one( |
209 | + '#member-count').get('innerHTML'); |
210 | + total_members = parseInt(total_members, 10) + 1; |
211 | + Y.one('#member-count').set( |
212 | + 'innerHTML', total_members); |
213 | + person_repr = Y.Node.create( |
214 | + '<li>' + person_html + '</li>'); |
215 | + members_section.setStyle('visibility', 'visible'); |
216 | + members_ul.insertBefore( |
217 | + person_repr, first_node); |
218 | + anim = Y.lazr.anim.green_flash( |
219 | + {node: person_repr}); |
220 | + anim.run(); |
221 | + disable_spinner(); |
222 | + }, |
223 | + failure: error_handler.getFailureHandler() |
224 | + }, |
225 | + accept: LP.client.XHTML |
226 | + }; |
227 | + lp_client.get(result.api_uri, config); |
228 | + }, |
229 | + failure: error_handler.getFailureHandler() |
230 | + }, |
231 | + parameters: { |
232 | + // XXX: Why do I always have to get absolute URIs out of the URIs |
233 | + // in the picker's result/client.links? |
234 | + reviewer: LP.client.get_absolute_uri(LP.client.links.me), |
235 | + person: LP.client.get_absolute_uri(result.api_uri) |
236 | + } |
237 | + }; |
238 | + |
239 | + lp_client.named_post( |
240 | + LP.client.cache.context.self_link, 'addMember', config); |
241 | +}; |
242 | + |
243 | +}, '0.1', {requires: [ |
244 | + 'node', 'lazr.anim', 'lp.picker', 'lp.errors', 'lp.client.plugins']}); |
245 | |
246 | === modified file 'lib/lp/app/templates/base-layout-macros.pt' |
247 | --- lib/lp/app/templates/base-layout-macros.pt 2009-12-08 16:43:44 +0000 |
248 | +++ lib/lp/app/templates/base-layout-macros.pt 2009-12-16 02:39:23 +0000 |
249 | @@ -181,6 +181,8 @@ |
250 | tal:attributes="src string:${lp_js}/lp/comment.js"></script> |
251 | <script type="text/javascript" |
252 | tal:attributes="src string:${lp_js}/lp/errors.js"></script> |
253 | + <script type="text/javascript" |
254 | + tal:attributes="src string:${lp_js}/registry/team.js"></script> |
255 | |
256 | </tal:devmode> |
257 | <tal:production condition="not:devmode"> |
258 | |
259 | === modified file 'lib/lp/bugs/tests/test_bugs_webservice.py' |
260 | --- lib/lp/bugs/tests/test_bugs_webservice.py 2009-12-01 12:21:56 +0000 |
261 | +++ lib/lp/bugs/tests/test_bugs_webservice.py 2009-12-16 02:39:22 +0000 |
262 | @@ -120,16 +120,6 @@ |
263 | self.assertEqual(response.status, 200) |
264 | |
265 | rendered_comment = response.body |
266 | - # XXX Bjorn Tillenius 2009-05-15 bug=377003 |
267 | - # The current request is a web service request when rendering |
268 | - # the HTML, causing canonical_url to produce links pointing to the |
269 | - # web service. Adjust the test to compensate for this, and accept |
270 | - # that the links will be incorrect for now. We should fix this |
271 | - # before using it for anything useful. |
272 | - rendered_comment = rendered_comment.replace( |
273 | - 'http://api.launchpad.dev/beta/', |
274 | - 'http://launchpad.dev/') |
275 | - |
276 | self.assertRenderedCommentsEqual( |
277 | rendered_comment, self.expected_comment_html) |
278 | |
279 | |
280 | === modified file 'lib/lp/registry/browser/configure.zcml' |
281 | --- lib/lp/registry/browser/configure.zcml 2009-12-05 18:37:28 +0000 |
282 | +++ lib/lp/registry/browser/configure.zcml 2009-12-16 02:39:22 +0000 |
283 | @@ -751,6 +751,9 @@ |
284 | for="lp.registry.interfaces.person.IPerson" |
285 | layer="canonical.launchpad.layers.BugsLayer" |
286 | name="+bugs"/> |
287 | + <adapter |
288 | + factory="lp.registry.browser.person.PersonXHTMLRepresentation" |
289 | + name="lazr.restful.EntryResource" /> |
290 | <browser:page |
291 | name="+review" |
292 | for="lp.registry.interfaces.person.IPerson" |
293 | |
294 | === modified file 'lib/lp/registry/browser/person.py' |
295 | --- lib/lp/registry/browser/person.py 2009-12-08 10:20:37 +0000 |
296 | +++ lib/lp/registry/browser/person.py 2009-12-16 02:39:23 +0000 |
297 | @@ -102,7 +102,7 @@ |
298 | from zope.interface import classImplements, implements, Interface |
299 | from zope.interface.exceptions import Invalid |
300 | from zope.interface.interface import invariant |
301 | -from zope.component import getUtility |
302 | +from zope.component import adapts, getUtility |
303 | from zope.publisher.interfaces import NotFound |
304 | from zope.publisher.interfaces.browser import IBrowserPublisher |
305 | from zope.schema import Bool, Choice, List, Text, TextLine |
306 | @@ -117,6 +117,7 @@ |
307 | from lazr.delegates import delegates |
308 | from lazr.config import as_timedelta |
309 | from lazr.restful.interface import copy_field, use_template |
310 | +from lazr.restful.interfaces import IWebServiceClientRequest |
311 | from canonical.lazr.utils import safe_hasattr |
312 | from canonical.database.sqlbase import flush_database_updates |
313 | |
314 | @@ -226,7 +227,8 @@ |
315 | logoutPerson, allowUnauthenticatedSession) |
316 | from canonical.launchpad.webapp.menu import get_current_view |
317 | from canonical.launchpad.webapp.publisher import LaunchpadView |
318 | -from canonical.launchpad.webapp.tales import DateTimeFormatterAPI |
319 | +from canonical.launchpad.webapp.tales import ( |
320 | + DateTimeFormatterAPI, PersonFormatterAPI) |
321 | from lazr.uri import URI, InvalidURIError |
322 | |
323 | from canonical.launchpad import _ |
324 | @@ -2460,7 +2462,7 @@ |
325 | |
326 | @property |
327 | def next_url(self): |
328 | - """Redirect back to the +languages page if request originated there.""" |
329 | + """Redirect back to the originating page.""" |
330 | redirection_url = self.request.get('redirection_url') |
331 | if redirection_url: |
332 | return redirection_url |
333 | @@ -2468,7 +2470,7 @@ |
334 | |
335 | @property |
336 | def cancel_url(self): |
337 | - """Redirect back to the +languages page if request originated there.""" |
338 | + """Redirect back to the originating page.""" |
339 | redirection_url = self.getRedirectionURL() |
340 | if redirection_url: |
341 | return redirection_url |
342 | @@ -2676,12 +2678,29 @@ |
343 | orderBy='-TeamMembership.date_proposed') |
344 | return members[:5] |
345 | |
346 | - @cachedproperty |
347 | - def has_recent_approved_or_proposed_members(self): |
348 | - """Does the team have recently approved or proposed members?""" |
349 | - approved = self.recently_approved_members.count() > 0 |
350 | - proposed = self.recently_proposed_members.count() > 0 |
351 | - return approved or proposed |
352 | + @property |
353 | + def recently_approved_hidden(self): |
354 | + """Optionally hide the div. |
355 | + |
356 | + The AJAX on the page needs the elements to be present |
357 | + but hidden in case it adds a member to the list. |
358 | + """ |
359 | + if self.recently_approved_members.count() == 0: |
360 | + return 'visibility: collapse' |
361 | + else: |
362 | + return '' |
363 | + |
364 | + @property |
365 | + def recently_proposed_hidden(self): |
366 | + """Optionally hide the div. |
367 | + |
368 | + The AJAX on the page needs the elements to be present |
369 | + but hidden in case it adds a member to the list. |
370 | + """ |
371 | + if self.recently_proposed_members.count() == 0: |
372 | + return 'visibility: collapse' |
373 | + else: |
374 | + return '' |
375 | |
376 | @cachedproperty |
377 | def openpolls(self): |
378 | @@ -5826,8 +5845,7 @@ |
379 | usedfor = ITeamIndexMenu |
380 | facet = 'overview' |
381 | title = 'Change team' |
382 | - links = ('edit', 'join', 'add_member', 'add_my_teams', |
383 | - 'leave') |
384 | + links = ('edit', 'join', 'add_my_teams', 'leave') |
385 | |
386 | |
387 | class TeamEditMenu(TeamNavigationMenuBase): |
388 | @@ -5843,3 +5861,16 @@ |
389 | classImplements(TeamIndexView, ITeamIndexMenu) |
390 | classImplements(TeamEditView, ITeamEditMenu) |
391 | classImplements(PersonIndexView, IPersonIndexMenu) |
392 | + |
393 | + |
394 | +class PersonXHTMLRepresentation: |
395 | + adapts(IPerson, IWebServiceClientRequest) |
396 | + implements(Interface) |
397 | + |
398 | + def __init__(self, person, request): |
399 | + self.person = person |
400 | + self.request = request |
401 | + |
402 | + def __call__(self): |
403 | + """Render `Person` as XHTML using the webservice.""" |
404 | + return PersonFormatterAPI(self.person).link(None) |
405 | |
406 | === modified file 'lib/lp/registry/browser/tests/teammembership-views.txt' |
407 | --- lib/lp/registry/browser/tests/teammembership-views.txt 2009-11-13 13:06:50 +0000 |
408 | +++ lib/lp/registry/browser/tests/teammembership-views.txt 2009-12-16 02:39:23 +0000 |
409 | @@ -63,6 +63,7 @@ |
410 | |
411 | >>> login_person(team_owner) |
412 | >>> super_team.addMember(team, team_owner) |
413 | + True |
414 | >>> membership = membership_set.getByPersonAndTeam(team, super_team) |
415 | >>> login_person(team.teamowner) |
416 | >>> view = TeamInvitationView(membership, request) |
417 | |
418 | === added file 'lib/lp/registry/browser/tests/test_person_webservice.py' |
419 | --- lib/lp/registry/browser/tests/test_person_webservice.py 1970-01-01 00:00:00 +0000 |
420 | +++ lib/lp/registry/browser/tests/test_person_webservice.py 2009-12-16 02:39:22 +0000 |
421 | @@ -0,0 +1,38 @@ |
422 | +# Copyright 2009 Canonical Ltd. This software is licensed under the |
423 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
424 | + |
425 | +__metaclass__ = type |
426 | + |
427 | +import unittest |
428 | + |
429 | +from canonical.launchpad.ftests import login |
430 | +from lp.testing import TestCaseWithFactory |
431 | +from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller |
432 | +from canonical.testing import DatabaseFunctionalLayer |
433 | + |
434 | + |
435 | +class TestPersonRepresentation(TestCaseWithFactory): |
436 | + layer = DatabaseFunctionalLayer |
437 | + |
438 | + def setUp(self): |
439 | + TestCaseWithFactory.setUp(self) |
440 | + login('guilherme.salgado@canonical.com ') |
441 | + self.person = self.factory.makePerson( |
442 | + name='test-person', displayname='Test Person') |
443 | + self.webservice = LaunchpadWebServiceCaller( |
444 | + 'launchpad-library', 'salgado-change-anything') |
445 | + |
446 | + def test_GET_xhtml_representation(self): |
447 | + response = self.webservice.get( |
448 | + '/~%s' % self.person.name, 'application/xhtml+xml') |
449 | + |
450 | + self.assertEqual(response.status, 200) |
451 | + |
452 | + rendered_comment = response.body |
453 | + self.assertEquals( |
454 | + rendered_comment, |
455 | + '<a href="/~test-person" class="sprite person">Test Person</a>') |
456 | + |
457 | + |
458 | +def test_suite(): |
459 | + return unittest.TestLoader().loadTestsFromName(__name__) |
460 | |
461 | === modified file 'lib/lp/registry/doc/teammembership-email-notification.txt' |
462 | --- lib/lp/registry/doc/teammembership-email-notification.txt 2009-08-24 03:01:12 +0000 |
463 | +++ lib/lp/registry/doc/teammembership-email-notification.txt 2009-12-16 02:39:23 +0000 |
464 | @@ -226,6 +226,7 @@ |
465 | >>> cprov = personset.getByName('cprov') |
466 | >>> marilize = personset.getByName('marilize') |
467 | >>> ubuntu_team.addMember(marilize, reviewer=cprov) |
468 | + True |
469 | >>> transaction.commit() |
470 | >>> len(stub.test_emails) |
471 | 6 |
472 | @@ -275,6 +276,7 @@ |
473 | >>> mirror_admins.getTeamAdminsEmailAddresses() |
474 | ['mark@example.com'] |
475 | >>> ubuntu_team.addMember(mirror_admins, reviewer=cprov) |
476 | + True |
477 | >>> transaction.commit() |
478 | >>> len(stub.test_emails) |
479 | 1 |
480 | @@ -326,6 +328,7 @@ |
481 | |
482 | >>> landscape = personset.getByName('landscape-developers') |
483 | >>> ubuntu_team.addMember(landscape, reviewer=cprov) |
484 | + True |
485 | |
486 | # Reset stub.test_emails as we don't care about the notification triggered |
487 | # by the addMember() call. |
488 | @@ -359,6 +362,7 @@ |
489 | |
490 | >>> launchpad = personset.getByName('launchpad') |
491 | >>> ubuntu_team.addMember(launchpad, reviewer=cprov, force_team_add=True) |
492 | + True |
493 | >>> flush_database_updates() |
494 | >>> transaction.commit() |
495 | >>> len(stub.test_emails) |
496 | @@ -812,6 +816,7 @@ |
497 | >>> member = factory.makePerson( |
498 | ... name='team-member', email='team-member@example.com') |
499 | >>> team_one.addMember(member, owner) |
500 | + True |
501 | >>> print_distinct_emails() |
502 | From: Team One ... |
503 | ---------------------------------------- |
504 | @@ -838,6 +843,7 @@ |
505 | >>> team_two = factory.makeTeam( |
506 | ... name='team-two', email='team-two@example.com', owner=owner) |
507 | >>> team_one.addMember(team_two, owner, force_team_add=True) |
508 | + True |
509 | >>> print_distinct_emails() |
510 | From: Team One ... |
511 | ---------------------------------------- |
512 | |
513 | === modified file 'lib/lp/registry/doc/teammembership.txt' |
514 | --- lib/lp/registry/doc/teammembership.txt 2009-11-30 20:18:42 +0000 |
515 | +++ lib/lp/registry/doc/teammembership.txt 2009-12-16 02:39:22 +0000 |
516 | @@ -132,7 +132,6 @@ |
517 | Other users must use the join method if they are going to add themselves |
518 | to a team. |
519 | |
520 | - >>> from zope.security.interfaces import Unauthorized |
521 | >>> mark = personset.getByName('mark') |
522 | >>> t3.addMember(salgado, reviewer=mark, status=TeamMembershipStatus.ADMIN) |
523 | Traceback (most recent call last): |
524 | @@ -142,8 +141,12 @@ |
525 | # Log in as the team owner. |
526 | >>> login_person(t3.teamowner) |
527 | |
528 | +addMember returns True if the member got added (i.e. he wasn't already a |
529 | +member of the team). |
530 | + |
531 | >>> t3.addMember( |
532 | ... salgado, reviewer=mark, status=TeamMembershipStatus.ADMIN) |
533 | + True |
534 | >>> from canonical.launchpad.interfaces import ITeamMembershipSet |
535 | >>> membershipset = getUtility(ITeamMembershipSet) |
536 | >>> flush_database_updates() |
537 | @@ -155,13 +158,27 @@ |
538 | >>> salgado in t3.activemembers |
539 | True |
540 | |
541 | +addMember returns True also when the member is added as a proposed |
542 | +member. |
543 | + |
544 | >>> marilize = personset.getByName('marilize') |
545 | >>> t3.addMember( |
546 | ... marilize, reviewer=mark, status=TeamMembershipStatus.PROPOSED) |
547 | + True |
548 | >>> flush_database_updates() |
549 | >>> marilize in t3.activemembers |
550 | False |
551 | |
552 | +If addMember is called with a person that is already a member, it |
553 | +returns False. |
554 | + |
555 | + >>> t3.addMember( |
556 | + ... salgado, reviewer=mark, status=TeamMembershipStatus.ADMIN) |
557 | + False |
558 | + >>> t3.addMember( |
559 | + ... marilize, reviewer=mark, status=TeamMembershipStatus.PROPOSED) |
560 | + False |
561 | + |
562 | As expected, the membership object implements ITeamMembership. |
563 | |
564 | >>> from canonical.launchpad.webapp.testing import verifyObject |
565 | @@ -175,6 +192,7 @@ |
566 | invitation before the team is made a member. |
567 | |
568 | >>> t1.addMember(t2, reviewer) |
569 | + True |
570 | >>> membership = membershipset.getByPersonAndTeam(t2, t1) |
571 | >>> membership.status == TeamMembershipStatus.INVITED |
572 | True |
573 | @@ -192,6 +210,7 @@ |
574 | A team admin can also decline an invitation made to his team. |
575 | |
576 | >>> t2.addMember(t3, reviewer=mark) |
577 | + True |
578 | >>> login_person(t3.teamowner) |
579 | >>> t3.declineInvitationToBeMemberOf(t2, comment='something') |
580 | >>> membership = membershipset.getByPersonAndTeam(t3, t2) |
581 | @@ -205,6 +224,7 @@ |
582 | |
583 | >>> login_person(t3.teamowner) |
584 | >>> t2.addMember(t3, reviewer=mark, force_team_add=True) |
585 | + True |
586 | >>> [m.displayname for m in t2.allmembers] |
587 | [u'Foo Bar', u'Guilherme Salgado', u't3'] |
588 | |
589 | @@ -226,6 +246,7 @@ |
590 | Adding t2 as a member of t5 will add all t2 members as t5 members too. |
591 | |
592 | >>> t5.addMember(t2, reviewer, force_team_add=True) |
593 | + True |
594 | >>> [m.displayname for m in t5.allmembers] |
595 | [u'Foo Bar', u'Guilherme Salgado', u't2', u't3'] |
596 | |
597 | @@ -233,7 +254,9 @@ |
598 | members too. |
599 | |
600 | >>> t4.addMember(t5, reviewer, force_team_add=True) |
601 | + True |
602 | >>> t4.addMember(t1, reviewer, force_team_add=True) |
603 | + True |
604 | >>> [m.displayname for m in t4.allmembers] |
605 | [u'Foo Bar', u'Guilherme Salgado', u't1', u't2', u't3', u't5'] |
606 | |
607 | @@ -327,6 +350,7 @@ |
608 | |
609 | >>> cprov = getUtility(IPersonSet).getByName('cprov') |
610 | >>> t3.addMember(cprov, reviewer) |
611 | + True |
612 | >>> [m.displayname for m in t3.allmembers] |
613 | [u'Celso Providelo', u'Foo Bar'] |
614 | |
615 | @@ -391,15 +415,22 @@ |
616 | None |
617 | |
618 | When we approve his membership, the datejoined will contain the date that it |
619 | -was approved. |
620 | +was approved. It returns True to indicate that the status was changed. |
621 | |
622 | >>> membership.setStatus(TeamMembershipStatus.APPROVED, foobar) |
623 | + True |
624 | >>> print membership.status.title |
625 | Approved |
626 | >>> utc_now = datetime.now(pytz.timezone('UTC')) |
627 | >>> membership.datejoined.date() == utc_now.date() |
628 | True |
629 | |
630 | +If setStatus is called again with the same status, it returns False, |
631 | +to indicate that the status didn't change. |
632 | + |
633 | + >>> membership.setStatus(TeamMembershipStatus.APPROVED, foobar) |
634 | + False |
635 | + |
636 | Other status updates won't change datejoined, regardless of the status. |
637 | That's because datejoined stores the date in which the membership was first |
638 | made active. |
639 | @@ -415,6 +446,7 @@ |
640 | |
641 | >>> foobar_on_buildd.setStatus( |
642 | ... TeamMembershipStatus.DEACTIVATED, foobar) |
643 | + True |
644 | >>> print foobar_on_buildd.status.title |
645 | Deactivated |
646 | >>> foobar_on_buildd.datejoined <= utc_now |
647 | @@ -422,6 +454,7 @@ |
648 | |
649 | >>> foobar_on_buildd.setStatus( |
650 | ... TeamMembershipStatus.APPROVED, foobar) |
651 | + True |
652 | >>> print foobar_on_buildd.status.title |
653 | Approved |
654 | >>> foobar_on_buildd.datejoined <= utc_now |
655 | @@ -790,6 +823,7 @@ |
656 | >>> admins = getUtility(IPersonSet).getByName('admins') |
657 | >>> login_person(t1.teamowner) |
658 | >>> t1.addMember(admins, reviewer=t1.teamowner, force_team_add=True) |
659 | + True |
660 | >>> flush_database_updates() |
661 | >>> print '\n'.join(sorted( |
662 | ... team.name for team in salgado.teams_participated_in)) |
663 | @@ -804,6 +838,7 @@ |
664 | for Salgado. |
665 | |
666 | >>> admins.addMember(t2, reviewer=admins.teamowner, force_team_add=True) |
667 | + True |
668 | >>> flush_database_updates() |
669 | >>> print '\n'.join(sorted( |
670 | ... team.name for team in salgado.teams_participated_in)) |
671 | @@ -845,6 +880,7 @@ |
672 | Or changed: |
673 | |
674 | >>> membership.setStatus(TeamMembershipStatus.DEACTIVATED, mark) |
675 | + True |
676 | >>> no_priv._inTeam_cache |
677 | {} |
678 | >>> no_priv.inTeam(admins) |
679 | @@ -902,3 +938,4 @@ |
680 | >>> bad_membership.sendAutoRenewalNotification() |
681 | |
682 | >>> bad_membership.setStatus(TeamMembershipStatus.EXPIRED, bad_user) |
683 | + True |
684 | |
685 | === modified file 'lib/lp/registry/interfaces/teammembership.py' |
686 | --- lib/lp/registry/interfaces/teammembership.py 2009-06-25 04:06:00 +0000 |
687 | +++ lib/lp/registry/interfaces/teammembership.py 2009-12-16 02:39:23 +0000 |
688 | @@ -224,6 +224,8 @@ |
689 | transition. |
690 | |
691 | The given status must be different than the current status. |
692 | + |
693 | + Return True if the status got changed, otherwise False. |
694 | """ |
695 | |
696 | |
697 | |
698 | === modified file 'lib/lp/registry/model/person.py' |
699 | --- lib/lp/registry/model/person.py 2009-12-05 18:37:28 +0000 |
700 | +++ lib/lp/registry/model/person.py 2009-12-16 02:39:23 +0000 |
701 | @@ -1236,6 +1236,7 @@ |
702 | status = TeamMembershipStatus.INVITED |
703 | event = TeamInvitationEvent |
704 | |
705 | + member_added = True |
706 | expires = self.defaultexpirationdate |
707 | tm = TeamMembership.selectOneBy(person=person, team=self) |
708 | if tm is None: |
709 | @@ -1250,11 +1251,12 @@ |
710 | # We can't use tm.setExpirationDate() here because the reviewer |
711 | # here will be the member themselves when they join an OPEN team. |
712 | tm.dateexpires = expires |
713 | - tm.setStatus(status, reviewer, comment) |
714 | + member_added = tm.setStatus(status, reviewer, comment) |
715 | |
716 | if not person.is_team and may_subscribe_to_list: |
717 | person.autoSubscribeToMailingList(self.mailing_list, |
718 | requester=reviewer) |
719 | + return member_added |
720 | |
721 | # The three methods below are not in the IPerson interface because we want |
722 | # to protect them with a launchpad.Edit permission. We could do that by |
723 | |
724 | === modified file 'lib/lp/registry/model/teammembership.py' |
725 | --- lib/lp/registry/model/teammembership.py 2009-06-25 04:06:00 +0000 |
726 | +++ lib/lp/registry/model/teammembership.py 2009-12-16 02:39:23 +0000 |
727 | @@ -272,7 +272,7 @@ |
728 | def setStatus(self, status, user, comment=None): |
729 | """See `ITeamMembership`.""" |
730 | if status == self.status: |
731 | - return |
732 | + return False |
733 | |
734 | approved = TeamMembershipStatus.APPROVED |
735 | admin = TeamMembershipStatus.ADMIN |
736 | @@ -357,10 +357,9 @@ |
737 | # When a member proposes himself, a more detailed notification is |
738 | # sent to the team admins by a subscriber of JoinTeamEvent; that's |
739 | # why we don't send anything here. |
740 | - if self.person == self.last_changed_by and self.status == proposed: |
741 | - return |
742 | - |
743 | - self._sendStatusChangeNotification(old_status) |
744 | + if self.person != self.last_changed_by or self.status != proposed: |
745 | + self._sendStatusChangeNotification(old_status) |
746 | + return True |
747 | |
748 | def _sendStatusChangeNotification(self, old_status): |
749 | """Send a status change notification to all team admins and the |
750 | |
751 | === modified file 'lib/lp/registry/templates/team-index.pt' |
752 | --- lib/lp/registry/templates/team-index.pt 2009-10-26 21:12:49 +0000 |
753 | +++ lib/lp/registry/templates/team-index.pt 2009-12-16 02:39:22 +0000 |
754 | @@ -17,6 +17,16 @@ |
755 | rel="meta" type="application/rdf+xml" |
756 | title="FOAF" href="+rdf" |
757 | /> |
758 | + <script type="text/javascript" |
759 | + tal:content="string: |
760 | + YUI().use('registry.team', function(Y) { |
761 | + Y.on('load', |
762 | + function(e) { |
763 | + Y.registry.team.setup_add_member_handler(); |
764 | + }, |
765 | + window); |
766 | + }); |
767 | + "/> |
768 | </tal:block> |
769 | </head> |
770 | |
771 | |
772 | === modified file 'lib/lp/registry/templates/team-portlet-membership.pt' |
773 | --- lib/lp/registry/templates/team-portlet-membership.pt 2009-10-23 21:11:12 +0000 |
774 | +++ lib/lp/registry/templates/team-portlet-membership.pt 2009-12-16 02:39:23 +0000 |
775 | @@ -18,7 +18,9 @@ |
776 | <div id="membership-summary"> |
777 | <div> |
778 | <img src="/@@/team" alt="team" /> |
779 | - <strong><tal:active content="context/all_member_count" /></strong> |
780 | + <strong id="member-count"> |
781 | + <tal:active content="context/all_member_count" /> |
782 | + </strong> |
783 | <a tal:attributes="href string:${context/fmt:url/+members}#active" |
784 | >active members</a><tal:invited |
785 | define="invited_member_count context/invited_member_count" |
786 | @@ -90,24 +92,33 @@ |
787 | <tal:can-view |
788 | condition="context/@@+restricted-membership/userCanViewMembership" |
789 | define="overview_menu context/menu:overview"> |
790 | - <table style="margin: 0px 0px .5em 0px;" |
791 | - tal:condition="view/has_recent_approved_or_proposed_members"> |
792 | + <table style="margin: 0px 0px .5em 0px;"> |
793 | <tr> |
794 | + <td style="padding: 0px 1em 1em 0px;" |
795 | + tal:define="link context/menu:overview/add_member"> |
796 | + <span id="add-member-spinner" class="update-in-progress-message" |
797 | + style="display: none"> |
798 | + Saving... |
799 | + </span> |
800 | + <tal:add-member replace="structure link/fmt:link-icon" /> |
801 | + </td> |
802 | <td style="padding: 0px 0px 1em 0px;" |
803 | tal:define="link context/menu:overview/mugshots"> |
804 | <tal:mugshots replace="structure link/fmt:link-icon" /> |
805 | </td> |
806 | </tr> |
807 | <tr> |
808 | - <td style="padding: 3px 3em 0px 0px;" id="recently-approved" |
809 | - tal:condition="view/recently_approved_members"> |
810 | - <h3 style="color:black; font-weight:bold; margin: 0px"> |
811 | - Recently approved |
812 | - </h3> |
813 | - <ul tal:condition="view/recently_approved_members"> |
814 | - <li tal:repeat="person view/recently_approved_members" |
815 | - tal:content="structure person/fmt:link" /> |
816 | - </ul> |
817 | + <td style="padding: 3px 3em 0px 0px;"> |
818 | + <div id="recently-approved" |
819 | + tal:attributes="style view/recently_approved_hidden"> |
820 | + <h3 style="color:black; font-weight:bold; margin: 0px"> |
821 | + Latest members |
822 | + </h3> |
823 | + <ul id="recently-approved-ul"> |
824 | + <li tal:repeat="person view/recently_approved_members" |
825 | + tal:content="structure person/fmt:link" /> |
826 | + </ul> |
827 | + </div> |
828 | </td> |
829 | <td style="padding: 0px;" id="recently-applied" |
830 | tal:condition="view/recently_proposed_members"> |
Summary
-------
This is the first branch on the way to adding a picker
to add members to a team with a link on its index page.
Salgado started this branch. I did a little bit of cleanup, but
the following things should be done in a followup branch.
* Add windmill test.
* Handle adding teams, which will go into a PROPOSED or INVITED status.
Implementation details ------- ------- -
-------
The picker widget now clears the search term by default when canonical/ launchpad/ javascript/ bugs/bugtask- index.js canonical/ launchpad/ javascript/ code/codereview .js
the SAVE event is processed (when an item is selected from the results).
The picker widget will also add an onclick handler for you.
lib/
lib/
The main part of this feature: canonical/ launchpad/ javascript/ registry/ team.js lp/app/ templates/ base-layout- macros. pt lp/registry/ browser/ configure. zcml lp/registry/ browser/ person. py lp/registry/ templates/ team-index. pt lp/registry/ browser/ tests/test_ person_ webservice. py lp/registry/ templates/ team-portlet- membership. pt
lib/
lib/
lib/
lib/
lib/
lib/
lib/
Change affecting existing tests: lp/registry/ model/person. py lp/registry/ model/teammembe rship.py lp/registry/ browser/ tests/teammembe rship-views. txt lp/registry/ doc/teammembers hip-email- notification. txt lp/registry/ doc/teammembers hip.txt
lib/
lib/
lib/
lib/
lib/
Tests
-----
./bin/test -vv -t 'test_person_ webservice| teammembership- views.txt| teammembership- email-notificat ion.txt| /teammembership .txt'
Demo and Q/A
------------
* Open http:// launchpad. dev/~guadamen
* The "Add member" link should be green, click on it to show the picker.
* When you click on a person in the picker, the progress spinner
should display where the (+) icon was.
* Then there should be a green flash where the person is added to the
"Latest members" list.