Merge lp:~raj-abhilash1/mailman/bug_1423756 into lp:mailman
- bug_1423756
- Merge into 3.0
Status: | Merged |
---|---|
Merge reported by: | Barry Warsaw |
Merged at revision: | not available |
Proposed branch: | lp:~raj-abhilash1/mailman/bug_1423756 |
Merge into: | lp:mailman |
Diff against target: |
907 lines (+247/-92) 17 files modified
src/mailman/app/registrar.py (+0/-2) src/mailman/commands/docs/create.rst (+1/-2) src/mailman/commands/tests/test_lists.py (+1/-1) src/mailman/database/alembic/versions/46e92facee7_add_serverowner_domainowner.py (+36/-0) src/mailman/interfaces/domain.py (+5/-8) src/mailman/model/docs/domains.rst (+26/-23) src/mailman/model/domain.py (+37/-16) src/mailman/model/tests/test_domain.py (+23/-1) src/mailman/model/user.py (+11/-1) src/mailman/rest/docs/addresses.rst (+1/-0) src/mailman/rest/docs/domains.rst (+8/-25) src/mailman/rest/docs/users.rst (+17/-0) src/mailman/rest/domains.py (+19/-6) src/mailman/rest/lists.py (+1/-1) src/mailman/rest/tests/test_domains.py (+12/-0) src/mailman/rest/users.py (+48/-5) src/mailman/testing/layers.py (+1/-1) |
To merge this branch: | bzr merge lp:~raj-abhilash1/mailman/bug_1423756 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Barry Warsaw | Needs Fixing | ||
Review via email: mp+254479@code.launchpad.net |
Commit message
Description of the change
Ability to define a server owner and domain owner.
@barry: I couldn't find where are variables filled in the confirm.txt template for verification of addresses so two tests related to the same are failing.
Barry Warsaw (barry) wrote : | # |
Also, the confirm.txt file lives in src/mailman/
- 7314. By Abhilash Raj
-
* Add `drop_column` inside sqlite check, fix indentation
* Change `Owner` to `DomainOwner`
* Fix indentation errors in docs
* add multiple owners using `add_owners`
* all dummy addresses should be using example.com, example.org to avoid conflict ever
* add dummy tests
Abhilash Raj (raj-abhilash1) wrote : | # |
I did check out that function before, saw where $confirm_url and $email_address variables are filled in from. But I couldn't find ${domain_name} and $contact_address.
Barry Warsaw (barry) wrote : | # |
A few inlined replies.
- 7315. By Abhilash Raj
-
* implement left over methods
* add and remove owners using the address
Abhilash Raj (raj-abhilash1) wrote : | # |
@Barry: I have Updated the branch as per your comments.
Barry Warsaw (barry) wrote : | # |
Here's some general feedback as I work through the merge:
* Be careful about single quotes vs. double quotes. In general, I prefer single quotes except for multiline strings (including docstrings) and of course any string with an apostrophe in it.
* I fixed the interface for IDomainManager.
* You might want to use a Python linter to check for basic problems. E.g. I use pyflakes (hooked up to Emacs) so I noticed immediately some unused imports.
* Don't use mutables (e.g. []) in argument lists. If the mutable were to change, it would affect *every* call to the function. Use and check for `None` instead.
* You don't need to `assert isinstance(owners, list)` because we should accept any sequence. Probably not worth type checking the argument, as iteration ought to work, but of course it'll do the wrong thing if say owner is a string. That's okay, "we're all adults here" :) (Also, assert is not a function call so it doesn't need parentheses.)
* Implementations of interface methods can just have their docstring refer to the interface.
* I left a note in Domain.
More to come as I continue with the merge.
Barry Warsaw (barry) wrote : | # |
Oh, I'm adding a lot more tests for corner cases and such. :)
Barry Warsaw (barry) wrote : | # |
Added copyright template to alembic migration file, and fixed a few style nits.
Sort lines in __all__, and always use trailing commas in multline lists.
Sort from-imports.
Doctests for is_server_owner (model and REST layers).
Made is_server_owner PATCH and PUT-able for user resources in REST API.
Barry Warsaw (barry) wrote : | # |
Added doctest for domain owners.
Barry Warsaw (barry) wrote : | # |
I'm not so sure about the OwnersForDomain resource.
First, there should be no arguments to DELETE. You should be able to say:
dump_json('http://
but you cannot currently do that.
Also, I wonder if it makes sense to allow for POSTing multiple email addresses to the <domain>/owners resource to add many owners.
Are there tests for OwnersForDomain? I'm writing some but they fail on these examples.
Barry Warsaw (barry) wrote : | # |
So, actually you will be able to add more than one owner at a time, either after the fact by posting to <domain>/owners, or when the domain is created. You need to use multiple `owner` keys in the POST data.
DELETEing <domain>/owners deletes all owners.
Preview Diff
1 | === modified file 'src/mailman/app/registrar.py' |
2 | --- src/mailman/app/registrar.py 2015-03-28 20:00:24 +0000 |
3 | +++ src/mailman/app/registrar.py 2015-04-05 22:33:23 +0000 |
4 | @@ -161,8 +161,6 @@ |
5 | # For i18n interpolation. |
6 | confirm_url = mlist.domain.confirm_url(event.token) |
7 | email_address = event.pendable['email'] |
8 | - domain_name = mlist.domain.mail_host |
9 | - contact_address = mlist.domain.contact_address |
10 | # Send a verification email to the address. |
11 | template = getUtility(ITemplateLoader).get( |
12 | 'mailman:///{0}/{1}/confirm.txt'.format( |
13 | |
14 | === modified file 'src/mailman/commands/docs/create.rst' |
15 | --- src/mailman/commands/docs/create.rst 2014-04-28 15:23:35 +0000 |
16 | +++ src/mailman/commands/docs/create.rst 2015-04-05 22:33:23 +0000 |
17 | @@ -44,8 +44,7 @@ |
18 | |
19 | >>> from mailman.interfaces.domain import IDomainManager |
20 | >>> getUtility(IDomainManager).get('example.xx') |
21 | - <Domain example.xx, base_url: http://example.xx, |
22 | - contact_address: postmaster@example.xx> |
23 | + <Domain example.xx, base_url: http://example.xx> |
24 | |
25 | You can also create mailing lists in existing domains without the |
26 | auto-creation flag. |
27 | |
28 | === modified file 'src/mailman/commands/tests/test_lists.py' |
29 | --- src/mailman/commands/tests/test_lists.py 2015-03-14 01:16:51 +0000 |
30 | +++ src/mailman/commands/tests/test_lists.py 2015-04-05 22:33:23 +0000 |
31 | @@ -48,7 +48,7 @@ |
32 | # LP: #1166911 - non-matching lists were returned. |
33 | getUtility(IDomainManager).add( |
34 | 'example.net', 'An example domain.', |
35 | - 'http://lists.example.net', 'postmaster@example.net') |
36 | + 'http://lists.example.net') |
37 | create_list('test1@example.com') |
38 | create_list('test2@example.com') |
39 | # Only this one should show up. |
40 | |
41 | === added file 'src/mailman/database/alembic/versions/46e92facee7_add_serverowner_domainowner.py' |
42 | --- src/mailman/database/alembic/versions/46e92facee7_add_serverowner_domainowner.py 1970-01-01 00:00:00 +0000 |
43 | +++ src/mailman/database/alembic/versions/46e92facee7_add_serverowner_domainowner.py 2015-04-05 22:33:23 +0000 |
44 | @@ -0,0 +1,36 @@ |
45 | +"""add_serverowner_domainowner |
46 | + |
47 | +Revision ID: 46e92facee7 |
48 | +Revises: 33e1f5f6fa8 |
49 | +Create Date: 2015-03-20 16:01:25.007242 |
50 | + |
51 | +""" |
52 | + |
53 | +# revision identifiers, used by Alembic. |
54 | +revision = '46e92facee7' |
55 | +down_revision = '33e1f5f6fa8' |
56 | + |
57 | +from alembic import op |
58 | +import sqlalchemy as sa |
59 | + |
60 | + |
61 | +def upgrade(): |
62 | + op.create_table('domain_owner', |
63 | + sa.Column('user_id', sa.Integer(), nullable=False), |
64 | + sa.Column('domain_id', sa.Integer(), nullable=False), |
65 | + sa.ForeignKeyConstraint(['domain_id'], ['domain.id'], ), |
66 | + sa.ForeignKeyConstraint(['user_id'], ['user.id'], ), |
67 | + sa.PrimaryKeyConstraint('user_id', 'domain_id') |
68 | + ) |
69 | + op.add_column('user', sa.Column('is_server_owner', sa.Boolean(), |
70 | + nullable=True)) |
71 | + if op.get_bind().dialect.name != 'sqlite': |
72 | + op.drop_column('domain', 'contact_address') |
73 | + |
74 | + |
75 | +def downgrade(): |
76 | + if op.get_bind().dialect.name != 'sqlite': |
77 | + op.drop_column('user', 'is_server_owner') |
78 | + op.add_column('domain', sa.Column('contact_address', sa.VARCHAR(), |
79 | + nullable=True)) |
80 | + op.drop_table('domain_owner') |
81 | |
82 | === modified file 'src/mailman/interfaces/domain.py' |
83 | --- src/mailman/interfaces/domain.py 2015-01-05 01:22:39 +0000 |
84 | +++ src/mailman/interfaces/domain.py 2015-04-05 22:33:23 +0000 |
85 | @@ -88,9 +88,8 @@ |
86 | description = Attribute( |
87 | 'The human readable description of the domain name.') |
88 | |
89 | - contact_address = Attribute("""\ |
90 | - The contact address for the human at this domain. |
91 | - E.g. postmaster@example.com""") |
92 | + owners = Attribute("""\ |
93 | + The relationship with the user database representing domain owners""") |
94 | |
95 | mailing_lists = Attribute( |
96 | """All mailing lists for this domain. |
97 | @@ -112,7 +111,7 @@ |
98 | class IDomainManager(Interface): |
99 | """The manager of domains.""" |
100 | |
101 | - def add(mail_host, description=None, base_url=None, contact_address=None): |
102 | + def add(mail_host, description=None, base_url=None, owner_id=None): |
103 | """Add a new domain. |
104 | |
105 | :param mail_host: The email host name for the domain. |
106 | @@ -123,10 +122,8 @@ |
107 | interface of the domain. If not given, it defaults to |
108 | http://`mail_host`/ |
109 | :type base_url: string |
110 | - :param contact_address: The email contact address for the human |
111 | - managing the domain. If not given, defaults to |
112 | - postmaster@`mail_host` |
113 | - :type contact_address: string |
114 | + :param owners: List of owners of the domain, defaults to None |
115 | + :type owners: list |
116 | :return: The new domain object |
117 | :rtype: `IDomain` |
118 | :raises `BadDomainSpecificationError`: when the `mail_host` is |
119 | |
120 | === modified file 'src/mailman/model/docs/domains.rst' |
121 | --- src/mailman/model/docs/domains.rst 2014-12-13 18:26:05 +0000 |
122 | +++ src/mailman/model/docs/domains.rst 2015-04-05 22:33:23 +0000 |
123 | @@ -9,6 +9,10 @@ |
124 | >>> manager = getUtility(IDomainManager) |
125 | >>> manager.remove('example.com') |
126 | <Domain example.com...> |
127 | + >>> from mailman.interfaces.usermanager import IUserManager |
128 | + >>> user_manager = getUtility(IUserManager) |
129 | + >>> user = user_manager.create_user('test@example.org') |
130 | + >>> config.db.commit() |
131 | |
132 | Domains are how Mailman interacts with email host names and web host names. |
133 | :: |
134 | @@ -28,17 +32,14 @@ |
135 | is the only required piece. The other parts are inferred from that. |
136 | |
137 | >>> manager.add('example.org') |
138 | - <Domain example.org, base_url: http://example.org, |
139 | - contact_address: postmaster@example.org> |
140 | + <Domain example.org, base_url: http://example.org> |
141 | >>> show_domains() |
142 | - <Domain example.org, base_url: http://example.org, |
143 | - contact_address: postmaster@example.org> |
144 | + <Domain example.org, base_url: http://example.org> |
145 | |
146 | We can remove domains too. |
147 | |
148 | >>> manager.remove('example.org') |
149 | - <Domain example.org, base_url: http://example.org, |
150 | - contact_address: postmaster@example.org> |
151 | + <Domain example.org, base_url: http://example.org> |
152 | >>> show_domains() |
153 | no domains |
154 | |
155 | @@ -46,30 +47,34 @@ |
156 | web interface for the domain. |
157 | |
158 | >>> manager.add('example.com', base_url='https://mail.example.com') |
159 | - <Domain example.com, base_url: https://mail.example.com, |
160 | - contact_address: postmaster@example.com> |
161 | + <Domain example.com, base_url: https://mail.example.com> |
162 | >>> show_domains() |
163 | - <Domain example.com, base_url: https://mail.example.com, |
164 | - contact_address: postmaster@example.com> |
165 | + <Domain example.com, base_url: https://mail.example.com> |
166 | |
167 | -Domains can have explicit descriptions and contact addresses. |
168 | +Domains can have explicit descriptions. |
169 | :: |
170 | |
171 | >>> manager.add( |
172 | ... 'example.net', |
173 | ... base_url='http://lists.example.net', |
174 | - ... contact_address='postmaster@example.com', |
175 | - ... description='The example domain') |
176 | + ... description='The example domain', |
177 | + ... owners=['user@domain.com']) |
178 | <Domain example.net, The example domain, |
179 | - base_url: http://lists.example.net, |
180 | - contact_address: postmaster@example.com> |
181 | + base_url: http://lists.example.net> |
182 | |
183 | >>> show_domains() |
184 | - <Domain example.com, base_url: https://mail.example.com, |
185 | - contact_address: postmaster@example.com> |
186 | + <Domain example.com, base_url: https://mail.example.com> |
187 | <Domain example.net, The example domain, |
188 | - base_url: http://lists.example.net, |
189 | - contact_address: postmaster@example.com> |
190 | + base_url: http://lists.example.net> |
191 | + |
192 | +Domains can have multiple owners, ideally one of the owners should have a |
193 | +verified preferred address. However this is not checked right now and |
194 | +contact_address from config can be used as a fallback. |
195 | +:: |
196 | + |
197 | + >>> net_domain = manager['example.net'] |
198 | + >>> net_domain.add_owner('test@example.org') |
199 | + |
200 | |
201 | Domains can list all associated mailing lists with the mailing_lists property. |
202 | :: |
203 | @@ -105,8 +110,7 @@ |
204 | |
205 | >>> print(manager['example.net']) |
206 | <Domain example.net, The example domain, |
207 | - base_url: http://lists.example.net, |
208 | - contact_address: postmaster@example.com> |
209 | + base_url: http://lists.example.net> |
210 | |
211 | As with dictionaries, you can also get the domain. If the domain does not |
212 | exist, ``None`` or a default is returned. |
213 | @@ -114,8 +118,7 @@ |
214 | |
215 | >>> print(manager.get('example.net')) |
216 | <Domain example.net, The example domain, |
217 | - base_url: http://lists.example.net, |
218 | - contact_address: postmaster@example.com> |
219 | + base_url: http://lists.example.net> |
220 | |
221 | >>> print(manager.get('doesnotexist.com')) |
222 | None |
223 | |
224 | === modified file 'src/mailman/model/domain.py' |
225 | --- src/mailman/model/domain.py 2015-01-05 01:40:47 +0000 |
226 | +++ src/mailman/model/domain.py 2015-04-05 22:33:23 +0000 |
227 | @@ -28,11 +28,15 @@ |
228 | from mailman.interfaces.domain import ( |
229 | BadDomainSpecificationError, DomainCreatedEvent, DomainCreatingEvent, |
230 | DomainDeletedEvent, DomainDeletingEvent, IDomain, IDomainManager) |
231 | +from mailman.interfaces.usermanager import IUserManager |
232 | from mailman.model.mailinglist import MailingList |
233 | +from mailman.model.user import User, DomainOwner |
234 | from urllib.parse import urljoin, urlparse |
235 | from sqlalchemy import Column, Integer, Unicode |
236 | +from sqlalchemy.orm import relationship, backref |
237 | from zope.event import notify |
238 | from zope.interface import implementer |
239 | +from zope.component import getUtility |
240 | |
241 | |
242 | |
243 | |
244 | @@ -44,15 +48,17 @@ |
245 | |
246 | id = Column(Integer, primary_key=True) |
247 | |
248 | - mail_host = Column(Unicode) # TODO: add index? |
249 | + mail_host = Column(Unicode) |
250 | base_url = Column(Unicode) |
251 | description = Column(Unicode) |
252 | - contact_address = Column(Unicode) |
253 | + owners = relationship("User", |
254 | + secondary="domain_owner", |
255 | + backref="domains") |
256 | |
257 | def __init__(self, mail_host, |
258 | description=None, |
259 | base_url=None, |
260 | - contact_address=None): |
261 | + owners=[]): |
262 | """Create and register a domain. |
263 | |
264 | :param mail_host: The host name for the email interface. |
265 | @@ -63,18 +69,16 @@ |
266 | scheme. If not given, it will be constructed from the |
267 | `mail_host` using the http protocol. |
268 | :type base_url: string |
269 | - :param contact_address: The email address to contact a human for this |
270 | - domain. If not given, postmaster@`mail_host` will be used. |
271 | - :type contact_address: string |
272 | + :param owners: List of `User` who are the owners of this domain |
273 | + :type owners: list |
274 | """ |
275 | self.mail_host = mail_host |
276 | self.base_url = (base_url |
277 | if base_url is not None |
278 | else 'http://' + mail_host) |
279 | self.description = description |
280 | - self.contact_address = (contact_address |
281 | - if contact_address is not None |
282 | - else 'postmaster@' + mail_host) |
283 | + if len(owners): |
284 | + self.add_owners(owners) |
285 | |
286 | @property |
287 | def url_host(self): |
288 | @@ -103,13 +107,29 @@ |
289 | def __repr__(self): |
290 | """repr(a_domain)""" |
291 | if self.description is None: |
292 | - return ('<Domain {0.mail_host}, base_url: {0.base_url}, ' |
293 | - 'contact_address: {0.contact_address}>').format(self) |
294 | + return ('<Domain {0.mail_host}, base_url: {0.base_url}>').format(self) |
295 | else: |
296 | return ('<Domain {0.mail_host}, {0.description}, ' |
297 | - 'base_url: {0.base_url}, ' |
298 | - 'contact_address: {0.contact_address}>').format(self) |
299 | - |
300 | + 'base_url: {0.base_url}>').format(self) |
301 | + |
302 | + def add_owner(self, owner): |
303 | + """Add a domain owner""" |
304 | + user_manager = getUtility(IUserManager) |
305 | + user = user_manager.get_user(owner) |
306 | + if user is None: |
307 | + user = user_manager.create_user(owner) |
308 | + self.owners.append(user) |
309 | + |
310 | + def add_owners(self, owners): |
311 | + """Add multiple owners""" |
312 | + assert(isinstance(owners, list)) |
313 | + for owner in owners: |
314 | + self.add_owner(owner) |
315 | + |
316 | + def remove_owner(self, owner): |
317 | + """ Remove a domain owner""" |
318 | + user_manager = getUtility(IUserManager) |
319 | + self.owners.remove(user_manager.get_user(owner)) |
320 | |
321 | |
322 | |
323 | @implementer(IDomainManager) |
324 | @@ -121,15 +141,16 @@ |
325 | mail_host, |
326 | description=None, |
327 | base_url=None, |
328 | - contact_address=None): |
329 | + owners=[]): |
330 | """See `IDomainManager`.""" |
331 | # Be sure the mail_host is not already registered. This is probably |
332 | # a constraint that should (also) be maintained in the database. |
333 | if self.get(mail_host) is not None: |
334 | raise BadDomainSpecificationError( |
335 | 'Duplicate email host: %s' % mail_host) |
336 | + |
337 | notify(DomainCreatingEvent(mail_host)) |
338 | - domain = Domain(mail_host, description, base_url, contact_address) |
339 | + domain = Domain(mail_host, description, base_url, owners) |
340 | store.add(domain) |
341 | notify(DomainCreatedEvent(domain)) |
342 | return domain |
343 | |
344 | === modified file 'src/mailman/model/tests/test_domain.py' |
345 | --- src/mailman/model/tests/test_domain.py 2015-01-05 01:22:39 +0000 |
346 | +++ src/mailman/model/tests/test_domain.py 2015-04-05 22:33:23 +0000 |
347 | @@ -26,9 +26,11 @@ |
348 | import unittest |
349 | |
350 | from mailman.app.lifecycle import create_list |
351 | +from mailman.config import config |
352 | from mailman.interfaces.domain import ( |
353 | DomainCreatedEvent, DomainCreatingEvent, DomainDeletedEvent, |
354 | - DomainDeletingEvent, IDomainManager) |
355 | + DomainDeletingEvent, IDomainManager, BadDomainSpecificationError) |
356 | +from mailman.interfaces.usermanager import IUserManager |
357 | from mailman.interfaces.listmanager import IListManager |
358 | from mailman.testing.helpers import event_subscribers |
359 | from mailman.testing.layers import ConfigLayer |
360 | @@ -78,6 +80,26 @@ |
361 | # Trying to delete a missing domain gives you a KeyError. |
362 | self.assertRaises(KeyError, self._manager.remove, 'doesnotexist.com') |
363 | |
364 | + def test_domain_create_with_owner(self): |
365 | + domain = self._manager.add('example.org', |
366 | + owners=['someuser@example.org']) |
367 | + self.assertEqual(len(domain.owners), 1) |
368 | + self.assertEqual(domain.owners[0].addresses[0].email, |
369 | + 'someuser@example.org') |
370 | + |
371 | + def test_add_domain_owner(self): |
372 | + domain = self._manager.add('example.org') |
373 | + domain.add_owner('someuser@example.org') |
374 | + self.assertEqual(len(domain.owners), 1) |
375 | + self.assertEqual(domain.owners[0].addresses[0].email, |
376 | + 'someuser@example.org') |
377 | + |
378 | + def test_remove_domain_owner(self): |
379 | + domain = self._manager.add('example.org', |
380 | + owners=['someuser@example.org']) |
381 | + domain.remove_owner('someuser@example.org') |
382 | + self.assertEqual(len(domain.owners), 0) |
383 | + |
384 | |
385 | |
386 | |
387 | class TestDomainLifecycleEvents(unittest.TestCase): |
388 | |
389 | === modified file 'src/mailman/model/user.py' |
390 | --- src/mailman/model/user.py 2015-03-20 16:38:00 +0000 |
391 | +++ src/mailman/model/user.py 2015-04-05 22:33:23 +0000 |
392 | @@ -19,6 +19,7 @@ |
393 | |
394 | __all__ = [ |
395 | 'User', |
396 | + 'DomainOwner' |
397 | ] |
398 | |
399 | |
400 | @@ -34,7 +35,7 @@ |
401 | from mailman.model.roster import Memberships |
402 | from mailman.utilities.datetime import factory as date_factory |
403 | from mailman.utilities.uid import UniqueIDFactory |
404 | -from sqlalchemy import Column, DateTime, ForeignKey, Integer, Unicode |
405 | +from sqlalchemy import Column, DateTime, ForeignKey, Integer, Unicode, Boolean |
406 | from sqlalchemy.orm import relationship, backref |
407 | from zope.event import notify |
408 | from zope.interface import implementer |
409 | @@ -55,6 +56,7 @@ |
410 | _password = Column('password', Unicode) |
411 | _user_id = Column(UUID, index=True) |
412 | _created_on = Column(DateTime) |
413 | + is_server_owner = Column(Boolean, default=False) |
414 | |
415 | addresses = relationship( |
416 | 'Address', backref='user', |
417 | @@ -176,3 +178,11 @@ |
418 | @property |
419 | def memberships(self): |
420 | return Memberships(self) |
421 | + |
422 | + |
423 | +class DomainOwner(Model): |
424 | + """Domain to owners(user) association class""" |
425 | + |
426 | + __tablename__ = 'domain_owner' |
427 | + user_id = Column(Integer, ForeignKey('user.id'), primary_key=True) |
428 | + domain_id = Column(Integer, ForeignKey('domain.id'), primary_key=True) |
429 | |
430 | === modified file 'src/mailman/rest/docs/addresses.rst' |
431 | --- src/mailman/rest/docs/addresses.rst 2015-02-14 01:35:35 +0000 |
432 | +++ src/mailman/rest/docs/addresses.rst 2015-04-05 22:33:23 +0000 |
433 | @@ -190,6 +190,7 @@ |
434 | created_on: 2005-08-01T07:49:23 |
435 | display_name: Cris X. Person |
436 | http_etag: "..." |
437 | + is_server_owner: False |
438 | password: ... |
439 | self_link: http://localhost:9001/3.0/users/1 |
440 | user_id: 1 |
441 | |
442 | === modified file 'src/mailman/rest/docs/domains.rst' |
443 | --- src/mailman/rest/docs/domains.rst 2014-12-16 01:01:53 +0000 |
444 | +++ src/mailman/rest/docs/domains.rst 2015-04-05 22:33:23 +0000 |
445 | @@ -29,14 +29,12 @@ |
446 | >>> domain_manager.add( |
447 | ... 'example.com', 'An example domain', 'http://lists.example.com') |
448 | <Domain example.com, An example domain, |
449 | - base_url: http://lists.example.com, |
450 | - contact_address: postmaster@example.com> |
451 | + base_url: http://lists.example.com> |
452 | >>> transaction.commit() |
453 | |
454 | >>> dump_json('http://localhost:9001/3.0/domains') |
455 | entry 0: |
456 | base_url: http://lists.example.com |
457 | - contact_address: postmaster@example.com |
458 | description: An example domain |
459 | http_etag: "..." |
460 | mail_host: example.com |
461 | @@ -51,24 +49,19 @@ |
462 | |
463 | >>> domain_manager.add( |
464 | ... 'example.org', |
465 | - ... base_url='http://mail.example.org', |
466 | - ... contact_address='listmaster@example.org') |
467 | - <Domain example.org, base_url: http://mail.example.org, |
468 | - contact_address: listmaster@example.org> |
469 | + ... base_url='http://mail.example.org') |
470 | + <Domain example.org, base_url: http://mail.example.org> |
471 | >>> domain_manager.add( |
472 | ... 'lists.example.net', |
473 | ... 'Porkmasters', |
474 | - ... 'http://example.net', |
475 | - ... 'porkmaster@example.net') |
476 | + ... 'http://example.net') |
477 | <Domain lists.example.net, Porkmasters, |
478 | - base_url: http://example.net, |
479 | - contact_address: porkmaster@example.net> |
480 | + base_url: http://example.net> |
481 | >>> transaction.commit() |
482 | |
483 | >>> dump_json('http://localhost:9001/3.0/domains') |
484 | entry 0: |
485 | base_url: http://lists.example.com |
486 | - contact_address: postmaster@example.com |
487 | description: An example domain |
488 | http_etag: "..." |
489 | mail_host: example.com |
490 | @@ -76,7 +69,6 @@ |
491 | url_host: lists.example.com |
492 | entry 1: |
493 | base_url: http://mail.example.org |
494 | - contact_address: listmaster@example.org |
495 | description: None |
496 | http_etag: "..." |
497 | mail_host: example.org |
498 | @@ -84,7 +76,6 @@ |
499 | url_host: mail.example.org |
500 | entry 2: |
501 | base_url: http://example.net |
502 | - contact_address: porkmaster@example.net |
503 | description: Porkmasters |
504 | http_etag: "..." |
505 | mail_host: lists.example.net |
506 | @@ -103,7 +94,6 @@ |
507 | |
508 | >>> dump_json('http://localhost:9001/3.0/domains/lists.example.net') |
509 | base_url: http://example.net |
510 | - contact_address: porkmaster@example.net |
511 | description: Porkmasters |
512 | http_etag: "..." |
513 | mail_host: lists.example.net |
514 | @@ -165,7 +155,6 @@ |
515 | |
516 | >>> dump_json('http://localhost:9001/3.0/domains/lists.example.com') |
517 | base_url: http://lists.example.com |
518 | - contact_address: postmaster@lists.example.com |
519 | description: None |
520 | http_etag: "..." |
521 | mail_host: lists.example.com |
522 | @@ -176,9 +165,7 @@ |
523 | :: |
524 | |
525 | >>> domain_manager['lists.example.com'] |
526 | - <Domain lists.example.com, |
527 | - base_url: http://lists.example.com, |
528 | - contact_address: postmaster@lists.example.com> |
529 | + <Domain lists.example.com, base_url: http://lists.example.com> |
530 | |
531 | # Unlock the database. |
532 | >>> transaction.abort() |
533 | @@ -190,8 +177,7 @@ |
534 | >>> dump_json('http://localhost:9001/3.0/domains', { |
535 | ... 'mail_host': 'my.example.com', |
536 | ... 'description': 'My new domain', |
537 | - ... 'base_url': 'http://allmy.example.com', |
538 | - ... 'contact_address': 'helpme@example.com' |
539 | + ... 'base_url': 'http://allmy.example.com' |
540 | ... }) |
541 | content-length: 0 |
542 | date: ... |
543 | @@ -200,7 +186,6 @@ |
544 | |
545 | >>> dump_json('http://localhost:9001/3.0/domains/my.example.com') |
546 | base_url: http://allmy.example.com |
547 | - contact_address: helpme@example.com |
548 | description: My new domain |
549 | http_etag: "..." |
550 | mail_host: my.example.com |
551 | @@ -208,9 +193,7 @@ |
552 | url_host: allmy.example.com |
553 | |
554 | >>> domain_manager['my.example.com'] |
555 | - <Domain my.example.com, My new domain, |
556 | - base_url: http://allmy.example.com, |
557 | - contact_address: helpme@example.com> |
558 | + <Domain my.example.com, My new domain, base_url: http://allmy.example.com> |
559 | |
560 | # Unlock the database. |
561 | >>> transaction.abort() |
562 | |
563 | === modified file 'src/mailman/rest/docs/users.rst' |
564 | --- src/mailman/rest/docs/users.rst 2014-12-22 18:40:30 +0000 |
565 | +++ src/mailman/rest/docs/users.rst 2015-04-05 22:33:23 +0000 |
566 | @@ -34,6 +34,7 @@ |
567 | created_on: 2005-08-01T07:49:23 |
568 | display_name: Anne Person |
569 | http_etag: "..." |
570 | + is_server_owner: False |
571 | self_link: http://localhost:9001/3.0/users/1 |
572 | user_id: 1 |
573 | http_etag: "..." |
574 | @@ -50,11 +51,13 @@ |
575 | created_on: 2005-08-01T07:49:23 |
576 | display_name: Anne Person |
577 | http_etag: "..." |
578 | + is_server_owner: False |
579 | self_link: http://localhost:9001/3.0/users/1 |
580 | user_id: 1 |
581 | entry 1: |
582 | created_on: 2005-08-01T07:49:23 |
583 | http_etag: "..." |
584 | + is_server_owner: False |
585 | self_link: http://localhost:9001/3.0/users/2 |
586 | user_id: 2 |
587 | http_etag: "..." |
588 | @@ -76,6 +79,7 @@ |
589 | created_on: 2005-08-01T07:49:23 |
590 | display_name: Anne Person |
591 | http_etag: "..." |
592 | + is_server_owner: False |
593 | self_link: http://localhost:9001/3.0/users/1 |
594 | user_id: 1 |
595 | http_etag: "..." |
596 | @@ -86,6 +90,7 @@ |
597 | entry 0: |
598 | created_on: 2005-08-01T07:49:23 |
599 | http_etag: "..." |
600 | + is_server_owner: False |
601 | self_link: http://localhost:9001/3.0/users/2 |
602 | user_id: 2 |
603 | http_etag: "..." |
604 | @@ -120,6 +125,7 @@ |
605 | >>> dump_json('http://localhost:9001/3.0/users/3') |
606 | created_on: 2005-08-01T07:49:23 |
607 | http_etag: "..." |
608 | + is_server_owner: False |
609 | password: {plaintext}... |
610 | self_link: http://localhost:9001/3.0/users/3 |
611 | user_id: 3 |
612 | @@ -131,6 +137,7 @@ |
613 | >>> dump_json('http://localhost:9001/3.0/users/cris@example.com') |
614 | created_on: 2005-08-01T07:49:23 |
615 | http_etag: "..." |
616 | + is_server_owner: False |
617 | password: {plaintext}... |
618 | self_link: http://localhost:9001/3.0/users/3 |
619 | user_id: 3 |
620 | @@ -158,6 +165,7 @@ |
621 | created_on: 2005-08-01T07:49:23 |
622 | display_name: Dave Person |
623 | http_etag: "..." |
624 | + is_server_owner: False |
625 | password: {plaintext}... |
626 | self_link: http://localhost:9001/3.0/users/4 |
627 | user_id: 4 |
628 | @@ -190,6 +198,7 @@ |
629 | created_on: 2005-08-01T07:49:23 |
630 | display_name: Elly Person |
631 | http_etag: "..." |
632 | + is_server_owner: False |
633 | password: {plaintext}supersekrit |
634 | self_link: http://localhost:9001/3.0/users/5 |
635 | user_id: 5 |
636 | @@ -214,6 +223,7 @@ |
637 | created_on: 2005-08-01T07:49:23 |
638 | display_name: David Person |
639 | http_etag: "..." |
640 | + is_server_owner: False |
641 | password: {plaintext}... |
642 | self_link: http://localhost:9001/3.0/users/4 |
643 | user_id: 4 |
644 | @@ -238,6 +248,7 @@ |
645 | created_on: 2005-08-01T07:49:23 |
646 | display_name: David Person |
647 | http_etag: "..." |
648 | + is_server_owner: False |
649 | password: {plaintext}clockwork angels |
650 | self_link: http://localhost:9001/3.0/users/4 |
651 | user_id: 4 |
652 | @@ -260,6 +271,7 @@ |
653 | created_on: 2005-08-01T07:49:23 |
654 | display_name: David Personhood |
655 | http_etag: "..." |
656 | + is_server_owner: False |
657 | password: {plaintext}the garden |
658 | self_link: http://localhost:9001/3.0/users/4 |
659 | user_id: 4 |
660 | @@ -343,6 +355,7 @@ |
661 | created_on: 2005-08-01T07:49:23 |
662 | display_name: Fred Person |
663 | http_etag: "..." |
664 | + is_server_owner: False |
665 | self_link: http://localhost:9001/3.0/users/6 |
666 | user_id: 6 |
667 | |
668 | @@ -350,6 +363,7 @@ |
669 | created_on: 2005-08-01T07:49:23 |
670 | display_name: Fred Person |
671 | http_etag: "..." |
672 | + is_server_owner: False |
673 | self_link: http://localhost:9001/3.0/users/6 |
674 | user_id: 6 |
675 | |
676 | @@ -357,6 +371,7 @@ |
677 | created_on: 2005-08-01T07:49:23 |
678 | display_name: Fred Person |
679 | http_etag: "..." |
680 | + is_server_owner: False |
681 | self_link: http://localhost:9001/3.0/users/6 |
682 | user_id: 6 |
683 | |
684 | @@ -364,6 +379,7 @@ |
685 | created_on: 2005-08-01T07:49:23 |
686 | display_name: Fred Person |
687 | http_etag: "..." |
688 | + is_server_owner: False |
689 | self_link: http://localhost:9001/3.0/users/6 |
690 | user_id: 6 |
691 | |
692 | @@ -382,6 +398,7 @@ |
693 | created_on: 2005-08-01T07:49:23 |
694 | display_name: Elly Person |
695 | http_etag: "..." |
696 | + is_server_owner: False |
697 | password: {plaintext}supersekrit |
698 | self_link: http://localhost:9001/3.0/users/5 |
699 | user_id: 5 |
700 | |
701 | === modified file 'src/mailman/rest/domains.py' |
702 | --- src/mailman/rest/domains.py 2015-01-05 01:40:47 +0000 |
703 | +++ src/mailman/rest/domains.py 2015-04-05 22:33:23 +0000 |
704 | @@ -25,10 +25,12 @@ |
705 | |
706 | from mailman.interfaces.domain import ( |
707 | BadDomainSpecificationError, IDomainManager) |
708 | +from mailman.interfaces.usermanager import IUserManager |
709 | from mailman.rest.helpers import ( |
710 | BadRequest, CollectionMixin, NotFound, bad_request, child, created, etag, |
711 | no_content, not_found, okay, path_to) |
712 | from mailman.rest.lists import ListsForDomain |
713 | +from mailman.rest.users import OwnersForDomain |
714 | from mailman.rest.validator import Validator |
715 | from zope.component import getUtility |
716 | |
717 | @@ -41,7 +43,6 @@ |
718 | """See `CollectionMixin`.""" |
719 | return dict( |
720 | base_url=domain.base_url, |
721 | - contact_address=domain.contact_address, |
722 | description=domain.description, |
723 | mail_host=domain.mail_host, |
724 | self_link=path_to('domains/{0}'.format(domain.mail_host)), |
725 | @@ -88,6 +89,17 @@ |
726 | else: |
727 | return BadRequest(), [] |
728 | |
729 | + @child() |
730 | + def owners(self, request, segments): |
731 | + """/domains/<domain>/owners""" |
732 | + if len(segments) == 0: |
733 | + domain = getUtility(IDomainManager).get(self._domain) |
734 | + if domain is None: |
735 | + return NotFound() |
736 | + return OwnersForDomain(domain) |
737 | + else: |
738 | + return BadRequest(), [] |
739 | + |
740 | |
741 | class AllDomains(_DomainBase): |
742 | """The domains.""" |
743 | @@ -99,12 +111,13 @@ |
744 | validator = Validator(mail_host=str, |
745 | description=str, |
746 | base_url=str, |
747 | - contact_address=str, |
748 | + owners=list, |
749 | _optional=('description', 'base_url', |
750 | - 'contact_address')) |
751 | - domain = domain_manager.add(**validator(request)) |
752 | - except BadDomainSpecificationError: |
753 | - bad_request(response, b'Domain exists') |
754 | + 'owners')) |
755 | + values = validator(request) |
756 | + domain = domain_manager.add(**values) |
757 | + except BadDomainSpecificationError as error: |
758 | + bad_request(response, str(error)) |
759 | except ValueError as error: |
760 | bad_request(response, str(error)) |
761 | else: |
762 | |
763 | === modified file 'src/mailman/rest/lists.py' |
764 | --- src/mailman/rest/lists.py 2015-01-05 01:40:47 +0000 |
765 | +++ src/mailman/rest/lists.py 2015-04-05 22:33:23 +0000 |
766 | @@ -1,4 +1,4 @@ |
767 | -# Copyright (C) 2010-2015 by the Free Software Foundation, Inc. |
768 | + # Copyright (C) 2010-2015 by the Free Software Foundation, Inc. |
769 | # |
770 | # This file is part of GNU Mailman. |
771 | # |
772 | |
773 | === modified file 'src/mailman/rest/tests/test_domains.py' |
774 | --- src/mailman/rest/tests/test_domains.py 2015-01-05 01:40:47 +0000 |
775 | +++ src/mailman/rest/tests/test_domains.py 2015-04-05 22:33:23 +0000 |
776 | @@ -27,6 +27,7 @@ |
777 | from mailman.app.lifecycle import create_list |
778 | from mailman.database.transaction import transaction |
779 | from mailman.interfaces.listmanager import IListManager |
780 | +from mailman.interfaces.domain import IDomainManager |
781 | from mailman.testing.helpers import call_api |
782 | from mailman.testing.layers import RESTLayer |
783 | from urllib.error import HTTPError |
784 | @@ -41,6 +42,17 @@ |
785 | with transaction(): |
786 | self._mlist = create_list('test@example.com') |
787 | |
788 | + def test_create_domains(self): |
789 | + """Test Create domain via REST""" |
790 | + data = {'mail_host': 'example.org', |
791 | + 'description': 'Example domain', |
792 | + 'base_url': 'http://example.org', |
793 | + 'owners': ['someone@example.com', |
794 | + 'secondowner@example.com',]} |
795 | + content, response = call_api('http://localhost:9001/3.0/domains', |
796 | + data, method="POST") |
797 | + self.assertEqual(response.status, 201) |
798 | + |
799 | def test_bogus_endpoint_extension(self): |
800 | # /domains/<domain>/lists/<anything> is not a valid endpoint. |
801 | with self.assertRaises(HTTPError) as cm: |
802 | |
803 | === modified file 'src/mailman/rest/users.py' |
804 | --- src/mailman/rest/users.py 2015-03-20 16:38:00 +0000 |
805 | +++ src/mailman/rest/users.py 2015-04-05 22:33:23 +0000 |
806 | @@ -22,6 +22,7 @@ |
807 | 'AddressUser', |
808 | 'AllUsers', |
809 | 'Login', |
810 | + 'OwnersForDomain', |
811 | ] |
812 | |
813 | |
814 | @@ -67,8 +68,9 @@ |
815 | email=str, |
816 | display_name=str, |
817 | password=str, |
818 | - _optional=('display_name', 'password'), |
819 | - ) |
820 | + is_server_owner=bool, |
821 | + _optional=('display_name', 'password', 'is_server_owner'), |
822 | +) |
823 | |
824 | |
825 | |
826 | |
827 | @@ -108,7 +110,8 @@ |
828 | user_id=user_id, |
829 | created_on=user.created_on, |
830 | self_link=path_to('users/{}'.format(user_id)), |
831 | - ) |
832 | + is_server_owner=user.is_server_owner, |
833 | + ) |
834 | # Add the password attribute, only if the user has a password. Same |
835 | # with the real name. These could be None or the empty string. |
836 | if user.password: |
837 | @@ -293,7 +296,8 @@ |
838 | del fields['email'] |
839 | fields['user_id'] = int |
840 | fields['auto_create'] = as_boolean |
841 | - fields['_optional'] = fields['_optional'] + ('user_id', 'auto_create') |
842 | + fields['_optional'] = fields['_optional'] + ('user_id', 'auto_create', |
843 | + 'is_server_owner') |
844 | try: |
845 | validator = Validator(**fields) |
846 | arguments = validator(request) |
847 | @@ -328,7 +332,8 @@ |
848 | # Process post data and check for an existing user. |
849 | fields = CREATION_FIELDS.copy() |
850 | fields['user_id'] = int |
851 | - fields['_optional'] = fields['_optional'] + ('user_id', 'email') |
852 | + fields['_optional'] = fields['_optional'] + ('user_id', 'email', |
853 | + 'is_server_owner') |
854 | try: |
855 | validator = Validator(**fields) |
856 | arguments = validator(request) |
857 | @@ -377,3 +382,41 @@ |
858 | no_content(response) |
859 | else: |
860 | forbidden(response) |
861 | + |
862 | +class OwnersForDomain(_UserBase): |
863 | + """Owners for a particular domain.""" |
864 | + |
865 | + def __init__(self, domain): |
866 | + self._domain = domain |
867 | + |
868 | + def on_get(self, request, response): |
869 | + """/domains/<domain>/owners""" |
870 | + resource = self._make_collection(request) |
871 | + okay(response, etag(resource)) |
872 | + |
873 | + def on_post(self, request, response): |
874 | + """POST to /domains/<domain>/owners """ |
875 | + validator = Validator(owner=GetterSetter(str)) |
876 | + try: |
877 | + values = validator(request) |
878 | + except ValueError as error: |
879 | + bad_request(response, str(error)) |
880 | + return |
881 | + self._domain.add_owner(values['owner']) |
882 | + return no_content(response) |
883 | + |
884 | + def on_delete(self, request, response): |
885 | + """DELETE to /domains/<domain>/owners""" |
886 | + validator = Validator(owner=GetterSetter(str)) |
887 | + try: |
888 | + values = validator(request) |
889 | + except ValueError as error: |
890 | + bad_request(response, str(error)) |
891 | + return |
892 | + self._domain.remove_owner(owner) |
893 | + return no_content(response) |
894 | + |
895 | + @paginate |
896 | + def _get_collection(self, request): |
897 | + """See `CollectionMixin`.""" |
898 | + return list(self._domain.owners) |
899 | |
900 | === modified file 'src/mailman/testing/layers.py' |
901 | --- src/mailman/testing/layers.py 2015-01-05 01:22:39 +0000 |
902 | +++ src/mailman/testing/layers.py 2015-04-05 22:33:23 +0000 |
903 | @@ -200,7 +200,7 @@ |
904 | with transaction(): |
905 | getUtility(IDomainManager).add( |
906 | 'example.com', 'An example domain.', |
907 | - 'http://lists.example.com', 'postmaster@example.com') |
908 | + 'http://lists.example.com') |
909 | |
910 | @classmethod |
911 | def testTearDown(cls): |
Thanks Abhilash. Looks pretty good so far, but I have a number of questions and comments inlined.