From 989b032a98dd921dd88e05c27bfc391bf6a38a43 Mon Sep 17 00:00:00 2001 From: Rohan Gupta Date: Tue, 28 Jun 2022 00:31:15 -0700 Subject: [PATCH] atomic transactions, cleanup, and some views --- backend/clubs/models.py | 20 ++- backend/clubs/serializers.py | 14 +- backend/clubs/views.py | 285 ++++++++++++++++++++++++----------- 3 files changed, 221 insertions(+), 98 deletions(-) diff --git a/backend/clubs/models.py b/backend/clubs/models.py index 3284388e2..8fd25c9f4 100644 --- a/backend/clubs/models.py +++ b/backend/clubs/models.py @@ -1574,7 +1574,9 @@ class ApplicationCommittee(models.Model): name = models.TextField(blank=True) application = models.ForeignKey( - ClubApplication, related_name="committees", on_delete=models.CASCADE, + ClubApplication, + related_name="committees", + on_delete=models.CASCADE, ) def get_word_limit(self): @@ -1626,7 +1628,9 @@ class ApplicationMultipleChoice(models.Model): value = models.TextField(blank=True) question = models.ForeignKey( - ApplicationQuestion, related_name="multiple_choice", on_delete=models.CASCADE, + ApplicationQuestion, + related_name="multiple_choice", + on_delete=models.CASCADE, ) @@ -1707,7 +1711,7 @@ class Cart(models.Model): Represents an instance of a ticket cart for a user """ - owner = models.ForeignKey( + owner = models.OneToOneField( get_user_model(), related_name="cart", on_delete=models.CASCADE ) @@ -1724,12 +1728,18 @@ class Ticket(models.Model): type = models.CharField(max_length=100) owner = models.ForeignKey( get_user_model(), - related_name="tickets", + related_name="owned_tickets", + on_delete=models.SET_NULL, + blank=True, + null=True, + ) + holder = models.ForeignKey( + get_user_model(), + related_name="held_tickets", on_delete=models.SET_NULL, blank=True, null=True, ) - held = models.BooleanField(default=False) holding_expiration = models.DateTimeField(null=True, blank=True) carts = models.ManyToManyField(Cart, related_name="tickets", blank=True) diff --git a/backend/clubs/serializers.py b/backend/clubs/serializers.py index 42a95b333..fe4ec4452 100644 --- a/backend/clubs/serializers.py +++ b/backend/clubs/serializers.py @@ -340,8 +340,12 @@ class ClubEventSerializer(serializers.ModelSerializer): image_url = serializers.SerializerMethodField("get_image_url") large_image_url = serializers.SerializerMethodField("get_large_image_url") url = serializers.SerializerMethodField("get_event_url") + ticketed = serializers.SerializerMethodField("get_ticketed") creator = serializers.HiddenField(default=serializers.CurrentUserDefault()) + def get_ticketed(self, obj): + return Event.tickets.exists() + def get_event_url(self, obj): # if no url, return that if not obj.url: @@ -2007,7 +2011,9 @@ def get_clubs(self, obj): # hide non public memberships if not superuser if user is None or not user.has_perm("clubs.manage_club"): queryset = queryset.filter( - membership__person=obj, membership__public=True, approved=True, + membership__person=obj, + membership__public=True, + approved=True, ) serializer = MembershipClubListSerializer( @@ -2408,7 +2414,8 @@ def save(self): ApplicationMultipleChoice.objects.filter(question=question_obj).delete() for choice in multiple_choice: ApplicationMultipleChoice.objects.create( - value=choice["value"], question=question_obj, + value=choice["value"], + question=question_obj, ) # manually create committee choices as Django does not @@ -2697,7 +2704,8 @@ def save(self): for name in committees: if name not in prev_committee_names: ApplicationCommittee.objects.create( - name=name, application=application_obj, + name=name, + application=application_obj, ) return application_obj diff --git a/backend/clubs/views.py b/backend/clubs/views.py index 3417952a9..dfadeeaa2 100644 --- a/backend/clubs/views.py +++ b/backend/clubs/views.py @@ -25,6 +25,7 @@ from django.core.management import call_command, get_commands, load_command_class from django.core.serializers.json import DjangoJSONEncoder from django.core.validators import validate_email +from django.db import transaction from django.db.models import Count, DurationField, ExpressionWrapper, F, Prefetch, Q from django.db.models.expressions import RawSQL, Value from django.db.models.functions import Lower, Trunc @@ -1065,7 +1066,8 @@ def get_queryset(self): self.request.user, get_user_model() ): SearchQuery( - person=self.request.user, query=self.request.query_params.get("search"), + person=self.request.user, + query=self.request.query_params.get("search"), ).save() # select subset of clubs if requested @@ -1658,7 +1660,9 @@ def constitutions(self, request, *args, **kwargs): query = ( Club.objects.filter(badges=badge, archived=False) .order_by(Lower("name")) - .prefetch_related(Prefetch("asset_set", to_attr="prefetch_asset_set"),) + .prefetch_related( + Prefetch("asset_set", to_attr="prefetch_asset_set"), + ) ) if request.user.is_authenticated: query = query.prefetch_related( @@ -2217,10 +2221,39 @@ def get_serializer_class(self): return EventWriteSerializer return EventSerializer + def update_holds(self): + """ + Update ticket holds for *all* tickets + """ + held_tickets = Ticket.objects.filter(holder__isnull=False).all() + for ticket in held_tickets: + if ticket.holding_expiration <= timezone.now(): + ticket.holder = None + ticket.save() + + @action(detail=False, methods=["get"]) + def view_cart(self): + """ + View contents of the cart + --- + requestBody: {} + responses: + "200": + content: + application/json: + schema: + allOf: + - $ref: "#/components/schemas/Cart" + + """ + cart = Cart.objects.get_or_create(owner=self.request.user) + return None # Response(CartSerializer(cart.data)) + @action(detail=True, methods=["post"]) - def cart(self, request, *args, **kwargs): + @transaction.atomic + def add_to_cart(self, request, *args, **kwargs): """ - Add a certain number of tickets to cart + Add a certain number of tickets to the cart --- requestBody: content: @@ -2237,40 +2270,84 @@ def cart(self, request, *args, **kwargs): content: application/json: schema: - allOf: - - $ref: "#/components/schemas/Ticket" + properties: + detail: + type: string + "403": + content: + application/json: + schema: + properties: + detail: + type: string --- """ - # Update holding status for all held tickets - held_tickets = Ticket.objects.filter(held=True).all() - for ticket in held_tickets: - if ticket.holding_expiration <= timezone.now(): - ticket.held = False - ticket.save() + self.update_holds() type = request.data.get("type") count = request.data.get("count") - event = Event.objects.get(id=self.get_object().id) - cart = Cart.objects.filter(owner=self.request.user).first() - if not cart: - new_cart = Cart(owner=self.request.user) - new_cart.save() - - # Try to get count unowned ticket of requested type - tickets = Ticket.objects.filter( - event=event, type=type, owner__isnull=True, held=False - ).exclude(carts__owner=self.request.user) + event = self.get_object() + cart = Cart.objects.get_or_create(owner=self.request.user) + + # count unowned/unheld tickets of requested type + tickets = ( + Ticket.objects.select_for_update(skip_locked=True) + .filter(event=event, type=type, owner__isnull=True, holder__isnull=True) + .exclude(carts__owner=self.request.user) + ) + if tickets.count() < count: return Response( {"detail": f"Not enough tickets of type {type} left!"}, status=status.HTTP_403_FORBIDDEN, ) else: - for ticket in tickets[:count]: - ticket.carts.add(cart) - ticket.save() + cart.tickets.add(*tickets[:count]) + cart.save() return Response({"detail": "Successfully added to cart"}) + @action(detail=True, methods=["post"]) + @transaction.atomic + def remove_from_cart(self, request, *args, **kwargs): + """ + Remove a certain type/number of tickets from the cart + --- + requestBody: + content: + application/json: + schema: + type: object + properties: + type: + type: string + count: + type: integer + responses: + "200": + content: + application/json: + schema: + properties: + detail: + type: string + --- + """ + + self.update_holds() + type = request.data.get("type") + event = self.get_object() + cart = get_object_or_404(owner=self.request.user) + tickets = cart.tickets.filter(type=type, event=event) + + # Ensure we don't try to remove more tickets than we can + count = min(request.data.get("count"), tickets.count()) + + cart.tickets.remove(*tickets[:count]) + cart.save() + + return Response({"detail": "Successfully removed from cart"}) + @action(detail=False, methods=["post"]) + @transaction.atomic def validate_cart(self, request, *args, **kwargs): """ Validate tickets in a cart @@ -2288,72 +2365,112 @@ def validate_cart(self, request, *args, **kwargs): - $ref: "#/components/schemas/Ticket" --- """ - # Update holding status for all held tickets - held_tickets = Ticket.objects.filter(held=True).all() - for ticket in held_tickets: - if ticket.holding_expiration <= timezone.now(): - ticket.held = False - ticket.save() + self.update_holds() - cart = Cart.objects.filter(owner=self.request.user).first() + cart = get_object_or_404(Cart, owner=self.request.user) + sold_out_flag = False for ticket in cart.tickets.all(): - if ticket.owner or ticket.held: - new_ticket = Ticket.objects.filter( - event=ticket.event, type=ticket.type, owner__isnull=True, held=False - ).first() + # if ticket in cart has been bought, try to replace + if ticket.owner or ticket.holder: + # lock new ticket until transaction is completed + new_ticket = ( + Ticket.objects.select_for_update(skip_locked=True) + .filter( + event=ticket.event, + type=ticket.type, + owner__isnull=True, + holder__isnull=True, + ) + .first() + ) cart.tickets.remove(ticket) if new_ticket: cart.tickets.add(new_ticket) - cart.save() - return Response({"detail": "Cart validated"}) + else: + sold_out_flag = True + cart.save() + + return Response( + {"detail": "Validated" + ("" if not sold_out_flag else " with changes")} + ) @action(detail=False, methods=["post"]) + @transaction.atomic def checkout(self, request, *args, **kwargs): """ - Checkout all tickets in cart, assumes all tickets are unowned and unheld + Checkout all tickets in cart, to be called after validate_cart + + NOTE: this does NOT buy tickets, it simply initiates a checkout process + which includes a 10-minute ticket hold --- requestBody: content: application/json: schema: + type: object + responses: "200": content: application/json: schema: - allOf: - - $ref: "#/components/schemas/Ticket" + properties: + detail: + type: string --- """ - cart = Cart.objects.filter(owner=self.request.user).first() + cart = get_object_or_404(Cart, owner=self.request.user) - for ticket in cart.tickets.all(): - ticket.held = True + # The assumption is that this filter query should return all tickets in the cart (if run after validate_cart); + # however we cannot guarantee atomicity between validate_cart and checkout + # + # customers will be prompted to review the cart before payment + + for ticket in cart.tickets.select_for_update().filter( + owner__isnull=True, holder__isnull=True + ): + ticket.holder = self.request.user ticket.holding_expiration = timezone.now() + datetime.timedelta(minutes=10) ticket.save() - # Should only run if Stripe call succeeds - for ticket in cart.tickets.all(): + return Response({"detail": "Successfully initated checkout"}) + + @action(detail=False, methods=["post"]) + @transaction.atomic + def checkout_success_callback(self, request, *args, **kwargs): + """ + Callback after third party payment succeeds + --- + requestBody: {} + responses: + "200": + content: + application/json: + schema: + properties: + detail: + type: string + + """ + cart = get_object_or_404(Cart, owner=self.request.user) + + for ticket in cart.tickets.select_for_update().all(): ticket.owner = request.user ticket.carts.remove(cart) - ticket.save() # ticket.send_confirmation_email() - return Response({"detail": "Successfully checked out!"}) + ticket.save() + + return Response({"detail": "callback successful"}) @action(detail=True, methods=["post"]) + @transaction.atomic def buy(self, request, *args, **kwargs): """ - Buy a ticket + Buy tickets in a cart --- requestBody: - content: - application/json: - schema: - type: object - properties: - type: - type: string + content: {} responses: "200": content: @@ -2363,35 +2480,16 @@ def buy(self, request, *args, **kwargs): - $ref: "#/components/schemas/Ticket" --- """ - type = request.data.get("type") - event = self.get_object() - # If ticket already owned, do nothing - if Ticket.objects.filter(event=event, owner=request.user.id).first(): - return Response( - {"detail": "Ticket to event already owned by user"}, - status=status.HTTP_400_BAD_REQUEST, - ) + # Some logic here to serialize all held tickets down to whatever + # format third party asks for - # Otherwise get first unowned ticket of requested type - ticket = Ticket.objects.filter( - event=event, type=type, owner__isnull=True, held=False - ).first() + cart = get_object_or_404(Cart, owner=self.request.user) - # Stripe call here + for ticket in cart.tickets.filter(holder=self.request.user): + pass - # If Stripe call succeeds - if ticket: - ticket.held = True - ticket.owner = request.user - ticket.save() - ticket.send_confirmation_email() - return Response(TicketSerializer(ticket).data) - else: - return Response( - {"detail": f"No tickets of type {type} left!"}, - status=status.HTTP_403_FORBIDDEN, - ) + pass @action(detail=True, methods=["get"]) def tickets(self, request, *args, **kwargs): @@ -2467,8 +2565,6 @@ def create_tickets(self, request, *args, **kwargs): schema: type: object properties: - name: - type: string quantities: type: array items: @@ -2487,7 +2583,7 @@ def create_tickets(self, request, *args, **kwargs): properties: detail: type: string - description: A success or error message. + description: Empty array for success """ event = self.get_object() quantities = request.data.get("quantities") @@ -2525,7 +2621,6 @@ def upload(self, request, *args, **kwargs): description: Returned if the file was successfully uploaded. content: &upload_resp application/json: - schema: type: object properties: detail: @@ -3246,7 +3341,9 @@ def destroy(self, request, *args, **kwargs): def get_queryset(self): return MembershipRequest.objects.filter( - person=self.request.user, withdrew=False, club__archived=False, + person=self.request.user, + withdrew=False, + club__archived=False, ) @@ -4370,7 +4467,9 @@ def get(self, request): try: response = zoom_api_call( - request.user, "GET", "https://api.zoom.us/v2/users/{uid}/settings", + request.user, + "GET", + "https://api.zoom.us/v2/users/{uid}/settings", ) except requests.exceptions.HTTPError as e: raise DRFValidationError( @@ -4491,7 +4590,9 @@ def get_operation_id(self, **kwargs): def get_object(self): user = self.request.user prefetch_related_objects( - [user], "profile__school", "profile__major", + [user], + "profile__school", + "profile__major", ) return user @@ -4758,7 +4859,9 @@ def question_response(self, *args, **kwargs): } ) submission = ApplicationSubmission.objects.create( - user=self.request.user, application=application, committee=committee, + user=self.request.user, + application=application, + committee=committee, ) for question_pk in questions: question = ApplicationQuestion.objects.filter(pk=question_pk).first() @@ -4779,7 +4882,9 @@ def question_response(self, *args, **kwargs): text = question_data.get("text", None) if text is not None and text != "": obj = ApplicationQuestionResponse.objects.create( - text=text, question=question, submission=submission, + text=text, + question=question, + submission=submission, ).save() response = Response(ApplicationQuestionResponseSerializer(obj).data) elif question_type == ApplicationQuestion.MULTIPLE_CHOICE: