Implement API to check if user paid for event or not #280
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new API endpoint to check if a customer has paid for an event. The main changes include the addition of a new view class File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @lcduong - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using consistent response types throughout the view (either Response or JsonResponse) for better maintainability.
- The order_list query using complex Q objects might impact performance for large datasets. Consider optimizing this query or adding appropriate database indexes.
- Ensure proper authentication and authorization checks are in place to protect sensitive customer information.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| event = Event.objects.get(slug=kwargs["event"], organizer=organizer) | ||
| customer = Customer.objects.get(identifier=kwargs["customer_id"], organizer=organizer) | ||
|
|
||
| except Organizer.DoesNotExist: |
There was a problem hiding this comment.
suggestion: Simplify error handling using a dictionary
Consider using a dictionary to map exception types to error messages. This can reduce code duplication and improve maintainability.
exceptions = {
Organizer.DoesNotExist: "Organizer not found.",
Event.DoesNotExist: "Event not found.",
Customer.DoesNotExist: "Customer not found.",
}
for exception, message in exceptions.items():
try:
raise exception
except exception:
return JsonResponse(status=404, data={"error": message})
| return JsonResponse(status=404, data={"error": "Customer not found."}) | ||
|
|
||
| # Get all orders of customer which belong to this event | ||
| order_list = (Order.objects.filter(Q(event=event) |
There was a problem hiding this comment.
suggestion (performance): Optimize and simplify the order query
The current query is complex and might be inefficient. Consider simplifying it and avoiding the use of iexact for email comparison, which could be slow for large datasets.
| order_list = (Order.objects.filter(Q(event=event) | |
| order_list = Order.objects.filter( | |
| event=event, | |
| customer__in=[customer, None], | |
| email__in=[customer.email, ''] | |
| ).select_related('event').order_by('-datetime') |
| if not order_list: | ||
| return JsonResponse(status=404, data={"error": "Customer has no orders for this event."}) | ||
|
|
||
| for order in order_list: |
There was a problem hiding this comment.
suggestion (performance): Optimize paid order check by modifying the initial query
Instead of looping through all orders to check for paid status, consider adding a filter for paid orders in the initial database query.
| for order in order_list: | |
| if order_list.filter(status='p').exists(): | |
| return JsonResponse(status=200, data={"message": "Customer has paid orders."}) |
As part of this issue fossasia/eventyay-talk#133
Implement an API for talk component to check if customer ordered ticket yet or not
Summary by Sourcery
Implement an API endpoint to verify if a customer has paid for a specific event by checking their orders.
New Features: