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

Verwendungszweck mit : und + wurde nicht richtig getrennt #142

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

lenilsas
Copy link
Contributor

@lenilsas lenilsas commented Jul 5, 2024

Bei mir wurden die Verwendungszwecke nicht richtig getrennt. Mit dieser Änderung funktioniert es richtig.
Außerdem gibt es bei meiner Bank (GLS) noch den Tag TAN1, den habe ich hinzugefügt, damit der auch getrennt wird.

@willuhn
Copy link
Owner

willuhn commented Jul 5, 2024

Der boolean-Parameter ist extra für diesen konkreten Fall da. Bei allen anderen Aufrufen wird ja "true" übergeben. Mit der Änderung schlägt TestVerwendungszweckUtil.testParse010 fehl.

@lenilsas
Copy link
Contributor Author

lenilsas commented Jul 5, 2024

Bei test 10 steht

String [] test =
    {
        "SVWZ+ Das geht sogar",
        " gemischt ",
        "IBAN: DE1234567890 "
    };

    Map<Tag,String> map = VerwendungszweckUtil.parse(test);
    assertEquals("Das geht sogar gemischt IBAN: DE1234567890",map.get(Tag.SVWZ),"SVWZ falsch");
    assertEquals("DE1234567890",map.get(Tag.IBAN),"IBAN falsch");

Ich würde hier aber "Das geht sogar gemischt" als SVWZ erwarten ohne IBAN

@willuhn
Copy link
Owner

willuhn commented Jul 5, 2024

Bis e841b3c war das auch so. Dort ist die IBAN explizit mit in dem Assert gelandet. Ich weiss den Grund dafür nicht mehr. Da ich den Tesr aber explizit dahingehend geändert hatte, gehe ich davon aus, dass es einen Grund hatte.

@willuhn
Copy link
Owner

willuhn commented Jul 5, 2024

Möglicherweise war es auch ein Zugeständnis, weil sich sonst testParse012 nicht lösen liess.

@willuhn
Copy link
Owner

willuhn commented Jul 5, 2024

Vielleicht war es aber auch Absicht, weil es Fälle gab, in denen dieser Text so im Verwendungszweck auftauchte und so auch drin bleiben sollte. Ich weiss es leider nicht mehr. Bemerkenswert ist jedenfalls, dass die IBAN da nicht versehentlich im assert steht sondern manuell von mir da eingetragen wurde. Das muss einen validen Grund gehabt haben.

@lenilsas
Copy link
Contributor Author

lenilsas commented Jul 5, 2024

Ich habe den Test auch mit meiner Änderung ausgeführt und testParse012 war erfolgreich

@willuhn
Copy link
Owner

willuhn commented Jul 5, 2024

Ja, ich weiss. Aber wie gesagt: Ich habe das assert in dem Commit ja nicht versehentlich um die IBAN ergänzt. Das muss einen Grund gehabt haben.

@lenilsas
Copy link
Contributor Author

lenilsas commented Jul 6, 2024

Ich habe gefunden, dass du auf diesen Beitrag hin den Commit gemacht hast:
https://homebanking-hilfe.de/forum/topic.php?t=19713
Falls das Mail von CBC noch vorhanden ist, könnte man herausfinden, ob da es einen weiterhin bestehen Grund für die Änderung gab.
Wie auch immer, ich werde die Änderung auf jeden Fall bei mir lokal weiterverwenden. Evtl. könnte man es ja auch in den Settings konfigurierbar machen. Vor allem bei der Buchungsübernahme nach Syntax finde ich es störend, wenn hier alle weiteren Daten (Iban, Bic, Tan...) im Verwendungszweck stehen.

@willuhn
Copy link
Owner

willuhn commented Jul 7, 2024

Ja, ich hab die Mail noch. Die Verwendungszwecke sehen genauso aus, wie die in dem Unit-Test - ich hatte nur die Zahlen anonymisiert. Das Problem war, dass hinter "SVWZ+" kein Text kommt sondern direkt ein weiteres Tag (in dem Fall "BIC:GENODED1SAM") mit ":" folgt. Das Ergebnis war, dass der Verwendungszweck dann leer war. Er sollte in dem Fall aber stattdessen "BIC:GENODED1SAM....." enthalten.

@lenilsas
Copy link
Contributor Author

lenilsas commented Jul 7, 2024

Mein Vermutung wäre, dass du den Parameter leadingSvwz eingeführt hast um diesen Fall zu lösen, und dann auch testParse010 angepasst hast, es aber schlussendlich gar nicht nötig war da parseTest012 auch so funktioniert. Ich sehe keinen Grund, warum neben dem eigentlichen Verwendungszweck auch noch die anderen Daten angezeigt werden sollen. Und für den Fall, dass man sie sehen will, kann man ja auch Alle Daten des Verwendungszwecks anzeigen aktivieren.
In openjverein/jverein#259 wird auch nach dieser Änderung gefragt

@willuhn willuhn merged commit 4b377bd into willuhn:master Jul 9, 2024
willuhn added a commit that referenced this pull request Jul 9, 2024
aufgerufen wird, kann der Parameter auch ganz entfallen.
@lenilsas lenilsas deleted the verwendugszweck branch July 9, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants