Merge lp:~postfuturist/mailman/rest-api-delivery_mode into lp:mailman

[Stephen A. Goss]

Currently, in the REST API for members/subscriptions you can create a subscription with a specific delivery_mode (regular or various digest modes). However, you can't get this information back from the REST API or change it. This branch addresses these shortcomings by adding delivery_mode to the information returned by the API when GETing member info, it also allows you to PATCH the delivery_mode of a member. It handles edge/error cases, like patching nonexistent members, patching invalid parameters, etc. All cases are tested (I think). This resolves 833132 and 833376.

[Barry Warsaw (barry)]

Thanks for the branch! It's generally better to provide separate branches for each of the separate bugs though, otherwise it's more difficult to review and apply. In this case, no worries, I'll work it out.

[Barry Warsaw (barry)]

Oh, one thing I'm going to remove is the prohibition against PATCHing with no attributes. I think that should be fine (and the member won't change).

=== modified file 'src/mailman/rest/docs/addresses.txt'
--- src/mailman/rest/docs/addresses.txt	2011-04-26 02:23:05 +0000
+++ src/mailman/rest/docs/addresses.txt	2011-08-25 00:14:24 +0000
@@ -169,6 +169,7 @@
     ...           '[e-mail]/memberships')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -176,6 +177,7 @@
         user: http://localhost:9001/3.0/users/2
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -204,6 +206,7 @@
     ...           '[e-mail]/memberships')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -211,6 +214,7 @@
         user: http://localhost:9001/3.0/users/2
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -224,6 +228,7 @@
     ...           '[e-mail]/memberships')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member

=== modified file 'src/mailman/rest/docs/membership.rst'
--- src/mailman/rest/docs/membership.rst	2011-08-17 23:10:39 +0000
+++ src/mailman/rest/docs/membership.rst	2011-08-25 00:14:24 +0000
@@ -42,6 +42,7 @@
     >>> dump_json('http://localhost:9001/3.0/members')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -55,6 +56,7 @@
 
     >>> dump_json('http://localhost:9001/3.0/members/1')
     address: [e-mail]
+    delivery_mode: regular
     fqdn_listname: [e-mail]
     http_etag: ...
     role: member
@@ -68,6 +70,7 @@
     >>> dump_json('http://localhost:9001/3.0/members')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -75,6 +78,7 @@
         user: http://localhost:9001/3.0/users/1
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -93,6 +97,7 @@
     >>> dump_json('http://localhost:9001/3.0/members')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -100,6 +105,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -107,6 +113,7 @@
         user: http://localhost:9001/3.0/users/1
     entry 2:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -129,6 +136,7 @@
     >>> dump_json('http://localhost:9001/3.0/members')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -136,6 +144,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -143,6 +152,7 @@
         user: http://localhost:9001/3.0/users/2
     entry 2:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -150,6 +160,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 3:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -157,6 +168,7 @@
         user: http://localhost:9001/3.0/users/1
     entry 4:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -172,6 +184,7 @@
     ...     'http://localhost:9001/3.0/lists/[e-mail]/roster/member')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -179,6 +192,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -203,6 +217,7 @@
     >>> dump_json('http://localhost:9001/3.0/members')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: moderator
@@ -210,6 +225,7 @@
         user: http://localhost:9001/3.0/users/4
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -217,6 +233,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 2:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -224,6 +241,7 @@
         user: http://localhost:9001/3.0/users/2
     entry 3:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: owner
@@ -231,6 +249,7 @@
         user: http://localhost:9001/3.0/users/2
     entry 4:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -238,6 +257,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 5:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -245,6 +265,7 @@
         user: http://localhost:9001/3.0/users/1
     entry 6:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -260,6 +281,7 @@
     ...     'http://localhost:9001/3.0/lists/[e-mail]/roster/owner')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: owner
@@ -278,6 +300,7 @@
     >>> dump_json('http://localhost:9001/3.0/lists/'
     ...           '[e-mail]/owner/[e-mail]')
     address: [e-mail]
+    delivery_mode: regular
     fqdn_listname: [e-mail]
     http_etag: ...
     role: owner
@@ -292,6 +315,7 @@
     ...           })
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -299,6 +323,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -315,6 +340,7 @@
     ...           })
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -322,6 +348,7 @@
         user: http://localhost:9001/3.0/users/3
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -329,6 +356,7 @@
         user: http://localhost:9001/3.0/users/1
     entry 2:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -336,12 +364,13 @@
         user: http://localhost:9001/3.0/users/2
     entry 3:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: owner
         self_link: http://localhost:9001/3.0/members/7
         user: http://localhost:9001/3.0/users/2
-    http_etag: "66836d0f23bed36fa9e0cda1e5dec7e5b0797743"
+    http_etag: "..."
     start: 0
     total_size: 4
 
