fix: Fixed multiple tickets error & refactored order invoice logic

[mrsaicharan1]

Fixes #6132

Short description of what this resolves:

The order invoice function was trying to query for one entry alluding to the order ID. But, It found multiple entries pertaining to the order. Fixed this by taking only the first entry going into the model.

Changes proposed in this pull request:

• Refactored order invoice logic to loop through OrderTicket objects
• Queried all the OrderTicket instances to ensure everything was getting transmitted

Checklist

• I have read the Contribution & Best practices Guide and my PR follows them.
• My branch is up-to-date with the Upstream development branch.
• The unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

• All the functions created/modified in this PR contain relevant docstrings.

[codecov]

Codecov Report

Merging #6133 into development will not change coverage.
The diff coverage is 0%.

@@             Coverage Diff              @@
##           development    #6133   +/-   ##
============================================
  Coverage        66.37%   66.37%           
============================================
  Files              286      286           
  Lines            14359    14359           
============================================
  Hits              9531     9531           
  Misses            4828     4828

Impacted Files	Coverage Δ
app/api/helpers/order.py	40.9% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94489bb...f26c422. Read the comment docs.

[iamareebjamal]

Then you should filter by user_id as well. Current approach looks brittle, it'll just select first

[mrsaicharan1]

Then you should filter by user_id as well. Current approach looks brittle, it'll just select first

I agree. Changing it right now.

[mrsaicharan1]

Then you should filter by user_id as well. Current approach looks brittle, it'll just select first

The problem is that there is no user_id field associated with OrderTicket Model. Also, we just require the quantity of the a specific ticket associated with an Order ID. So whichever entry gets filtered, the quantity would be the same.

[iamareebjamal]

Let's say there are 3 OrderTicket associated with an order, meaning there are 3 tickets, then quantity of all 3 tickets can be different. Right? Even the price can be different

[mrsaicharan1]

Let's say there are 3 OrderTicket associated with an order, meaning there are 3 tickets, then quantity of all 3 tickets can be different. Right? Even the price can be different

Sorry for the confusion. The quantity will be different. I agree. The thing is, to represent each ticket with their quantity in a table in the order invoice, I had used OrderTicket Model. I will think of another approach.

[iamareebjamal]

Then first will give wrong results, right? You need to loop over each ticket in OrderTicket relation

[mrsaicharan1]

Yes, just got that. I'm doing the same loop in Jinja template too. I'll push it right away

…

On Tue, 2 Jul 2019 at 3:21 PM, Areeb Jamal ***@***.***> wrote: Let's say there are 3 OrderTicket associated with an order, meaning there are 3 tickets, then quantity of all 3 tickets can be different. Right? Even the price can be different — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6133?email_source=notifications&email_token=AGAHUW2ARFV5LAJNOCTHP53P5MQLHA5CNFSM4H4YFXEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZAXGXQ#issuecomment-507605854>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGAHUW2XS2OTQNHICHAHFV3P5MQLHANCNFSM4H4YFXEA> .

[mrsaicharan1]

@iamareebjamal Refactored the jinja logic to match the OrderTicket loop too.

[mrsaicharan1]

Here's a test invoice
Full Event-Invoice-07d5f25f-5b2e-4561-a8cf-7abf84a7c15d.pdf

[mrsaicharan1]

Umm, there isn't a ticket attribute associated with it. Ticket_id, order_id and quantity are the 3 attributes present in the model

…

On Tue, 2 Jul 2019 at 4:34 PM, Areeb Jamal ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/templates/pdf/order_invoice.html <#6133 (comment)> : > - <td style="text-align:center">{{ ticket.name }}</td> - <td style="text-align:center">{{ event.payment_currency | currency_symbol }}{{ ticket.price }}</td> - <td style="text-align:center">{{ order_tickets_info.quantity }}</td> - <td style="text-align:center">{{ event.payment_currency | currency_symbol }}{{ order_tickets_info.quantity*ticket.price }}</td> - {% if tax %} - <td style="text-align:center">{{ tax.rate }}%</td> - {% else %} - <td style="text-align:center">{{ ("0%") }}</td> - {% endif %} - {% if tax %} - <td style="text-align:center">{{ event.payment_currency | currency_symbol }}{{ tax.rate*ticket.price/100 }}</td> - {% else %} - <td style="text-align:center">{{ event.payment_currency | currency_symbol }}0</td> - {% endif %} - </tr> + {% for ticket in tickets %} Why not just use order_ticket.ticket? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6133?email_source=notifications&email_token=AGAHUWYNJB4WWT65OQDMDXTP5MY2TA5CNFSM4H4YFXEKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB5HMFFA#pullrequestreview-256819860>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGAHUWZHCOP6FU5EMECAFX3P5MY2TANCNFSM4H4YFXEA> .

[iamareebjamal]

If there is ticket_id, there is ticket as related model

[mrsaicharan1]

If there is ticket_id, there is ticket as related model

@iamareebjamal Oh yeah! SQLalchmey relations. I'm updating it right now.