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 some new documents - trade republic #3807

Conversation

christen90
Copy link
Contributor

add US Quellensteuer
add Steuerlicher Umtausch
add Spin-Off

add Steuerlicher Umtausch
add Spin-Off
@christen90 christen90 changed the title add some new documents add some new documents - trade republic Feb 21, 2024
@Nirus2000 Nirus2000 added the pdf label Feb 22, 2024
Copy link
Member

@Nirus2000 Nirus2000 left a comment

Choose a reason for hiding this comment

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

Hello @christen90
thank you very much for this pull request.
I have looked at it and suggested some minor changes and corrections.
All in all it looks good.

What do you think? Could you take a look and check if necessary?
I would also be happy if you could include this PDF debug.
Would you have time for this?
https://forum.portfolio-performance.info/t/pdf-import-von-trade-republic/5107/486

Greetings
Alex

Comment on lines 1117 to 1119
.wrap(t -> {
return new TransactionItem(t);
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.wrap(t -> {
return new TransactionItem(t);
});
.wrap(TransactionItem::new);

{
DocumentType type = new DocumentType("(STEUERLICHER UMTAUSCH)", (context, lines) -> {
Pattern pDate = Pattern.compile("^(.*) DATUM (?<date>[\\d]{2}\\.[\\d]{2}\\.[\\d]{4})$");
context.put("transactionPosition", "0");
Copy link
Member

Choose a reason for hiding this comment

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

Is this variable reused? No? Remove it 👍🏻

Suggested change
context.put("transactionPosition", "0");

Comment on lines 1098 to 1109
.section("position", "name", "shares", "nameContinued", "isin")
.match("^(?<position>[\\d]) (Einbuchung|Ausbuchung) (?<name>.*) (?<shares>[\\.,\\d]+) Stk\\.$")
.match("^(?<nameContinued>.*)$")
.match("^(?<isin>[A-Z]{2}[A-Z0-9]{9}[0-9])$")
.assign((t, v) -> {
Map<String, String> context = type.getCurrentContext();
context.put("position", v.get("position"));
t.setDateTime(asDate(context.get("date")));
t.setShares(asShares(v.get("shares")));
t.setSecurity(getOrCreateSecurity(v));
t.setCurrencyCode(asCurrencyCode(t.getSecurity().getCurrencyCode()));
t.setAmount(0L);
Copy link
Member

Choose a reason for hiding this comment

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

Is this variable "position" reused?

With the change for reading the date, .documentContext("date") is now used. access to the variable v.get("date") is now possible. In addition, we now check whether this exists.

Suggested change
.section("position", "name", "shares", "nameContinued", "isin")
.match("^(?<position>[\\d]) (Einbuchung|Ausbuchung) (?<name>.*) (?<shares>[\\.,\\d]+) Stk\\.$")
.match("^(?<nameContinued>.*)$")
.match("^(?<isin>[A-Z]{2}[A-Z0-9]{9}[0-9])$")
.assign((t, v) -> {
Map<String, String> context = type.getCurrentContext();
context.put("position", v.get("position"));
t.setDateTime(asDate(context.get("date")));
t.setShares(asShares(v.get("shares")));
t.setSecurity(getOrCreateSecurity(v));
t.setCurrencyCode(asCurrencyCode(t.getSecurity().getCurrencyCode()));
t.setAmount(0L);
.section("name", "shares", "nameContinued", "isin")
.documentContext("date") //
.match("^[\\d] (Einbuchung|Ausbuchung) (?<name>.*) (?<shares>[\\.,\\d]+) Stk\\.$")
.match("^(?<nameContinued>.*)$")
.match("^(?<isin>[A-Z]{2}[A-Z0-9]{9}[0-9])$")
.assign((t, v) -> {
t.setDateTime(v.get("date"));
t.setShares(asShares(v.get("shares")));
t.setSecurity(getOrCreateSecurity(v));
t.setCurrencyCode(asCurrencyCode(t.getSecurity().getCurrencyCode()));
t.setAmount(0L);

Comment on lines 1046 to 1058
DocumentType type = new DocumentType("(STEUERLICHER UMTAUSCH)", (context, lines) -> {
Pattern pDate = Pattern.compile("^(.*) DATUM (?<date>[\\d]{2}\\.[\\d]{2}\\.[\\d]{4})$");
context.put("transactionPosition", "0");

for (String line : lines)
{
Matcher mDate = pDate.matcher(line);
if (mDate.matches())
context.put("date", mDate.group("date"));

}
});
this.addDocumentTyp(type);
Copy link
Member

Choose a reason for hiding this comment

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

You could extract the date from the document in this way.
It should also be possible to transfer the security data in this way.

Suggested change
DocumentType type = new DocumentType("(STEUERLICHER UMTAUSCH)", (context, lines) -> {
Pattern pDate = Pattern.compile("^(.*) DATUM (?<date>[\\d]{2}\\.[\\d]{2}\\.[\\d]{4})$");
context.put("transactionPosition", "0");
for (String line : lines)
{
Matcher mDate = pDate.matcher(line);
if (mDate.matches())
context.put("date", mDate.group("date"));
}
});
this.addDocumentTyp(type);
final DocumentType type = new DocumentType("STEUERLICHER UMTAUSCH", //
documentContext -> documentContext //
// @formatter:off
// Straße 1 DATUM 07.12.2023
// @formatter:on
.section("date") //
.match("^(.*) DATUM (?<date>[\\d]{2}\\.[\\d]{2}\\.[\\d]{4})$") //
.assign((ctx, v) -> ctx.put("date", asDate(v.get("date")))));
this.addDocumentTyp(type);

Comment on lines +1077 to +1087
.section("type")
.match("(^|^[\\d] )(?<type>"
+ "(Einbuchung|Ausbuchung)"
+ ") .* ([\\.,\\d]+ Stk\\.)$")
.assign((t, v) -> {
if ("Einbuchung".equals(v.get("type")))
{
t.setType(PortfolioTransaction.Type.DELIVERY_INBOUND);
t.setNote("Einstandskurs fehlt");
}
})
Copy link
Member

Choose a reason for hiding this comment

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

Missing translation "note" but i think... remove this line.

Suggested change
.section("type")
.match("(^|^[\\d] )(?<type>"
+ "(Einbuchung|Ausbuchung)"
+ ") .* ([\\.,\\d]+ Stk\\.)$")
.assign((t, v) -> {
if ("Einbuchung".equals(v.get("type")))
{
t.setType(PortfolioTransaction.Type.DELIVERY_INBOUND);
t.setNote("Einstandskurs fehlt");
}
})
.section("type")
.match("^[\\d] (?<type>(Einbuchung|Ausbuchung)) .* [\\.,\\d]+ Stk\\.$")
.assign((t, v) -> {
if ("Einbuchung".equals(v.get("type")))
t.setType(PortfolioTransaction.Type.DELIVERY_INBOUND);
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wenn im Dokument in Deutsch Ein/Ausbuchung steht wird der Nutzer auch "Einstandskurs fehlt" verstehen. Finde den Hinweis wichtig, da sich daraus ja die zu zahlenden Steuern bei späterer Veräußerung berechnen. Der Import jeglicher Einlieferungen mit Wert 0 führt ja zur Verfälschung des Einstandskurs (zum Beispiel bei Berichte-> Performance-> Wertpapiere in den Spalten Einstandskurs und Nicht realisierte Kurserfolge).

Finde den Hinweis hilfreich. Wenn irgendwann anderssprachige Trade Republic Dokumente auftauchen stimme ich dir zu - dann brauchen wir eine Übersetzung.

Copy link
Member

@Nirus2000 Nirus2000 Feb 23, 2024

Choose a reason for hiding this comment

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

Wenn der "Einstandskurs fehlt" sollten wir dann nicht mit "cancellation" oder "not supported" den Import markieren?
Wäre das nicht sinnvoller?

BTW ich sehe auch, dass die reguläre expressions immer unterschliedlich sind:
L1059 --> (^|^[\\d] )(Einbuchung|Ausbuchung) .* ([\\.,\\d]+ Stk\\.)$
L1075 --> ^[\\d] (?<type>(Einbuchung|Ausbuchung)) .* [\\.,\\d]+ Stk\\.$
L1097 --> ^(?<position>[\\d]) (Einbuchung|Ausbuchung) (?<name>.*) (?<shares>[\\.,\\d]+) Stk\\.$

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naja um den aktuellen Bestand abzubilden, ist das als Platzhalter ja schonmal nicht schlecht.
Die steuerlichen Details sehe ich persönlich eher optional.
Das Problem zieht sich durch sehr viele Importer, dass der Kurs bei Einlieferung hart auf 0 gesetzt wird. Das müsste man dann alles rausnehmen und Funktionsumfang reduzieren. Ich denke mit dem Einfügen einer Notiz ist der Gewinn größer.

L1059 kann ich noch anpassen. Ansonsten sind die doch gleich?

Comment on lines +1140 to +1141
.section("position", "amount", "currency", "date") //
.match("^(?<position>[\\d]) Kapitalertragsteuer \\-[\\.,\\d]+ ([\\w]{3})$") //
Copy link
Member

Choose a reason for hiding this comment

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

Is this variable "position" reused?

Suggested change
.section("position", "amount", "currency", "date") //
.match("^(?<position>[\\d]) Kapitalertragsteuer \\-[\\.,\\d]+ ([\\w]{3})$") //
.section("amount", "currency", "date") //
.match("^[\\d] Kapitalertragsteuer \\-[\\.,\\d]+ ([\\w]{3})$") //

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops. added the comparison from in/outbound delivery with tax booking

adopting some of Alex recommendations
@christen90
Copy link
Contributor Author

updated PR

buchen pushed a commit that referenced this pull request Feb 26, 2024
* add US Quellensteuer
* add Steuerlicher Umtausch
* add Spin-Off
* add Zwangsübernahme

Issue: #3807
@buchen
Copy link
Member

buchen commented Feb 26, 2024

squashed, rebased, and merged

@buchen buchen closed this Feb 26, 2024
@christen90 christen90 deleted the adds-tax-bookings-trade-republic branch March 7, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants