Fix membership request handlers and tests

bounce/cli.py

@@ -10,7 +10,7 @@
 from .server import Server
 from .server.api.auth import LoginEndpoint
 from .server.api.clubs import ClubEndpoint, ClubsEndpoint
-from .server.api.membership import MembershipEndpoint, MembershipsEndpoint
+from .server.api.membership import MembershipEndpoint
 from .server.api.users import UserEndpoint, UsersEndpoint
 from .server.config import ServerConfig
 
@@ -78,7 +78,7 @@ def start(port, secret, pg_host, pg_port, pg_user, pg_password, pg_database,
     # Register your new endpoints here
     endpoints = [
         UsersEndpoint, UserEndpoint, ClubsEndpoint, ClubEndpoint,
-        LoginEndpoint, MembershipEndpoint, MembershipsEndpoint
+        LoginEndpoint, MembershipEndpoint
     ]
     serv = Server(conf, endpoints)
     serv.start()

bounce/db/club.py

@@ -4,7 +4,6 @@
 """
 
 from sqlalchemy import Column, Integer, String, func
-from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import relationship
 from sqlalchemy.types import TIMESTAMP
 

bounce/db/membership.py

@@ -1,13 +1,10 @@
 """Defines the schema for the Memberships table in our DB."""
 
 from sqlalchemy import Column, ForeignKey, Integer, func
-from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import relationship
 from sqlalchemy.types import TIMESTAMP
 
 from . import BASE
-from .club import Club
-from .user import User
 
 
 class Membership(BASE):
@@ -43,10 +40,19 @@ def to_dict(self):
         }
 
 
-def insert(session, user_id, club_id):
-    """Insert a new user into the Users table."""
-    membership = Membership(user_id=user_id, club_id=club_id)
-    session.add(membership)
+def insert(session, club_name, user_id):
+    """Creates a new membership that associates the given user with the given
+    club. """
+    # For now we do nothing on conflict, but when we have roles on these
+    # memberships we need to update on conflict.
+    query = f"""
+        INSERT INTO memberships (user_id, club_id) VALUES (
+            '{user_id}',
+            (SELECT id FROM clubs WHERE name = '{club_name}')
+        )
+        ON CONFLICT DO NOTHING
+    """
+    session.execute(query)
     session.commit()
 
 
@@ -68,9 +74,9 @@ def select(session, club_name, user_id=None):
     result_proxy = session.execute(query)
     results = []
     for row in result_proxy.fetchall():
-        results.append({
-            key: row[i] for i, key in enumerate(result_proxy.keys())
-        })
+        results.append(
+            {key: row[i]
+             for i, key in enumerate(result_proxy.keys())})
     return results
 
 

bounce/server/api/membership.py

@@ -5,12 +5,12 @@
 from sanic import response
 from sqlalchemy.exc import IntegrityError
 
-from . import APIError, Endpoint, util, verify_token
+from . import APIError, Endpoint, verify_token
 from ...db import club, membership
 from ..resource import validate
 from ..resource.membership import (DeleteMembershipRequest,
                                    GetMembershipRequest, GetMembershipResponse,
-                                   PostMembershipRequest, PutMembershipRequest)
+                                   PutMembershipRequest)
 
 
 class MembershipEndpoint(Endpoint):
@@ -40,22 +40,44 @@ async def get(self, request, club_name):
             self.server.db_session, club_name, user_id=user_id)
         return response.json(membership_info, status=200)
 
+    # pylint: disable=unused-argument
+    @verify_token()
+    @validate(PutMembershipRequest, None)
+    async def put(self, request, club_name, id_from_token=None):
+        """Handles a PUT /memberships/<club_name>?user_id=<user_id> request by
+        creating or updating the membership for the given user and club."""
+        # Decode the club name
+        club_name = unquote(club_name)
+        try:
+            user_id = int(request.args.get('user_id'))
+        except Exception:
+            raise APIError('Invalid user ID', status=400)
+        try:
+            membership.insert(self.server.db_session, club_name, user_id)
+        except IntegrityError:
+            raise APIError('Invalid user or club ID', status=400)
+        return response.text('', status=201)
+
+    # pylint: enable=unused-argument
+
     @verify_token()
     @validate(DeleteMembershipRequest, None)
     async def delete(self, request, club_name, id_from_token=None):
         """
         Handles a DELETE /memberships/<club_name>?user_id=<user_id> request
         by deleting the membership that associates the given user with the
-        given club. If no user ID is given, deletes all memberships for the
-        given club.
+        given club..
         """
         # TODO: fix this when we have user roles set up. A user should only be
         # able to delete their own memberships and memberships on clubs they
         # are an admin/owner of.
 
         # Decode the club name
         club_name = unquote(club_name)
-        user_id = request.args.get('user_id', None)
+        try:
+            user_id = int(request.args.get('user_id'))
+        except ValueError:
+            raise APIError('Invalid user ID', status=400)
 
         if id_from_token != user_id:
             # Regular members can only delete their own memberships
@@ -69,21 +91,3 @@ async def delete(self, request, club_name, id_from_token=None):
         # Delete the memberships
         membership.delete(self.server.db_session, club_name, user_id=user_id)
         return response.text('', status=204)
-
-
-class MembershipsEndpoint(Endpoint):
-    """Handles requests to /memberships."""
-
-    __uri__ = '/memberships'
-
-    @validate(PostMembershipRequest, None)
-    async def post(self, request):
-        """Handles a POST /users request by creating a new user."""
-        body = request.json
-        # Put the membership in the DB
-        try:
-            membership.insert(self.server.db_session, body['user_id'],
-                              body['club_id'])
-        except IntegrityError:
-            raise APIError('Invalid user or club ID', status=400)
-        return response.text('', status=201)

bounce/server/resource/membership.py

@@ -1,4 +1,4 @@
-'''Create a Membership resource in bounce/server/resource/membership.py that specifies all the information required for creating a new membership.'''
+"""Resources for the /memberships endpoint."""
 
 from . import ResourceMeta
 
@@ -47,47 +47,26 @@ class GetMembershipResponse(metaclass=ResourceMeta):
     }
 
 
-class PostMembershipRequest(metaclass=ResourceMeta):
-    """Defines the schema for a POST /membership request."""
-    __body__ = {
-        'type': 'object',
-        'required': ['user_id', 'club_id'],
-        'additionalProperties': False,
-        'properties': {
-            'user_id': {
-                'type': 'integer',
-                'minimum': 0,
-            },
-            'club_id': {
-                'type': 'integer',
-                'minimum': 0,
-            },
-        }
-    }
-
-
 class PutMembershipRequest(metaclass=ResourceMeta):
-    """Defines the schema for a PUT /users/<username> request."""
-    __body__ = {
+    """Defines the schema for a PUT /memberships/<club_name> request."""
+    __params__ = {
         'type': 'object',
         'additionalProperties': False,
+        'required': ['user_id'],
         'properties': {
             'user_id': {
-                'type': 'integer',
-                'minimum': 0,
-            },
-            'created_at': {
-                'type': 'integer',
+                'type': 'string',
             },
         }
     }
 
 
 class DeleteMembershipRequest(metaclass=ResourceMeta):
-    """Defines the schema for a DELETE /members/<club_name> request."""
+    """Defines the schema for a DELETE /memberships/<club_name> request."""
     __params__ = {
         'type': 'object',
         'additionalProperties': False,
+        'required': ['user_id'],
         'properties': {
             'user_id': {
                 'type': 'string',

schema/schema.sql

@@ -22,8 +22,8 @@ CREATE TABLE users (
 
 DROP TABLE IF EXISTS memberships;
 CREATE TABLE memberships (
-    user_id INT REFERENCES users(id) ON DELETE CASCADE,
-    club_id INT REFERENCES clubs(id) ON DELETE CASCADE,
+    user_id INT NOT NULL REFERENCES users(id) ON DELETE CASCADE,
+    club_id INT NOT NULL REFERENCES clubs(id) ON DELETE CASCADE,
     PRIMARY KEY (user_id, club_id),
     created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT (now() at time zone 'utc')
 );

setup.cfg

@@ -6,4 +6,5 @@ disable=
     too-few-public-methods,
     broad-except,
     duplicate-code,
-    too-many-instance-attributes
+    too-many-instance-attributes,
+    fixme

tests/api/test_endpoints.py

@@ -179,27 +179,54 @@ def test_delete_club__success(server):
     assert response.status == 204
 
 
-def test_post_memberships__success(server):
+def test_put_memberships__success(server):
     _, response = server.app.test_client.post(
-        '/memberships',
+        '/users',
         data=json.dumps({
-            'user_id': '384928',
-            'club_id': '2342',
+            'username': 'mrguy',
+            'full_name': 'Hello WOrld',
+            'email': 'something@anotherthing.com',
+            'password': 'Val1dPassword!'
         }))
     assert response.status == 201
+    token = util.create_jwt(2, server.config.secret)
+    _, response = server.app.test_client.put(
+        '/memberships/newtest?user_id=2',
+        headers={'Authorization': token})
+    assert response.status == 201
 
 
-def test_post_memberships__failure(server):
-    _, response = server.app.test_client.post(
-        '/clubs',
-        data=json.dumps({
-            'user_id': '12313',
-            'club_id': '212314',
-        }))
-    assert response.status == 409
-    assert 'error' in response.json
+def test_put_memberships__failure(server):
+    token = util.create_jwt(2, server.config.secret)
+    _, response = server.app.test_client.put(
+        '/memberships/doesnotexist?user_id=2',
+        headers={'Authorization': token})
+    assert response.status == 400
 
-def test_delete_membership_success(server):
-    _, response = server.app.test_client.delete('/memberships/test')
-    assert response.status == 204
 
+def test_get_memberships__success(server):
+    _, response = server.app.test_client.get(
+        '/memberships/newtest?user_id=2')
+    assert response.status == 200
+    assert len(response.json) == 1
+    membership = response.json[0]
+    assert membership['user_id'] == 2
+    assert membership['full_name'] == 'Hello WOrld'
+    assert membership['username'] == 'mrguy'
+    assert isinstance(membership['created_at'], int)
+
+
+def test_delete_membership__failure(server):
+    token = util.create_jwt(1, server.config.secret)
+    _, response = server.app.test_client.delete(
+        '/memberships/newtest?user_id=2',
+        headers={'Authorization': token})
+    assert response.status == 403
+
+
+def test_delete_membership__success(server):
+    token = util.create_jwt(2, server.config.secret)
+    _, response = server.app.test_client.delete(
+        '/memberships/newtest?user_id=2',
+        headers={'Authorization': token})
+    assert response.status == 204