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

fix: Ticket downloads for multi-attendee orders #6095

Merged
merged 3 commits into from
Jun 22, 2019

Conversation

abhinavk96
Copy link
Contributor

Fixes #6091

Short description of what this resolves:

There were several issues at storage and permission level which prevented ticket downloads for multiple attendees.

  • There are separate ticket PDFs generated for all ticket holders, and then individual ticket holders.
  • These files were overwriting each other, and as such only the last attendee was present in the generated ticket
  • Admin, or ticket holders were not being allowed to download tickets due to wrong logical implementation on permission checks

Changes proposed in this pull request:

  • Create a new path for combined tickets
  • Move away permission control to user model
  • Amend permission control to allow admin, and ticket holders to download tickets.

@abhinavk96 abhinavk96 requested a review from iamareebjamal June 22, 2019 09:56
@abhinavk96 abhinavk96 changed the title Fix: Ticket downloads for multi-attendee orders fix: Ticket downloads for multi-attendee orders Jun 22, 2019
@auto-label auto-label bot added the fix label Jun 22, 2019
@codecov
Copy link

codecov bot commented Jun 22, 2019

Codecov Report

Merging #6095 into development will decrease coverage by <.01%.
The diff coverage is 14.28%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6095      +/-   ##
===============================================
- Coverage        66.17%   66.17%   -0.01%     
===============================================
  Files              285      285              
  Lines            14133    14136       +3     
===============================================
+ Hits              9353     9354       +1     
- Misses            4780     4782       +2
Impacted Files Coverage Δ
app/api/helpers/order.py 39.06% <ø> (ø) ⬆️
app/api/helpers/storage.py 59.29% <ø> (ø) ⬆️
app/api/auth.py 23.55% <0%> (+0.2%) ⬆️
app/models/user.py 55.55% <20%> (-0.72%) ⬇️

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 6d69104...f628796. Read the comment docs.

@iamareebjamal
Copy link
Member

What about existing ticket PDFs present in old location?

@abhinavk96
Copy link
Contributor Author

abhinavk96 commented Jun 22, 2019

@iamareebjamal deleted them (manually), the endpoint will recreate them if not found.
*Not deleted on the production server yet

try:
                return return_tickets(file_path, order_identifier)
except FileNotFoundError:
                create_pdf_tickets_for_holder(order)
                return return_tickets(file_path, order_identifier)

This ensures it ^ .

@iamareebjamal iamareebjamal merged commit 5822b1a into fossasia:development Jun 22, 2019
iamareebjamal pushed a commit to iamareebjamal/open-event-server that referenced this pull request Aug 2, 2019
* Introduce can_download_tickets property in user model

* Specify separate storage path for all ticket pdf

* fix: make it possible to download all attendee tickets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multi-attendee ticket download fails
2 participants