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

[Bug]: getLowestRate() doesn't account for difference currencies #421

Closed
pauldraper opened this issue Nov 14, 2023 · 3 comments
Closed

[Bug]: getLowestRate() doesn't account for difference currencies #421

pauldraper opened this issue Nov 14, 2023 · 3 comments
Labels
triage Issue is being researched

Comments

@pauldraper
Copy link

pauldraper commented Nov 14, 2023

Software Version

Language Version

Operating System

What happened?

getLowestRate() gives a bad result with different currencies, e.g. mixing CAD and USD sometimes chooses a numerically lower USD even if CAD quote is better.

I can't imagine any case where this is desirable behavior.

getLowestRate(rates, carriers = null, services = null) {

What was expected?

Not sure, but the existence of getLowestRate() is a footgun.

Sample Code

No response

Relevant logs

No response

@pauldraper pauldraper added the triage Issue is being researched label Nov 14, 2023
@nwithan8
Copy link
Member

nwithan8 commented Nov 14, 2023

Hello, thank you for reaching out! We are triaging the issue currently. Do you perhaps have any logs you could share to help us reproduce the issue, specifically how you retrieved a combination of USD and non-USD rates?

@Harrison-Burgess
Copy link

Harrison-Burgess commented Nov 14, 2023

Hey, what I did to produce this was creating a shipment
const shipmentData = { to_address: toAddress, from_address: fromAddress, parcel: await easypost.Parcel.create({ weight: getLineItemWeight(productWithQty), }), options: { currency: "USD", }, mode, };

shipment = await easypost.Shipment.create(shipmentData);
Where the addresses were from 2 different countries
This requested rates where we preferred them in USD. Some of the carriers didn't seem to work for the currency option though and still gave us back rates in the local currency.
const lowestRate = shipment.lowestRate([ "UPS", "CanadaPost", "FedExDefault", "USPSReturns", ]);

@Justintime50
Copy link
Member

Hey @Harrison-Burgess, thanks for providing additional details. This particular issue sounds like it could be better solved at the API level as this client library's function behaved correctly based on its intended use-case - it can only determine the lowest rate with good data. With that said, you can reach our support team at [email protected] and describe the perceived bug to them. They'll work with our internal teams if possible to correct this so the carrier (if possible) can return currencies in the requested type. I will note that per our documentation, the currency option does state that only carriers that allow conversion will respect the shipment option of defining an explicit currency, if I had to guess, I assume CanadaPost does not at present, though it's possible that could change if requested.

Screenshot 2024-08-27 at 9 51 36 AM

As this issue happens at the API level, I'm going to close this issue since we do not intend to support currency conversion directly in our client libraries.

@Justintime50 Justintime50 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Issue is being researched
Projects
None yet
Development

No branches or pull requests

4 participants