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.

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

feat: Single endpoint for creating creating an order #6635

Merged
merged 16 commits into from
Jan 5, 2020
86 changes: 82 additions & 4 deletions app/api/custom/orders.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
import json

Choose a reason for hiding this comment

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

'json' imported but unused


from flask import Blueprint, jsonify, request
from flask_jwt_extended import current_user, jwt_required
from flask_limiter.util import get_remote_address
from sqlalchemy.orm.exc import NoResultFound


from app import limiter
from app.models import db
from app.api.auth import return_file
from app.api.helpers.db import safe_query
from app.api.helpers.db import safe_query, get_count
from app.api.helpers.mail import send_email_to_attendees
from app.api.helpers.errors import ForbiddenError, UnprocessableEntityError, NotFoundError
from app.api.helpers.order import calculate_order_amount, create_pdf_tickets_for_holder
from app.api.helpers.storage import UPLOAD_PATHS
from app.api.helpers.storage import generate_hash
from app.api.helpers.ticketing import TicketingManager
from app.api.schema.attendees import AttendeeSchema
from app.api.schema.orders import OrderSchema
from app.api.helpers.permission_manager import has_access
from app.models.discount_code import DiscountCode
from app.models.order import Order
from app.models.order import OrderTicket
from app.models.ticket import Ticket
from app.models.ticket_holder import TicketHolder

order_blueprint = Blueprint('order_blueprint', __name__, url_prefix='/v1/orders')
ticket_blueprint = Blueprint('ticket_blueprint', __name__, url_prefix='/v1/tickets')
Expand Down Expand Up @@ -82,8 +88,11 @@ def resend_emails():

@order_blueprint.route('/calculate-amount', methods=['POST'])
@jwt_required
def calculate_amount():
data = request.get_json()
def calculate_amount(*args):
if not args:
data = request.get_json()
else:
data = json.loads(args[0])
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned previously, this should not be done

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

No benefit of merging an intermediate state PR

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal I will start working on that PR as soon as this gets merged. I wanted to keep them separate as this PR already contains a big change. And the second change is more of a refactor.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's why it can be taken in this PR only. It's not a huge change compared to what has been done already. It makes no sense to break something first and then fix later in a separate PR

tickets = data['tickets']
discount_code = None
if 'discount-code' in data:
Expand All @@ -93,3 +102,72 @@ def calculate_amount():
return UnprocessableEntityError({'source': 'discount-code'}, 'Discount Usage Exceeded').respond()

return jsonify(calculate_order_amount(tickets, discount_code))


@order_blueprint.route('/create-order', methods=['POST'])
@jwt_required
def create_order():
data = request.get_json()
tickets = data['tickets']
attendee = data['attendee']
prateekj117 marked this conversation as resolved.
Show resolved Hide resolved
for attribute in attendee:
attendee[attribute.replace('-', '_')] = attendee.pop(attribute)
schema = AttendeeSchema()
json_api_attendee = {"data": {"attributes": data['attendee'], "type": "attendee"}}
result = schema.load(json_api_attendee)
if result.errors:
return jsonify(result.errors)
Copy link
Member

Choose a reason for hiding this comment

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

This will send HTTP status code as 200, which means OK. Which means order was created successfully

ticket_ids = [int(ticket['id']) for ticket in tickets]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ticket_ids = [int(ticket['id']) for ticket in tickets]
ticket_ids = {int(ticket['id']) for ticket in tickets}

ticket_info_list = db.session.query(Ticket).filter(Ticket.id.in_(ticket_ids)).filter_by(event_id=data['event_id'],
Copy link
Member

Choose a reason for hiding this comment

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

Why not tickets or ticket_list

deleted_at=None).all()
ticket_ids_found = [ticket_information.id for ticket_information in ticket_info_list]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ticket_ids_found = [ticket_information.id for ticket_information in ticket_info_list]
ticket_ids_found = {ticket_information.id for ticket_information in ticket_info_list}

tickets_not_found = list(set(ticket_ids) - set(ticket_ids_found))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
tickets_not_found = list(set(ticket_ids) - set(ticket_ids_found))
tickets_not_found = ticket_ids - ticket_ids_found

if tickets_not_found:
return jsonify(status='Order Unsuccessful', error='Ticket with id {} was not found.'.format(tickets_not_found))
Copy link
Member

Choose a reason for hiding this comment

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

There can be multiple tickets not found. Also, send event ID as well. Ticket 324 may exist but not under the event requested

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal I am sending the list of all the tickets not found. Regarding example of ticket 324, I am already passing event_id as well which fetching the tickets, so those tickets will also throw tickets not found.

Copy link
Member

Choose a reason for hiding this comment

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

It will throw: Ticket with ID 324, 325, 326 was not found

Everyone wants to see: Tickets with ID 324, 325, 326 were not found in Event 432

Hence, again #6635 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

It will throw: Ticket with ID 324, 325, 326 was not found in Event 432

Everyone wants to see: Tickets with ID 324, 325, 326 were not found in Event 432

Copy link
Member

Choose a reason for hiding this comment

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

This will send HTTP status code as 200, which means OK. Which means order was created successfully

for ticket in tickets:
ticket_info = None
for ticket_information in ticket_info_list:
Copy link
Member

Choose a reason for hiding this comment

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

Nested for loop = Automatically not approved. There is no need of nested for loop here

if ticket_information.id == int(ticket['id']):
ticket_info = ticket_information
if (ticket_info.quantity - get_count(db.session.query(TicketHolder.id).filter_by(
ticket_id=int(ticket['id']), deleted_at=None))) < ticket['quantity']:
return jsonify(status='Order Unsuccessful', error='Ticket already sold out.')
Copy link
Member

Choose a reason for hiding this comment

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

These checks should be in calculate amount function itself

Copy link
Member

Choose a reason for hiding this comment

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

This will send HTTP status code as 200, which means OK. Which means order was created successfully

attendee_list = []
for ticket in tickets:
for ticket_amount in range(ticket['quantity']):
prateekj117 marked this conversation as resolved.
Show resolved Hide resolved
prateekj117 marked this conversation as resolved.
Show resolved Hide resolved
attendee = TicketHolder(**result[0], event_id=int(data['event_id']), ticket_id=int(ticket['id']))
prateekj117 marked this conversation as resolved.
Show resolved Hide resolved
db.session.add(attendee)
attendee_list.append(attendee)
ticket_pricing = calculate_amount(json.dumps(data))
if not has_access('is_coorganizer', event_id=data['event_id']):
data['status'] = 'initializing'
# create on site attendees
# check if order already exists for this attendee.
# check for free tickets and verified user
ticket_pricing = json.loads(ticket_pricing.data)
order = Order(amount=ticket_pricing['total_amount'], user_id=current_user.id, event_id=int(data['event_id']),
status=data['status'])
db.session.add(order)
db.session.commit()
db.session.refresh(order)
order_tickets = {}
for holder in attendee_list:
holder.order_id = order.id
db.session.add(holder)
if not order_tickets.get(holder.ticket_id):
order_tickets[holder.ticket_id] = 1
else:
order_tickets[holder.ticket_id] += 1

create_pdf_tickets_for_holder(order)

for ticket in order_tickets:
od = OrderTicket(order_id=order.id, ticket_id=ticket, quantity=order_tickets[ticket])
db.session.add(od)

order.quantity = order.tickets_count
db.session.add(order)
db.session.commit()
db.session.refresh(order)
order_schema = OrderSchema()
return order_schema.dump(order)