Add backend support for ownership requests #740

gabeweng · 2024-10-15T18:45:55Z

Implement backend for new Ownership Requests feature and update Membership Request feature.

New:

OwnershipRequest model
UserMembershipRequestSerializer and OwnershipRequestViewSet to see/create/destroy Ownership Requests for a user
Routes for users to manage their own requests under api/requests/ownership
OwnershipRequestSerializer and OwnershipRequestManagementViewSet for management (including accept and deny) of Ownership Requests by club admin and site admin
Routes for clubs and superusers to manage requests under api/clubs/{club_code}/ownershiprequests
Email template for sending out ownership requests

Modified:

MembershipRequest model (person renamed to requester, withdrew renamed to withdrawn, related names for MembershipRequest added for requester and club)
api/requests/membership used instead of api/requests/ for membership requests

…s and migrate

rm03

Off to a great start! Left some comments; feel free to reply/reach out with any questions. Please also add some test cases when you get a chance 🙂

backend/clubs/admin.py

backend/clubs/migrations/0114_ownershiprequest.py

backend/clubs/models.py

backend/clubs/views.py

backend/templates/emails/ownershiprequest.html

aviupadhyayula

Thanks for the PR, this looks like a great start! Rohan was (very) thorough, not much to add. One last nit: consider updating the PR's title and description to be a little more descriptive. (Take a look at past PRs for examples.) It's a small detail, but when you're reviewing code years in the future, it's great to understand what the author was thinking.

…mentViewSet

codecov · 2024-10-18T22:23:03Z

Codecov Report

Attention: Patch coverage is 90.90909% with 13 lines in your changes missing coverage. Please review.

Project coverage is 72.60%. Comparing base (b280dc0) to head (ecd4de6).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
backend/clubs/admin.py	66.66%	6 Missing ⚠️
backend/clubs/views.py	93.10%	4 Missing ⚠️
backend/clubs/models.py	91.30%	2 Missing ⚠️
backend/clubs/permissions.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #740      +/-   ##
==========================================
+ Coverage   71.95%   72.60%   +0.64%     
==========================================
  Files          32       32              
  Lines        6953     7055     +102     
==========================================
+ Hits         5003     5122     +119     
+ Misses       1950     1933      -17

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…manage_club

… viewing membership requests

…/penn-clubs into feat/ownership-requests

rm03

Thanks for resolving the comments from earlier! Left a few more, but mostly looks good to me!

backend/clubs/migrations/0119_rename_withdrew_ownershiprequest_withdrawn_and_more.py

backend/clubs/models.py

backend/clubs/serializers.py

backend/clubs/views.py

backend/tests/clubs/test_views.py

aviupadhyayula

Great work Gabe! Rohan did a great job with his review, so not much to add outside of minor comments.

backend/tests/clubs/test_views.py

backend/clubs/models.py

backend/clubs/serializers.py

backend/clubs/views.py

backend/templates/emails/ownership_request.html

…equest models and combine migration files

…lizer already in model

rm03

lgtm aside from a few nits! sorry for taking so long to get to this

rm03 · 2025-01-01T19:14:47Z

backend/clubs/models.py

@@ -1077,49 +1077,89 @@ def __str__(self):
        return "<SearchQuery: {} at {}>".format(self.query, self.created_at)


-class MembershipRequest(models.Model):
+class Request(models.Model):


nit: something more descriptive like JoinRequest

rm03 · 2025-01-01T19:15:05Z

backend/clubs/views.py

+    accept:
+    Accept an ownership request as a club owner.
+
+    old_requests:


nit: make this consistent with the new route

rm03 · 2025-01-01T19:15:10Z

backend/clubs/views.py

        request_object.delete()
        return Response({"success": True})

+    @action(detail=False, methods=["get"], permission_classes=[IsSuperuser])
+    def all_requests(self, request, *args, **kwargs):


aviupadhyayula

LGTM! Thanks for fixing up the tests.

gabeweng added 2 commits October 13, 2024 10:57

Make models, perms, serializers, urls, and views for ownershiprequest…

42e953b

…s and migrate

add API view for admin to see all ownershiprequests older than a week

9439212

gabeweng requested a review from rm03 October 15, 2024 18:45

merge conflicting migrations

32baced

rm03 requested changes Oct 16, 2024

View reviewed changes

aviupadhyayula reviewed Oct 16, 2024

View reviewed changes

gabeweng added 7 commits October 18, 2024 00:41

Merge branch 'master' into feat/ownership-requests

38224e3

migrate ownershiprequests

dc828b2

edit descriptions of OwnershipRequestSerializer and OwnershipRequest

