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

Allow to select an existing security to replace a newly imported one #3494

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

buchen
Copy link
Member

@buchen buchen commented Aug 9, 2023

When reviewing the newly created items, the user can choose to replace a new security with an existing security. All transactions are then imported and added to the existing security.

  • Extended ExtractedEntry with security dependency (= if an extracted entry depends on another extracted security) and security override (= if the user chose to replace the imported security with an existing security)
  • Refactored the import logic into ImportController to allow sharing between PDF and CSV imports
Bildschirmaufnahme.2023-08-09.um.16.41.28.mp4

When reviewing the newly created items, the user can choose to replace
a new security with an existing security. All transactions are then
imported and added to the existing security.

* Extended ExtractedEntry with security dependency (= if an extracted
  entry depends on another extracted security) and security override
  (= if the user chose to replace the imported security with an existing
  security)
* Refactored the import logic into ImportController to allow sharing
  between PDF and CSV imports
Comment on lines 62 to +64
return maxCode == Status.Code.OK
|| (maxCode == Status.Code.WARNING && isImported != null && isImported.booleanValue()
|| (maxCode == Status.Code.WARNING && item.isInvestmentPlanItem()));
|| (maxCode == Status.Code.WARNING && item.isInvestmentPlanItem()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that way better for understanding the code?

    // Import if the status is OK or the user explicitly overrides warnings
    if (maxCode == Status.Code.OK) {
        return true;
    }

    // Import if warnings are explicitly allowed or if it's an investment plan item
    return maxCode == Status.Code.WARNING && (isImported != null && isImported.booleanValue()
            || item.isInvestmentPlanItem());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was only a formatting change - maybe I should remove even the formatting from the commit.

I am also not sure if the statements are identical due to the missing parenthesis.

@Nirus2000
Copy link
Member

This is very nice... i ❤️‍🔥 it...

buchen and others added 2 commits August 9, 2023 19:45
…s/datatransfer/ImportController.java

Co-authored-by: Nirus2000 <45203494+Nirus2000@users.noreply.github.com>
…es.properties

Co-authored-by: Nirus2000 <45203494+Nirus2000@users.noreply.github.com>
@buchen buchen merged commit a7446a8 into master Aug 9, 2023
2 checks passed
@buchen buchen deleted the feature_pick_existing_security_during_import branch August 10, 2023 09:32
@siprbaum
Copy link

On a technical level, I understand what the feature can do, but I am struggling a little for which use cases I would use it.

Would this be helpful e.g. when a security was merged/replaced by another?
E.g. in the past, the Lyxor MSCI World (WKN: ETF110) was merged/replaced by Amundi MSCI World (WKN: ETF018).
The bank automatically exchanged the existing ETF110 in the depot with Amundi ETF018 shares.

Any pointer to existing uses cases/documentation when to use this feature would be appreciated.

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

Successfully merging this pull request may close these issues.

3 participants