Open-event-server: Server showing 409: tickets sold out when they are not

[cweitat]

Describe the bug
409 payload error message shows when booking a ticket. This can be verified by multiple users.

To Reproduce
Steps to reproduce the behaviour:

1. Go to event (https://eventyay.com/e/d99b1f7e)
2. Select 1 Free ticket and click order now
3. 409 payload error message appears

Expected behaviour
Should display the information page

Screenshots

Desktop (please complete the following information):
Firefox, Windows
Chrome, Windows

[iamareebjamal]

Ticket is sold out. Proper info should be shown in frontend

[cweitat]

@iamareebjamal ticket is not sold out. please don't jump to conclusion without asking anything

[iamareebjamal]

This is the error coming from backend. I am not jumping to any conclusion. Open the network panel and see for yourself. I never said it is not a bug. So not sure why you are triggered

So, it's a bug from server. Either way, the backend is showing tickets are sold out

[cweitat]

I'm a user, not a developer of this project. Just reporting what attendees who failed signed up are facing and not obliged to do so as well. And no one is triggered here. The short reply given earlier without any image is misleading.

So thanks for the more detailed reply of why attendees are seeing the error message.

[iamareebjamal]

We are also all volunteers here with no obligation :)

[codedsun]

@iamareebjamal Taking this up! Thanks

[codedsun]

@iamareebjamal The error is not in the attendees api else it would be failing for every event. How can i get the logs for the server? and also how can i know what values are present their in the table for this event for debugging?

[prateekj117]

@iamareebjamal While working on the New Orders API, I did had a hard time understanding the old logic for tickets left and it seemed wrong to me. It would be great if you could give it a look as well. I will get back with more details after my exam tomorrow.

[iamareebjamal]

The error is not in the attendees api

It cannot be anywhere else

else it would be failing for every event

No, it can fail for certain events where some people bought tickets and then the pending ticket holders remained from being expired

[iamareebjamal]

Well, you can see why it is broken. In order statistics API, this is how you calculate the sold tickets:

https://github.com/fossasia/open-event-server/blob/80b47a62bb40d81e796ee2933645cc4ed4a43290/app/api/order_statistics/events.py#L49-L50

This returns 19

https://github.com/fossasia/open-event-server/blob/80b47a62bb40d81e796ee2933645cc4ed4a43290/app/api/attendees.py#L58-L60

This returns 50. Simply because it does not filter any expired order. So, any any ticket holder created with no deleted_at is going to be considered a valid ticket holder.

However, both logic are wrong. The first query runs:

select sum(quantity) from orders_tickets, orders where order_id = orders.id and ticket_id = 1280 and orders_tickets.deleted_at is null and orders.status = 'completed';

And returns 19, which is at least better than 50.

But it doesn't filter out deleted orders. Other example why deleted_at and multiple order state ruins the logic.

So, the actual query should be:

select sum(quantity) from orders_tickets, orders where order_id = orders.id and ticket_id = 1280 and orders_tickets.deleted_at is null and orders.status = 'completed' and orders.deleted_at is null;

Which correctly returns 18. However, orders_tickets is a redundant table and should be removed. Which order has which tickets and how many attendees is already saved in ticket_holders table.

The current tickets_holder query is:

select count(*) from ticket_holders where ticket_id = 1280 and deleted_at is null;

Which is completely wrong and doesn't take completed orders into account.

Change it to

select count(*) from ticket_holders, orders where order_id = orders.id and ticket_id = 1280 and orders.status = 'completed' and ticket_holders.deleted_at is null;

And you get the correct number 18. However, if the current logic is changed. There will be a race condition through which more tickets than allowed can be bought.

Suppose there are 10 tickets left and I buy 5 of them, but my order is not complete. In current scenario, a new attendee will see 5 tickets left and will not be able to buy more than 5, which is incorrect because my order may never be completed. So, it should be able to buy those tickets. This means, once there are 10 pending ticket holders, no one can buy new tickets even if there were 0 tickets sold.

If we change it to correct count query, then remaining tickets are 10. New attendee comes and buys 5 tickets. Another attendee 2 comes and sees remaining tickets as 10 as well because previous order is not completed. And buys 10 tickets. Now order 1 gets completed and 5 tickets remain unsold. A third attendee comes and buys those 5 tickets. Now, Order 2 gets completed, and 15 tickets have been sold when only 10 were left, and once order 3 will be completed, 20 tickets will be sold.

So, another way needs to be thought of how to mitigate that. In my opinion, the ticket reservation mechanism is dubious at best and a source of a lot of problems. At the end of the day, either it will prevent actual people to buy unsold tickets, or make them buy more tickets than allowed, or fail at the end of procedure saying that sorry, tickets are sold out.

@mariobehling Need your input

[iamareebjamal]

And needless to say that order expiry logic is not working as well. It is also something which is difficult to get right in current design

Related: https://github.com/fossasia/open-event-server/issues/5105 https://github.com/fossasia/open-event-server/pull/5108
https://github.com/fossasia/open-event-server/issues/5085 https://github.com/fossasia/open-event-server/pull/5087

[kushthedude]

The Pending Mechanism holds the tickets only for 30 min , Needless to say, the reservation is needed as order may get payment failure or some other issue. Talking about the other ticket buyer when he comes to buy the ticket, There we had an idea of implementing the order waitlist as it is done in other similar platforms.
Now we have two options, One to remove the reservation feature which may hit too hard on the buyers since payment gateways may cause issues sometimes.
Second, implement an ordered waitlist which will redirect the pending attendee into the waitlist and once the waitlist is cleared we can mail him for the orders or else the tickets are gone.
@iamareebjamal @mariobehling

[kushthedude]

Also this is related to https://github.com/fossasia/open-event-server/issues/6433 https://github.com/fossasia/open-event-server/pull/6432#issuecomment-549126183

[kushthedude]

The Pending Mechanism holds the tickets only for 30 min , Needless to say, the reservation is needed as order may get payment failure or some other issue. Talking about the other ticket buyer when he comes to buy the ticket, There we had an idea of implementing the order waitlist as it is done in other similar platforms.
Now we have two options, One to remove the reservation feature which may hit too hard on the buyers since payment gateways may cause issues sometimes.
Second, implement an ordered waitlist which will redirect the pending attendee into the waitlist and once the waitlist is cleared we can mail him for the orders or else the tickets are gone.
@iamareebjamal @mariobehling

Also, There is other discrepancy while ordering the ticket through offline payment. All the ticket which are bought through offline mode of payment gets a placed label, not to mention these tickets are counted in Ticket Statistics Table. But here comes the twist, Suppose I have 10 tickets and I buy all the 10 using offline mode, All of the 10 Tickets will be with the label placed and the ticket statistic will show 0 remaining tickets. But I will be able to buy more tickets without any issue.

[iamareebjamal]

The Pending Mechanism holds the tickets only for 30 min

And keeps holding it. It is not implemented correctly

[kushthedude]

The Pending Mechanism holds the tickets only for 30 min

And keeps holding it. It is not implemented correctly

The expiry mechanisn is not working then, I suppose

[iamareebjamal]

Duplicate: #6433 but there are 2 issues.

1. Count query of sold tickets. Solution: Calculate only placed orders + initializing orders which were created under order expiry time
2. The frontend creating dummy attendees without order ID. So, even if count query and reservation mechanism is fixed, we will not be able to track entries of attendees with dummy data since they are not linked to any order. This is the primary reason when I manually ran the query to expire orders, no attendees were deleted. Solution: Delete any attendee without order_id after order_expiry_time.

This will solve this issue

Closing older issue as this has more discussion

[iamareebjamal]

@kushthedude Is there any case where an attendee without order_id is valid?

I see some attendees without order_id but having a ticket PDF

[iamareebjamal]

I've deleted dummy attendees manually, so, it should work for now temporarily while we work on a fix

[kushthedude]

@iamareebjamal I will take a look at the order process thoroughly tonight.

On Thu, 12 Dec, 2019, 22:39 Areeb Jamal, [e-mail] wrote:

I've deleted dummy attendees manually, so, it should work for now
temporarily while we work on a fix

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/fossasia/open-event-server/issues/6653?email_source=notifications&email_token=AKQMTLT652Y3XNW3SF3B6WDQYJV47A5CNFSM4JZKR7FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGXLJ2A#issuecomment-565097704,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AKQMTLXMYDVXSYTSKSOSCMDQYJV47ANCNFSM4JZKR7FA
.

[codedsun]

@iamareebjamal https://github.com/fossasia/open-event-server/issues/6653#issuecomment-565030517

For this shall I create the sub issue and send the PR for it?

[iamareebjamal]

No need for sub issue. This is the issue itself

[kushthedude]

Duplicate: #6433 but there are 2 issues.

1. Count query of sold tickets. Solution: Calculate only placed orders + initializing orders which were created under order expiry time

@iamareebjamal For your point one:
It makes sense to count the queries for placed order, But why intializing ? Since Initialising is only used untill the order is changed to pending or expired. Shouldn't it be placed & completed ?

[iamareebjamal]

Then the reservation system will be broken. We also need to count reserved tickets - thus initializing tickets as well

[kushthedude]

Ohk, so we will include all the expired and pending initially for initialising status which will be
removed by the job which is currently being implemented by @codedsun, hence
only completed tickets will be stored.

On Sat, 14 Dec, 2019, 21:23 Areeb Jamal, [e-mail] wrote:

Then the reservation system will be broken.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/fossasia/open-event-server/issues/6653?email_source=notifications&email_token=AKQMTLS4LOX223HK2KWJRW3QYT6PJA5CNFSM4JZKR7FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG4FO7A#issuecomment-565729148,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AKQMTLT2B4RKQ4PTNPWUCEDQYT6PJANCNFSM4JZKR7FA
.

[iamareebjamal]

count = completed orders + (initializing orders created within order expiry time) + (placed orders created within 10 minutes)

What @codedsun has made is a job to remove orphan ticket holders which have no order ID. They won't be counted, and can't be counted.

[iamareebjamal]

What is the current state transition of orders is this:

1. initializing - wait for order_expiry_time and then change to expired
   User is entering the information and can enter upto order_expiry_time
2. placed - wait for 10 minutes and then change to expired
   User has entered the information and now in the process of payment, and must complete the payment within 10 minutes
3. completed - User has paid for the order

@prateekj117 @kushthedude Please correct me if I am wrong

[iamareebjamal]

These are the states in the DB:

+--------------+---------+
| status       |   count |
|--------------+---------|
| initializing |       4 |
| initialized  |      27 |
| placed       |      04 |
| cancelled    |      15 |
| deleted      |      51 |
| completed    |      54 |
| expired      |      37 |
+--------------+---------+

Why is initialized used? Why not just placed? And what's the difference between deleted and orders where deleted_at is not null? This is confusing

[iamareebjamal]

Wow, there's also trashed_at field in orders along with deleted_at. There are more trashed_at orders than deleted_at in production. I'm sure there are wrong sales and tickets sold data because of this. Every new state is like salt to the wound which is the order API. If someone has reason for this, please help

[kushthedude]

What is the current state transition of orders is this:

1. initializing - wait for order_expiry_time and then change to expired
   User is entering the information and can enter upto order_expiry_time
2. placed - wait for 10 minutes and then change to expired
   User has entered the information and now in the process of payment, and must complete the payment within 10 minutes
3. completed - User has paid for the order

@prateekj117 @kushthedude Please correct me if I am wrong

No, There are some minor changes @iamareebjamal :

