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

Add currency symbol for displayPrice product property #6162

Closed
3 tasks done
willopez opened this issue Mar 27, 2020 · 3 comments
Closed
3 tasks done

Add currency symbol for displayPrice product property #6162

willopez opened this issue Mar 27, 2020 · 3 comments
Labels
core work For issues that track feature development work being done by core Reaction developers discussion For issues in which discussion is expected but no further action is needed priority low

Comments

@willopez
Copy link
Member

willopez commented Mar 27, 2020

Prerequisites

  • Are you running the latest version?
  • Are you able to consistently reproduce the issue?
  • Did you search the issue queue for existing issue? Search issues

Issue Description

The pricing prop in products/variants has a property displayPrice that is missing the currency symbol.

Possible Solution

Add current shop's settings to app context, which would include the shop's currency. With currency information available in the app context, it would be trivial to add the currency symbol the displayProp

@willopez willopez added discussion For issues in which discussion is expected but no further action is needed needs triage For issues that are awaiting triage by the core development team labels Mar 27, 2020
@aldeed
Copy link
Contributor

aldeed commented Mar 27, 2020

@willopez There is no such thing as a "current" shop. Shop always needs to be looked up based on the data in question. So for product pricing it would be something like.

import getCurrencyDefinitionByCode from "@reactioncommerce/api-utils/getCurrencyDefinitionByCode.js";

const shop = await Shops.findOne({ _id: productOrVariant.shopId }, { projection: { currency: 1 } });
const currencyDefinition = getCurrencyDefinitionByCode(shop.currency);
const displayPrice = getDisplayPrice(minPrice, maxPrice, currencyDefinition);

@aldeed aldeed added core work For issues that track feature development work being done by core Reaction developers and removed needs triage For issues that are awaiting triage by the core development team labels Mar 27, 2020
@mikemurray
Copy link
Member

Getting the currency was added in this PR: #6159 but, I think it could be improved further by, either...

  • checking if displayPrice was requested then calculating it, only if it was
  • or, maybe by adding / update a separate resolver for display price to query when needed

@aldeed
Copy link
Contributor

aldeed commented Mar 27, 2020

Agree. What I would really prefer is to store all of the pricing data, including displayPrice, in the database whenever the price is changed. Then nothing needs to be transformed at read time. But this would require a large initial migration plus a way to schedule a migration whenever someone changes the shop currency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core work For issues that track feature development work being done by core Reaction developers discussion For issues in which discussion is expected but no further action is needed priority low
Projects
None yet
Development

No branches or pull requests

4 participants