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

Incorrect account preselection for CSV import #4325

Open
verglor opened this issue Nov 2, 2024 · 2 comments
Open

Incorrect account preselection for CSV import #4325

verglor opened this issue Nov 2, 2024 · 2 comments
Labels

Comments

@verglor
Copy link
Contributor

verglor commented Nov 2, 2024

Describe the bug
For CSV imports accounts are pre-selected according to the last used account for any other CSV import so they are shared between imports from different banks/brokers. This may cause hard to recover mistakes of importing transactions to incorrect accounts.

Forum: https://forum.portfolio-performance.info/t/csv-import-improvements/29924

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'File > Import > CSV files' (or select CSV template for 'Broker A')
  2. Select CSV file from 'Broker A'
  3. Adjust CSV configuration and column mappings
  4. Click on 'Next >'
  5. Select desired cash and securities accounts (e.g. 'Broker A')
  6. Click on 'Finish'
  7. Go to 'File > Import > CSV files' (or select CSV template for 'Broker B')
  8. Select CSV file from 'Broker B'
  9. Adjust CSV configuration and column mappings
  10. Click on 'Next >'
  11. Cash and securities accounts are pre-selected to 'Broker A'

Expected behavior
The target accounts should not be pre-selected to 'Broker A' if I'm importing transactions for 'Broker B'.

Desktop

  • OS: Windows
  • Version: 0.71.2

Details
There was Remember the last selected account/portfolio by the type of PDF document implemented which does not fit well for generic CSV imports.
I tried to disable pre-selection according to extractor in ReviewExtractedItemsPage.preselectDropDowns() but instead of CSVExtractor the private proxy is used.

Example of stored accounts:

import-target-account-Account\ Transactions=2bea6733-fe86-4e70-a0d2-507df4168945
import-target-account-Portfolio\ Transactions=2bea6733-fe86-4e70-a0d2-507df4168945
import-target-account-Trade\ Republic\ Bank\ GmbH=2bea6733-fe86-4e70-a0d2-507df4168945

The first two are from CSV extrators that are generic (only "Type of data" is used as discriminator) and the last one is from PDF extractor that is bank/broker specific.

Suggested solution

  1. We should make account pre-selection feature optional (configurable) since even if it works correctly it may not be desirable if someone has multiple accounts for the same bank/broker.
  2. We should prevent account pre-selection for generic CSV import.
  3. (Optionally) When CSV template is used we should store and pre-select accounts separately for each template (adding template label to preferences key)
@verglor verglor added the bug label Nov 2, 2024
@pfalcon
Copy link
Contributor

pfalcon commented Nov 2, 2024

I wouldn't call this "bug". But it definitely would be a nice feature if PP remembered account pair selected per CSV import template (because such a template would usually correspond to a particular broker), instead of just remembering the last pair used.

I don't think that removing remembering of the last used account pair (and starting from empty selection, thus forcing user to make selections on each import) is the right "fix", because most users don't have more than 1 account, and for them forcing repetitive selection would be significant drop in UX. Likewise, I don't think that making it configurable makes sense - PP already has too many configuration knobs, and most users don't know what's configurable and what's not and where a particular config option is located.

@verglor
Copy link
Contributor Author

verglor commented Nov 2, 2024

Since suggestion 3 (pre-seleciton according to csv tempate) would be nice to have I think that suggestion 2 (prevent pre-selection for generic CSV import) is quite needed to prevent mistakes - this should not impact how it works for PDF imports though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants