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: Made invoice similar to Eventbrite #5952

Merged
merged 2 commits into from
Jun 3, 2019

Conversation

mrsaicharan1
Copy link
Member

@mrsaicharan1 mrsaicharan1 commented May 24, 2019

Screenshot 2019-06-03 at 1 10 02 AM

Fixes #5891

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:

Structured the order invoice with order and related ticket info

Changes proposed in this pull request:

  • Added VAT and VAT amount to invoice
  • Added checks for billing info

@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented May 24, 2019

@shreyanshdwivedi @uds5501 Could you suggest some design changes to this invoice structure?

@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

Merging #5952 into development will decrease coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5952      +/-   ##
===============================================
- Coverage        66.46%   66.46%   -0.01%     
===============================================
  Files              284      284              
  Lines            13905    13907       +2     
===============================================
+ Hits              9242     9243       +1     
- Misses            4663     4664       +1
Impacted Files Coverage Δ
app/api/helpers/order.py 39.06% <33.33%> (+0.35%) ⬆️

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 eb349a6...57baefc. Read the comment docs.

@uds5501
Copy link
Contributor

uds5501 commented May 25, 2019

@mrsaicharan1 I will pull the changes on my local and suggest edits in a while

@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented May 25, 2019 via email

Copy link
Contributor

@uds5501 uds5501 left a comment

Choose a reason for hiding this comment

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

Try this on your local and post results, it seems to be working for me

@mrsaicharan1 mrsaicharan1 force-pushed the invoice-fix branch 2 times, most recently from 6d19142 to 3128633 Compare May 25, 2019 07:27
Copy link
Member

@kushthedude kushthedude left a comment

Choose a reason for hiding this comment

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

@mrsaicharan1 Some suggestions for invoice -:

  • The above two sub heading Order and order details can be replace with Order Details centre aligned .
  • Begin the alignment of Order Details from extreme right margin .
  • Try to have Margins for the tables and highlight the Order Details Headings as done in invoices generated by E-Com Sites .
  • Ensure the following order of details in the Order Details table -:
    • Ticket Type
    • Price
    • Quantity
    • Discount (If Exist)
    • Net Amount
    • Tax Rate
    • Tax Amount
    • Total Amount

@uds5501 @shreyanshdwivedi @iamareebjamal Your views upon the following suggestions

@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 3, 2019

@mrsaicharan1 Some suggestions for invoice -:

  • The above two sub heading Order and order details can be replace with Order Details centre aligned .

  • Begin the alignment of Order Details from extreme right margin .

  • Try to have Margins for the tables and highlight the Order Details Headings as done in invoices generated by E-Com Sites .

  • Ensure the following order of details in the Order Details table -:

    • Ticket Type
    • Price
    • Quantity
    • Discount (If Exist)
    • Net Amount
    • Tax Rate
    • Tax Amount
    • Total Amount

@uds5501 @shreyanshdwivedi @iamareebjamal Your views upon the following suggestions

Keeping the headings bold is a way of highlighting them. Even though I try to place the margins, they aren't showing up for some weird reason.
Apart from this, I'll take care of the rest.

@mrsaicharan1
Copy link
Member Author

@shreyanshdwivedi @uds5501 @iamareebjamal Please review. The order invoice looks like this now.
Screenshot 2019-06-03 at 1 10 02 AM

@uds5501
Copy link
Contributor

uds5501 commented Jun 3, 2019

@mrsaicharan1 how would it look without any billing information? in case the buyer chooses not to provide any billing info?

@mrsaicharan1
Copy link
Member Author

@mrsaicharan1 how would it look without any billing information? in case the buyer chooses not to provide any billing info?

Screenshot 2019-06-03 at 1 21 06 AM

@kushthedude
Copy link
Member

@mrsaicharan1 Can you please show an invoice with Discount Amount ?

@kushthedude
Copy link
Member

I still dont see Net Amount and Price of Ticket in invoice .

@mrsaicharan1
Copy link
Member Author

@kushthedude Please try to check clearly. Net amount is Sub-total(net). Price of ticket is price

uds5501
uds5501 previously approved these changes Jun 3, 2019
Copy link
Contributor

@uds5501 uds5501 left a comment

Choose a reason for hiding this comment

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

Please see the comment

@kushthedude
Copy link
Member

kushthedude commented Jun 3, 2019

@kushthedude Please try to check clearly. Net amount is Sub-total(net). Price of ticket is price

Can you shift that column to the last and shift tax columns before Net , It would look nice

Re-arranged the order items
@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 3, 2019

@shreyanshdwivedi @uds5501 Hound issues fixed. Please review. :)

@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 3, 2019

Screenshot 2019-06-03 at 1 31 04 AM

EventBrite follows this format. Therefore, I wouldn't want to change that.

@iamareebjamal iamareebjamal merged commit 1f3b50f into fossasia:development Jun 3, 2019
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.

Make Order Invoice similar to EventBrite
5 participants