Add option to pass transaction fees onto buyers #762
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ticketing-v2 #762 +/- ##
================================================
+ Coverage 71.99% 72.03% +0.04%
================================================
Files 32 32
Lines 6963 6977 +14
================================================
+ Hits 5013 5026 +13
- Misses 1950 1951 +1 ☔ View full report in Codecov by Sentry. |
backend/clubs/models.py
Outdated
| event = models.OneToOneField( | ||
| Event, on_delete=models.CASCADE, related_name="ticket_settings" | ||
| ) | ||
| order_limit = models.IntegerField(null=True, blank=True) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| ) | ||
| order_limit = models.IntegerField(null=True, blank=True) | ||
| drop_time = models.DateTimeField(null=True, blank=True) | ||
| fee_charged_to_buyer = models.BooleanField(default=False) |
There was a problem hiding this comment.
Setting transaction fees to be shouldered by the club by default.
| # 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(): |
There was a problem hiding this comment.
Since we won't be supporting ticket holds Q(holder__isnull=False) will be unnecessary
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
backend/clubs/models.py
Outdated
| event = models.OneToOneField( | ||
| Event, on_delete=models.CASCADE, related_name="ticket_settings" | ||
| ) | ||
| order_limit = models.IntegerField(null=True, blank=True) |
There was a problem hiding this comment.
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
Refactors out event-wide settings for ticketed events into a new
TicketSettingsmodel. Adds backend support for club leaders to pass transaction fees onto buyers. (This isn't reflected in calculated cart totals yet. We're waiting on Penn Finance to determine how transaction fees will be collected before implementing this.)Addresses #754.