fix: Show "Free" instead of $0.00 in ticket price #5369

sachinchauhan2889 · 2020-10-23T18:11:01Z

Fixes #5364

Short description of what this resolves:

In the column "Price" and "Subtotal" for free tickets show "Free" instead of $0.00.

Changes proposed in this pull request:

-In the column "Price" and "Subtotal" for free tickets show "Free" instead of $0.00.

Checklist

I have read the Contribution & Best practices Guide.
My branch is up-to-date with the Upstream development branch.
The acceptance, integration, unit tests and linter 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)

screenshots
before

after

vercel · 2020-10-23T18:11:07Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/p8leld8kn
✅ Preview: https://open-event-frontend-git-fix-5364.eventyay.vercel.app

codecov · 2020-10-23T18:24:39Z

Codecov Report

Merging #5369 into development will decrease coverage by 0.05%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           development    #5369      +/-   ##
===============================================
- Coverage        22.78%   22.72%   -0.06%     
===============================================
  Files              491      491              
  Lines             5245     5245              
  Branches            37       37              
===============================================
- Hits              1195     1192       -3     
- Misses            4045     4048       +3     
  Partials             5        5

Impacted Files	Coverage Δ
app/models/event.js	`31.25% <0.00%> (-18.75%)`	⬇️

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 bbdbe76...e2d14b9. Read the comment docs.

mariobehling

Thank you. Could you also make this work in the same way once the user presses "Order" and show "Free" on the next order page, please?

sachinchauhan2889 · 2020-10-24T13:13:26Z

@iamareebjamal @mariobehling implemented suggested changes.

snitin315

Good Job!

iamareebjamal · 2020-10-24T13:19:57Z

app/templates/components/orders/order-summary.hbs

+                    {{t "Free"}}
+                  {{else}}
+                    {{currency-symbol this.eventCurrency}} {{format-money ticket.price}}
+                  {{/if}}


Make this an HTML only component to avoid repetition. Just move it into its own HTML file and replace ticket.type and ticket.price to @ticket.type and vice versa. Then use that component and pass ticket like @ticket={{ticket}}

Name the component ticket-price.hbs

iamareebjamal · 2020-10-25T22:25:59Z

@sachinchauhan2889 Status?

sachinchauhan2889 · 2020-10-26T04:57:19Z

@sachinchauhan2889 Status?

@iamareebjamal sir i will implement your suggested changes by tomorrow morning. Sorry for delay sir i was busy in my exam. I will surly commit changes suggested by you by tomorrow morning.

iamareebjamal · 2020-10-26T05:48:20Z

No issues

sachinchauhan2889 · 2020-10-27T09:15:50Z

@iamareebjamal sir i have created a new component for solving this issue(as suggested by you).

iamareebjamal · 2020-10-27T09:16:37Z

app/components/orders/ticket-price.js

+import Component from '@ember/component';
+
+@classic
+export default class TicketPrice extends Component {}


No need of this file, please remove it

iamareebjamal · 2020-10-27T09:17:27Z

app/templates/components/orders/order-summary.hbs

+                <Orders::TicketPrice
+                  @ticket-type={{ticket.type}}
+                  @display-symbol={{currency-symbol this.eventCurrency}}
+                  @display-price={{format-money ticket.price}}


Simply pass the ticket. What's the point of creating a component if you'll be doing all calculations here

iamareebjamal · 2020-10-27T09:18:51Z

I have reviewed again. Please read my previous review again and what it is asking to do. You have used a significantly more complex solution

… into fix-5364

sachinchauhan2889 · 2020-11-01T10:10:17Z

@iamareebjamal sir, have a look at this. i have done calculations inside ticket-price.hbs. i passesd ticket as @ticket={{ticket}}.
For accessing eventCurrency and attendees inside ticket-price.hbs i have also passed them as parameter.
I have also passed a parameter named display which will decide what to display if ticket is not free.

iamareebjamal · 2020-11-01T10:11:24Z

app/templates/components/orders/ticket-price.hbs

+        {{currency-symbol @eventCurrency}} {{format-money @ticket.subTotal}}
+    {{else if (eq @display 'order-subTotal')}}
+        {{currency-symbol @eventCurrency}} {{format-money (mult (sub @ticket.price @ticket.discount) (ticket-attendees @attendees @ticket.attendees))}}
+    {{/if}}


Why are you making it so so difficult? You just have to check the type and show either free or passed value. Pass the amount as an argument and don't do these complex calculations here. I don't know why everyone likes so much complicated solutions. Passing attendees and multiplying and switch casing. I don't know what to say except changing it myself will take much much less time than reviewing this

@iamareebjamal sorry sir.

iamareebjamal · 2020-11-01T10:19:59Z

Please see the changes now

sachinchauhan2889 · 2020-11-01T10:30:09Z

@iamareebjamal sir, there is a misunderstanding between us. please look at my previous commits. i have given this solution. but u told me to do calculations inside ticket-price.hbs.
i only need to pass ticket as @ticket={{ticket}}.

iamareebjamal · 2020-11-01T10:37:08Z

Because you were formatting money and doing currency conversions here, which can be moved in the component. See the end result. Why would I ask you to write more code than you had written? Is the end result shorter than your version or not?

sachinchauhan2889 · 2020-11-01T10:54:52Z

Because you were formatting money and doing currency conversions here, which can be moved in the component. See the end result. Why would I ask you to write more code than you had written? Is the end result shorter than your version or not?

@iamareebjamal sir, yes you are right. I will take care of this from now.
now this is able to merge.
i have remove (or @amount @ticket.price) this because in subtotal column initially subtotal is 0 and price is non-zero. so initially subtotal column is showing ticket-price instead of $0.00

iamareebjamal · 2020-11-01T11:10:55Z

i have remove (or @amount @ticket.price) this because in subtotal column initially subtotal is 0 and price is non-zero. so initially subtotal column is showing ticket-price instead of $0.00

OK, thank you

fix-5364

1fb0ffe

vercel bot deployed to Preview October 23, 2020 18:11 View deployment

auto-label bot added the fix label Oct 23, 2020

mariobehling requested changes Oct 24, 2020

View reviewed changes

free-at-order-summary

0157cea

vercel bot deployed to Preview October 24, 2020 13:12 View deployment

sachinchauhan2889 requested a review from mariobehling October 24, 2020 13:13

snitin315 previously approved these changes Oct 24, 2020

View reviewed changes

iamareebjamal reviewed Oct 24, 2020

View reviewed changes

Abhijay007 mentioned this pull request Oct 24, 2020

fix #5364 Public event page and order summary: Show "Free" instead of $0.00 #5370

Closed

5 tasks

iamareebjamal mentioned this pull request Oct 25, 2020

Public Event Page: Free Tickets - Show "Free" instead of $0.00 #5364 #5381

Closed

5 tasks

add-new-component

e2d14b9

sachinchauhan2889 dismissed snitin315’s stale review via e2d14b9 October 27, 2020 09:09

vercel bot deployed to Preview October 27, 2020 09:09 View deployment

iamareebjamal reviewed Oct 27, 2020

View reviewed changes

Merge branch 'development' of github.com:fossasia/open-event-frontend…

52fedb6

… into fix-5364

vercel bot deployed to Preview November 1, 2020 10:03 View deployment

update-price.hbs

2ac2320

vercel bot deployed to Preview November 1, 2020 10:08 View deployment

iamareebjamal reviewed Nov 1, 2020

View reviewed changes

fix

bb2ee2e

vercel bot deployed to Preview November 1, 2020 10:19 View deployment

final

18a1215

vercel bot deployed to Preview November 1, 2020 10:50 View deployment

iamareebjamal force-pushed the fix-5364 branch from 18a1215 to bb2ee2e Compare November 1, 2020 10:54

vercel bot deployed to Preview November 1, 2020 10:55 View deployment

sachinchauhan2889 force-pushed the fix-5364 branch from c2e4472 to 18a1215 Compare November 1, 2020 10:58

test

56f61fb

vercel bot deployed to Preview November 1, 2020 11:09 View deployment

iamareebjamal changed the title ~~fix: Public Event Page: Free Tickets - Show "Free" instead of $0.00~~ fix: Show "Free" instead of $0.00 in ticket price Nov 1, 2020

iamareebjamal approved these changes Nov 1, 2020

View reviewed changes

snitin315 approved these changes Nov 1, 2020

View reviewed changes

iamareebjamal merged commit 0f07185 into fossasia:development Nov 1, 2020

sachinchauhan2889 deleted the fix-5364 branch November 3, 2020 03:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Show "Free" instead of $0.00 in ticket price #5369

fix: Show "Free" instead of $0.00 in ticket price #5369

sachinchauhan2889 commented Oct 23, 2020

vercel bot commented Oct 23, 2020 •

edited

Loading

codecov bot commented Oct 23, 2020 •

edited

Loading

mariobehling left a comment

sachinchauhan2889 commented Oct 24, 2020

snitin315 left a comment

iamareebjamal Oct 24, 2020

iamareebjamal Oct 24, 2020

iamareebjamal commented Oct 25, 2020

sachinchauhan2889 commented Oct 26, 2020

iamareebjamal commented Oct 26, 2020

sachinchauhan2889 commented Oct 27, 2020

iamareebjamal Oct 27, 2020

iamareebjamal Oct 27, 2020

iamareebjamal commented Oct 27, 2020

sachinchauhan2889 commented Nov 1, 2020 •

edited

Loading

iamareebjamal Nov 1, 2020 •

edited

Loading

sachinchauhan2889 Nov 1, 2020

iamareebjamal commented Nov 1, 2020

sachinchauhan2889 commented Nov 1, 2020

iamareebjamal commented Nov 1, 2020

sachinchauhan2889 commented Nov 1, 2020

iamareebjamal commented Nov 1, 2020

fix: Show "Free" instead of $0.00 in ticket price #5369

fix: Show "Free" instead of $0.00 in ticket price #5369

Conversation

sachinchauhan2889 commented Oct 23, 2020

Short description of what this resolves:

Changes proposed in this pull request:

Checklist

vercel bot commented Oct 23, 2020 • edited Loading

codecov bot commented Oct 23, 2020 • edited Loading

Codecov Report

mariobehling left a comment

Choose a reason for hiding this comment

sachinchauhan2889 commented Oct 24, 2020

snitin315 left a comment

Choose a reason for hiding this comment

iamareebjamal Oct 24, 2020

Choose a reason for hiding this comment

iamareebjamal Oct 24, 2020

Choose a reason for hiding this comment

iamareebjamal commented Oct 25, 2020

sachinchauhan2889 commented Oct 26, 2020

iamareebjamal commented Oct 26, 2020

sachinchauhan2889 commented Oct 27, 2020

iamareebjamal Oct 27, 2020

Choose a reason for hiding this comment

iamareebjamal Oct 27, 2020

Choose a reason for hiding this comment

iamareebjamal commented Oct 27, 2020

sachinchauhan2889 commented Nov 1, 2020 • edited Loading

iamareebjamal Nov 1, 2020 • edited Loading

Choose a reason for hiding this comment

sachinchauhan2889 Nov 1, 2020

Choose a reason for hiding this comment

iamareebjamal commented Nov 1, 2020

sachinchauhan2889 commented Nov 1, 2020

iamareebjamal commented Nov 1, 2020

sachinchauhan2889 commented Nov 1, 2020

iamareebjamal commented Nov 1, 2020

vercel bot commented Oct 23, 2020 •

edited

Loading

codecov bot commented Oct 23, 2020 •

edited

Loading

sachinchauhan2889 commented Nov 1, 2020 •

edited

Loading

iamareebjamal Nov 1, 2020 •

edited

Loading