From fe57b60b6bfd0f05ae6a8592337f81eff4883049 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Aur=C3=A9lien=20Bompard?= <aurelien@bompard.org>
Date: Wed, 13 Jan 2016 00:29:13 +0100
Subject: [PATCH] Add the self_link for bans and factor some code

---
 src/mailman/rest/bans.py             | 43 ++++++++++++++++++++++--------------
 src/mailman/rest/docs/membership.rst | 16 ++++++++++++--
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/src/mailman/rest/bans.py b/src/mailman/rest/bans.py
index bf331f46e..b6c25b749 100644
--- a/src/mailman/rest/bans.py
+++ b/src/mailman/rest/bans.py
@@ -26,17 +26,32 @@ __all__ = [
 from mailman.interfaces.bans import IBanManager
 from mailman.rest.helpers import (
     CollectionMixin, bad_request, child, created, etag, no_content, not_found,
-    okay)
+    okay, path_to)
 from mailman.rest.validator import Validator
 
 
 
-class BannedEmail:
-    """A banned email."""
+class _BannedBase:
+    """Common base class."""
 
-    def __init__(self, mailing_list, email):
+    def __init__(self, mailing_list):
         self._mlist = mailing_list
         self.ban_manager = IBanManager(self._mlist)
+
+    def _location(self, email):
+        if self._mlist is None:
+            base_location = ''
+        else:
+            base_location = 'lists/{}/'.format(self._mlist.list_id)
+        return path_to(
+            '{}bans/{}'.format(base_location, email), self.api_version)
+
+
+class BannedEmail(_BannedBase):
+    """A banned email."""
+
+    def __init__(self, mailing_list, email):
+        super().__init__(mailing_list)
         self._email = email
 
     def on_get(self, request, response):
@@ -47,7 +62,11 @@ class BannedEmail:
             not_found(
                 response, 'Email {} is not banned'.format(self._email))
         else:
-            resource = dict(email=self._email)
+            resource = dict(
+                email=self._email,
+                list_id=self._mlist.list_id if self._mlist else None,
+                self_link=self._location(self._email),
+                )
             okay(response, etag(resource))
 
     def on_delete(self, request, response):
@@ -62,18 +81,15 @@ class BannedEmail:
             no_content(response)
 
 
-class BannedEmails(CollectionMixin):
+class BannedEmails(_BannedBase, CollectionMixin):
     """The list of all banned emails."""
 
-    def __init__(self, mailing_list):
-        self._mlist = mailing_list
-        self.ban_manager = IBanManager(self._mlist)
-
     def _resource_as_dict(self, ban):
         """See `CollectionMixin`."""
         return dict(
             email=ban.email,
             list_id=ban.list_id,
+            self_link=self._location(ban.email),
             )
 
     def _get_collection(self, request):
@@ -97,12 +113,7 @@ class BannedEmails(CollectionMixin):
             bad_request(response, b'Address is already banned')
         else:
             self.ban_manager.ban(email)
-            if self._mlist is None:
-                base_location = ''
-            else:
-                base_location = 'lists/{}/'.format(self._mlist.list_id)
-            location = self.path_to('{}bans/{}'.format(base_location, email))
-            created(response, location)
+            created(response, self._location(email))
 
     @child(r'^(?P<email>[^/]+)')
     def email(self, request, segments, **kw):
diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst
index 8470eb3c6..4c46e8e29 100644
--- a/src/mailman/rest/docs/membership.rst
+++ b/src/mailman/rest/docs/membership.rst
@@ -988,13 +988,18 @@ issuing a GET request on the /bans child::
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com/bans')
     entry 0:
         email: banned@example.com
+        http_etag: "..."
+        list_id: ant.example.com
+        self_link: http://localhost:9001/3.0/lists/ant.example.com/bans/banned@example.com
     ...
 
 Or checking if a single address is banned:
 
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com/bans/banned@example.com')
     email: banned@example.com
-    http_etag: ...
+    http_etag: "..."
+    list_id: ant.example.com
+    self_link: http://localhost:9001/3.0/lists/ant.example.com/bans/banned@example.com
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com/bans/someone-else@example.com')
     Traceback (most recent call last):
     ...
@@ -1025,14 +1030,21 @@ To ban an address from subscribing to every list, you can use the global /bans e
     ...           {'email': 'banned@example.com'})
     content-length: 0
     ...
+    location: http://localhost:9001/3.0/bans/banned@example.com
+    ...
     status: 201
     >>> dump_json('http://localhost:9001/3.0/bans')
     entry 0:
         email: banned@example.com
+        http_etag: "..."
+        list_id: None
+        self_link: http://localhost:9001/3.0/bans/banned@example.com
     ...
     >>> dump_json('http://localhost:9001/3.0/bans/banned@example.com')
     email: banned@example.com
-    http_etag: ...
+    http_etag: "..."
+    list_id: None
+    self_link: http://localhost:9001/3.0/bans/banned@example.com
     >>> dump_json('http://localhost:9001/3.0/bans/banned@example.com',
     ...           method='DELETE')
     content-length: 0
-- 
2.11.4.GIT