Skip to content

Conversation

@uds5501
Copy link
Contributor

@uds5501 uds5501 commented Mar 27, 2019

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)

Short description of what this resolves:

We can't use paymentCurrency of an event based on location because on a location, different events with different currencies maybe present and they can't be displayed under a single tab using a single payment Currency unless standardised.

Changes proposed in this pull request:

  • Use USD as default rendering currency

Fixes #2402

<td>{{entry.locationName}}</td>
<td class="right aligned">{{entry.sales.completed.ticket_count}}</td>
<td class="right aligned">{{currency-symbol entry.paymentCurrency}} {{number-format entry.sales.completed.sales_total}}</td>
<td class="right aligned">US$ {{number-format entry.sales.completed.sales_total}}</td>
Copy link
Member

Choose a reason for hiding this comment

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

This is not the valid fix @uds5501 as the line {{currency-symbol entry.paymentCurrency}} was written to correctly render the currency parser and by setting it to default 'US $' , You are not making valid fix.

Copy link
Contributor Author

@uds5501 uds5501 Mar 27, 2019

Choose a reason for hiding this comment

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

@kushthedude It actually is a valid fix . Did you read the pertaining argument in the issue itself?
The location part of the admin/sales is not yet ready for event based currency parsing.

At a location ABC suppose there are 3 events. 2 of them use USD and one of them uses CAD. What would you show then, USD or CAD? :)

Copy link
Member

Choose a reason for hiding this comment

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

Thats what i was saying , How did you came to know location part cant render event bases currency parsing?

Copy link
Member

Choose a reason for hiding this comment

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

How can i make them ready for currency parsing?
@uds5501

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kushthedude Regarding that, I have proposed #2253 to be worked during GSoC period. I will standardise the amount of other currency into USD and that should do it!

@abhinavk96 abhinavk96 merged commit 74dc037 into fossasia:development Apr 1, 2019
@kushthedude
Copy link
Member

@CosmicCoder96 shouldnt this supposed to be in #2376

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admin/sales statistics not being rendered

3 participants