Skip to content

Implement discount (promo) codes#798

Open
julianweng wants to merge 5 commits into
masterfrom
discount-codes
Open

Implement discount (promo) codes#798
julianweng wants to merge 5 commits into
masterfrom
discount-codes

Conversation

@julianweng
Copy link
Copy Markdown
Member

  • Implement auto-generation of discount code
  • Fix implementation of group discounts
  • Misc mobile responsivity improvements

adding discounts

seeing selling information for a given ticket

checkout

Copy link
Copy Markdown
Member

@aviupadhyayula aviupadhyayula left a comment

Choose a reason for hiding this comment

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

Took a look at the backend and left a couple comments. At a high level: I'm not a huge fan of moving pricing-related stuff (group discounts, promo codes) under the Ticket class. It feels crowded and sort of against the principles of OOP.

The way I was envisioning we'd implement this is as a separate model (e.g. TicketPromoCode), with a link back to the ticket type. When a user checks out (or when we call the cart route), they apply as many promo codes as they want through the frontend. We check if the promo codes are valid, and then use them in the cart total calculation.

This feels cleaner to me, and also unlocks more capability for event owners. With the current approach, event owners can't create multiple promo codes per ticket.

I think the work Rohan was doing with the Ticket and TicketClass models is kind of what we want. I don't know what the best course of action here is: should we wait until the checkout flow rewrite's done, and then migrate this work over to that branch?

Let me know what you think. (One last nit: we'll be merging this PR into ticketing-v2, so we'll need to change the target branch in the PR.)

Comment thread backend/clubs/models.py
blank=True,
)
group_size = models.PositiveIntegerField(null=True, blank=True)
code_discount = models.DecimalField(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: should this be discount_code?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, I think we should delineate between promo codes and group discounts throughout the codebase. Let's refer to these as promo codes instead.

Comment thread backend/clubs/views.py
Comment on lines +2650 to +2660
if discount_code:
valid_discount_code = tickets.first().discount_code
if discount_code != valid_discount_code:
return Response(
{
"detail": f"Invalid discount code for {type}",
"success": False,
},
status=status.HTTP_400_BAD_REQUEST,
)
tickets.update(discount_code_applied=True)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will the promo code be applied when the user adds tickets to their cart? I was under the impression that we'd let users add promo codes before checkout (e.g. on the cart page).

Comment thread backend/clubs/views.py
tickets = Ticket.objects.filter(event=event)
# if for purchasing purposes, non-buyable tickets are unavailable
# otherwise, all non-owned/held tickets are available
for_purchasing = self.request.query_params.get("for_purchasing", "true")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: let's do validation here (has to be true or false) and then convert to a boolean.

Comment thread backend/clubs/views.py
Comment on lines +2853 to +2854
for ticket_type in tickets.values_list("type", flat=True).distinct():
ticket = tickets.filter(type=ticket_type).first()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't we just get a ticket of each type in a single query? This looks expensive.

Comment thread backend/clubs/views.py
available_list = []
for ticket_type in tickets.values_list("type", flat=True).distinct():
ticket = tickets.filter(type=ticket_type).first()
if ticket:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: use guard clause pattern instead.

Comment thread backend/clubs/views.py

# Group discounts must be between 0 and 1
if item.get("group_discount", 0) < 0 or item.get("group_discount", 0) > 1:
if item.get("group_discount", 0) is not None and (
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be if item.get("group_discount", 0" is None or ? We want to check that if the group_discount is specified, it's non-null.

Comment thread backend/clubs/views.py
)

# Code discounts must be between 0 and 1
if item.get("code_discount", 0) is not None and (
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants