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 support for purchase price/currency to portfolio #243

Merged
merged 15 commits into from
Oct 24, 2021

Conversation

lyricnz
Copy link
Collaborator

@lyricnz lyricnz commented Oct 20, 2021

Add support for declaring a BuyPrice and BuyCurrency in portfolio.
eg: ["Algorand", "125.4", "0.8", "USD"]

Add optional (default off) columns to portfolio:
"buy_price", "buy_currency", "profit", "profit_percent"

TODO:

  • currency conversion
  • (maybe) merge price/currency into one column
  • add to "cointop holdings" output
  • column sorting shortcuts
  • alerts based on profit
  • column formatting improvements?

eg: ["Algorand", "125.4", "0.8", "USD"]

Add optional (default off) columns to portfolio:
"buy_price", "buy_currency", "profit", "profit_percent"

TODO:
- currency conversion
- (maybe) merge price/currency into one column
- add to "cointop holdings" output
- column sorting shortcuts
@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

With the following config

[portfolio]
  columns = ["rank", "name", "symbol", "price", "holdings", "balance", "1h_change", "24h_change", "7d_change", "percent_holdings", "last_updated"]
  compact_notation = false
  holdings = [["Algorand", "100", "1.95", "USD"], ["Binance Coin", "3.2", "401.45", "USD"], ["Bitcoin", "0.1", "34849.83", "USD"]]

The following output
Screen Shot 2021-10-21 at 11 14 08 am

@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

Added "cost" and changed some of the formatting.

[portfolio]
  holdings = [["Algorand", "100", "1.95", "USD"], ["Binance Coin", "3.2", "401.45", "USD"], ["Bitcoin", "0.1", "34849.83", "USD"]]
  columns = ["rank", "name", "symbol", "price", "holdings", "balance", "buy_price", "buy_currency", "cost", "profit", "profit_percent"]

Screen Shot 2021-10-21 at 3 37 23 pm

@matthiasbeyer
Copy link

Nice! I Think in the next step it would also be interesting to add the date the purchase was made, so one can see the differences in performance over time - I don't think, though, that this should go in the same PR, should it? (Of course the maintainers have to decide!)

@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

I don't know how to make new edit fields, and in fact are in the middle of a radical overhaul of color/keybinding in another PR. Can you test this PR, see if it works for you? (or help?) There are still some todo's here, in particular currency conversion.

@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

  • currency conversion
  • (maybe) merge price/currency into one column
  • add to "cointop holdings" output ==> future PR
  • column sorting shortcuts
  • alerts based on profit ==> future PR
  • column formatting improvements

There's still an issue with changing currency conversion - it takes a while for the table to update, so the numbers are wrong for a bit.

@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

Priced in SATs

image

@lyricnz lyricnz linked an issue Oct 21, 2021 that may be closed by this pull request
@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

The slow-update is caused by #178 which has to do with updating all the pages of coins, one after another, before going back to the top. I think this PR is ready for review. Will raise feature-requests for pnl-alerts and pnl "cointop holdings" output.

@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

Renamed and combined buy-price and buy-currency to "cost price".

With the following portfolio (cost in diverse currencies) and column configuration, and the currency set to GBP (British Pound).

[portfolio]
  columns = ["rank", "name", "symbol", "price", "holdings", "balance", "cost_price", "cost", "profit", "profit_percent"]
  holdings = [["Algorand", "100", "1.95", "USD"], ["Binance Coin", "3.2", "401.45", "USD"], ["Bitcoin", "0.1", "50100.83", "AUD"]]

it looks like this:

image

@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 21, 2021

Oh! There's still no support for editing the cost price in the UI. I have no idea how to do this yet.

@lyricnz lyricnz marked this pull request as ready for review October 21, 2021 22:32
miguelmota added a commit that referenced this pull request Oct 23, 2021
@miguelmota
Copy link
Member

miguelmota commented Oct 23, 2021

For consistency, I renamed 'profit' column keys to 'pnl'. I also added the cost and pnl columns to the default list of column but they'll only render if there's at least one portfolio entry with a specified cost basis. fe94000

Overall it works great! nice job @lyricnz

@lyricnz lyricnz merged commit b5b6883 into cointop-sh:master Oct 24, 2021
@lyricnz lyricnz deleted the feature/portfolio-buy branch October 24, 2021 01:09
lyricnz pushed a commit that referenced this pull request Oct 24, 2021
@lyricnz
Copy link
Collaborator Author

lyricnz commented Oct 24, 2021

Oops, I didn't notice your commits weren't in this branch/PR, but were in another! I tried to update your branch from but got all kinds of conflicts, so I create a new branch and cherry-picked the two commits above. Hope this is OK! #250

@miguelmota
Copy link
Member

sorry about that! I didn't realize that it was in a different branch. The other PR looks good @lyricnz

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.

Cointop keeps overwriting my non-default column selection
3 participants