-
Notifications
You must be signed in to change notification settings - Fork 0
Issue 51 create api endpoint for registration form #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heya! can you please merge main into your branch and resolve merge conflicts?
I think you will also need to convert these endpoints into rest endpoints and specify the types of requests they would take, i.e. PUT and GET. Will need to get someone with more django knowledge to confirm the specifics though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of changes, as we are nearly at the end, I have included the code to go with it as well. These are crucial changes and might need to be addressed.
server/event_registration/views.py
Outdated
class RegistrationListCreate(generics.ListCreateAPIView): | ||
queryset = Registration.objects.all() | ||
serializer_class = RegistrationSerializer | ||
permission_classes = [permissions.IsAuthenticated] | ||
|
||
|
||
class RegistrantDetailRetrieveUpdate(generics.RetrieveUpdateAPIView): | ||
queryset = RegistrantDetail.objects.all() | ||
serializer_class = RegistrantDetailSerializer | ||
permission_classes = [permissions.IsAuthenticated] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current code exposes all users' data to any authenticated user - This can be a security issue
it should be replaced by something like this:
class RegistrationListCreate(generics.ListCreateAPIView):
serializer_class = RegistrationSerializer
permission_classes = [permissions.IsAuthenticated]
def get_queryset(self):
return Registration.objects.filter(user=self.request.user) # Only user's own registrations
def perform_create(self, serializer):
registration = serializer.save(user=self.request.user) # Auto-assign to current user
# Auto-create empty registrant details for form continuation
RegistrantDetail.objects.create(registration=registration)
class RegistrantDetailRetrieveUpdate(generics.RetrieveUpdateAPIView):
serializer_class = RegistrantDetailSerializer
permission_classes = [permissions.IsAuthenticated]
def get_queryset(self):
return RegistrantDetail.objects.filter(registration__user=self.request.user) # Only details of user's registrations
Here the queryset is replaced by a function which only returns the form allowed to view by the authenticated user.
Also the perform_create method here created an empty registrant_details form to avoid multiple POST requsets.
server/event_registration/urls.py
Outdated
urlpatterns = [ | ||
path('event_registration/registrations/', views.RegistrationListCreate.as_view(), name='registration-list-create'), | ||
path('event_registration/registrant-details/<int:pk>/', RegistrantDetailRetrieveUpdate.as_view(), name='registrant-detail-retrieve-update'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Maybe remove the
event_registration
from the url as it is a bit unclear here(these registrations are just to register a person in the system, not in an event) - Also models are UUID primary keys to replace the
<int:pk>
with<uuid:pl>
from rest_framework import permissions, generics | ||
from .models import Registration, RegistrantDetail | ||
from .serializers import RegistrationSerializer, RegistrantDetailSerializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might need to import the Registration Details here
from .models import Registration, RegistrantDetail
…nt_for_registration_form
…ttps://github.com/codersforcauses/transplant into issue-51-Create_API_endpoint_for_registration_form
…er and set user automatically on creation
Change Summary
Registration
for current user.Registration
for current user.RegistrantDetail
by uuidChange Form
Fill this up (NA if not available). If a certain criteria is not met, can you please give a reason.
I used postman to test listing all & creating new
registration
, retrieving and updatingRegistrantDetails
.Other Information
Related issue