Merge lp:~sinzui/launchpad/closed-teams-0 into lp:launchpad
- closed-teams-0
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12018 |
Proposed branch: | lp:~sinzui/launchpad/closed-teams-0 |
Merge into: | lp:launchpad |
Diff against target: |
762 lines (+264/-144) 15 files modified
lib/canonical/launchpad/vocabularies/dbobjects.py (+2/-0) lib/canonical/launchpad/webapp/vocabulary.py (+5/-1) lib/canonical/widgets/popup.py (+21/-13) lib/lp/answers/vocabulary.py (+1/-2) lib/lp/app/widgets/tests/test_popup.py (+43/-0) lib/lp/blueprints/vocabularies/specificationdependency.py (+1/-0) lib/lp/code/vocabularies/branch.py (+1/-0) lib/lp/registry/browser/configure.zcml (+1/-1) lib/lp/registry/browser/person.py (+7/-0) lib/lp/registry/browser/tests/test_team.py (+14/-0) lib/lp/registry/doc/vocabularies.txt (+1/-93) lib/lp/registry/javascript/team.js (+2/-2) lib/lp/registry/templates/team-portlet-membership.pt (+4/-2) lib/lp/registry/tests/test_person_vocabularies.py (+99/-18) lib/lp/registry/vocabularies.py (+62/-12) |
To merge this branch: | bzr merge lp:~sinzui/launchpad/closed-teams-0 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+42500@code.launchpad.net |
Commit message
Description of the change
Need a closed team and user vocabulary and picker
Launchpad bug: https:/
Pre-
Test command: ./bin/test -vv \
-t test_popup -t test_team -t test_person_
There are several cases where Launchpad must prevent users from selecting a
team with an open subscription policy because it compromises security or
discloses information. The person picker and forms need a vocabulary that is
composed on closed teams (restricted and moderated), and users. The
picker/vocabulary is needed is needed in:
1. closed team owner role
2. closed team +addmember
3. moderated team +proposemember
-------
RULES
* Update the ValidTeamMember
to exclude open teams when the context team is closed.
* Update IHugeVocabulary, <intermediate classes>, VocabularyPicke
to use the vocabs step_title so that users understand what can and
cannot be searched for. This issue was discovered in user testing.
* Update all IHugeVocabularies to have a step_title
* Update the add member ajax feature to also use the vocabulary's
step_title
QA
Using a moderated or restricted team to test.
* Verify that open teams to not appear in the person picker to add
a new member from the team's front page.
* Verify that open teams to not appear in the person picker to add
a new member from the teams +addmember page.
* Verify that open teams to not appear in the person picker to change
the team owner
LINT
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
IMPLEMENTATION
Updated ValidTeamMember
open teams when the context team is closed.
lib/
lib/
Added step_title to the vocabulary. The value is the same that appears in
the UI now.
lib/
lib/
lib/
lib/
lib/
lib/
Updated VocabularyPicke
no direct tests of this widget. Moving the whole module would make the diff
hard to read so I create a test where the module should be moved.
lib/
lib/
lib/
lib/
lib/
lib/
Updated the TeamIndexView and team membership portlet to use the step_title
from the vocabulary.
lib/
lib/
lib/
lib/
lib/
Curtis Hovey (sinzui) wrote : | # |
On Thu, 2010-12-02 at 17:12 +0000, Gavin Panella wrote:
> Review: Approve
> Hi Curtis. Some incredibly minor comments, +1.
>
>
> [1]
>
> js = js_template % simplejson.
> # If the YUI widget or javascript is not supported in the browser,
> # it will degrade to being this "Find..." link instead of the
> @@ -154,8 +163,7 @@
> css = ''
> return ('<span class="%s">(<a id="%s" href="/people/">'
> 'Find…
> - '\n<script>
> - ) % (css, self.show_
> + '\n<script>
>
> It's not part of your change, but using simplejson.dumps() can be
> hazardous for embedding javascript into <script> tags. See:
>
> http://
>
> Instead the following should be used:
>
> simplejson.
>
> Or:
>
> simplejson.
Interesting. We cannot do this right now because Lp is using 2.0.9. I
can see it my version of 2.1.1
> [2]
>
> + Y.lp.registry.
> + '${step_title}');
>
> The lack of escaping and all that jazz makes me a little uneasy, but
> it's not user-supplied so I'm not too worried.
This content is in a tal:content rule and is escaped. A single-quote
will break the script. I updated the method to escape quotes and added a
test.
> [3]
>
> + def test_open_
>
> s/or/of/
fixed
> [4]
>
> + def test_open_
>
> s/or/of/
fixed
> [5]
>
> + return 'Search for a restricted or moderated team or person'
>
> This could be parsed as:
>
> Search for a (restricted or moderated) (team or person)
>
> Perhaps the following would be less ambiguous:
>
> Search for a restricted team, a moderated team, or a person
I accept your proposal.
> [6]
>
> + Person.
>
> Maybe it's worth taking a defensive approach here (to someone adding
> another TeamSubscriptio
>
> In(Person.
> TeamSubscriptio
> TeamSubscriptio
We have not changed the policies in 5 years. I do not think we will be
adding other policies. If I am wrong, we may need to update the proposed
revision. Since this issue is about OPEN, I think the clause should
remain as is.
--
__Curtis C. Hovey_________
http://
Preview Diff
1 | === modified file 'lib/canonical/launchpad/vocabularies/dbobjects.py' | |||
2 | --- lib/canonical/launchpad/vocabularies/dbobjects.py 2010-11-09 08:38:23 +0000 | |||
3 | +++ lib/canonical/launchpad/vocabularies/dbobjects.py 2010-12-03 00:04:45 +0000 | |||
4 | @@ -141,6 +141,7 @@ | |||
5 | 141 | class BugTrackerVocabulary(SQLObjectVocabularyBase): | 141 | class BugTrackerVocabulary(SQLObjectVocabularyBase): |
6 | 142 | """All web and email based external bug trackers.""" | 142 | """All web and email based external bug trackers.""" |
7 | 143 | displayname = 'Select a bug tracker' | 143 | displayname = 'Select a bug tracker' |
8 | 144 | step_title = 'Search' | ||
9 | 144 | implements(IHugeVocabulary) | 145 | implements(IHugeVocabulary) |
10 | 145 | _table = BugTracker | 146 | _table = BugTracker |
11 | 146 | _filter = True | 147 | _filter = True |
12 | @@ -401,6 +402,7 @@ | |||
13 | 401 | Person.q.id == Archive.q.ownerID, | 402 | Person.q.id == Archive.q.ownerID, |
14 | 402 | Archive.q.purpose == ArchivePurpose.PPA) | 403 | Archive.q.purpose == ArchivePurpose.PPA) |
15 | 403 | displayname = 'Select a PPA' | 404 | displayname = 'Select a PPA' |
16 | 405 | step_title = 'Search' | ||
17 | 404 | 406 | ||
18 | 405 | def toTerm(self, archive): | 407 | def toTerm(self, archive): |
19 | 406 | """See `IVocabulary`.""" | 408 | """See `IVocabulary`.""" |
20 | 407 | 409 | ||
21 | === modified file 'lib/canonical/launchpad/webapp/vocabulary.py' | |||
22 | --- lib/canonical/launchpad/webapp/vocabulary.py 2010-11-09 08:38:23 +0000 | |||
23 | +++ lib/canonical/launchpad/webapp/vocabulary.py 2010-12-03 00:04:45 +0000 | |||
24 | @@ -70,7 +70,10 @@ | |||
25 | 70 | """ | 70 | """ |
26 | 71 | 71 | ||
27 | 72 | displayname = Attribute( | 72 | displayname = Attribute( |
29 | 73 | 'A name for this vocabulary, to be displayed in the popup window.') | 73 | 'A name for this vocabulary, to be displayed in the picker window.') |
30 | 74 | |||
31 | 75 | step_title = Attribute( | ||
32 | 76 | 'The search step title in the picker window.') | ||
33 | 74 | 77 | ||
34 | 75 | def searchForTerms(query=None): | 78 | def searchForTerms(query=None): |
35 | 76 | """Return a `CountableIterator` of `SimpleTerm`s that match the query. | 79 | """Return a `CountableIterator` of `SimpleTerm`s that match the query. |
36 | @@ -365,6 +368,7 @@ | |||
37 | 365 | implements(IHugeVocabulary) | 368 | implements(IHugeVocabulary) |
38 | 366 | _orderBy = 'name' | 369 | _orderBy = 'name' |
39 | 367 | displayname = None | 370 | displayname = None |
40 | 371 | step_title = 'Search' | ||
41 | 368 | # The iterator class will be used to wrap the results; its iteration | 372 | # The iterator class will be used to wrap the results; its iteration |
42 | 369 | # methods should return SimpleTerms, as the reference implementation | 373 | # methods should return SimpleTerms, as the reference implementation |
43 | 370 | # CountableIterator does. | 374 | # CountableIterator does. |
44 | 371 | 375 | ||
45 | === modified file 'lib/canonical/widgets/popup.py' | |||
46 | --- lib/canonical/widgets/popup.py 2010-08-24 10:45:57 +0000 | |||
47 | +++ lib/canonical/widgets/popup.py 2010-12-03 00:04:45 +0000 | |||
48 | @@ -37,7 +37,7 @@ | |||
49 | 37 | style = '' | 37 | style = '' |
50 | 38 | cssClass = '' | 38 | cssClass = '' |
51 | 39 | 39 | ||
53 | 40 | step_title = 'Search' | 40 | step_title = None |
54 | 41 | # Defaults to self.vocabulary.displayname. | 41 | # Defaults to self.vocabulary.displayname. |
55 | 42 | header = None | 42 | header = None |
56 | 43 | 43 | ||
57 | @@ -79,19 +79,20 @@ | |||
58 | 79 | 79 | ||
59 | 80 | def inputField(self): | 80 | def inputField(self): |
60 | 81 | d = { | 81 | d = { |
62 | 82 | 'formToken' : cgi.escape(self.formToken, quote=True), | 82 | 'formToken': cgi.escape(self.formToken, quote=True), |
63 | 83 | 'name': self.name, | 83 | 'name': self.name, |
64 | 84 | 'displayWidth': self.displayWidth, | 84 | 'displayWidth': self.displayWidth, |
65 | 85 | 'displayMaxWidth': self.displayMaxWidth, | 85 | 'displayMaxWidth': self.displayMaxWidth, |
66 | 86 | 'onKeyPress': self.onKeyPress, | 86 | 'onKeyPress': self.onKeyPress, |
67 | 87 | 'style': self.style, | 87 | 'style': self.style, |
70 | 88 | 'cssClass': self.cssClass | 88 | 'cssClass': self.cssClass, |
71 | 89 | } | 89 | } |
72 | 90 | return """<input type="text" value="%(formToken)s" id="%(name)s" | 90 | return """<input type="text" value="%(formToken)s" id="%(name)s" |
73 | 91 | name="%(name)s" size="%(displayWidth)s" | 91 | name="%(name)s" size="%(displayWidth)s" |
74 | 92 | maxlength="%(displayMaxWidth)s" | 92 | maxlength="%(displayMaxWidth)s" |
75 | 93 | onKeyPress="%(onKeyPress)s" style="%(style)s" | 93 | onKeyPress="%(onKeyPress)s" style="%(style)s" |
76 | 94 | class="%(cssClass)s" />""" % d | 94 | class="%(cssClass)s" />""" % d |
77 | 95 | |||
78 | 95 | @property | 96 | @property |
79 | 96 | def suffix(self): | 97 | def suffix(self): |
80 | 97 | return self.name.replace('.', '-') | 98 | return self.name.replace('.', '-') |
81 | @@ -126,23 +127,31 @@ | |||
82 | 126 | % (choice.context, choice.__name__)) | 127 | % (choice.context, choice.__name__)) |
83 | 127 | return choice.vocabularyName | 128 | return choice.vocabularyName |
84 | 128 | 129 | ||
90 | 129 | def chooseLink(self): | 130 | def js_template_args(self): |
91 | 130 | js_file = os.path.join(os.path.dirname(__file__), | 131 | """return a dict of args to configure the picker javascript.""" |
87 | 131 | 'templates/vocabulary-picker.js.template') | ||
88 | 132 | js_template = open(js_file).read() | ||
89 | 133 | |||
92 | 134 | if self.header is None: | 132 | if self.header is None: |
93 | 135 | header = self.vocabulary.displayname | 133 | header = self.vocabulary.displayname |
94 | 136 | else: | 134 | else: |
95 | 137 | header = self.header | 135 | header = self.header |
96 | 138 | 136 | ||
98 | 139 | args = dict( | 137 | if self.step_title is None: |
99 | 138 | step_title = self.vocabulary.step_title | ||
100 | 139 | else: | ||
101 | 140 | step_title = self.step_title | ||
102 | 141 | |||
103 | 142 | return dict( | ||
104 | 140 | vocabulary=self.vocabulary_name, | 143 | vocabulary=self.vocabulary_name, |
105 | 141 | header=header, | 144 | header=header, |
107 | 142 | step_title=self.step_title, | 145 | step_title=step_title, |
108 | 143 | show_widget_id=self.show_widget_id, | 146 | show_widget_id=self.show_widget_id, |
109 | 144 | input_id=self.name, | 147 | input_id=self.name, |
110 | 145 | extra_no_results_message=self.extra_no_results_message) | 148 | extra_no_results_message=self.extra_no_results_message) |
111 | 149 | |||
112 | 150 | def chooseLink(self): | ||
113 | 151 | js_file = os.path.join(os.path.dirname(__file__), | ||
114 | 152 | 'templates/vocabulary-picker.js.template') | ||
115 | 153 | js_template = open(js_file).read() | ||
116 | 154 | args = self.js_template_args() | ||
117 | 146 | js = js_template % simplejson.dumps(args) | 155 | js = js_template % simplejson.dumps(args) |
118 | 147 | # If the YUI widget or javascript is not supported in the browser, | 156 | # If the YUI widget or javascript is not supported in the browser, |
119 | 148 | # it will degrade to being this "Find..." link instead of the | 157 | # it will degrade to being this "Find..." link instead of the |
120 | @@ -154,8 +163,7 @@ | |||
121 | 154 | css = '' | 163 | css = '' |
122 | 155 | return ('<span class="%s">(<a id="%s" href="/people/">' | 164 | return ('<span class="%s">(<a id="%s" href="/people/">' |
123 | 156 | 'Find…</a>)</span>' | 165 | 'Find…</a>)</span>' |
126 | 157 | '\n<script>\n%s\n</script>' | 166 | '\n<script>\n%s\n</script>') % (css, self.show_widget_id, js) |
125 | 158 | ) % (css, self.show_widget_id, js) | ||
127 | 159 | 167 | ||
128 | 160 | @property | 168 | @property |
129 | 161 | def nonajax_uri(self): | 169 | def nonajax_uri(self): |
130 | 162 | 170 | ||
131 | === modified file 'lib/lp/answers/vocabulary.py' | |||
132 | --- lib/lp/answers/vocabulary.py 2010-08-20 20:31:18 +0000 | |||
133 | +++ lib/lp/answers/vocabulary.py 2010-12-03 00:04:45 +0000 | |||
134 | @@ -24,6 +24,7 @@ | |||
135 | 24 | implements(IHugeVocabulary) | 24 | implements(IHugeVocabulary) |
136 | 25 | 25 | ||
137 | 26 | displayname = 'Select a FAQ' | 26 | displayname = 'Select a FAQ' |
138 | 27 | step_title = 'Search' | ||
139 | 27 | 28 | ||
140 | 28 | def __init__(self, context): | 29 | def __init__(self, context): |
141 | 29 | """Create a new vocabulary for the context. | 30 | """Create a new vocabulary for the context. |
142 | @@ -72,5 +73,3 @@ | |||
143 | 72 | """See `IHugeVocabulary`.""" | 73 | """See `IHugeVocabulary`.""" |
144 | 73 | results = self.context.findSimilarFAQs(query) | 74 | results = self.context.findSimilarFAQs(query) |
145 | 74 | return CountableIterator(results.count(), results, self.toTerm) | 75 | return CountableIterator(results.count(), results, self.toTerm) |
146 | 75 | |||
147 | 76 | |||
148 | 77 | 76 | ||
149 | === added directory 'lib/lp/app/widgets' | |||
150 | === added file 'lib/lp/app/widgets/__init__.py' | |||
151 | === added directory 'lib/lp/app/widgets/tests' | |||
152 | === added file 'lib/lp/app/widgets/tests/__init__.py' | |||
153 | === added file 'lib/lp/app/widgets/tests/test_popup.py' | |||
154 | --- lib/lp/app/widgets/tests/test_popup.py 1970-01-01 00:00:00 +0000 | |||
155 | +++ lib/lp/app/widgets/tests/test_popup.py 2010-12-03 00:04:45 +0000 | |||
156 | @@ -0,0 +1,43 @@ | |||
157 | 1 | # Copyright 2010 Canonical Ltd. This software is licensed under the | ||
158 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
159 | 3 | |||
160 | 4 | __metaclass__ = type | ||
161 | 5 | |||
162 | 6 | from zope.schema.vocabulary import getVocabularyRegistry | ||
163 | 7 | |||
164 | 8 | from canonical.launchpad.webapp.servers import LaunchpadTestRequest | ||
165 | 9 | from canonical.testing.layers import DatabaseFunctionalLayer | ||
166 | 10 | from canonical.widgets.popup import VocabularyPickerWidget | ||
167 | 11 | from lp.registry.interfaces.person import ITeam | ||
168 | 12 | from lp.testing import TestCaseWithFactory | ||
169 | 13 | |||
170 | 14 | |||
171 | 15 | class TestVocabularyPickerWidget(TestCaseWithFactory): | ||
172 | 16 | |||
173 | 17 | layer = DatabaseFunctionalLayer | ||
174 | 18 | |||
175 | 19 | def setUp(self): | ||
176 | 20 | super(TestVocabularyPickerWidget, self).setUp() | ||
177 | 21 | context = self.factory.makeTeam() | ||
178 | 22 | field = ITeam['teamowner'] | ||
179 | 23 | self.bound_field = field.bind(context) | ||
180 | 24 | vocabulary_registry = getVocabularyRegistry() | ||
181 | 25 | self.vocabulary = vocabulary_registry.get(context, 'ValidTeamOwner') | ||
182 | 26 | self.request = LaunchpadTestRequest() | ||
183 | 27 | |||
184 | 28 | def test_js_template_args(self): | ||
185 | 29 | picker_widget = VocabularyPickerWidget( | ||
186 | 30 | self.bound_field, self.vocabulary, self.request) | ||
187 | 31 | js_template_args = picker_widget.js_template_args() | ||
188 | 32 | self.assertEqual( | ||
189 | 33 | 'ValidTeamOwner', js_template_args['vocabulary']) | ||
190 | 34 | self.assertEqual( | ||
191 | 35 | self.vocabulary.displayname, js_template_args['header']) | ||
192 | 36 | self.assertEqual( | ||
193 | 37 | self.vocabulary.step_title, js_template_args['step_title']) | ||
194 | 38 | self.assertEqual( | ||
195 | 39 | 'show-widget-field-teamowner', js_template_args['show_widget_id']) | ||
196 | 40 | self.assertEqual( | ||
197 | 41 | 'field.teamowner', js_template_args['input_id']) | ||
198 | 42 | self.assertEqual( | ||
199 | 43 | None, js_template_args['extra_no_results_message']) | ||
200 | 0 | 44 | ||
201 | === modified file 'lib/lp/blueprints/vocabularies/specificationdependency.py' | |||
202 | --- lib/lp/blueprints/vocabularies/specificationdependency.py 2010-11-01 03:36:04 +0000 | |||
203 | +++ lib/lp/blueprints/vocabularies/specificationdependency.py 2010-12-03 00:04:45 +0000 | |||
204 | @@ -58,6 +58,7 @@ | |||
205 | 58 | _table = Specification | 58 | _table = Specification |
206 | 59 | _orderBy = 'name' | 59 | _orderBy = 'name' |
207 | 60 | displayname = 'Select a blueprint' | 60 | displayname = 'Select a blueprint' |
208 | 61 | step_title = 'Search' | ||
209 | 61 | 62 | ||
210 | 62 | def _is_valid_candidate(self, spec, check_target=False): | 63 | def _is_valid_candidate(self, spec, check_target=False): |
211 | 63 | """Is `spec` a valid candidate spec for self.context? | 64 | """Is `spec` a valid candidate spec for self.context? |
212 | 64 | 65 | ||
213 | === modified file 'lib/lp/code/vocabularies/branch.py' | |||
214 | --- lib/lp/code/vocabularies/branch.py 2010-09-20 01:06:36 +0000 | |||
215 | +++ lib/lp/code/vocabularies/branch.py 2010-12-03 00:04:45 +0000 | |||
216 | @@ -44,6 +44,7 @@ | |||
217 | 44 | _table = Branch | 44 | _table = Branch |
218 | 45 | _orderBy = ['name', 'id'] | 45 | _orderBy = ['name', 'id'] |
219 | 46 | displayname = 'Select a branch' | 46 | displayname = 'Select a branch' |
220 | 47 | step_title = 'Search' | ||
221 | 47 | 48 | ||
222 | 48 | def toTerm(self, branch): | 49 | def toTerm(self, branch): |
223 | 49 | """The display should include the URL if there is one.""" | 50 | """The display should include the URL if there is one.""" |
224 | 50 | 51 | ||
225 | === modified file 'lib/lp/registry/browser/configure.zcml' | |||
226 | --- lib/lp/registry/browser/configure.zcml 2010-11-30 20:34:31 +0000 | |||
227 | +++ lib/lp/registry/browser/configure.zcml 2010-12-03 00:04:45 +0000 | |||
228 | @@ -1127,7 +1127,7 @@ | |||
229 | 1127 | template="../templates/team-portlet-map.pt"/> | 1127 | template="../templates/team-portlet-map.pt"/> |
230 | 1128 | <browser:page | 1128 | <browser:page |
231 | 1129 | for="lp.registry.interfaces.person.ITeam" | 1129 | for="lp.registry.interfaces.person.ITeam" |
233 | 1130 | class="lp.registry.browser.person.PersonIndexView" | 1130 | class="lp.registry.browser.person.TeamIndexView" |
234 | 1131 | permission="zope.Public" | 1131 | permission="zope.Public" |
235 | 1132 | name="+portlet-membership" | 1132 | name="+portlet-membership" |
236 | 1133 | template="../templates/team-portlet-membership.pt"/> | 1133 | template="../templates/team-portlet-membership.pt"/> |
237 | 1134 | 1134 | ||
238 | === modified file 'lib/lp/registry/browser/person.py' | |||
239 | --- lib/lp/registry/browser/person.py 2010-11-30 12:53:20 +0000 | |||
240 | +++ lib/lp/registry/browser/person.py 2010-12-03 00:04:45 +0000 | |||
241 | @@ -3567,6 +3567,13 @@ | |||
242 | 3567 | return 'portlet' | 3567 | return 'portlet' |
243 | 3568 | return 'portlet private' | 3568 | return 'portlet private' |
244 | 3569 | 3569 | ||
245 | 3570 | @property | ||
246 | 3571 | def add_member_step_title(self): | ||
247 | 3572 | """A string for setup_add_member_handler with escaped quotes.""" | ||
248 | 3573 | vocabulary_registry = getVocabularyRegistry() | ||
249 | 3574 | vocabulary = vocabulary_registry.get(self.context, 'ValidTeamMember') | ||
250 | 3575 | return vocabulary.step_title.replace("'", "\\'").replace('"', '\\"') | ||
251 | 3576 | |||
252 | 3570 | 3577 | ||
253 | 3571 | class PersonCodeOfConductEditView(LaunchpadView): | 3578 | class PersonCodeOfConductEditView(LaunchpadView): |
254 | 3572 | """View for the ~person/+codesofconduct pages.""" | 3579 | """View for the ~person/+codesofconduct pages.""" |
255 | 3573 | 3580 | ||
256 | === modified file 'lib/lp/registry/browser/tests/test_team.py' | |||
257 | --- lib/lp/registry/browser/tests/test_team.py 2010-11-23 04:42:24 +0000 | |||
258 | +++ lib/lp/registry/browser/tests/test_team.py 2010-12-03 00:04:45 +0000 | |||
259 | @@ -135,3 +135,17 @@ | |||
260 | 135 | self.assertEqual( | 135 | self.assertEqual( |
261 | 136 | "You can't add a team that doesn't have any active members.", | 136 | "You can't add a team that doesn't have any active members.", |
262 | 137 | view.errors[0]) | 137 | view.errors[0]) |
263 | 138 | |||
264 | 139 | |||
265 | 140 | class TestTeamIndexView(TestCaseWithFactory): | ||
266 | 141 | |||
267 | 142 | layer = DatabaseFunctionalLayer | ||
268 | 143 | |||
269 | 144 | def setUp(self): | ||
270 | 145 | super(TestTeamIndexView, self).setUp() | ||
271 | 146 | self.team = self.factory.makeTeam(name='test-team') | ||
272 | 147 | login_person(self.team.teamowner) | ||
273 | 148 | |||
274 | 149 | def test_add_member_step_title(self): | ||
275 | 150 | view = create_initialized_view(self.team, '+index') | ||
276 | 151 | self.assertEqual('Search', view.add_member_step_title) | ||
277 | 138 | 152 | ||
278 | === modified file 'lib/lp/registry/doc/vocabularies.txt' | |||
279 | --- lib/lp/registry/doc/vocabularies.txt 2010-11-05 14:35:36 +0000 | |||
280 | +++ lib/lp/registry/doc/vocabularies.txt 2010-12-03 00:04:45 +0000 | |||
281 | @@ -1052,103 +1052,11 @@ | |||
282 | 1052 | [(u'Ubuntu Team', u'Mark Shuttleworth')] | 1052 | [(u'Ubuntu Team', u'Mark Shuttleworth')] |
283 | 1053 | 1053 | ||
284 | 1054 | 1054 | ||
285 | 1055 | === ValidTeamMember === | ||
286 | 1056 | |||
287 | 1057 | With the exception of all teams that have this team as a member and the | ||
288 | 1058 | team itself, all valid persons and teams are valid members. | ||
289 | 1059 | |||
290 | 1060 | >>> login('foo.bar@canonical.com') | ||
291 | 1061 | >>> team = person_set.getByName('ubuntu-team') | ||
292 | 1062 | >>> team2 = person_set.getByName('guadamen') | ||
293 | 1063 | >>> person = person_set.getByName('name16') | ||
294 | 1064 | |||
295 | 1065 | ValidTeamMember needs a context: | ||
296 | 1066 | |||
297 | 1067 | >>> vocab = get_naked_vocab(None, "ValidTeamMember") | ||
298 | 1068 | Traceback (most recent call last): | ||
299 | 1069 | ... | ||
300 | 1070 | AssertionError: ... | ||
301 | 1071 | |||
302 | 1072 | ValidTeamMember's context must implement ITeam: | ||
303 | 1073 | |||
304 | 1074 | >>> vocab = get_naked_vocab(person, "ValidTeamMember") | ||
305 | 1075 | Traceback (most recent call last): | ||
306 | 1076 | ... | ||
307 | 1077 | AssertionError: ... | ||
308 | 1078 | |||
309 | 1079 | 'name16' is a valid member for 'ubuntu-team': | ||
310 | 1080 | |||
311 | 1081 | >>> vocab = get_naked_vocab(team, "ValidTeamMember") | ||
312 | 1082 | >>> vocab.displayname | ||
313 | 1083 | 'Select a Person or Team' | ||
314 | 1084 | |||
315 | 1085 | >>> person in vocab | ||
316 | 1086 | True | ||
317 | 1087 | >>> [person.name for person in vocab.search('foo.bar')] | ||
318 | 1088 | [u'name16'] | ||
319 | 1089 | |||
320 | 1090 | 'ubuntu-team' is not a valid member for itself: | ||
321 | 1091 | |||
322 | 1092 | >>> team in vocab | ||
323 | 1093 | False | ||
324 | 1094 | >>> [person.name for person in vocab.search('ubuntu-team')] | ||
325 | 1095 | [u'name18', u'ubuntu-security'] | ||
326 | 1096 | |||
327 | 1097 | 'ubuntu-team' is a member of 'guadamen', so 'guadamen' can't be a member | ||
328 | 1098 | of 'ubuntu-team'. | ||
329 | 1099 | |||
330 | 1100 | >>> team2 in vocab | ||
331 | 1101 | False | ||
332 | 1102 | >>> [person.name for person in vocab.search('guadamen')] | ||
333 | 1103 | [] | ||
334 | 1104 | |||
335 | 1105 | |||
336 | 1106 | === ValidTeamOwner === | ||
337 | 1107 | |||
338 | 1108 | With the exception of the team itself and all teams owned by that team, | ||
339 | 1109 | all valid persons and teams are valid owners for the team in context. | ||
340 | 1110 | |||
341 | 1111 | ValidTeamOwner needs a context. | ||
342 | 1112 | |||
343 | 1113 | >>> vocab = get_naked_vocab(None, "ValidTeamOwner") | ||
344 | 1114 | Traceback (most recent call last): | ||
345 | 1115 | ... | ||
346 | 1116 | AssertionError: ... | ||
347 | 1117 | |||
348 | 1118 | ValidTeamOwner's context must provide IPerson or IPersonSet. | ||
349 | 1119 | |||
350 | 1120 | >>> get_naked_vocab(team, "ValidTeamOwner") | ||
351 | 1121 | <...ValidTeamOwnerVocabulary... | ||
352 | 1122 | >>> get_naked_vocab(person_set, "ValidTeamOwner") | ||
353 | 1123 | <...ValidTeamOwnerVocabulary... | ||
354 | 1124 | >>> get_naked_vocab(firefox, "ValidTeamOwner") | ||
355 | 1125 | Traceback (most recent call last): | ||
356 | 1126 | ... | ||
357 | 1127 | AssertionError: ... | ||
358 | 1128 | |||
359 | 1129 | 'ubuntu-team' is not a valid owner for itself. | ||
360 | 1130 | |||
361 | 1131 | >>> vocab = get_naked_vocab(team, "ValidTeamOwner") | ||
362 | 1132 | >>> vocab.displayname | ||
363 | 1133 | 'Select a Person or Team' | ||
364 | 1134 | |||
365 | 1135 | >>> team in vocab | ||
366 | 1136 | False | ||
367 | 1137 | >>> [person.name for person in vocab.search('ubuntu-team')] | ||
368 | 1138 | [u'name18', u'ubuntu-security'] | ||
369 | 1139 | |||
370 | 1140 | 'name16' is a valid owner for 'ubuntu-team'. | ||
371 | 1141 | |||
372 | 1142 | >>> person in vocab | ||
373 | 1143 | True | ||
374 | 1144 | >>> [person.name for person in vocab.search('foo.bar')] | ||
375 | 1145 | [u'name16'] | ||
376 | 1146 | |||
377 | 1147 | |||
378 | 1148 | === ValidPerson === | 1055 | === ValidPerson === |
379 | 1149 | 1056 | ||
380 | 1150 | All 'valid' persons who are not a team. | 1057 | All 'valid' persons who are not a team. |
381 | 1151 | 1058 | ||
382 | 1059 | >>> login('foo.bar@canonical.com') | ||
383 | 1152 | >>> vocab = get_naked_vocab(None, "ValidPerson") | 1060 | >>> vocab = get_naked_vocab(None, "ValidPerson") |
384 | 1153 | >>> vocab.displayname | 1061 | >>> vocab.displayname |
385 | 1154 | 'Select a Person' | 1062 | 'Select a Person' |
386 | 1155 | 1063 | ||
387 | === modified file 'lib/lp/registry/javascript/team.js' | |||
388 | --- lib/lp/registry/javascript/team.js 2010-10-21 22:06:57 +0000 | |||
389 | +++ lib/lp/registry/javascript/team.js 2010-12-03 00:04:45 +0000 | |||
390 | @@ -14,14 +14,14 @@ | |||
391 | 14 | * | 14 | * |
392 | 15 | * @method setup_add_member_handler | 15 | * @method setup_add_member_handler |
393 | 16 | */ | 16 | */ |
395 | 17 | module.setup_add_member_handler = function() { | 17 | module.setup_add_member_handler = function(step_title) { |
396 | 18 | if (Y.UA.ie) { | 18 | if (Y.UA.ie) { |
397 | 19 | return; | 19 | return; |
398 | 20 | } | 20 | } |
399 | 21 | 21 | ||
400 | 22 | var config = { | 22 | var config = { |
401 | 23 | header: 'Add a member', | 23 | header: 'Add a member', |
403 | 24 | step_title: 'Search', | 24 | step_title: step_title, |
404 | 25 | picker_activator: '.menu-link-add_member' | 25 | picker_activator: '.menu-link-add_member' |
405 | 26 | }; | 26 | }; |
406 | 27 | 27 | ||
407 | 28 | 28 | ||
408 | === modified file 'lib/lp/registry/templates/team-portlet-membership.pt' | |||
409 | --- lib/lp/registry/templates/team-portlet-membership.pt 2010-11-10 15:33:47 +0000 | |||
410 | +++ lib/lp/registry/templates/team-portlet-membership.pt 2010-12-03 00:04:45 +0000 | |||
411 | @@ -101,14 +101,16 @@ | |||
412 | 101 | <table style="margin: 0px 0px .5em 0px;"> | 101 | <table style="margin: 0px 0px .5em 0px;"> |
413 | 102 | <tr> | 102 | <tr> |
414 | 103 | <td style="padding: 0px 1em 1em 0px;" | 103 | <td style="padding: 0px 1em 1em 0px;" |
416 | 104 | tal:define="link context/menu:overview/add_member" | 104 | tal:define="link context/menu:overview/add_member; |
417 | 105 | step_title view/add_member_step_title;" | ||
418 | 105 | tal:condition="link/enabled"> | 106 | tal:condition="link/enabled"> |
419 | 106 | <script type="text/javascript" | 107 | <script type="text/javascript" |
420 | 107 | tal:content="string: | 108 | tal:content="string: |
421 | 108 | LPS.use('lp.registry.team', function(Y) { | 109 | LPS.use('lp.registry.team', function(Y) { |
422 | 109 | Y.on('load', | 110 | Y.on('load', |
423 | 110 | function(e) { | 111 | function(e) { |
425 | 111 | Y.lp.registry.team.setup_add_member_handler(); | 112 | Y.lp.registry.team.setup_add_member_handler( |
426 | 113 | '${step_title}'); | ||
427 | 112 | }, | 114 | }, |
428 | 113 | window); | 115 | window); |
429 | 114 | }); | 116 | }); |
430 | 115 | 117 | ||
431 | === modified file 'lib/lp/registry/tests/test_person_vocabularies.py' | |||
432 | --- lib/lp/registry/tests/test_person_vocabularies.py 2010-10-20 15:45:40 +0000 | |||
433 | +++ lib/lp/registry/tests/test_person_vocabularies.py 2010-12-03 00:04:45 +0000 | |||
434 | @@ -11,36 +11,117 @@ | |||
435 | 11 | 11 | ||
436 | 12 | from canonical.launchpad.ftests import login_person | 12 | from canonical.launchpad.ftests import login_person |
437 | 13 | from canonical.testing.layers import DatabaseFunctionalLayer | 13 | from canonical.testing.layers import DatabaseFunctionalLayer |
439 | 14 | from lp.registry.interfaces.person import PersonVisibility | 14 | from lp.registry.interfaces.person import ( |
440 | 15 | PersonVisibility, | ||
441 | 16 | TeamSubscriptionPolicy, | ||
442 | 17 | ) | ||
443 | 15 | from lp.testing import TestCaseWithFactory | 18 | from lp.testing import TestCaseWithFactory |
444 | 16 | 19 | ||
445 | 17 | 20 | ||
447 | 18 | class TestValidTeamMemberVocabulary(TestCaseWithFactory): | 21 | class VocabularyTestBase: |
448 | 22 | |||
449 | 23 | vocabulary_name = None | ||
450 | 24 | |||
451 | 25 | def setUp(self): | ||
452 | 26 | super(VocabularyTestBase, self).setUp() | ||
453 | 27 | self.vocabulary_registry = getVocabularyRegistry() | ||
454 | 28 | |||
455 | 29 | def getVocabulary(self, context): | ||
456 | 30 | return self.vocabulary_registry.get(context, self.vocabulary_name) | ||
457 | 31 | |||
458 | 32 | def searchVocabulary(self, context, text): | ||
459 | 33 | Store.of(context).flush() | ||
460 | 34 | vocabulary = self.getVocabulary(context) | ||
461 | 35 | return removeSecurityProxy(vocabulary)._doSearch(text) | ||
462 | 36 | |||
463 | 37 | def test_open_team_cannot_be_a_member_of_a_closed_team(self): | ||
464 | 38 | context_team = self.factory.makeTeam( | ||
465 | 39 | subscription_policy=TeamSubscriptionPolicy.MODERATED) | ||
466 | 40 | open_team = self.factory.makeTeam( | ||
467 | 41 | subscription_policy=TeamSubscriptionPolicy.OPEN) | ||
468 | 42 | moderated_team = self.factory.makeTeam( | ||
469 | 43 | subscription_policy=TeamSubscriptionPolicy.MODERATED) | ||
470 | 44 | restricted_team = self.factory.makeTeam( | ||
471 | 45 | subscription_policy=TeamSubscriptionPolicy.RESTRICTED) | ||
472 | 46 | user = self.factory.makePerson() | ||
473 | 47 | all_possible_members = self.searchVocabulary(context_team, '') | ||
474 | 48 | self.assertNotIn(open_team, all_possible_members) | ||
475 | 49 | self.assertIn(moderated_team, all_possible_members) | ||
476 | 50 | self.assertIn(restricted_team, all_possible_members) | ||
477 | 51 | self.assertIn(user, all_possible_members) | ||
478 | 52 | |||
479 | 53 | def test_open_team_can_be_a_member_of_an_open_team(self): | ||
480 | 54 | context_team = self.factory.makeTeam( | ||
481 | 55 | subscription_policy=TeamSubscriptionPolicy.OPEN) | ||
482 | 56 | open_team = self.factory.makeTeam( | ||
483 | 57 | subscription_policy=TeamSubscriptionPolicy.OPEN) | ||
484 | 58 | moderated_team = self.factory.makeTeam( | ||
485 | 59 | subscription_policy=TeamSubscriptionPolicy.MODERATED) | ||
486 | 60 | restricted_team = self.factory.makeTeam( | ||
487 | 61 | subscription_policy=TeamSubscriptionPolicy.RESTRICTED) | ||
488 | 62 | user = self.factory.makePerson() | ||
489 | 63 | all_possible_members = self.searchVocabulary(context_team, '') | ||
490 | 64 | self.assertIn(open_team, all_possible_members) | ||
491 | 65 | self.assertIn(moderated_team, all_possible_members) | ||
492 | 66 | self.assertIn(restricted_team, all_possible_members) | ||
493 | 67 | self.assertIn(user, all_possible_members) | ||
494 | 68 | |||
495 | 69 | def test_vocabulary_displayname(self): | ||
496 | 70 | context_team = self.factory.makeTeam( | ||
497 | 71 | subscription_policy=TeamSubscriptionPolicy.OPEN) | ||
498 | 72 | vocabulary = self.getVocabulary(context_team) | ||
499 | 73 | self.assertEqual( | ||
500 | 74 | 'Select a Team or Person', vocabulary.displayname) | ||
501 | 75 | |||
502 | 76 | def test_open_team_vocabulary_step_title(self): | ||
503 | 77 | context_team = self.factory.makeTeam( | ||
504 | 78 | subscription_policy=TeamSubscriptionPolicy.OPEN) | ||
505 | 79 | vocabulary = self.getVocabulary(context_team) | ||
506 | 80 | self.assertEqual('Search', vocabulary.step_title) | ||
507 | 81 | |||
508 | 82 | def test_closed_team_vocabulary_step_title(self): | ||
509 | 83 | context_team = self.factory.makeTeam( | ||
510 | 84 | subscription_policy=TeamSubscriptionPolicy.MODERATED) | ||
511 | 85 | vocabulary = self.getVocabulary(context_team) | ||
512 | 86 | self.assertEqual( | ||
513 | 87 | 'Search for a restricted team, a moderated team, or a person', | ||
514 | 88 | vocabulary.step_title) | ||
515 | 89 | |||
516 | 90 | |||
517 | 91 | class TestValidTeamMemberVocabulary(VocabularyTestBase, TestCaseWithFactory): | ||
518 | 19 | """Test that the ValidTeamMemberVocabulary behaves as expected.""" | 92 | """Test that the ValidTeamMemberVocabulary behaves as expected.""" |
519 | 20 | 93 | ||
520 | 21 | layer = DatabaseFunctionalLayer | 94 | layer = DatabaseFunctionalLayer |
527 | 22 | 95 | vocabulary_name = 'ValidTeamMember' | |
522 | 23 | def searchVocabulary(self, team, text): | ||
523 | 24 | vocabulary_registry = getVocabularyRegistry() | ||
524 | 25 | naked_vocabulary = removeSecurityProxy( | ||
525 | 26 | vocabulary_registry.get(team, 'ValidTeamMember')) | ||
526 | 27 | return naked_vocabulary._doSearch(text) | ||
528 | 28 | 96 | ||
529 | 29 | def test_public_team_cannot_be_a_member_of_itself(self): | 97 | def test_public_team_cannot_be_a_member_of_itself(self): |
530 | 30 | # A public team should be filtered by the vocab.extra_clause | 98 | # A public team should be filtered by the vocab.extra_clause |
531 | 31 | # when provided a search term. | 99 | # when provided a search term. |
537 | 32 | team_owner = self.factory.makePerson() | 100 | team = self.factory.makeTeam() |
538 | 33 | login_person(team_owner) | 101 | self.assertNotIn(team, self.searchVocabulary(team, team.name)) |
534 | 34 | team = self.factory.makeTeam(owner=team_owner) | ||
535 | 35 | Store.of(team).flush() | ||
536 | 36 | self.assertFalse(team in self.searchVocabulary(team, team.name)) | ||
539 | 37 | 102 | ||
540 | 38 | def test_private_team_cannot_be_a_member_of_itself(self): | 103 | def test_private_team_cannot_be_a_member_of_itself(self): |
541 | 39 | # A private team should be filtered by the vocab.extra_clause | 104 | # A private team should be filtered by the vocab.extra_clause |
542 | 40 | # when provided a search term. | 105 | # when provided a search term. |
543 | 41 | team_owner = self.factory.makePerson() | ||
544 | 42 | login_person(team_owner) | ||
545 | 43 | team = self.factory.makeTeam( | 106 | team = self.factory.makeTeam( |
549 | 44 | owner=team_owner, visibility=PersonVisibility.PRIVATE) | 107 | visibility=PersonVisibility.PRIVATE) |
550 | 45 | Store.of(team).flush() | 108 | login_person(team.teamowner) |
551 | 46 | self.assertFalse(team in self.searchVocabulary(team, team.name)) | 109 | self.assertNotIn(team, self.searchVocabulary(team, team.name)) |
552 | 110 | |||
553 | 111 | |||
554 | 112 | class TestValidTeamOwnerVocabulary(VocabularyTestBase, TestCaseWithFactory): | ||
555 | 113 | """Test that the ValidTeamOwnerVocabulary behaves as expected.""" | ||
556 | 114 | |||
557 | 115 | layer = DatabaseFunctionalLayer | ||
558 | 116 | vocabulary_name = 'ValidTeamOwner' | ||
559 | 117 | |||
560 | 118 | def test_team_cannot_own_itself(self): | ||
561 | 119 | context_team = self.factory.makeTeam() | ||
562 | 120 | results = self.searchVocabulary(context_team, context_team.name) | ||
563 | 121 | self.assertNotIn(context_team, results) | ||
564 | 122 | |||
565 | 123 | def test_team_cannot_own_its_owner(self): | ||
566 | 124 | context_team = self.factory.makeTeam() | ||
567 | 125 | owned_team = self.factory.makeTeam(owner=context_team) | ||
568 | 126 | results = self.searchVocabulary(context_team, owned_team.name) | ||
569 | 127 | self.assertNotIn(owned_team, results) | ||
570 | 47 | 128 | ||
571 | === modified file 'lib/lp/registry/vocabularies.py' | |||
572 | --- lib/lp/registry/vocabularies.py 2010-11-09 08:38:23 +0000 | |||
573 | +++ lib/lp/registry/vocabularies.py 2010-12-03 00:04:45 +0000 | |||
574 | @@ -47,6 +47,7 @@ | |||
575 | 47 | 'SourcePackageNameVocabulary', | 47 | 'SourcePackageNameVocabulary', |
576 | 48 | 'UserTeamsParticipationVocabulary', | 48 | 'UserTeamsParticipationVocabulary', |
577 | 49 | 'UserTeamsParticipationPlusSelfVocabulary', | 49 | 'UserTeamsParticipationPlusSelfVocabulary', |
578 | 50 | 'ValidPersonOrClosedTeamVocabulary', | ||
579 | 50 | 'ValidPersonOrTeamVocabulary', | 51 | 'ValidPersonOrTeamVocabulary', |
580 | 51 | 'ValidPersonVocabulary', | 52 | 'ValidPersonVocabulary', |
581 | 52 | 'ValidTeamMemberVocabulary', | 53 | 'ValidTeamMemberVocabulary', |
582 | @@ -143,6 +144,7 @@ | |||
583 | 143 | IPersonSet, | 144 | IPersonSet, |
584 | 144 | ITeam, | 145 | ITeam, |
585 | 145 | PersonVisibility, | 146 | PersonVisibility, |
586 | 147 | TeamSubscriptionPolicy, | ||
587 | 146 | ) | 148 | ) |
588 | 147 | from lp.registry.interfaces.pillar import IPillarName | 149 | from lp.registry.interfaces.pillar import IPillarName |
589 | 148 | from lp.registry.interfaces.product import ( | 150 | from lp.registry.interfaces.product import ( |
590 | @@ -219,6 +221,7 @@ | |||
591 | 219 | class ProductVocabulary(SQLObjectVocabularyBase): | 221 | class ProductVocabulary(SQLObjectVocabularyBase): |
592 | 220 | """All `IProduct` objects vocabulary.""" | 222 | """All `IProduct` objects vocabulary.""" |
593 | 221 | implements(IHugeVocabulary) | 223 | implements(IHugeVocabulary) |
594 | 224 | step_title = 'Search' | ||
595 | 222 | 225 | ||
596 | 223 | _table = Product | 226 | _table = Product |
597 | 224 | _orderBy = 'displayname' | 227 | _orderBy = 'displayname' |
598 | @@ -271,6 +274,7 @@ | |||
599 | 271 | _table = ProjectGroup | 274 | _table = ProjectGroup |
600 | 272 | _orderBy = 'displayname' | 275 | _orderBy = 'displayname' |
601 | 273 | displayname = 'Select a project group' | 276 | displayname = 'Select a project group' |
602 | 277 | step_title = 'Search' | ||
603 | 274 | 278 | ||
604 | 275 | def __contains__(self, obj): | 279 | def __contains__(self, obj): |
605 | 276 | where = "active='t' and id=%d" | 280 | where = "active='t' and id=%d" |
606 | @@ -361,6 +365,7 @@ | |||
607 | 361 | 365 | ||
608 | 362 | _orderBy = ['displayname'] | 366 | _orderBy = ['displayname'] |
609 | 363 | displayname = 'Select a Person or Team' | 367 | displayname = 'Select a Person or Team' |
610 | 368 | step_title = 'Search' | ||
611 | 364 | 369 | ||
612 | 365 | def __contains__(self, obj): | 370 | def __contains__(self, obj): |
613 | 366 | return obj in self._select() | 371 | return obj in self._select() |
614 | @@ -391,6 +396,7 @@ | |||
615 | 391 | 396 | ||
616 | 392 | _orderBy = ['displayname'] | 397 | _orderBy = ['displayname'] |
617 | 393 | displayname = 'Select a Person to Merge' | 398 | displayname = 'Select a Person to Merge' |
618 | 399 | step_title = 'Search' | ||
619 | 394 | must_have_email = True | 400 | must_have_email = True |
620 | 395 | 401 | ||
621 | 396 | def __contains__(self, obj): | 402 | def __contains__(self, obj): |
622 | @@ -439,7 +445,7 @@ | |||
623 | 439 | implements(IHugeVocabulary) | 445 | implements(IHugeVocabulary) |
624 | 440 | 446 | ||
625 | 441 | displayname = 'Select a Person or Team' | 447 | displayname = 'Select a Person or Team' |
627 | 442 | 448 | step_title = 'Search' | |
628 | 443 | # This is what subclasses must change if they want any extra filtering of | 449 | # This is what subclasses must change if they want any extra filtering of |
629 | 444 | # results. | 450 | # results. |
630 | 445 | extra_clause = True | 451 | extra_clause = True |
631 | @@ -722,11 +728,32 @@ | |||
632 | 722 | cache_table_name = 'ValidPersonCache' | 728 | cache_table_name = 'ValidPersonCache' |
633 | 723 | 729 | ||
634 | 724 | 730 | ||
636 | 725 | class ValidTeamMemberVocabulary(ValidPersonOrTeamVocabulary): | 731 | class TeamVocabularyMixin: |
637 | 732 | """Common methods for team vocabularies.""" | ||
638 | 733 | |||
639 | 734 | displayname = 'Select a Team or Person' | ||
640 | 735 | |||
641 | 736 | @property | ||
642 | 737 | def is_closed_team(self): | ||
643 | 738 | return self.team.subscriptionpolicy != TeamSubscriptionPolicy.OPEN | ||
644 | 739 | |||
645 | 740 | @property | ||
646 | 741 | def step_title(self): | ||
647 | 742 | """See `IHugeVocabulary`.""" | ||
648 | 743 | if self.is_closed_team: | ||
649 | 744 | return ( | ||
650 | 745 | 'Search for a restricted team, a moderated team, or a person') | ||
651 | 746 | else: | ||
652 | 747 | return 'Search' | ||
653 | 748 | |||
654 | 749 | |||
655 | 750 | class ValidTeamMemberVocabulary(TeamVocabularyMixin, | ||
656 | 751 | ValidPersonOrTeamVocabulary): | ||
657 | 726 | """The set of valid members of a given team. | 752 | """The set of valid members of a given team. |
658 | 727 | 753 | ||
659 | 728 | With the exception of all teams that have this team as a member and the | 754 | With the exception of all teams that have this team as a member and the |
661 | 729 | team itself, all valid persons and teams are valid members. | 755 | team itself, all valid persons and teams are valid members. Restricted |
662 | 756 | and moderated teams cannot have open teams as members. | ||
663 | 730 | """ | 757 | """ |
664 | 731 | 758 | ||
665 | 732 | def __init__(self, context): | 759 | def __init__(self, context): |
666 | @@ -740,19 +767,29 @@ | |||
667 | 740 | "Got %s" % str(context)) | 767 | "Got %s" % str(context)) |
668 | 741 | 768 | ||
669 | 742 | ValidPersonOrTeamVocabulary.__init__(self, context) | 769 | ValidPersonOrTeamVocabulary.__init__(self, context) |
671 | 743 | self.extra_clause = """ | 770 | |
672 | 771 | @property | ||
673 | 772 | def extra_clause(self): | ||
674 | 773 | clause = SQL(""" | ||
675 | 744 | Person.id NOT IN ( | 774 | Person.id NOT IN ( |
676 | 745 | SELECT team FROM TeamParticipation | 775 | SELECT team FROM TeamParticipation |
677 | 746 | WHERE person = %d | 776 | WHERE person = %d |
678 | 747 | ) | 777 | ) |
683 | 748 | """ % self.team.id | 778 | """ % self.team.id) |
684 | 749 | 779 | if self.is_closed_team: | |
685 | 750 | 780 | clause = And( | |
686 | 751 | class ValidTeamOwnerVocabulary(ValidPersonOrTeamVocabulary): | 781 | clause, |
687 | 782 | Person.subscriptionpolicy != TeamSubscriptionPolicy.OPEN) | ||
688 | 783 | return clause | ||
689 | 784 | |||
690 | 785 | |||
691 | 786 | class ValidTeamOwnerVocabulary(TeamVocabularyMixin, | ||
692 | 787 | ValidPersonOrTeamVocabulary): | ||
693 | 752 | """The set of Persons/Teams that can be owner of a team. | 788 | """The set of Persons/Teams that can be owner of a team. |
694 | 753 | 789 | ||
695 | 754 | With the exception of the team itself and all teams owned by that team, | 790 | With the exception of the team itself and all teams owned by that team, |
697 | 755 | all valid persons and teams are valid owners for the team. | 791 | all valid persons and teams are valid owners for the team. Restricted |
698 | 792 | and moderated teams cannot have open teams as members. | ||
699 | 756 | """ | 793 | """ |
700 | 757 | 794 | ||
701 | 758 | def __init__(self, context): | 795 | def __init__(self, context): |
702 | @@ -760,9 +797,7 @@ | |||
703 | 760 | raise AssertionError('ValidTeamOwnerVocabulary needs a context.') | 797 | raise AssertionError('ValidTeamOwnerVocabulary needs a context.') |
704 | 761 | 798 | ||
705 | 762 | if IPerson.providedBy(context): | 799 | if IPerson.providedBy(context): |
709 | 763 | self.extra_clause = """ | 800 | self.team = context |
707 | 764 | (person.teamowner != %d OR person.teamowner IS NULL) AND | ||
708 | 765 | person.id != %d""" % (context.id, context.id) | ||
710 | 766 | elif IPersonSet.providedBy(context): | 801 | elif IPersonSet.providedBy(context): |
711 | 767 | # The context is an IPersonSet, which means we're creating a new | 802 | # The context is an IPersonSet, which means we're creating a new |
712 | 768 | # team and thus we don't need any extra_clause --any valid person | 803 | # team and thus we don't need any extra_clause --any valid person |
713 | @@ -774,6 +809,17 @@ | |||
714 | 774 | "or IPersonSet.") | 809 | "or IPersonSet.") |
715 | 775 | ValidPersonOrTeamVocabulary.__init__(self, context) | 810 | ValidPersonOrTeamVocabulary.__init__(self, context) |
716 | 776 | 811 | ||
717 | 812 | @property | ||
718 | 813 | def extra_clause(self): | ||
719 | 814 | clause = SQL(""" | ||
720 | 815 | (person.teamowner != %d OR person.teamowner IS NULL) AND | ||
721 | 816 | person.id != %d""" % (self.team.id, self.team.id)) | ||
722 | 817 | if self.is_closed_team: | ||
723 | 818 | clause = And( | ||
724 | 819 | clause, | ||
725 | 820 | Person.subscriptionpolicy != TeamSubscriptionPolicy.OPEN) | ||
726 | 821 | return clause | ||
727 | 822 | |||
728 | 777 | 823 | ||
729 | 778 | class AllUserTeamsParticipationVocabulary(ValidTeamVocabulary): | 824 | class AllUserTeamsParticipationVocabulary(ValidTeamVocabulary): |
730 | 779 | """The set of teams where the current user is a member. | 825 | """The set of teams where the current user is a member. |
731 | @@ -844,6 +890,7 @@ | |||
732 | 844 | implements(IHugeVocabulary) | 890 | implements(IHugeVocabulary) |
733 | 845 | 891 | ||
734 | 846 | displayname = 'Select an active mailing list.' | 892 | displayname = 'Select an active mailing list.' |
735 | 893 | step_title = 'Search' | ||
736 | 847 | 894 | ||
737 | 848 | def __init__(self, context): | 895 | def __init__(self, context): |
738 | 849 | assert context is None, ( | 896 | assert context is None, ( |
739 | @@ -965,6 +1012,7 @@ | |||
740 | 965 | implements(IHugeVocabulary) | 1012 | implements(IHugeVocabulary) |
741 | 966 | 1013 | ||
742 | 967 | displayname = 'Select a Product Release' | 1014 | displayname = 'Select a Product Release' |
743 | 1015 | step_title = 'Search' | ||
744 | 968 | _table = ProductRelease | 1016 | _table = ProductRelease |
745 | 969 | # XXX carlos Perello Marin 2005-05-16 bugs=687: | 1017 | # XXX carlos Perello Marin 2005-05-16 bugs=687: |
746 | 970 | # Sorting by version won't give the expected results, because it's just a | 1018 | # Sorting by version won't give the expected results, because it's just a |
747 | @@ -1032,6 +1080,7 @@ | |||
748 | 1032 | implements(IHugeVocabulary) | 1080 | implements(IHugeVocabulary) |
749 | 1033 | 1081 | ||
750 | 1034 | displayname = 'Select a Release Series' | 1082 | displayname = 'Select a Release Series' |
751 | 1083 | step_title = 'Search' | ||
752 | 1035 | _table = ProductSeries | 1084 | _table = ProductSeries |
753 | 1036 | _order_by = [Product.name, ProductSeries.name] | 1085 | _order_by = [Product.name, ProductSeries.name] |
754 | 1037 | _clauseTables = ['Product'] | 1086 | _clauseTables = ['Product'] |
755 | @@ -1265,6 +1314,7 @@ | |||
756 | 1265 | 1314 | ||
757 | 1266 | _table = Product | 1315 | _table = Product |
758 | 1267 | _orderBy = 'displayname' | 1316 | _orderBy = 'displayname' |
759 | 1317 | step_title = 'Search' | ||
760 | 1268 | 1318 | ||
761 | 1269 | @property | 1319 | @property |
762 | 1270 | def displayname(self): | 1320 | def displayname(self): |
Hi Curtis. Some incredibly minor comments, +1.
[1]
js = js_template % simplejson. dumps(args)
'Find& hellip; </a>)</ span>' \n%s\n< /script> ' widget_ id, js) \n%s\n< /script> ') % (css, self.show_ widget_ id, js)
# If the YUI widget or javascript is not supported in the browser,
# it will degrade to being this "Find..." link instead of the
@@ -154,8 +163,7 @@
css = ''
return ('<span class="%s">(<a id="%s" href="/people/">'
- '\n<script>
- ) % (css, self.show_
+ '\n<script>
It's not part of your change, but using simplejson.dumps() can be
hazardous for embedding javascript into <script> tags. See:
http:// code.google. com/p/simplejso n/issues/ detail? id=66
Instead the following should be used:
simplejson. dumps(args, cls=simplejson. encoder. JSONEncoderForH TML)
Or:
simplejson. encoder. JSONEncoderForH TML().encode( obj)
[2]
+ Y.lp.registry. team.setup_ add_member_ handler(
+ '${step_title}');
The lack of escaping and all that jazz makes me a little uneasy, but
it's not user-supplied so I'm not too worried.
[3]
+ def test_open_ team_cannot_ be_a_member_ or_a_closed_ team(self) :
s/or/of/
[4]
+ def test_open_ team_can_ be_a_member_ or_an_open_ team(self) :
s/or/of/
[5]
+ return 'Search for a restricted or moderated team or person'
This could be parsed as:
Search for a (restricted or moderated) (team or person)
Perhaps the following would be less ambiguous:
Search for a restricted team, a moderated team, or a person
?
[6]
+ Person. subscriptionpol icy != TeamSubscriptio nPolicy. OPEN)
Maybe it's worth taking a defensive approach here (to someone adding nPolicy member) instead:
another TeamSubscriptio
In(Person. subscriptionpol icy, ( riptionPolicy. RESTRICTED, riptionPolicy. MODERATED) )
TeamSubsc
TeamSubsc
The same clause appears twice.