Skip to content
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.

Sign up for GitHub

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

Add option to pass transaction fees onto buyers #762

Merged
merged 6 commits into from
Jan 4, 2025
Merged

Add option to pass transaction fees onto buyers #762

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0311a34
Add TicketSettings model
aviupadhyayula Jan 2, 2025
d7f8054
Update views to use ticket settings
aviupadhyayula Jan 2, 2025
a44c5bd
Update tests
aviupadhyayula Jan 2, 2025
0efdc96
Update tests for codecov
aviupadhyayula Jan 2, 2025
58f36f3
Minor tweaks to tests
aviupadhyayula Jan 3, 2025
b4882c9
Revert to default order limit
aviupadhyayula Jan 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions backend/clubs/migrations/0118_remove_event_ticket_drop_time_and_more.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Generated by Django 5.0.4 on 2025-01-02 07:07

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("clubs", "0117_clubapprovalresponsetemplate"),
]

operations = [
migrations.RemoveField(model_name="event", name="ticket_drop_time",),
migrations.RemoveField(model_name="event", name="ticket_order_limit",),
migrations.CreateModel(
name="TicketSettings",
fields=[
(
"id",
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
("order_limit", models.IntegerField(blank=True, null=True)),
("drop_time", models.DateTimeField(blank=True, null=True)),
("fee_charged_to_buyer", models.BooleanField(default=False)),
(
"event",
models.OneToOneField(
on_delete=django.db.models.deletion.CASCADE,
related_name="ticket_settings",
to="clubs.event",
),
),
],
),
]
23 changes: 21 additions & 2 deletions backend/clubs/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,8 +940,6 @@
parent_recurring_event = models.ForeignKey(
RecurringEvent, on_delete=models.CASCADE, blank=True, null=True
)
ticket_order_limit = models.IntegerField(default=10)
ticket_drop_time = models.DateTimeField(null=True, blank=True)

OTHER = 0
RECRUITMENT = 1
Expand Down Expand Up @@ -969,6 +967,10 @@
def create_thumbnail(self, request=None):
return create_thumbnail_helper(self, request, 400)

@property
def has_tickets(self):
return self.tickets.exists()

def __str__(self):
return self.name

Expand Down Expand Up @@ -1821,6 +1823,23 @@
checkout_context = models.CharField(max_length=8297, blank=True, null=True)


class TicketSettings(models.Model):
"""
Configuration settings for events that have tickets.
Only created when an event has associated tickets created.
"""

event = models.OneToOneField(
Event, on_delete=models.CASCADE, related_name="ticket_settings"
)
order_limit = models.IntegerField(null=True, blank=True)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the default order limit of 10 -- I don't see a reason why we should have it enabled by default. Happy to change it back if you guys disagree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but I think it was more of a safety net for say club officers that wouldn't know why they might want to limit orders per customer. Having a default limit might force them to think about this and opt out after making an informed decision. Also, since we will be opening ticket purchases to non-penn people, we have a larger pool of potential bad actors

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Since there'll eventually be ticketed events with free tickets, letting non-Penn users check out all of them would be disastrous. Let's re-implement the limit.

drop_time = models.DateTimeField(null=True, blank=True)
fee_charged_to_buyer = models.BooleanField(default=False)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting transaction fees to be shouldered by the club by default.


def __str__(self):
return f"Ticket settings for {self.event.name}"

Check warning on line 1840 in backend/clubs/models.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/models.py#L1840 

Added line #L1840 was not covered by tests


class TicketQuerySet(models.query.QuerySet):
def delete(self):
if self.filter(transaction_record__isnull=False).exists():
Expand Down
136 changes: 85 additions & 51 deletions backend/clubs/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
Tag,
Testimonial,
Ticket,
TicketSettings,
TicketTransactionRecord,
TicketTransferRecord,
Year,
Expand Down Expand Up @@ -2464,6 +2465,14 @@
---
"""
event = self.get_object()

# Check if event has any tickets
if not event.has_tickets:
return Response(

Check warning on line 2471 in backend/clubs/views.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/views.py#L2471 

Added line #L2471 was not covered by tests
{"detail": "This event does not have any tickets", "success": False},
status=status.HTTP_403_FORBIDDEN,
)

cart, _ = Cart.objects.get_or_create(owner=self.request.user)

# Check if the event has already ended
Expand All @@ -2474,7 +2483,10 @@
)

# Cannot add tickets that haven't dropped yet
if event.ticket_drop_time and timezone.now() < event.ticket_drop_time:
if (
event.ticket_settings.drop_time
and timezone.now() < event.ticket_settings.drop_time
):
return Response(
{"detail": "Ticket drop time has not yet elapsed", "success": False},
status=status.HTTP_403_FORBIDDEN,
Expand All @@ -2490,11 +2502,14 @@
num_requested = sum(item["count"] for item in quantities)
num_carted = cart.tickets.filter(event=event).count()

if num_requested + num_carted > event.ticket_order_limit:
if (
event.ticket_settings.order_limit
and num_requested + num_carted > event.ticket_settings.order_limit
):
return Response(
{
"detail": f"Order exceeds the maximum ticket limit of "
f"{event.ticket_order_limit}.",
f"{event.ticket_settings.order_limit}.",
"success": False,
},
status=status.HTTP_403_FORBIDDEN,
Expand Down Expand Up @@ -2680,20 +2695,22 @@
---
"""
event = self.get_object()
tickets = Ticket.objects.filter(event=event)

if event.ticket_drop_time and timezone.now() < event.ticket_drop_time:
if not event.has_tickets or (
event.ticket_settings.drop_time
and timezone.now() < event.ticket_settings.drop_time
):
return Response({"totals": [], "available": []})

# Take price of first ticket of given type for now
totals = (
tickets.values("type")
event.tickets.values("type")
.annotate(price=Max("price"))
.annotate(count=Count("type"))
.order_by("type")
)
available = (
tickets.filter(owner__isnull=True, holder__isnull=True, buyable=True)
event.tickets.filter(owner__isnull=True, holder__isnull=True, buyable=True)
.values("type")
.annotate(price=Max("price"))
.annotate(count=Count("type"))
Expand All @@ -2705,7 +2722,11 @@
@transaction.atomic
def create_tickets(self, request, *args, **kwargs):
"""
Create ticket offerings for event
Create or update ticket offerings for an event.

This endpoint allows configuring ticket types, quantities, prices, and settings.
Tickets cannot be modified after they have been dropped or sold.

---
requestBody:
content:
Expand All @@ -2717,6 +2738,11 @@
type: array
items:
type: object
required:
- type
- count
- price
- transferable
properties:
type:
type: string
Expand All @@ -2725,26 +2751,24 @@
price:
type: number
group_size:
type: number
required: false
type: integer
group_discount:
type: number
format: float
required: false
transferable:
type: boolean
buyable:
type: boolean
required: false
order_limit:
type: int
required: false
type: integer
drop_time:
type: string
format: date-time
required: false
fee_charged_to_buyer:
type: boolean
responses:
"200":
description: Tickets created successfully
content:
application/json:
schema:
Expand All @@ -2753,6 +2777,7 @@
detail:
type: string
"400":
description: Invalid request parameters
content:
application/json:
schema:
Expand All @@ -2761,6 +2786,7 @@
detail:
type: string
"403":
description: Tickets cannot be modified
content:
application/json:
schema:
Expand All @@ -2772,27 +2798,59 @@
"""
event = self.get_object()

# Tickets can't be edited after they've dropped
if event.ticket_drop_time and timezone.now() > event.ticket_drop_time:
# Tickets can't be edited after they've been sold or checked out
if event.tickets.filter(
Q(owner__isnull=False) | Q(holder__isnull=False)
).exists():
Comment on lines +2801 to +2804
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we won't be supporting ticket holds Q(holder__isnull=False) will be unnecessary

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, but I think I want to leave it in for now. If something changes with the redesign of checkout (e.g. we use holds in a different context), it'll be a pain to go back and add these checks to all the previous PRs. Once we commit to a specific implementation checkout, we can just go through and remove all the holding logic in one go.

return Response(
{"detail": "Tickets cannot be edited after they have dropped"},
{
"detail": "Tickets cannot be edited after they have been "
"sold or checked out"
},
status=status.HTTP_403_FORBIDDEN,
)

# Tickets can't be edited after they've been sold or held
ticket_settings, _ = TicketSettings.objects.get_or_create(event=event)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really matter, but can ticket_settings, _ = TicketSettings.objects.get_or_create(event=event) be moved down to after the if statement just below it since we don't use ticket_settings until after the if statement and in the case that the if is true we return immediately

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we're creating an event for the first time? Then there is no ticket_settings associated with the event, and the check below (if event.ticket_settings.drop_time is set) throws an exception, since TicketSettings is a one-to-one relationship.


# Tickets can't be edited after they've dropped
if (
Ticket.objects.filter(event=event)
.filter(Q(owner__isnull=False) | Q(holder__isnull=False))
.exists()
event.ticket_settings.drop_time
and timezone.now() > event.ticket_settings.drop_time
):
return Response(
{
"detail": "Tickets cannot be edited after they have been "
"sold or checked out"
},
{"detail": "Tickets cannot be edited after they have dropped"},
status=status.HTTP_403_FORBIDDEN,
)

order_limit = request.data.get("order_limit", None)
if order_limit is not None:
ticket_settings.order_limit = order_limit
ticket_settings.save()

drop_time = request.data.get("drop_time", None)
if drop_time is not None:
try:
drop_time = datetime.datetime.strptime(drop_time, "%Y-%m-%dT%H:%M:%S%z")
except ValueError as e:
return Response(

Check warning on line 2835 in backend/clubs/views.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/views.py#L2834-L2835 

Added lines #L2834 - L2835 were not covered by tests
{"detail": f"Invalid drop time: {str(e)}"},
status=status.HTTP_400_BAD_REQUEST,
)

if drop_time < timezone.now():
return Response(

Check warning on line 2841 in backend/clubs/views.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/views.py#L2841 

Added line #L2841 was not covered by tests
{"detail": "Specified drop time has already elapsed"},
status=status.HTTP_400_BAD_REQUEST,
)

ticket_settings.drop_time = drop_time
ticket_settings.save()

fee_charged_to_buyer = request.data.get("fee_charged_to_buyer", None)
if fee_charged_to_buyer is not None:
ticket_settings.fee_charged_to_buyer = fee_charged_to_buyer
ticket_settings.save()

quantities = request.data.get("quantities", [])
if not quantities:
return Response(
Expand Down Expand Up @@ -2855,35 +2913,11 @@
for item in quantities
for _ in range(item["count"])
]

Ticket.objects.bulk_create(tickets)

order_limit = request.data.get("order_limit", None)
if order_limit is not None:
event.ticket_order_limit = order_limit
event.save()

drop_time = request.data.get("drop_time", None)
if drop_time is not None:
try:
drop_time = datetime.datetime.strptime(drop_time, "%Y-%m-%dT%H:%M:%S%z")
except ValueError as e:
return Response(
{"detail": f"Invalid drop time: {str(e)}"},
status=status.HTTP_400_BAD_REQUEST,
)

if drop_time < timezone.now():
return Response(
{"detail": "Specified drop time has already elapsed"},
status=status.HTTP_400_BAD_REQUEST,
)

event.ticket_drop_time = drop_time
event.save()

cache.delete(f"clubs:{event.club.id}-authed")
cache.delete(f"clubs:{event.club.id}-anon")

return Response({"detail": "Successfully created tickets"})

@action(detail=True, methods=["post"])
Expand Down
Loading
Loading