• For the placed, it is incorrect . All the orders which are made using offline mode of payment are placed orders which have no expiry time.
• Pending orders have a window of 30 minutes after order_expiry_time to convert it into expired if payment is not done.
• Completed is either user has paid or ticket is free.
• Cancelled is the status assigned to an order by organiser, Like if on the event day any attendee does not do offline payment then he can change the status of placed into cancelled or vice versa to completed

[kushthedude]

These are the states in the DB:

+--------------+---------+
| status       |   count |
|--------------+---------|
| initializing |       4 |
| initialized  |      27 |
| placed       |      04 |
| cancelled    |      15 |
| deleted      |      51 |
| completed    |      54 |
| expired      |      37 |
+--------------+---------+

Why is initialized used? Why not just placed? And what's the difference between deleted and orders where deleted_at is not null? This is confusing

I have not idea about intialized and initialising, Most common guess of mine is there is some typo in our code for this!

[iamareebjamal]

For the placed, it is incorrect . All the orders which are made using offline mode of payment are placed orders which have no expiry time.

Why not completed?

[kushthedude]

For the placed, it is incorrect . All the orders which are made using offline mode of payment are placed orders which have no expiry time.

Why not completed?

This was the change suggested by @mariobehling .

[iamareebjamal]

OK, if they want to have a distinction between placed and completed, but this makes it more complicated and now there are 2 states for valid orders. placed and completed. I think there should be a single final state and maybe a boolean field to mark if the order is placed or not.

This will reduce the states to initializing, cancelled, expired, completed (once we have finalized and removed deleted state as well)
With only one valid state, greatly simplifying the queries for count and everything

[kushthedude]

(once we have finalized and removed deleted state as well)

There is a undue feature requested by @mariobehling for listing of all the deleted orders in the Admin Panel rather then showing it to the organisers and event rolees ?

[kushthedude]

OK, if they want to have a distinction between placed and completed, but this makes it more complicated and now there are 2 states for valid orders. placed and completed.

Exactly, And we are neglecting the placed orders in every aspect whether it be no of tickets sold checks or the final sales amount shown for the event.

I think there should be a single final state and maybe a boolean field to mark if the order is placed or not.

What will the final state make a difference ? Since we require the both completed and placed to show the organiser distinction between offline and online orders .

[iamareebjamal]

Final state will make query easier as you just have to check for order.status == 'completed', for revenue or ticket sold, online or offline. All in all, it being offline is not a state, it's an attribute of the order

listing of all the deleted orders in the Admin Panel

We already have trashed_at and deleted_at, why do we need deleted state?

[kushthedude]

Final state will make query easier as you just have to check for order.status == 'completed', for revenue or ticket sold, online or offline. All in all, it being offline is not a state, it's an attribute of the order

So what I can understand is we will have a boolean to check if the order is offline paid. If the order is offline we will assign it a completed state and for the FE we may show something like Completed Order [ Offline Mode] or something other. For ease of organiser .

[iamareebjamal]

Yes, or show it as placed. It doesn't really matter. Order will only have a few states, and offline or online will be notified using a boolean.

[iamareebjamal]

So, I just checked and all of the orders with initialized state have paid_via set to paypal or stripe, and also have paypal_token and stripe_token. Now, I really hope it is not what it seems that the orders were paid for and never got saved as completed :(

And there are 3 initializing orders from 12th Dec 2019 which were never expired, because the cron job expires orders which have no paid_via and status == 'initialized'

The cron job should expire both initializing and initialized orders. @codedsun Can you please open an issue for that.

In my opinion, this should be the order state flow.

initializing -> pending -> completed

Two more for invalid state : cancelled and expired.

That's it. Clear and simple to understand. Initializing when user is entering information. Pending when order is being waited or payment. Completed when it is paid for or the order is free.
Also, when new state is assigned, previous should be stored to know at which state the order was expired or cancelled, at initializing or pending, and make it easier to debug, correct bugs and mistakes

[kushthedude]

@iamareebjamal I think we should involve @mariobehling in this order status too. It is too complex rn and we can discuss what more further improvements cam be done