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
[RFC] Direct/Inverse Exchange Rates #10075
Comments
This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions. |
@venyii I think this limitation is redundand.
And it will not introduce a BC break. |
This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions. |
Thanks for your patience! I'd go for dropping the unique constraint for inversed exchange rates. It should be possible to have both EUR/USD and USD/EUR exchange rates. To preserve backwards compatibility, when doing the conversion from EUR to USD, it should first look up for EUR/USD and then fallback to USD/EUR. I would be happy to merge a PR that does that and includes tests for the added behaviour. |
After reading the documentation and working on the code (fixture to create exchange rates automatically based on currencies), I'm not sure how to handle Exchange Rates. From my standpoint, I'd like to have full control over every exchange rate and their ratios. For example:
Currently this is limited to EUR->USD 1.14675 and Sylius won't allow USD->EUR to be specified (
The currency pair must be unique
), it will always calculate the inverse exchange rate to calculate the price USD -> EUR. From what I read this isn't always the case since the bank may offer different ratios for a currency relation.From a data schema point of view, it's not a problem to store (which is why I didn't see a problem in the beginning, since creating/inserting models works nicely), but this came up while I was testing my code and got a
More than one result was found for query although one row or none was expected.
inExchangeRateRepository::findOneWithCurrencyPair
.I found the PR (#6781) this was originally built (the way I imagined it), which was changed shortly after without (visible) discussion based on a single review comment #6781 (comment).
So my question is: Is the current solution how it was/is intended and everything else should be customized?
Based on the answer, we should either
UniqueCurrencyPairValidator
checks only source -> target and not inverseBut before I change anything, I'd like to hear your opinion on this.
The text was updated successfully, but these errors were encountered: