Skip to content

Conversation

@mrsaicharan1
Copy link
Member

@mrsaicharan1 mrsaicharan1 commented May 15, 2019

Fixes #5878

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.

Short description of what this resolves:

Generates a proper pdf which is served over a route instead of the current snapshot technique for order invoices.

Changes proposed in this pull request:

  • Added method for generating invoices
  • Route for serving invoices
  • Added static HTML page for invoice

@auto-label auto-label bot added the fix label May 15, 2019
else:
return ForbiddenError({'source': ''}, 'Authentication Required to access ticket').respond()

@ticket_blueprint.route('/orders/invoices/<string:order_identifier>')

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@mrsaicharan1 mrsaicharan1 force-pushed the invoice-fix branch 2 times, most recently from 47dcde8 to a621c3c Compare May 15, 2019 11:06
@codecov
Copy link

codecov bot commented May 15, 2019

Codecov Report

Merging #5886 into development will decrease coverage by 0.06%.
The diff coverage is 11.76%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5886      +/-   ##
===============================================
- Coverage         66.6%   66.54%   -0.07%     
===============================================
  Files              286      286              
  Lines            13902    13919      +17     
===============================================
+ Hits              9260     9262       +2     
- Misses            4642     4657      +15
Impacted Files Coverage Δ
app/api/helpers/storage.py 59.29% <ø> (ø) ⬆️
app/api/helpers/order.py 38.7% <0%> (-0.64%) ⬇️
app/api/auth.py 19.02% <12.5%> (-0.56%) ⬇️

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 9317162...a067156. Read the comment docs.

@mrsaicharan1
Copy link
Member Author

@uds5501 @shreyanshdwivedi @iamareebjamal Please review

Copy link
Contributor

@abhinavk96 abhinavk96 left a comment

Choose a reason for hiding this comment

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

Can you give a preview of a sample invoice, also I don't see the billing info in the template.

@mrsaicharan1
Copy link
Member Author

Can you give a preview of a sample invoice, also I don't see the billing info in the template.
Will update the billing info and post a preview.

Added serving route and static page

Add billing info invoice
@mrsaicharan1
Copy link
Member Author

Can you give a preview of a sample invoice, also I don't see the billing info in the template.

Screenshot 2019-05-16 at 12 49 34 PM

@mariobehling
Copy link
Member

Thanks a lot! This is a step in the right direction. We need additional information in the invoice.

  • Event Information -> What is the invoice for
  • Tax of each item
  • Overall tax

We also need a way to reflect the different way tax is handled in Europe/Asia and the US:

  • In Europe tax is included in the ticket price (but we still need to show how much it is)
  • In the US tax is added on top of each item

This is also an option in the tax info.

Below is an example of an invoice from eventbrite (for events - we need just the same structure but for tickets)

difference-between-tax-invoice-and-sales-receipt-difference-between-tax-invoice-and-official-receipt-difference-between-tax-invoice-and-receipt

@mariobehling
Copy link
Member

To make the tax statement clearer I would appreciate if you could make this slightly different to eventbrite and show the tax in % and in numbers. Also show the price below the table as follows.

Screenshot from 2019-05-16 10-50-26

@abhinavk96
Copy link
Contributor

abhinavk96 commented May 16, 2019 via email

@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented May 16, 2019

@mariobehling @CosmicCoder96 Will make the issue related to this as a parent issue.
Will create another issue+PR for
Making the Order Invoice similar to EventBrite(Attaching the Tax amount too as well). If the order invoice generation is fine here, would request you to merge this.

@iamareebjamal iamareebjamal merged commit 5468dbc into fossasia:development May 16, 2019
@iamareebjamal
Copy link
Member

iamareebjamal commented May 16, 2019

-- via Internet Explorer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generate Order Invoice properly(akin to Tickets)

6 participants