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.

Sign up for GitHub

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

Jump to bottom

New pdf importer for Sunrise #3731

Merged
merged 1 commit into from
Mar 22, 2024
Merged

New pdf importer for Sunrise #3731

merged 1 commit into from
Mar 22, 2024

Conversation

auchri
Copy link
Contributor

@auchri auchri commented Jan 13, 2024 •
edited
Loading

New pdf importer for https://www.meetsunrise.com/

@auchri auchri force-pushed the feature/simpel_fix branch from 82b0ac0 to 8b35272 Compare January 13, 2024 14:09
@Nirus2000
Copy link
Member

Hello @auchri
Thanks for this pull request.
I would redesign the whole thing as a section and change the transaction type based on the character (plus or minus).
As an example --> here <--

You can also take a look at the new variant of the test cases... --> here <--
These are more simply structured....

Regards
Alex

@auchri
Copy link
Contributor Author

auchri commented Jan 15, 2024

I have no time to adjust the whole importer and the tests to the new structure. Feel free to change it 😊

@Nirus2000 Nirus2000 added the pdf label Jan 15, 2024
@Nirus2000
Copy link
Member

Nirus2000 commented Jan 15, 2024 •
edited
Loading

I have no time to adjust the whole importer and the tests to the new structure. Feel free to change it 😊

That's not a problem... 😇

Just set the pull request to "draft", then we'll wait until you've found the time. 👍

@Nirus2000 Nirus2000 self-assigned this Jan 15, 2024
@buchen buchen marked this pull request as draft January 28, 2024 19:55
@auchri auchri force-pushed the feature/simpel_fix branch 2 times, most recently from ab362fa to 7a6bf9d Compare February 5, 2024 17:03
@auchri auchri changed the title Anpassung für Sunrise PDF Import nach Umbenennung und Änderungen New pdf importer for Sunrise Feb 5, 2024
@auchri auchri force-pushed the feature/simpel_fix branch 3 times, most recently from a5909b9 to ee1bcca Compare February 5, 2024 17:08
@auchri auchri marked this pull request as ready for review February 5, 2024 17:09
@auchri
Copy link
Contributor Author

auchri commented Feb 5, 2024

@buchen @Nirus2000 I've created a new importer for the "new" bank. Please have a look now.

@auchri auchri force-pushed the feature/simpel_fix branch from ee1bcca to 8535c6d Compare February 5, 2024 17:19
@MonkeySon
Copy link
Contributor

Hello,

I have a quick question about the structure of PDF importer, since I am currently also working on a new one.

Does the importer class (in that case SunrisePDFExtractor) need to be added to the PDFImportAssistent to be usable in PP?
(name.abuchen.portfolio\src\name\abuchen\portfolio\datatransfer\pdf\PDFImportAssistant.java)

My assumption was, that if it is not added, test cases would pass but PP actually would not recognize the PDF.

Thanks!

@auchri
Copy link
Contributor Author

auchri commented Mar 8, 2024 •
edited
Loading

Why is this importer not included in the new release? 😔 @buchen

@Nirus2000
Copy link
Member

Hello @auchri
thats right... @MonkeySon

Add in PDFImportAssistant.java...

@Nirus2000 Nirus2000 requested a review from buchen March 13, 2024 05:58
@Nirus2000 Nirus2000 merged commit f8253a7 into portfolio-performance:master Mar 22, 2024
2 checks passed
@Nirus2000 Nirus2000 mentioned this pull request Mar 22, 2024
Merged
Nirus2000 added a commit that referenced this pull request Mar 22, 2024
@Nirus2000
Add Sunrise PDF-Importer to import assistant
3c36388 
Issue: #3731
@Nirus2000
Copy link
Member

Hello @auchri
Thx for the changes... 👍🏻
I add the importer in the import assistent.

Rebase and merge

@Nirus2000 Nirus2000 mentioned this pull request Mar 27, 2024
Merged
Nirus2000 added a commit that referenced this pull request Mar 27, 2024
@Nirus2000
Fix and Modify Sunrise PDF-Importer
447ca01 
https://forum.portfolio-performance.info/t/pdf-import-von-sunrise/26633
https://forum.portfolio-performance.info/t/pdf-import-von-sunrise/26633/5

The Sunrise PDF importer was implemented in #3731.

If the dividends and the taxes neutralize each other, then negative dividends were previously posted (EUR 0.00 minus taxes). This is incorrect, as a negative dividend is then displayed under "Payments".
The transactions are now recorded separately in this case.

A test case for the sale of securities does not exist. This has therefore been removed from the importer. (Validation)

The test cases have been updated.

Format source to standard eclipse format

Improve regulare expressions
Add missing currency detection
Remove obsolet source
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nirus2000 Nirus2000 Nirus2000 left review comments

@buchen buchen Awaiting requested review from buchen

Assignees
No one assigned
Labels
pdf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@auchri @Nirus2000 @MonkeySon @buchen