@@ -354,6 +383,7 @@
     ...           })
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -361,6 +391,7 @@
         user: http://localhost:9001/3.0/users/2
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: owner
@@ -378,6 +409,7 @@
     ...           })
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -385,6 +417,7 @@
         user: http://localhost:9001/3.0/users/2
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -403,6 +436,7 @@
     ...           })
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -450,6 +484,7 @@
     ...
     entry 3:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: ...
         role: member
@@ -487,6 +522,7 @@
     ...
     entry 4:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -509,6 +545,7 @@
     ...
     entry 4:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -565,7 +602,68 @@
     >>> memberships[0]
     <Member: Fred Person <[e-mail]>
              on [e-mail] as MemberRole.member>
-
+    >>> print memberships[0].delivery_mode.enumname
+    mime_digests
+
+    >>> dump_json('http://localhost:9001/3.0/members/10')
+    address: [e-mail]
+    delivery_mode: mime_digests
+    fqdn_listname: [e-mail]
+    http_etag: "..."
+    role: member
+    self_link: http://localhost:9001/3.0/members/10
+    user: http://localhost:9001/3.0/users/7
+
+Fred wants to change his delivery from MIME digest back to regular delivery.
+This can be done by PATCH'ing his member with the `delivery_mode`
+parameter.
+::
+
+    >>> transaction.abort()
+    >>> dump_json('http://localhost:9001/3.0/members/10', {
+    ...           'delivery_mode': 'regular',
+    ...           }, method='PATCH')
+    content-length: 0
+    date: ...
+    server: ...
+    status: 204
+
+    >>> dump_json('http://localhost:9001/3.0/members/10')
+    address: [e-mail]
+    delivery_mode: regular
+    fqdn_listname: [e-mail]
+    http_etag: "..."
+    role: member
+    self_link: http://localhost:9001/3.0/members/10
+    user: http://localhost:9001/3.0/users/7
+
+You can't PATCH a nonexistent member object.
+::
+
+    >>> dump_json('http://localhost:9001/3.0/members/99', {
+    ...           'delivery_mode': 'regular',
+    ...           }, method='PATCH')
+    Traceback (most recent call last):
+    ...
+    HTTPError: HTTP Error 404: 404 Not Found
+
+You can't PATCH a member with no attributes.
+::
+
+    >>> dump_json('http://localhost:9001/3.0/members/10', method='PATCH')
+    Traceback (most recent call last):
+    ...
+    HTTPError: HTTP Error 400: 400 Bad Request
+
+You can't PATCH a member with anything other than `address` or `delivery_mode`.
+::
+
+    >>> dump_json('http://localhost:9001/3.0/members/10', {
+    ...           'powers': 'super',
+    ...           }, method='PATCH')
+    Traceback (most recent call last):
+    ...
+    HTTPError: HTTP Error 400: Unexpected parameters: powers
 
 Changing delivery address
 =========================
@@ -601,6 +699,7 @@
     ...
     entry 5:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -609,6 +708,7 @@
     ...
     entry 10:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -664,6 +764,7 @@
     ...           '[e-mail]/memberships')
     entry 0:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member
@@ -671,6 +772,7 @@
         user: http://localhost:9001/3.0/users/8
     entry 1:
         address: [e-mail]
+        delivery_mode: regular
         fqdn_listname: [e-mail]
         http_etag: "..."
         role: member

=== modified file 'src/mailman/rest/members.py'
--- src/mailman/rest/members.py	2011-08-17 23:10:39 +0000
+++ src/mailman/rest/members.py	2011-08-25 00:14:24 +0000
@@ -59,6 +59,7 @@
             role=role,
             user=path_to('users/{0}'.format(member.user.user_id)),
             self_link=path_to('members/{0}'.format(member.member_id)),
+            delivery_mode=member.delivery_mode,
             )
 
     def _get_collection(self, request):
@@ -124,17 +125,28 @@
 
         This is how subscription changes are done.
         """
-        # Currently, only the `address` parameter can be patched.
-        values = Validator(address=unicode)(request)
-        assert len(values) == 1, 'Unexpected values'
-        email = values['address']
-        address = getUtility(IUserManager).get_address(email)
-        if address is None:
-            return http.bad_request([], b'Address not registered')
+        # Currently, only the `address` or `delivery_mode` parameters can be patched.
+        if self._member is None:
+            return http.not_found()
         try:
-            self._member.address = address
-        except (MembershipError, UnverifiedAddressError) as error:
+            values = Validator(address=unicode, 
+                    delivery_mode=enum_validator(DeliveryMode),
+                    _optional=('delivery_mode', 'address'))(request)
+        except ValueError as error:
             return http.bad_request([], str(error))
+        if len(values) == 0:
+            return http.bad_request()
+        if 'address' in values:
+            email = values['address']
+            address = getUtility(IUserManager).get_address(email)
+            if address is None:
+                return http.bad_request([], b'Address not registered')
+            try:
+                self._member.address = address
+            except (MembershipError, UnverifiedAddressError) as error:
+                return http.bad_request([], str(error))
+        if 'delivery_mode' in values:
+            self._member.preferences.delivery_mode = values['delivery_mode']
         return no_content()