add tests, fix docs, remove contact_address

3 files changed, 70 insertions, 28 deletions

diff --git a/src/mailman/model/docs/domains.rst b/src/mailman/model/docs/domains.rst
index abb594a62..491c0e612 100644
--- a/src/mailman/model/docs/domains.rst
+++ b/src/mailman/model/docs/domains.rst
@@ -9,6 +9,10 @@ Domains
     >>> manager = getUtility(IDomainManager)
     >>> manager.remove('example.com')
     <Domain example.com...>
+	>>> from mailman.interfaces.usermanager import IUserManager
+	>>> user_manager = getUtility(IUserManager)
+    >>> user = user_manager.create_user('test@example.org')
+	>>> config.db.commit()
 
 Domains are how Mailman interacts with email host names and web host names.
 ::
@@ -28,17 +32,14 @@ Adding a domain requires some basic information, of which the email host name
 is the only required piece.  The other parts are inferred from that.
 
     >>> manager.add('example.org')
-    <Domain example.org, base_url: http://example.org,
-            contact_address: postmaster@example.org>
+    <Domain example.org, base_url: http://example.org>
     >>> show_domains()
-    <Domain example.org, base_url: http://example.org,
-            contact_address: postmaster@example.org>
+    <Domain example.org, base_url: http://example.org>
 
 We can remove domains too.
 
     >>> manager.remove('example.org')
-    <Domain example.org, base_url: http://example.org,
-            contact_address: postmaster@example.org>
+    <Domain example.org, base_url: http://example.org>
     >>> show_domains()
     no domains
 
@@ -46,30 +47,34 @@ Sometimes the email host name is different than the base url for hitting the
 web interface for the domain.
 
     >>> manager.add('example.com', base_url='https://mail.example.com')
-    <Domain example.com, base_url: https://mail.example.com,
-            contact_address: postmaster@example.com>
+    <Domain example.com, base_url: https://mail.example.com>
     >>> show_domains()
-    <Domain example.com, base_url: https://mail.example.com,
-            contact_address: postmaster@example.com>
+    <Domain example.com, base_url: https://mail.example.com>
 
-Domains can have explicit descriptions and contact addresses.
+Domains can have explicit descriptions.
 ::
 
     >>> manager.add(
     ...     'example.net',
     ...     base_url='http://lists.example.net',
-    ...     contact_address='postmaster@example.com',
-    ...     description='The example domain')
+    ...     description='The example domain',
+    ...     owner_id=1)
     <Domain example.net, The example domain,
-            base_url: http://lists.example.net,
-            contact_address: postmaster@example.com>
+            base_url: http://lists.example.net>
 
     >>> show_domains()
-    <Domain example.com, base_url: https://mail.example.com,
-            contact_address: postmaster@example.com>
+    <Domain example.com, base_url: https://mail.example.com>
     <Domain example.net, The example domain,
-            base_url: http://lists.example.net,
-            contact_address: postmaster@example.com>
+            base_url: http://lists.example.net>
+
+Domains can have multiple number of owners, ideally one of the owners
+should  have a verified preferred address. However this is not checked
+right now and contact_address from config is used as a fallback.
+::
+
+   >>> net_domain = manager['example.net']
+   >>> net_domain.add_owner(user_manager.get_user('test@example.org'))
+
 
 Domains can list all associated mailing lists with the mailing_lists property.
 ::
@@ -105,8 +110,7 @@ In the global domain manager, domains are indexed by their email host name.
 
     >>> print(manager['example.net'])
     <Domain example.net, The example domain,
-            base_url: http://lists.example.net,
-            contact_address: postmaster@example.com>
+            base_url: http://lists.example.net>
 
 As with dictionaries, you can also get the domain.  If the domain does not
 exist, ``None`` or a default is returned.
@@ -114,8 +118,7 @@ exist, ``None`` or a default is returned.
 
     >>> print(manager.get('example.net'))
     <Domain example.net, The example domain,
-            base_url: http://lists.example.net,
-            contact_address: postmaster@example.com>
+            base_url: http://lists.example.net>
 
     >>> print(manager.get('doesnotexist.com'))
     None
diff --git a/src/mailman/model/domain.py b/src/mailman/model/domain.py
index 7167d211e..a3a059cb9 100644
--- a/src/mailman/model/domain.py
+++ b/src/mailman/model/domain.py
@@ -29,7 +29,7 @@ from mailman.interfaces.domain import (
     BadDomainSpecificationError, DomainCreatedEvent, DomainCreatingEvent,
     DomainDeletedEvent, DomainDeletingEvent, IDomain, IDomainManager)
 from mailman.model.mailinglist import MailingList
-from mailman.model.user import User
+from mailman.model.user import User, Owner
 from urllib.parse import urljoin, urlparse
 from sqlalchemy import Column, Integer, Unicode
 from sqlalchemy.orm import relationship, backref
@@ -105,14 +105,20 @@ class Domain(Model):
     def __repr__(self):
         """repr(a_domain)"""
         if self.description is None:
-            return ('<Domain {0.mail_host}, base_url: {0.base_url}').format(self)
+            return ('<Domain {0.mail_host}, base_url: {0.base_url}>').format(self)
         else:
             return ('<Domain {0.mail_host}, {0.description}, '
-                    'base_url: {0.base_url}, ').format(self)
+                    'base_url: {0.base_url}>').format(self)
 
     def add_owner(self, owner):
-        """ Add an owner to a domain"""
+        """ Add a domain owner"""
         self.owners.append(owner)
+
+    @dbconnection
+    def remove_owner(self, store, owner):
+        """ Remove a domain owner"""
+        self.owners.remove(owner)
+
 
 @implementer(IDomainManager)
 class DomainManager:
@@ -137,6 +143,7 @@ class DomainManager:
             if owner is None:
                 raise BadDomainSpecificationError(
                     'Owner of this domain does not exist')
+
         notify(DomainCreatingEvent(mail_host))
         domain = Domain(mail_host, description, base_url, owner)
         store.add(domain)
diff --git a/src/mailman/model/tests/test_domain.py b/src/mailman/model/tests/test_domain.py
index b4a6dd75c..ffa79e32e 100644
--- a/src/mailman/model/tests/test_domain.py
+++ b/src/mailman/model/tests/test_domain.py
@@ -26,9 +26,11 @@ __all__ = [
 import unittest
 
 from mailman.app.lifecycle import create_list
+from mailman.config import config
 from mailman.interfaces.domain import (
     DomainCreatedEvent, DomainCreatingEvent, DomainDeletedEvent,
-    DomainDeletingEvent, IDomainManager)
+    DomainDeletingEvent, IDomainManager, BadDomainSpecificationError)
+from mailman.interfaces.usermanager import IUserManager
 from mailman.interfaces.listmanager import IListManager
 from mailman.testing.helpers import event_subscribers
 from mailman.testing.layers import ConfigLayer
@@ -78,6 +80,32 @@ class TestDomainManager(unittest.TestCase):
         # Trying to delete a missing domain gives you a KeyError.
         self.assertRaises(KeyError, self._manager.remove, 'doesnotexist.com')
 
+    def test_domain_create_with_owner(self):
+        user = getUtility(IUserManager).create_user('someuser@somedomain.org')
+        config.db.commit()
+        domain = self._manager.add('example.org', owner_id=user.id)
+        self.assertEqual(len(domain.owners), 1)
+        self.assertEqual(domain.owners[0].id, user.id)
+
+    def test_domain_create_with_non_existent_owner(self):
+        with self.assertRaises(BadDomainSpecificationError):
+            self._manager.add('testdomain.org', owner_id=100)
+
+    def test_add_domain_owner(self):
+        user = getUtility(IUserManager).create_user('someuser@somedomain.org')
+        config.db.commit()
+        domain = self._manager.add('example.org')
+        domain.add_owner(user)
+        self.assertEqual(len(domain.owners), 1)
+        self.assertEqual(domain.owners[0].id, user.id)
+
+    def test_remove_domain_owner(self):
+        user = getUtility(IUserManager).create_user('someuser@somedomain.org')
+        config.db.commit()
+        domain = self._manager.add('example.org', owner_id=user.id)
+        domain.remove_owner(user)
+        self.assertEqual(len(domain.owners), 0)
+
 
 
 class TestDomainLifecycleEvents(unittest.TestCase):
@@ -110,3 +138,7 @@ class TestDomainLifecycleEvents(unittest.TestCase):
         self.assertEqual(listmanager.get('dog@example.org'), None)
         self.assertEqual(listmanager.get('ewe@example.com'), ewe)
         self.assertEqual(listmanager.get('fly@example.com'), fly)
+
+    def test_owners_are_deleted_when_domain_is(self):
+        #TODO: Complete this
+        pass