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

Added fees to account transfer dialog #3424

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RomanLangrehr
Copy link
Contributor

Let the user add fees for account transfers:
image

@buchen
Copy link
Member

buchen commented Jun 25, 2023

Does it really have no impact on client filters, reports on fees, etc? I haven't had the time to look into the details, but I pretty much expect that the newly introduced fees require changes in the code that reports, filters, and selects on fees.

@Nirus2000
Copy link
Member

Nirus2000 commented Jun 28, 2023

I think that definitely has an impact on the filters and so on. (See #3272)

Fees could also be specified in foreign currency.

Furthermore, we had also talked in a meeting that we wanted to include rebookings via importer with this problem.

@RomanLangrehr
Can you see if you could also integrate the test for the UI here?
As an example for the tests, you could take a few here and include them.
(#3370)

@RomanLangrehr
Copy link
Contributor Author

@buchen @Nirus2000 I missed that I have to make changes to the ClientPerformanceSnapshot to take into account the fees in transactions (for the calculation of fees and for the calculation of foreign currency gains/losses). I fixed that now. But I don't think any of the client filters need adaption (as far as I know there is no filter working based on fees).

I will add more unit tests (for the AccountTransferModel and the ClientPerformanceSnapshot ) when I have time

The motivation for me to add this feature was also to import automtically forex transactions in the future (for example from Interactive Brokers) and they typically include fees.

@Nirus2000
Copy link
Member

Hello @RomanLangrehr

The motivation for me to add this feature was also to import automtically forex transactions in the future (for example from Interactive Brokers) and they typically include fees.

Yes, that's right.
In the IB-Flex-Query Importer there are already test files with these transactions, which could not be processed yet, because this function does not exist yet. (Example testIBFlexStatementFile19.xml)
So... could you see if you can integrate this part or if we should implement it in a separate PR?

@chirlu
Copy link
Member

chirlu commented Nov 17, 2023

I’m very suspicious of this introducing unintended side effects.

Furthermore, UI-wise, how would the user distinguish between the two “fee” fields? If the accounts are of different currencies, OK, you can guess it from the currency; but for transfers without currency exchange?

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.

4 participants