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

Feature rework comdirect #3577

Closed
wants to merge 2 commits into from
Closed

Feature rework comdirect #3577

wants to merge 2 commits into from

Conversation

buchen
Copy link
Member

@buchen buchen commented Oct 2, 2023

No description provided.

Nirus2000 and others added 2 commits October 2, 2023 09:37
Closes #3553
Closes #3313
Closes #1131

https://forum.portfolio-performance.info/t/pdf-import-von-comdirect/1647/229

https://forum.portfolio-performance.info/t/pdf-import-von-comdirect/1647/236

https://forum.portfolio-performance.info/t/pdf-import-von-comdirect/1647/242

https://forum.portfolio-performance.info/t/pdf-import-von-comdirect/1647/254

https://forum.portfolio-performance.info/t/pdf-import-von-comdirect/1647/270

https://forum.portfolio-performance.info/t/pdf-import-von-comdirect/1647/276

https://forum.portfolio-performance.info/t/pdf-import-von-comdirect/1647/283

ComDirect provides two documents for the transaction.
The security transaction and the taxes treatment.
Both documents are provided as one PDF or as two PDFs.

The security transaction includes the fees, but not the correct taxes
and the taxes treatment includes all taxes (including withholding tax),
but not all fees.

Therefore, we use the documents based on their function and merge both documents,
if possible, as one transaction.

Always import the securities transaction and the taxes treatment for a correct transaction.
Due to rounding differences, the correct gross amount is not always shown in the securities transaction.
@buchen
Copy link
Member Author

buchen commented Oct 2, 2023

Hi @nirus,

please have a look at this commit: 4c23cef

It is a commit on top of your pull request #3555.

It addresses the issue that the exchange rate is stored permanently in the security. For one, this is polluting the XML. Furthermore, the current algorithm cannot handle if one import contains multiple such documents (as they override the exchange rate).

This change does the following:

  • it removes the handling of the exchange rate from parsing the transactions
  • instead it uses the postProcessing method to retrieve the exchange rate from the sale and attach it to the taxes

The only change in the test is that "failed" transactions (the ones that will never be imported), do not have a grossValue of 0 anymore.

As you can see, I added a method #matchSaleAndTaxTransactions which creates a list of pairs of sale and tax transaction. The same list could later be used to merge the sale and tax where applicable. I changed it so that I have directly already typed objects.

@buchen buchen requested a review from Nirus2000 October 2, 2023 09:13
@Nirus2000
Copy link
Member

Hallo @buchen
Yesterday I had to sit down at Thalia with a coffee...
Well, the weather was nice and I drank a coffee to understand the "Warum & Wieso" and how does that work?.
🤯 🤯 🤯

I modified your source code a bit so that I (sorry) understood it better when reading and added it to my pull request.
I added you as a co-author 🤪
698d6f3

As a side note:
If I now, for example, For example, if I looked at the Targobank importer, then I would have assumed that I could also use this method to solve the problem with the pay_day - but that's not the case, is it?

Thank you
Alex

@buchen
Copy link
Member Author

buchen commented Oct 4, 2023

If I now, for example, For example, if I looked at the Targobank importer, then I would have assumed that I could also use this method to solve the problem with the pay_day - but that's not the case, is it?

Yeah. I justed looked at the Targobank importer. The post processing looks very similar

@buchen
Copy link
Member Author

buchen commented Oct 4, 2023

Closing because it is now included in your pull request

@buchen buchen closed this Oct 4, 2023
@buchen buchen deleted the feature_rework_comdirect branch October 4, 2023 12:21
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.

2 participants