5fba7b8

rename, update ownershiprequest model fields

6b511f0

edit ownership_request.html template

f36d1f9

Combine club admin and superuser viewsets for ownership requests

c5b6c4c

Specify YAML documentation for old_requests in OwnershipRequestManage…

917c1fc

…mentViewSet

gabeweng added 9 commits October 20, 2024 13:23

Fix permissions for OwnershipRequest management to not specify clubs.…

ec1597f

…manage_club

Change url basename for user ownership requests

d8ee660

Use update_or_create logic for Ownership Requests

f905122

Write test for creating and viewing Ownership Requests

fa2b7e7

Return 204 for Requester view for destroy ownershiprequest

5462032

Edit OwnershipRequestAdmin for admin dashboard

a16d758

Create test for withdraw feature of OwnershipRequests

ffed858

Create test for accept feature of OwnershipRequests

60f0962

Create test for destroy feature of OwnershipRequests

d5e4b6c

gabeweng force-pushed the feat/ownership-requests branch from 8808888 to d5e4b6c Compare November 8, 2024 03:23

gabeweng and others added 6 commits November 7, 2024 22:28

Merge branch 'master' into feat/ownership-requests

d94a5f4

Change comments and assert status codes for test_ownership_requests

4e31e86

Change basename for membership requests and add test for creating and…

ac1f4de

… viewing membership requests

Add test for withdrawing membership requests

cf89829

Add test for accepting membership requests

038a1bb

Add test for destroying membership requests

826f54c

gabeweng added 2 commits November 8, 2024 18:06

Update Membership Requests model and logic to match Ownership Requests

ff1719e

Merge branch 'feat/ownership-requests' of https://github.com/pennlabs…

01dc669

…/penn-clubs into feat/ownership-requests

gabeweng added the backend label Nov 10, 2024

gabeweng marked this pull request as ready for review November 10, 2024 17:17

aviupadhyayula requested review from rm03 and aviupadhyayula November 11, 2024 17:37

aviupadhyayula changed the title ~~Feat/ownership requests~~ Add backend support for ownership requests Nov 13, 2024

rm03 reviewed Nov 15, 2024

View reviewed changes

aviupadhyayula reviewed Nov 15, 2024

View reviewed changes

gabeweng and others added 10 commits November 15, 2024 17:46

Make abstract base class Request for MembershipRequest and OwnershipR…

eb6e86f

…equest models and combine migration files

Modify and rename request.html email template to membership_request.html

c5d7941

Shorten and rename tests for membership and ownership requests

f77b281

minor edits to ownership requests accept and destroy tests

eb4fb0a

Fixing nits

7b923a1

Fix redundant UniqueTogether logic in Membership/OwnerhipRequestSeria…

b244e89

…lizer already in model

Merge branch 'master' into feat/ownership-requests

87af1ab

handle invalid club code in request creation

6315065

Change old-requests to all-requests

bf105d4

Update test_views.py

ecd4de6

rm03 approved these changes Jan 1, 2025

View reviewed changes

rm03 requested a review from aviupadhyayula January 1, 2025 19:16

aviupadhyayula approved these changes Jan 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add backend support for ownership requests #740

Add backend support for ownership requests #740

gabeweng commented Oct 15, 2024 •

edited

Loading

rm03 left a comment

aviupadhyayula left a comment •

edited

Loading

codecov bot commented Oct 18, 2024 •

edited

Loading

rm03 left a comment

aviupadhyayula left a comment

rm03 left a comment

rm03 Jan 1, 2025

rm03 Jan 1, 2025

rm03 Jan 1, 2025

aviupadhyayula left a comment

Add backend support for ownership requests #740

Are you sure you want to change the base?

Add backend support for ownership requests #740

Conversation

gabeweng commented Oct 15, 2024 • edited Loading

rm03 left a comment

Choose a reason for hiding this comment

aviupadhyayula left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Oct 18, 2024 • edited Loading

Codecov Report

rm03 left a comment

Choose a reason for hiding this comment

aviupadhyayula left a comment

Choose a reason for hiding this comment

rm03 left a comment

Choose a reason for hiding this comment

rm03 Jan 1, 2025

Choose a reason for hiding this comment

rm03 Jan 1, 2025

Choose a reason for hiding this comment

rm03 Jan 1, 2025

Choose a reason for hiding this comment

aviupadhyayula left a comment

Choose a reason for hiding this comment

gabeweng commented Oct 15, 2024 •

edited

Loading

aviupadhyayula left a comment •

edited

Loading

codecov bot commented Oct 18, 2024 •

edited

Loading