Allow user to select an explicit currency for dividends

[chrisaut]

Allows the user to select a different currency for dividend payments, different from either security currency and/or account currency.

Example use case: US stocks bought (and tracked) in EUR at a German exchange, but Dividend gets paid in USD.

I'm not sure if this is done yet, putting it up in the hope for a quick review if the approach is acceptable.

Some Forum discussion

[chirlu]

The custom drop-down menu doesn’t fit on my screen, and there is no way to scroll. If it is to stay at this place, it should be an actual drop-down (showing only the ISO codes), like when changing the currency of an account. But I would prefer a separate line with a label like “payment currency” or “dividend currency”. This would also provide enough space for a full currency drop-down, like in the dialogue for editing a security.

Another remark: There are lots of lines with dangling spaces or tabs at the end.

[chrisaut]

The custom drop-down menu doesn’t fit on my screen, and there is no way to scroll. If it is to stay at this place, it should be an actual drop-down (showing only the ISO codes), like when changing the currency of an account.

Well it's the same dropdown as under statement of assets. I don't particularly like it either (see #1876 ) but I still think it's better then the currency selection dropdown style IMO because at least it has the option of seperators for previously used currencies. I guess we could extend the other dropdowns with that too though. If other's disagree I can change it though, I'm not stuck on it.

I assume you mean this style?

There is also this currency selection:

I kinda think we should unify currency selection though, create a reusable component that looks the same everywhere and is easier to use. Try switching from EUR to USD quickly in the dropdown under accounts for example, it's a usability nightmare. Don't think this should be part of this though.

But I would prefer a separate line with a label like “payment currency” or “dividend currency”. This would also provide enough space for a full currency drop-down, like in the dialogue for editing a security.

I can put it on a new line, just thought it would waste space. Do others agree?

Another remark: There are lots of lines with dangling spaces or tabs at the end.

Yeah good point, will clean it up with next commit.

[buchen]

I would also propose to separate the two things:

• First: allow to record dividends in a currency other than the security currency.
• Second: have a better usable drop-down for selecting currencies

I propose to make this PR about the first topic.

Allowing other currencies makes sense. Particularly because the currency determines both: the currency of the security prices downloaded and the currency of the dividend statements. But one wants to be able to separates this: trade a security on a EUR exchange and still receive USD dividends.

The change of the UI, however, is only one part. I do not know for sure, but so far in many calculations I assume that the currency fits to the currency of the security. To merge the change, I would like to see a test case (similar to what you find in the n.a.p.tests/src/scenarios folder) that tests ClientSnapshot, ClientPerformanceSnapshot, and SecurityPerformanceSnapshot procure the results you expect.

I can put it on a new line, just thought it would waste space. Do others agree?

I think most of the time users will not touch the currency. I kind of like it right next to the "per share" item.

[chirlu]

Well it's the same dropdown as under statement of assets.

Oh, indeed, that one is unusable in the same way. :-) (But I never use it, so I never noticed.) I‘ve just created issue #1920 for that.

one wants to be able to separates this: trade a security on a EUR exchange and still receive USD dividends.

As an aside, I own some shares that used to pay dividends in DEM once upon a time and now pay in EUR. So that is a use case, too.

[chrisaut]

I would like to see a test case (similar to what you find in the n.a.p.tests/src/scenarios folder) that tests ClientSnapshot, ClientPerformanceSnapshot, and SecurityPerformanceSnapshot procure the results you expect.

Ok good, I will add tests and see if things break

I can put it on a new line, just thought it would waste space. Do others agree?

I think most of the time users will not touch the currency. I kind of like it right next to the "per share" item.

Ok, let's leave it where it is then

[tomasherman]

Hey everyone, i was wondering it this PR is going to be worked on further or merged :) Maybe one thing to consider would be adding Dividend currency as a paramter of Security (with default set to currency used to denominate the currency). So that the user can set it once when creating the security and then the dialog pics the current currency. What do you think?

[chrisaut]

I do still want this feature, very much so in fact, nearly all my dividends come from US but paid in EUR. If I remember correctly I got a bit stuck trying to setup a test context that would be able to exercise all the new cases. Then I unfortunately got sidetracked and busy. From the manual testing I had done it did seem to work though.

So, perhaps someone can take this as a starting point, or inspiration and finish it. If not I may pick it up sometime over the summer, but I just can't promise when. I can close the PR if you prefer.

[tomasherman]

i wanted to pick this up but having a really rough time with eclipse (intellij guy here :D) ... it keeps crashing on me, im not sure i will be able to get at it properly, it drives me crazy

[bbatsov]

Is the project built using Eclipse RCP? Haven't done any Java in ages, but I remember this was the last reason I had to use Eclipse for anything. :D

[ah4c]

I was curious so I copied the branch from chrisaut's repo and rebased it on buchen's master. There was one tiny merge conflict that was easy to resolve. I am not sure that I have a use case for this as I prefer to import my transactions from the bank's PDFs.

If someone wants to pick this up but is worried about rebasing over so many commits then you could take my branch. https://github.com/ah4c/portfolio/tree/feat/explicitDividendCurrency

git remote add ah4c [email protected]:ah4c/portfolio.git
git fetch ah4c
git checkout ah4c/feat/explicitDividendCurrency