From 3f6227564262141bae347af75805bbab35b2c863 Mon Sep 17 00:00:00 2001 From: Odin Mebesius Date: Sat, 23 Feb 2019 12:13:38 -0800 Subject: [PATCH 1/5] refactoring --- bounce/db/club.py | 16 ++++++------ bounce/server/api/clubs.py | 46 +++++++++++++++++++--------------- container/postgres.env.example | 3 --- container/web.env.example | 9 ------- requirements.txt | 14 +++++------ 5 files changed, 41 insertions(+), 47 deletions(-) delete mode 100644 container/postgres.env.example delete mode 100644 container/web.env.example diff --git a/bounce/db/club.py b/bounce/db/club.py index b738885..f4267ab 100644 --- a/bounce/db/club.py +++ b/bounce/db/club.py @@ -48,12 +48,12 @@ def to_dict(self): } -def select(session, name): +def select(session, identifier): """ - Returns the club with the given name or None if + Returns the club with the given identifier or None if there is no such club. """ - club = session.query(Club).filter(Club.name == name).first() + club = session.query(Club).filter(Club.identifier == identifier).first() return None if club is None else club.to_dict() @@ -93,11 +93,11 @@ def insert(session, name, description, website_url, facebook_url, session.commit() -def update(session, name, new_name, description, website_url, facebook_url, +def update(session, identifier, new_name, description, website_url, facebook_url, instagram_url, twitter_url): """Updates an existing club in the Clubs table and returns the updated club.""" - club = session.query(Club).filter(Club.name == name).first() + club = session.query(Club).filter(Club.identifier == identifier).first() if new_name: club.name = new_name if description: @@ -114,7 +114,7 @@ def update(session, name, new_name, description, website_url, facebook_url, return club.to_dict() -def delete(session, name): - """Deletes the club with the given name.""" - session.query(Club).filter(Club.name == name).delete() +def delete(session, identifier): + """Deletes the club with the given identifier.""" + session.query(Club).filter(Club.identifier == identifier).delete() session.commit() diff --git a/bounce/server/api/clubs.py b/bounce/server/api/clubs.py index d48aaa8..089f0cd 100644 --- a/bounce/server/api/clubs.py +++ b/bounce/server/api/clubs.py @@ -16,34 +16,38 @@ class ClubEndpoint(Endpoint): - """Handles requests to /clubs/.""" + """Handles requests to /clubs/.""" - __uri__ = "/clubs/" + __uri__ = "/clubs/" @validate(None, GetClubResponse) - async def get(self, _, name): - """Handles a GET /clubs/ request by returning the club with - the given name.""" - # Decode the name, since special characters will be URL-encoded - name = unquote(name) + async def get(self, _, identifier): + """Handles a GET /clubs/ request by returning the club with + the given identifier.""" + # Decode the identifier, since special characters will be URL-encoded + import pdb + pdb.set_trace() + identifier = unquote(identifier) #it no like this? + print(identifier) # Fetch club data from DB - club_data = club.select(self.server.db_session, name) + club_data = club.select(self.server.db_session, identifier) # or dis? + print(club_data) if not club_data: # Failed to find a club with that name raise APIError('No such club', status=404) return response.json(club_data, status=200) @validate(PutClubRequest, GetClubResponse) - async def put(self, request, name): - """Handles a PUT /clubs/ request by updating the club with - the given name and returning the updated club info.""" - # Decode the name, since special characters will be URL-encoded - name = unquote(name) + async def put(self, request, identifier): + """Handles a PUT /clubs/ request by updating the club with + the given identifier and returning the updated club info.""" + # Decode the identifier, since special characters will be URL-encoded + identifier = unquote(identifier) body = util.strip_whitespace(request.json) updated_club = club.update( self.server.db_session, name, - new_name=body.get('name', None), + new_name=body.get('name', None), # 'name' is not defined -> test put club success; clubs.id = 'doesnotexist' description=body.get('description', None), website_url=body.get('website_url', None), facebook_url=body.get('facebook_url', None), @@ -51,12 +55,12 @@ async def put(self, request, name): twitter_url=body.get('twitter_url', None)) return response.json(updated_club, status=200) - async def delete(self, _, name): - """Handles a DELETE /clubs/ request by deleting the club with - the given name. """ - # Decode the name, since special characters will be URL-encoded - name = unquote(name) - club.delete(self.server.db_session, name) + async def delete(self, _, identifier): + """Handles a DELETE /clubs/ request by deleting the club with + the given identifier. """ + # Decode the identifier, since special characters will be URL-encoded + identifier = unquote(identifier) + club.delete(self.server.db_session, identifier) return response.text('', status=204) @@ -71,6 +75,8 @@ async def post(self, request): # Put the club in the DB body = util.strip_whitespace(request.json) try: + #BUG: if no website_url, facebook_url, instagram_url, or twitter_url, + # this method will fail. Fix such that urls are not required. club.insert( self.server.db_session, body['name'].strip(), body['description'].strip(), body['website_url'].strip(), diff --git a/container/postgres.env.example b/container/postgres.env.example deleted file mode 100644 index 8b2a930..0000000 --- a/container/postgres.env.example +++ /dev/null @@ -1,3 +0,0 @@ -POSTGRES_USER= -POSTGRES_PASSWORD= -POSTGRES_DB= diff --git a/container/web.env.example b/container/web.env.example deleted file mode 100644 index 4673386..0000000 --- a/container/web.env.example +++ /dev/null @@ -1,9 +0,0 @@ -PORT= -BOUNCE_SECRET= -POSTGRES_HOST= -POSTGRES_PORT= -POSTGRES_USER= -POSTGRES_PASSWORD= -POSTGRES_DB= -ALLOWED_ORIGIN= -IMAGE_DIR= diff --git a/requirements.txt b/requirements.txt index 13f76b4..cce6bff 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,17 +9,17 @@ bcrypt==3.1.4 cffi==1.11.5 # via bcrypt click==6.7 ecdsa==0.13 # via python-jose -future==0.16.0 # via python-jose +future==0.17.1 # via python-jose httptools==0.0.11 # via sanic jsonschema==2.6.0 psycopg2==2.7.4 -pyasn1==0.4.4 # via rsa -pycparser==2.18 # via cffi +pyasn1==0.4.5 # via rsa +pycparser==2.19 # via cffi python-jose==3.0.0 -rsa==3.4.2 # via python-jose +rsa==4.0 # via python-jose sanic==0.7.0 -six==1.11.0 # via bcrypt, python-jose +six==1.12.0 # via bcrypt, python-jose sqlalchemy==1.2.8 ujson==1.35 # via sanic -uvloop==0.11.2 # via sanic -websockets==6.0 # via sanic +uvloop==0.12.0 # via sanic +websockets==7.0 # via sanic From 24069b6a1cfe37941533cfa8b09c1274ff502ec3 Mon Sep 17 00:00:00 2001 From: Odin Mebesius Date: Sat, 2 Mar 2019 13:27:55 -0800 Subject: [PATCH 2/5] passing all tests after refactoring club endpoint --- bounce/db/club.py | 25 ++++++++++---- bounce/db/membership.py | 34 +++++++++++++++++++ bounce/server/api/clubs.py | 52 +++++++++++++---------------- bounce/server/resource/user.py | 10 ++---- tests/api/endpoints/test_2_clubs.py | 21 ++++++------ 5 files changed, 89 insertions(+), 53 deletions(-) diff --git a/bounce/db/club.py b/bounce/db/club.py index 5451c39..daa52c9 100644 --- a/bounce/db/club.py +++ b/bounce/db/club.py @@ -64,12 +64,22 @@ def can_update(editor_role): return editor_role in [Roles.president.value, Roles.admin.value] -def select(session, name): +def select_by_club_id(session, identifier): """ Returns the club with the given identifier or None if there is no such club. """ # Anyone should be able to read info on the club (including non-members) + club = session.query(Club).filter(Club.identifier == identifier).first() + return None if club is None else club.to_dict() + + +def select(session, name): + """ + Returns the club with the given name or None if + there is no such club. + """ + # Anyone should be able to read info on the club (including non-members) club = session.query(Club).filter(Club.name == name).first() return None if club is None else club.to_dict() @@ -109,16 +119,17 @@ def insert(session, name, description, website_url, facebook_url, twitter_url=twitter_url) session.add(club) session.commit() + return club -def update(session, name, editors_role, new_name, description, website_url, - facebook_url, instagram_url, twitter_url): +def update(session, identifier, editors_role, new_name, description, + website_url, facebook_url, instagram_url, twitter_url): """Updates an existing club in the Clubs table and returns the updated club.""" # Only Presidents and Admins can update if not can_update(editors_role): raise PermissionError("Permission denied for updating the club.") - club = session.query(Club).filter(Club.name == name).first() + club = session.query(Club).filter(Club.identifier == identifier).first() if new_name: club.name = new_name if description: @@ -135,11 +146,11 @@ def update(session, name, editors_role, new_name, description, website_url, return club.to_dict() -def delete(session, name, editors_role): - """Deletes the club with the given name.""" +def delete(session, identifier, editors_role): + """Deletes the club with the given identifier.""" # Only Presidents can delete if not can_delete(editors_role): raise PermissionError("Permission denied for deleting the club.") - session.query(Club).filter(Club.name == name).delete() + session.query(Club).filter(Club.identifier == identifier).delete() session.commit() diff --git a/bounce/db/membership.py b/bounce/db/membership.py index 6ff77c0..d75ee85 100644 --- a/bounce/db/membership.py +++ b/bounce/db/membership.py @@ -214,6 +214,40 @@ def select_all(session, club_name, editors_role): return results +def select_by_club_id(session, club_id, user_id, editors_role): + """ + Returns returns the membership for the given user of the specified club. + """ + # All members can read all memberships + if not can_select(editors_role): + raise PermissionError('Permission denied for selecting membership.') + + query = f""" + SELECT users.id AS user_id, + memberships.created_at, memberships.position, + memberships.role, users.full_name, users.username + FROM memberships INNER JOIN users ON ( + memberships.user_id = users.id + ) + WHERE memberships.club_id IN ( + SELECT id FROM clubs WHERE id = :club_id + ) + AND user_id = :user_id + """ + result_proxy = session.execute(query, { + 'club_id': club_id, + 'user_id': user_id, + }) + results = [] + for row in result_proxy.fetchall(): + member_info = {} + for i, key in enumerate(result_proxy.keys()): + # for i, key in row: + member_info[key] = row[i] + results.append(member_info) + return results + + def select(session, club_name, user_id, editors_role): """ Returns returns the membership for the given user of the specified club. diff --git a/bounce/server/api/clubs.py b/bounce/server/api/clubs.py index 6dda004..81607a5 100644 --- a/bounce/server/api/clubs.py +++ b/bounce/server/api/clubs.py @@ -26,13 +26,9 @@ async def get(self, _, identifier): """Handles a GET /clubs/ request by returning the club with the given identifier.""" # Decode the identifier, since special characters will be URL-encoded - import pdb - pdb.set_trace() - identifier = unquote(identifier) #it no like this? - print(identifier) + identifier = unquote(identifier) # Fetch club data from DB - club_data = club.select(self.server.db_session, identifier) # or dis? - print(club_data) + club_data = club.select_by_club_id(self.server.db_session, identifier) if not club_data: # Failed to find a club with that name raise APIError('No such club', status=404) @@ -40,20 +36,20 @@ async def get(self, _, identifier): @verify_token() @validate(PutClubRequest, GetClubResponse) - async def put(self, request, name, id_from_token=None): - """Handles a PUT /clubs/ request by updating the club with + async def put(self, request, identifier, id_from_token=None): + """Handles a PUT /clubs/ request by updating the club with the given name and returning the updated club info.""" - # Decode the name, since special characters will be URL-encoded - name = unquote(name) + # Decode the identifier, since special characters will be URL-encoded + identifier = unquote(identifier) body = util.strip_whitespace(request.json) try: - editor_attr = membership.select(self.server.db_session, name, - id_from_token, - Roles.president.value) + editor_attr = membership.select_by_club_id( + self.server.db_session, identifier, id_from_token, + Roles.president.value) editors_role = editor_attr[0]['role'] updated_club = club.update( self.server.db_session, - name, + identifier, editors_role, new_name=body.get('name', None), description=body.get('description', None), @@ -67,18 +63,17 @@ async def put(self, request, name, id_from_token=None): @verify_token() @validate(DeleteClubRequest, None) - async def delete(self, _, name, id_from_token=None): - """Handles a DELETE /clubs/ request by deleting the club with - the given name.""" - # Decode the name, since special characters will be URL-encoded - - name = unquote(name) + async def delete(self, _, identifier, id_from_token=None): + """Handles a DELETE /clubs/ request by deleting the club with + the given identifier.""" + # Decode the identifier, since special characters will be URL-encoded + identifier = unquote(identifier) try: - membership_attr = membership.select(self.server.db_session, name, - id_from_token, - Roles.president.value) + membership_attr = membership.select_by_club_id( + self.server.db_session, identifier, id_from_token, + Roles.president.value) editors_role = membership_attr[0]['role'] - club.delete(self.server.db_session, name, editors_role) + club.delete(self.server.db_session, identifier, editors_role) except PermissionError: raise APIError('Forbidden', status=403) return response.text('', status=204) @@ -90,15 +85,16 @@ class ClubsEndpoint(Endpoint): __uri__ = '/clubs' @verify_token() - @validate(PostClubsRequest, None) + @validate(PostClubsRequest, GetClubResponse) async def post(self, request, id_from_token=None): """Handles a POST /clubs request by creating a new club.""" # Put the club in the DB body = util.strip_whitespace(request.json) try: - #BUG: if no website_url, facebook_url, instagram_url, or twitter_url, + # BUG: + # if no website_url, facebook_url, instagram_url, or twitter_url, # this method will fail. Fix such that urls are not required. - club.insert( + club_row = club.insert( self.server.db_session, name=body.get('name', None), description=body.get('description', None), @@ -112,7 +108,7 @@ async def post(self, request, id_from_token=None): membership.insert(self.server.db_session, body.get('name', None), id_from_token, Roles.president.value, Roles.president.value, 'Owner') - return response.text('', status=201) + return response.json(club_row.to_dict(), status=201) class SearchClubsEndpoint(Endpoint): diff --git a/bounce/server/resource/user.py b/bounce/server/resource/user.py index cf6695e..e0cd07a 100644 --- a/bounce/server/resource/user.py +++ b/bounce/server/resource/user.py @@ -60,14 +60,8 @@ class GetUserResponse(metaclass=ResourceMeta): """Defines the schema for a GET /users/ response.""" __body__ = { 'type': 'object', - 'required': [ - 'full_name', - 'username', - 'email', - 'bio', - 'id', - 'created_at' - ], + 'required': + ['full_name', 'username', 'email', 'bio', 'id', 'created_at'], 'additionalProperties': False, 'properties': { 'full_name': { diff --git a/tests/api/endpoints/test_2_clubs.py b/tests/api/endpoints/test_2_clubs.py index b26851a..3199a34 100644 --- a/tests/api/endpoints/test_2_clubs.py +++ b/tests/api/endpoints/test_2_clubs.py @@ -4,6 +4,8 @@ from bounce.server.api import util +test_post_clubs__success_club_identifier = None + def test_post_clubs__success(server): # A user is required to create the club @@ -63,10 +65,9 @@ def test_put_club__success(server): _, response = server.app.test_client.get('/users/founder') founder_id = response.json['id'] founder_token = util.create_jwt(founder_id, server.config.secret) - # test if the club is successfully edited by President _, response = server.app.test_client.put( - '/clubs/test', + '/clubs/1', data=json.dumps({ 'name': 'newtest', 'description': 'club with a new description', @@ -104,7 +105,7 @@ def test_put_club__success(server): # test if the club is successfully edited by Admin _, response = server.app.test_client.put( - '/clubs/newtest', + '/clubs/1', data=json.dumps({ 'name': 'newtest', 'description': 'club with a newer description', @@ -122,10 +123,9 @@ def test_put_club__failure(server): _, response = server.app.test_client.get('/users/founder') user_id = response.json['id'] token = util.create_jwt(user_id, server.config.secret) - # bad json data _, response = server.app.test_client.put( - '/clubs/newtest', + '/clubs/1', data=json.dumps({ 'garbage': True }), @@ -165,7 +165,7 @@ def test_put_club__failure(server): # now try editing the club with the Member membership token = util.create_jwt(user_id, server.config.secret) _, response = server.app.test_client.put( - '/clubs/newtest', + '/clubs/1', data=json.dumps({ 'name': 'newtest', 'description': 'new description', @@ -175,7 +175,7 @@ def test_put_club__failure(server): def test_get_club__success(server): - _, response = server.app.test_client.get('/clubs/newtest') + _, response = server.app.test_client.get('/clubs/1') assert response.status == 200 assert response.json['name'] == 'newtest' assert response.json['description'] == 'club with a newer description' @@ -184,7 +184,8 @@ def test_get_club__success(server): def test_get_club__failure(server): - _, response = server.app.test_client.get('/clubs/doesnotexist') + # use id that isn't in the club at this point + _, response = server.app.test_client.get('/clubs/9478') assert response.status == 404 @@ -196,7 +197,7 @@ def test_delete_club__failure(server): # fail when a user with a Member membership tries to delete the club _, response = server.app.test_client.delete( - '/clubs/newtest', headers={'Authorization': token}) + '/clubs/1', headers={'Authorization': token}) assert response.status == 403 @@ -208,7 +209,7 @@ def test_delete_club__success(server): token = util.create_jwt(editor_id, server.config.secret) _, response = server.app.test_client.delete( - '/clubs/newtest', headers={'Authorization': token}) + '/clubs/1', headers={'Authorization': token}) assert response.status == 204 From 9f8b070e8046830f10f099d9502cf240de0aa217 Mon Sep 17 00:00:00 2001 From: mebesius <38874722+mebesius@users.noreply.github.com> Date: Sat, 16 Mar 2019 11:20:41 -0700 Subject: [PATCH 3/5] removed the second return in comment This was in response to Josh's comment on the PR --- bounce/db/membership.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bounce/db/membership.py b/bounce/db/membership.py index d75ee85..2c67fda 100644 --- a/bounce/db/membership.py +++ b/bounce/db/membership.py @@ -216,7 +216,7 @@ def select_all(session, club_name, editors_role): def select_by_club_id(session, club_id, user_id, editors_role): """ - Returns returns the membership for the given user of the specified club. + Returns the membership for the given user of the specified club. """ # All members can read all memberships if not can_select(editors_role): From e3ad75b9d0deee7dc1fc095b8bca913369ca8acd Mon Sep 17 00:00:00 2001 From: mebesius <38874722+mebesius@users.noreply.github.com> Date: Sat, 16 Mar 2019 11:22:03 -0700 Subject: [PATCH 4/5] Update test_2_clubs.py In response to Bruno's comment on the PR --- tests/api/endpoints/test_2_clubs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/api/endpoints/test_2_clubs.py b/tests/api/endpoints/test_2_clubs.py index 3199a34..312e5d6 100644 --- a/tests/api/endpoints/test_2_clubs.py +++ b/tests/api/endpoints/test_2_clubs.py @@ -4,7 +4,6 @@ from bounce.server.api import util -test_post_clubs__success_club_identifier = None def test_post_clubs__success(server): From 0ce531c96667f78866b6e9143f3fa0495491c9d7 Mon Sep 17 00:00:00 2001 From: Bruno Bachmann Date: Sat, 23 Mar 2019 14:46:53 -0700 Subject: [PATCH 5/5] Fix lint error --- bounce/server/api/clubs.py | 12 +++++------- tests/api/endpoints/test_2_clubs.py | 1 - 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/bounce/server/api/clubs.py b/bounce/server/api/clubs.py index c688c25..d7a6e0f 100644 --- a/bounce/server/api/clubs.py +++ b/bounce/server/api/clubs.py @@ -44,8 +44,7 @@ async def put(self, session, request, identifier, id_from_token=None): body = util.strip_whitespace(request.json) try: editor_attr = membership.select_by_club_id( - session, identifier, id_from_token, - Roles.president.value) + session, identifier, id_from_token, Roles.president.value) editors_role = editor_attr[0]['role'] updated_club = club.update( session, @@ -70,8 +69,7 @@ async def delete(self, session, _, identifier, id_from_token=None): identifier = unquote(identifier) try: membership_attr = membership.select_by_club_id( - session, identifier, id_from_token, - Roles.president.value) + session, identifier, id_from_token, Roles.president.value) editors_role = membership_attr[0]['role'] club.delete(session, identifier, editors_role) except PermissionError: @@ -102,9 +100,9 @@ async def post(self, session, request, id_from_token=None): except IntegrityError: raise APIError('Club already exists', status=409) # Give the creator of the club a President membership - membership.insert(session, body.get('name', None), - id_from_token, Roles.president.value, - Roles.president.value, 'Owner') + membership.insert(session, body.get('name', None), id_from_token, + Roles.president.value, Roles.president.value, + 'Owner') return response.json(club_row.to_dict(), status=201) diff --git a/tests/api/endpoints/test_2_clubs.py b/tests/api/endpoints/test_2_clubs.py index d11360e..4ac8d45 100644 --- a/tests/api/endpoints/test_2_clubs.py +++ b/tests/api/endpoints/test_2_clubs.py @@ -5,7 +5,6 @@ from bounce.server.api import util - def test_post_clubs__success(server): # A user is required to create the club _, response = server.app.test_client.post(