Add response body to add-member API

Change the add-group-membership API's response from a 204 No Content to
a 200 OK with a JSON representation of the added membership as the body.

Fixes #9141

docs/_extra/api-reference/hypothesis-v1.yaml

@@ -265,6 +265,8 @@ components:
       $ref: './schemas/user-new.yaml#/User'
     UserUpdate:
       $ref: './schemas/user-update.yaml#/User'
+    Membership:
+      $ref: './schemas/membership.yaml#/Membership'
 
 # -----------------------------------------------------------------------------
 # API OPERATIONS
@@ -895,8 +897,12 @@ paths:
         - $ref: '#/components/parameters/GroupID'
         - $ref: '#/components/parameters/UserID'
       responses:
-        '204':
-          $ref: '#/components/responses/NoContent'
+        '200':
+          description: Success
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Membership'
 
     # ----------------------------------------------------------
     # DELETE groups/{id}/members/{user} - Remove user from group

docs/_extra/api-reference/hypothesis-v2.yaml

@@ -263,6 +263,8 @@ components:
       $ref: './schemas/user-new.yaml#/User'
     UserUpdate:
       $ref: './schemas/user-update.yaml#/User'
+    Membership:
+      $ref: './schemas/membership.yaml#/Membership'
 
 # -----------------------------------------------------------------------------
 # API OPERATIONS
@@ -893,8 +895,12 @@ paths:
         - $ref: '#/components/parameters/GroupID'
         - $ref: '#/components/parameters/UserID'
       responses:
-        '204':
-          $ref: '#/components/responses/NoContent'
+        '200':
+          description: Success
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Membership'
 
     # ----------------------------------------------------------
     # DELETE groups/{id}/members/{user} - Remove user from group

docs/_extra/api-reference/schemas/membership.yaml

@@ -0,0 +1,41 @@
+Membership:
+  type: object
+  properties:
+    authority:
+      type: string
+      example: "hypothes.is"
+    userid:
+      type: string
+      description: "The member's unique ID"
+      example: "acct:felicity_nunsun@hypothes.is"
+    username:
+      type: string
+      description: "The member's username"
+      example: "felicity_nunsun"
+    display_name:
+      type: string
+      description: "The member's display name"
+      example: "Felicity Nunsun"
+    roles:
+      type: array
+      items:
+        type: string
+      description: "The member's roles in the group"
+      example: ["moderator"]
+    actions:
+      type: array
+      items:
+        type: string
+      description: "The actions that the authenticated user can take against this membership"
+      example: ["delete", "updates.roles.member"]
+    created:
+      type: string
+      format: date-time
+      description: "When the member joined the group"
+      example: "1970-01-01T00:00:00.000000+00:00"
+    updated:
+      type: string
+      format: date-time
+      description: "When this membership was last updated (for example to change the role)"
+      example: "1970-01-01T00:00:00.000000+00:00"
+

h/services/group_members.py

@@ -111,15 +111,15 @@ def update_members(self, group, userids):
         for userid in userids_for_removal:
             self.member_leave(group, userid)
 
-    def member_join(self, group, userid):
+    def member_join(self, group, userid) -> GroupMembership:
         """Add `userid` to the member list of `group`."""
         user = self.user_fetcher(userid)
 
         existing_membership = self.get_membership(group, user)
 
         if existing_membership:
             # The user is already a member of the group.
-            return
+            return existing_membership
 
         membership = GroupMembership(group=group, user=user)
         self.db.add(membership)
@@ -130,6 +130,8 @@ def member_join(self, group, userid):
         log.info("Added group membership: %r", membership)
         self.publish("group-join", group.pubid, userid)
 
+        return membership
+
     def member_leave(self, group, userid):
         """Remove `userid` from the member list of `group`."""
         user = self.user_fetcher(userid)

h/views/api/group_members.py

@@ -96,14 +96,14 @@ def remove_member(context: GroupMembershipContext, request):
     permission=Permission.Group.MEMBER_ADD,
 )
 def add_member(context: GroupMembershipContext, request):
-    group_members_service = request.find_service(name="group_members")
-
     if context.user.authority != context.group.authority:
         raise HTTPNotFound()
 
-    group_members_service.member_join(context.group, context.user.userid)
+    group_members_service = request.find_service(name="group_members")
 
-    return HTTPNoContent()
+    membership = group_members_service.member_join(context.group, context.user.userid)
+
+    return GroupMembershipJSONPresenter(request, membership).asdict()
 
 
 @api_config(

tests/functional/api/groups/members_test.py

@@ -357,7 +357,7 @@ def test_me_alias_without_forwarded_user(self, do_request):
 
     @pytest.fixture
     def do_request(self, db_session, app, group, user, headers):
-        def do_request(pubid=group.pubid, userid=user.userid, status=204):
+        def do_request(pubid=group.pubid, userid=user.userid, status=200):
             db_session.commit()
             return app.post_json(
                 f"/api/groups/{pubid}/members/{userid}", headers=headers, status=status

tests/unit/h/services/group_members_test.py

@@ -183,22 +183,25 @@ def test_it_adds_user_to_group(
         caplog.set_level(logging.INFO)
         user = factories.User()
         group = factories.Group()
-        group_members_service.member_join(group, user.userid)
+        returned = group_members_service.member_join(group, user.userid)
 
         membership = db_session.scalars(
             select(GroupMembership)
             .where(GroupMembership.user == user)
             .where(GroupMembership.group == group)
         ).one_or_none()
         assert membership
+        assert returned == membership
         assert caplog.messages == [f"Added group membership: {membership!r}"]
 
     def test_it_is_idempotent(self, group_members_service, factories):
         user = factories.User()
         group = factories.Group()
-        group_members_service.member_join(group, user.userid)
-        group_members_service.member_join(group, user.userid)
+        existing_membership = group_members_service.member_join(group, user.userid)
+
+        returned = group_members_service.member_join(group, user.userid)
 
+        assert returned == existing_membership
         assert group.members.count(user) == 1
 
     def test_it_publishes_join_event(self, group_members_service, factories, publish):

tests/unit/h/views/api/group_members_test.py

@@ -141,13 +141,23 @@ def context(self, factories):
 
 @pytest.mark.usefixtures("group_members_service")
 class TestAddMember:
-    def test_it(self, pyramid_request, group_members_service, context):
+    def test_it(
+        self,
+        pyramid_request,
+        group_members_service,
+        context,
+        GroupMembershipJSONPresenter,
+    ):
         response = views.add_member(context, pyramid_request)
 
         group_members_service.member_join.assert_called_once_with(
             context.group, context.user.userid
         )
-        assert isinstance(response, HTTPNoContent)
+        GroupMembershipJSONPresenter.assert_called_once_with(
+            pyramid_request, group_members_service.member_join.return_value
+        )
+        GroupMembershipJSONPresenter.return_value.asdict.assert_called_once_with()
+        assert response == GroupMembershipJSONPresenter.return_value.asdict.return_value
 
     def test_it_with_authority_mismatch(self, pyramid_request, context):
         context.group.authority = "other"