nemiah / phpFinTS

PHP library to communicate with FinTS/HBCI servers
MIT License
132 stars 42 forks source link

Was fehlt noch in der FinTsNew-Implementierung? #197

Closed Philipp91 closed 4 years ago

Philipp91 commented 4 years ago

Momentan existieren die alte FinTs-Klasse und FinTsNew nebeneinander. Wenn die neue Klasse mit der Mehrheit der Anwendungen gut funktioniert, können wir sie zur Hauptimplementierung machen (umbenennen) und die alte löschen.

Ich würde vorschlagen, in diesem GitHub Issue die Dinge zu sammeln, die noch fehlen oder nicht funktionieren. Für meine Anwendungszwecke funktioniert FinTsNew bereits wie gewünscht, aber vermutlich haben andere Anwendungen bzw. Banken noch andere Ansprüche. @nemiah @fbett @MartinWeidner @Metabor

PS: Vielleicht kann @nemiah ein Release mit dem aktuellen Stand machen, dann kann man es einfacher testen. Ich habe auch ein paar "deprecation notices" vorbereitet, die wir zu einem geeigneten Zeitpunkt einbauen können.

nemiah commented 4 years ago

Ich brauche Umsatzabfrage, Lastschriften und Überweisungen mit regulärer und mit photoTAN (bzw mit Bild). Ich fände es super, wenn wir die API gleich lassen könnten, denn sonst muss ich meine Anwendung wieder anpassen.

Kann ich machen, dann mache ich einfach mal die 2.0 aus dem aktuellen Stand…

ampaze commented 4 years ago

Wäre es nicht sinnvoller gewesen mit 2.0 zu warten bis die Klasse umbenannt ist?

Und den jetzigen Stand 1.7 oder so?

ampaze commented 4 years ago

Die neue Klasse ist gänzlich anders zu verwenden, da wirst du nicht um Anwendung anpassen herumkommen...

nemiah commented 4 years ago

Ich fand, die Änderungen rechtfertigen den Sprung. Im Moment scheint es ja auch größtenteils zu funktionieren…

Welche Vorteile hat denn die neue Klasse?

Philipp91 commented 4 years ago

2.0 hin oder her -- wenn die alte FinTs-Klasse gelöscht oder ersetzt wird, dann muss das eine Hauptversion sein.

Die größten Vorteile der neuen Klasse sind im Innern (Wartbarkeit, Testabdeckung, weniger duplizierter Code, es werden durchgehend die neuen Segmente verwendet, die einfacher zu debuggen sind).

Aus Nutzersicht ist (meiner subjektiven Meinung nach) die Schnittstelle einfacher zu verstehen -- aber als "Insider" verstehst du natürlich die bestehende Klasse mindestens genauso gut.

nemiah commented 4 years ago

Da können wir dann ja die 3.0 machen. Mal sehen, wie es läuft…

Mit der Tatsache, dass ich ein Objekt an den Konstruktor übergeben muss, bin ich nicht einverstanden, das weißt du :blush:

Philipp91 commented 4 years ago

Ganz so bestimmt hatte ich deine Meinung nicht in Erinnerung. Wieso denn nicht? Hat es irgendwelche Nachteile?

Ich finde es leserlicher, wenn die Namen der Parameter dranstehen. ctor($productName, $bankCode) kann man zu ctor($bankCode, $productName) vertauschen, ohne dass man das Problem sieht. Auch der "Compiler" beschwert sich nicht, weil beides strings sind. $options->productName = $bankCode würde hingegen auffallen.

Auch optionale Parameter sind viel einfacher zu handhaben. Wenn jemand nur das Response-Timeout ändern will, braucht er das Connect-Timeout nicht ändern. Mit einem langen Konstruktor wäre es hingegen: ctor(..., ..., 15, 31).

Wenn der Konstruktor eine lange Liste von Parametern entgegennimmt, wird jede Änderung, die nicht einfach einen Parameter hinten anfügt, zu einem Breaking Change. Man hat außerdem keine rechte Möglichkeit, einzelne Konstruktor-Parameter als deprecated zu markieren.

Im konkreten Fall hat es noch den Vorteil, dass man dieselben beiden Objekte auch an den SanitizingCLILogger([$options, $credentials]) übergeben kann.

nemiah commented 4 years ago

Ich denke, die Änderungen an den Parametern im Konstruktor sind durch und diese API ist stabil. zusätzliche Optionen würde ich dann über Methoden (z.B. setTimeout) regeln, wie wir es ja auch mit dem Logger gemacht haben. Die muss man dann auch nur aufrufen, wenn man sie braucht. Meiner Ansicht nach macht es das nur unnötig kompliziert, wenn im Konstruktor selber die Objekte auch erzeugt werden könnten. Z.B. mit setLogger könnte sich der SanitizingCLILogger dann die Infos auch aus der Klasse holen und der Entwickler muss es gar nicht selbst an den Logger übergeben. Aber da hat wohl jeder so seinen Stil :blush:

Philipp91 commented 4 years ago

Aus heutiger Sicht sind die meisten APIs stabil (oder deprecated oder "TODO"), denn sonst würde man sie ja jetzt schon ändern. Man kann nie wissen, was die Zukunft bringt.

Verstehe ich deinen Vorschlag richtig, dass der Konstruktor viele Parameter hat, aber dann intern ein Credentials- und ein FinTsOptions-Objekt erzeugt, die rumgereicht werden können? Wo platzieren wir in dem Fall die Dokumentation für die einzelnen Optionen (Konstruktor @params oder da, wo sie heute ist)?

Und wären dann alle Pflicht-Optionen im Konstruktor und alle optionalen Parameter als Setter? Oder gibt es eine andere Aufteilung?

Der setLogger()-Funktion kann man einen beliebigen Logger übergeben. Man könnte prüfen, ob es sich um einen SanitizingCLILogger handelt und ggf. die addSensitiveMaterial() aufrufen, aber das wäre ein recht überraschendes Verhalten für einen Setter -- nachdem man sie aufgerufen hat, verhält sich der übergebene Logger auch anderswo anders als zuvor.

nemiah commented 4 years ago

Naja, das mit dem internen erzeugen deiner Objekte aus den Parametern des Konstruktors ist nur mein Weg, beide unsere Ansichten unter einen Hut zu bekommen :blush:

Die Doku für den Konstruktor würde ich beim Konstruktor einbauen und die Doku für das FinTsOptions/Credentials-Objekt einfach bei dem Objekt. Wenn man es sich holt, dann hat man ja damit zu tun. Ich codiere Informationen gerne an der Stelle, an der man sie braucht. Wenn man etwas gerade nicht braucht, muss man auch nichts drüber wissen.

Ich mache es in meiner Anwendung seit 12 Jahren so und in den ~180k Zeilen Code kommt es nur ein paar mal vor, dass eine Methode mehr als ein paar Parameter hat. Deswegen finde ich, dass wir es nicht unnötig kompliziert machen müssen. Der Anwendungsfall von phpFinTS ist ja recht spezifisch und damit eingeschränkt.

Meiner Erfahrung nach kommen die wichtigsten Parameter für jede Methode sowieso immer zuerst, weil man die Methode damit entwickelt. Die neuen Parameter können dann in der Regel optional sein. Mir ist es bisher nur wirklich selten passiert, dass ich nachträglich einen mandatorischen Parameter gebraucht habe.

Die Entscheidung, ob ich einen optionalen Parameter in einen Konstruktor einbaue oder einen setter dafür mache, ist vermutlich eine Bauchentscheidung. Grundsätzlich würde ich immer sinnvolle Werte vorgeben, so dass alles mit so wenig Input wie möglich einfach läuft. Wenn es dann ein Problem gibt, kann man noch nachjustieren (zum Beispiel einen Timeout). Wenn der Konstruktor zum Beispiel schon 8 Parameter hat, dann kann der 9. nicht so wichtig für die allgemeine Funktionalität sein.

Ich würde an den übergebenen Logger im Setter einfach immer $this übergeben und was er sich dann daraus holt oder nicht, bleibt ihm überlassen. dann ist der Logger maximal flexibel und meiner Ansicht nach erschließt sich ja aus dem Namen/der Doku des Loggers schon, was er macht.

ampaze commented 4 years ago

Ich sehe nicht unbedingt den Vorteil es unter einen Hut bringen zu wollen. Du kannst deinen bisherigen Code eh nicht mit FinTsNew weiterverwenden.

Für deine Anwendung und alle bestehenden Anwendung wäre es besser einfach das bisherige weiter zu verwenden.

Soll heißen:

Dann kann jemand der neu anfängt 3.0 verwenden und die anderen bleiben bei 2.0.

Ich finde die Objekte als Parameter inzwischen sehr praktisch, wir haben nämlich mehrere Banken (und pro Bank tlw. auch mehrere Zugänge) und ich kann mir von einer Methode die Objekte liefern lassen und muss nicht alle Einzelparameter zurückgeben.

Philipp91 commented 4 years ago

das mit dem internen erzeugen deiner Objekte aus den Parametern des Konstruktors

Wenn man die Objekte nicht erzeugt, kann man die Parameter auch nicht zusammen weiterreichen. Dann muss man sie überall einzeln weiterreichen und speichern, so wie hier und hier via hier. Das ist alles unnötiger Boilerplate-Code. Und auch dort können Fehler passieren wie dass man die Reihenfolge verwechselt, wenn man sie ändern will.

Die Doku für den Konstruktor würde ich beim Konstruktor einbauen und die Doku für das FinTsOptions/Credentials-Objekt einfach bei dem Objekt.

Also müsste man z.B. productName sowohl hier als auch hier und hier dokumentieren? Die alte Implementierung umgeht das Problem ja, indem sie überhaupt keine Dokumentation hat :-)

in den ~180k Zeilen Code kommt es nur ein paar mal vor, dass eine Methode mehr als ein paar Parameter hat

Sowas ist auch in meiner Erfahrung selten. Aber hier ist es eben der Fall. Es gibt Projekte, wo schon ab 3 Parametern der Linter Alarm schlägt und ein Parameter-Objekt empfiehlt, bei ESLint ist es beispielsweise der Standard.

an den übergebenen Logger im Setter einfach immer $this übergeben

Das ist nicht möglich, \Psr\LoggerInterface hat keine Funktion, an die man das übergeben könnte.

Nochmal zur Ausgangsfrage: "Hat es irgendwelche Nachteile" (wenn man diese Parameter-Objekte verwendet)?

Ich stimme zu, dass es für die aufrufende Aufwendung ein bisschen komplizierter ist. Aber nicht viel komplizierter. Und vor allem nicht "unnötig kompliziert", denn es hat ja die genannten Vorteile. Auch im Anwendungscode ergibt sich der Vorteil, dass man sieht, welcher Parameter welcher ist, weil man sie alle beim Namen nennt.

Du kannst deinen bisherigen Code eh nicht mit FinTsNew weiterverwenden.

Stimmt. Aber ist das denn so viel Code? Ich habe früher natürlich auch die FinTs-Klasse verwendet. Auf FinTsNew umzustellen, hat nicht besonders lange gedauert. Viel größer war der Aufwand, ein TAN-Eingabefeld in meine UI einzubauen und so weiter -- und das muss man ja ohnehin. Anders als bei häufig genutzten Funktionen wie file_get_contents(), wo es ärgerlich wäre, wenn sich bspw. die Parameterreihenfolge ändert, haben die meisten Anwendungen doch vermutlich nur wenige Stellen, an denen diese Bibliothek aufgerufen wird.

ampaze commented 4 years ago

Stimmt. Aber ist das denn so viel Code?

Es funktioniert einfach gänzlich anders, auch wenn es nicht viel Code mag sein, erfordert es eine Umstrukturierung. Wenn es für jemanden bislang gut funktioniert, braucht er ja nicht seine Anwendung umschreiben müssen und alles neu durchtesten, was inbesondere mit TANs schnell mal zu Sperrung des Zugangs führen kann.

Da ich auch gerade auf FinTsNew umstelle, wäre meine Empfehlung: Wenn es keine Probleme gibt, ruhig beim alten zu bleiben.

nemiah commented 4 years ago

Neenee, einen Branch machen wir nicht. Das gibt nur unnötige Arbeit, wenn im einen was funktioniert und im anderen nicht. Vorher mache ich einen Wrapper um die FinTs(New), der das übersetzt und es wieder so aussieht, wie ich das mag :blush:

Das ist nicht möglich, \Psr\LoggerInterface hat keine Funktion, an die man das übergeben könnte.

Lol! Wisst ihr, ich programmiere schon so lange, da gab es diesen ganzen dependency-Käse nicht, der meiner Ansicht mehr kompliziert macht, als er wirklich bringt. Die 5% Funktionalität, die ich daraus brauche, schreibe ich in der Zeit neu, die ich brauche, um zu verstehen, wie ich das Ding verwende.

Also müsste man z.B. productName sowohl hier als auch hier und hier dokumentieren? Die alte Implementierung umgeht das Problem ja, indem sie überhaupt keine Dokumentation hat :-)

Das hat mich nie interessiert, da es für mich darum ging, etwas zu erzeugen, das funktioniert und keine Schönheit :nail_care: :grin:

Sowas ist auch in meiner Erfahrung selten. Aber hier ist es eben der Fall. Es gibt Projekte, wo schon ab 3 Parametern der Linter Alarm schlägt und ein Parameter-Objekt empfiehlt, bei ESLint ist es beispielsweise der Standard.

Ob der Linter Alarm schlägt oder nicht, stört mich vergleichsweise wenig :grin:

Auch wenn ich diese Diskussion sehr interessant finde, da wir eine unterschiedliche Herangehensweise ans Programmieren haben - von eher technisch strukturiert zu meinem machmer mal nach Bauchgefühl, klappt schon - ist vermutlich das Meiste eine Geschmackssache und mir fehlt die Zeit, das alles detailliert durchzudiskutieren. Vor allem, da Ihr scheinbar Großes mit der Bibliothek vor habt und ich mir immer nur denke "passt schon, muss laufen und keinen Stress machen". Deswegen an dieser Stelle meine Frage: wem von euch beiden darf ich dieses Repository übergeben? :grin:

Philipp91 commented 4 years ago

einen Branch machen wir nicht. Das gibt nur unnötige Arbeit, wenn im einen was funktioniert und im anderen nicht.

Dem stimme ich voll und ganz zu. Das ist auch der Grund, warum ich meine Im-Grunde-Neu-Implementierung der Bibliothek hier im Repository gemacht habe anstatt auf der grünen Wiese, und warum ich versuche, sie zur Haupt-Implementierung zu machen und die alte zu ersetzen. Denn im Moment haben wir ja sowas wie einen Branch, nur eben nicht auf Git-Ebene.

Da ich auch gerade auf FinTsNew umstelle, wäre meine Empfehlung: Wenn es keine Probleme gibt, ruhig beim alten zu bleiben.

Das steht nicht direkt im Konflikt mit dem obigen. Wenn die FinTs-Klasse aus dem 2.0-Release für jemanden funktioniert, kann er dabei bleiben. Sobald aber eine Bank für denjenigen dazukommt oder eine bestehende ihre Schnittstelle ändert, steht (unter Umständen sehr plötzlich, siehe #200) die Migration an. Kann jeder für sich selbst entscheiden.

Die "unnötige Arbeit, wenn im einen was funktioniert und im anderen nicht" vermeiden wir, indem wir das 2.0-Release nicht weiter unterstützen. Wenn es mit einer Bank nicht funktioniert, ist es eben Pech, deswegen machen wir kein 2.0.1.

einen Wrapper um die FinTs(New), der das übersetzt und es wieder so aussieht, wie ich das mag

Das würde in der Tat funktionieren. Es könnte auch eine FinTsNew::create()-Funktion sein. Im Unterschied zu Konstruktoren, die man nicht überladen kann, kann man ja beliebit viele statische Funktionen mit verschiedenen Namen hinzufügen.

da Ihr scheinbar Großes mit der Bibliothek vor habt

Für meine Bedürfnisse ist jetzt schon alles fertig. Alles was ich vorhabe, ist Umsätze abholen, und das funktioniert in meiner Anwendung wie gewünscht. Das einzige verbleibende Vorhaben wäre, FinTsNew zu FinTs umzubenennen. (Ich könnte das einfach in meinem eigenen Fork machen, oder einfach für immer FinTsNew benutzen, aber das finde ich genauso suboptimal wie einen Branch.) Da ich weiß, dass für andere Nutzer noch ein paar Kleinigkeiten (?) fehlen, habe ich diesen Thread gestartet.

wem von euch beiden darf ich dieses Repository übergeben?

Ich kann gerne die Pflege der "neuen" Implementierung übernehmen. An die alte traue ich micht ran -- mangels Tests kann es da leicht passieren, dass man es für die X-Bank zum Laufen bringt, aber unbemerkt für eine andere Bank kaputt macht.

Es funktioniert einfach gänzlich anders, auch wenn es nicht viel Code mag sein, erfordert es eine Umstrukturierung.

Ich musste meine Anwendung vor allem wegen der neuen TAN-Eingaben umstrukturieren. Ich habe im gleichen Schritt von FinTs zu FinTsNew gewechselt. Wenn jemand seine Anwendung bereits auf Basis von FinTs für die TANs umgebaut hat, dann ist es jetzt noch ein zusätzlicher Wechsel. Den habe ich selbst zwar nie "erlebt", aber ich kann mir nicht vorstellen, dass es eine große Umstrukturierung sein kann. Das deswegen, weil ich glaube, dass man die alte FinTs-Klasse komplett als Wrapper um die neue implementieren könnte. Also ist es nicht so, dass einem an gewissen Stellen im Anwendungscode plötzlich Daten "fehlen" oder so, d.h. die Struktur der Anwendung muss sich nicht wirklich verändern.

nemiah commented 4 years ago

Ich kann gerne die Pflege der "neuen" Implementierung übernehmen.

Hmm, ok, im Moment bringt das noch nix…

An die alte traue ich micht ran -- mangels Tests kann es da leicht passieren, dass man es für die X-Bank zum Laufen bringt, aber unbemerkt für eine andere Bank kaputt macht.

War bisher kein Problem :wink: Unit-Tests werden überbewertet: https://augustl.com/blog/2019/best_bug_predictor_is_organizational_complexity/

Das einzige verbleibende Vorhaben wäre, FinTsNew zu FinTs umzubenennen. (Ich könnte das einfach in meinem eigenen Fork machen, oder einfach für immer FinTsNew benutzen, aber das finde ich genauso suboptimal wie einen Branch.) Da ich weiß, dass für andere Nutzer noch ein paar Kleinigkeiten (?) fehlen, habe ich diesen Thread gestartet.

Wie schon geschrieben, brauche ich halt noch die Überweisungen und Lastschriften. Beide Klassen können wir nicht lassen, das müssen wir aufräumen.

Mit einer ::create-Methode kann ich auch leben :blush:

ampaze commented 4 years ago

Wie schon geschrieben, brauche ich halt noch die Überweisungen und Lastschriften.

Lastschriften hatte ich implementiert, inkl. Sammellastschriften (Action\SendSEPADirectDebit) Überweisungen gibt es auch schon (Action\SendSEPATransfer)

Vor allem, da Ihr scheinbar Großes mit der Bibliothek vor habt und ich mir immer nur denke "passt schon, muss laufen und keinen Stress machen".

Meine aktuellen Vorhaben sind noch Auslandsüberweisungen zu implementieren.

Deswegen an dieser Stelle meine Frage: wem von euch beiden darf ich dieses Repository übergeben?

Die alte Implementation möchte ich auch nicht mehr anfassen ;) Sie läuft aber jetzt schon seit einiger Zeit ohne zu mucken. Diesen Stand als 2.0 einzufrieren wäre daher ohne Probleme möglich.

Philipp91 commented 4 years ago

Unit-Tests werden überbewertet: https://augustl.com/blog/2019/best_bug_predictor_is_organizational_complexity/

😄 Naja, die Anzahl von Bugs vorhersagen und ihre genaue Stelle und Lösung finden sind ja schon verschiedene Vorhaben..

Was diese Bibliothek braucht sind nicht Unit-Tests (außer halt ein paar, die übliche gezielte Dosis), sondern Integration-Tests, mit einer Fake-Bank im Hintergrund. Denn einerseits kann man nicht dauernd mit den echten Banken testen, weil die keine richtigen Sandbox-Umgebungen zur Verfügung stellen. Und andererseits hält sich niemand so hundertprozentig an die FinTS-Spezifikation, also braucht man pro (wichtiger) Bank einen Test, um sicherzustellen, dass man sie nicht versehentlich kaputt macht.

Mit einer ::create-Methode kann ich auch leben 😊

Wenn das mit dem SanitizingCLILogger gelöst wäre, könnte ich auch mit den 6 alten Konstruktor-Parametern leben. Es stimmt schon, dass sie jetzt eher stabil scheinen. Aber ich befürchte, dass das mit dem Logger nicht elegant hinzukriegen ist. Soll ich einen PR für die eine ::create-Funktion schicken? Oder willst du die, bzw. den anderen Konstruktor, einfach in deine FinTsCool extends FinTsNew Klasse einbauen?

nemiah commented 4 years ago

Wenn das mit dem SanitizingCLILogger gelöst wäre, könnte ich auch mit den 6 alten Konstruktor-Parametern leben. Es stimmt schon, dass sie jetzt eher stabil scheinen.

Ich glaube, php kann auch Interfaces extenden: interface FinTsLoggerInterface extends LoggerInterface { public function blubb(…); } Elegant genug? :wink:

Alleine für die ::create()-Methode würde ich keine neue Klasse machen…

Philipp91 commented 4 years ago

interface FinTsLoggerInterface extends LoggerInterface

Das würde zwar das Problem nicht lösen, weil die meisten den Logger nicht selber implementieren, sondern von ihrem Framework bekommen (z.B. Monolog in meinem Fall), aber auf den zweiten Blick würde ich sagen, dass es auch gar nicht wünschenswert ist, alle diese Daten in irgendeine Datei/Datenbank zu loggen.

Ich werde den SanitizingCLILogger aufsplitten in "Sanitizing" (ein Wrapper) und "CLI" (ein simpler Logger zum Testen). Wenn man dann setLogger() aufruft und etwas übergibt, was nicht eh schon ein SanitizingLogger ist, dann wird der übergebene Logger gewrappt, sodass niemand aus Versehen seine Passwörter loggen kann. Damit ist auch das Interface-Problem gelöst.

Philipp91 commented 4 years ago

Also hattest du es dir in etwa so vorgestellt? https://github.com/nemiah/phpFinTS/commit/d6e0d96e60ac31f3346de9d5bff38751a43ca80a

nemiah commented 4 years ago

Yes, genau so :+1: :blush:

ampaze commented 4 years ago

Also hattest du es dir in etwa so vorgestellt? d6e0d96

Kannst du da nicht noch FinTsNew::create mit den $options und $credentials zur Verfügung stellen?

Philipp91 commented 4 years ago

Kannst du da nicht noch FinTsNew::create mit den $options und $credentials zur Verfügung stellen?

Das wäre dann ein bisschen seltsam, dann würde die Factory-Methode die beiden Objekte zerlegen und verwerfen, und der Konstruktor würde dann neue Objekte mit denselben Inhalten wieder aufbauen. Das könnte dann allein schon dadurch Verwirrung stiften, dass ein Aufrufer vielleicht die Objekte hat und weiter verändert, die an create() übergeben wurden.

Im Gegenteil, wenn wir den Konstruktor so wollen, dann habe ich konsequenterweise die Objekte aus dem öffentlich sichtbareren Teil der Bibliothek entfernt (siehe die anderen beiden Commits auf der Branch). Die Anwendung verwendet diese Klassen dann gar nicht.

ampaze commented 4 years ago

Nungut, das Erstellen des SanitizingLoggers kann ich ja relative einfach abändern und die ganzen Daten einzeln angeben, aber wie setzt man dann timeoutConnect und timeoutResponse?

Philipp91 commented 4 years ago

aber wie setzt man dann timeoutConnect und timeoutResponse?

Das ist dann der letzte Commit auf der Branch: https://github.com/Philipp91/phpFinTS/commit/7afe168afce66333a2c1ddc0bc038c86ea141b46

ampaze commented 4 years ago

Okay, wenn @nemiah es so will. Komfortabler ist es dadurch nicht geworden.

nemiah commented 4 years ago

Ich wäre ja auch mit der ::create()-Methode zufrieden gewesen. :woman_shrugging: :blush:

ampaze commented 4 years ago

Noch ist es ja nicht zu spät! 😁

nemiah commented 4 years ago

Neenee, alles gut, ich finde es schöner so :grin:

Philipp91 commented 4 years ago

Wenn sonst nichts mehr bei der neuen Implementierung fehlt, könnte man die alte dann jetzt löschen (bzw. auf einer "old"-Branch weiterleben lassen)? (Kann gerne einen PR dafür schicken.)

nemiah commented 4 years ago

Beispiele für Überweisungen und Lastschriften wären noch super :blush:

Philipp91 commented 4 years ago

Beispiele für Überweisungen und Lastschriften wären noch super 😊

Ich verwende diese Features beide nicht. @ampaze Vielleicht hast du passenden Beispielcode? Insbesondere was die Zusammenarbeit mit der XML-Bibliothek angeht.

ampaze commented 4 years ago

Ich benutze beides, aber nicht die XML-Bibliothek von nemiah.

Wie man an das XML kommt, war eine Diskussion in #165 an der sich niemand weiter beteiligt hat.

Der einzige Unterschied zur Umsatzabfrage ist, dass eine andere Aktion instanziiert wird und keine Daten zurückgeliefert werden.

Philipp91 commented 4 years ago

Stört es jemand, wenn wir Daueraufträge für den Moment nicht mehr unterstützen? Die alte Bibliothek hat da merkwürdigerweise nur Auflisten und Löschen untersützt, aber nicht Hinzufügen/Ändern/Pausieren.

ampaze commented 4 years ago

Mich stört es nicht.

nemiah commented 4 years ago

Mit den Daueraufträgen habe ich damals die TAN-Unterstützung implementiert, um nicht jedes mal eine Überweisung/Lastschrift auszuführen :grin:

Kann draußen bleiben, brauche ich nicht mehr.

Philipp91 commented 4 years ago

Mit den Daueraufträgen habe ich damals die TAN-Unterstützung implementiert, um nicht jedes mal eine Überweisung/Lastschrift auszuführen 😁

Das ist eine sehr clevere Idee 👍 Wir hatten uns doch gefragt, ob es einen "TAN-Verbrennen"-Geschäftsfall gibt, mit dem man testen kann. Daueraufträge anlegen/ändern/löschen kommt dem ziemlich nahe. Wenn es wieder TAN-Probleme geben sollte, wäre das schon ein Grund, die Daueraufträge wieder zu implementieren.

nemiah commented 4 years ago

:grin:

valb3r commented 4 years ago

Hello all, @Philipp91 saw your message

Was diese Bibliothek braucht sind nicht Unit-Tests (außer halt ein paar, die übliche gezielte Dosis), sondern Integration-Tests, mit einer Fake-Bank im Hintergrund. Denn einerseits kann man nicht dauernd mit den echten Banken testen, weil die keine richtigen Sandbox-Umgebungen zur Verfügung stellen. Und andererseits hält sich niemand so hundertprozentig an die FinTS-Spezifikation, also braucht man pro (wichtiger) Bank einen Test, um sicherzustellen, dass man sie nicht versehentlich kaputt macht.

I'm the contributor of OpenBankingGateway project (https://github.com/adorsys/open-banking-gateway).

We have developed HBCI/FinTs Sandbox - https://github.com/adorsys/open-banking-gateway/tree/develop/opba-protocols/sandboxes/hbci-sandbox/src/main/java/de/adorsys/opba/protocol/sandbox/hbci that supports payments, account, transaction listing. It is written in Java, but in the era of Docker that shouldn't be of much issue.

If there is any interest in using our Sandbox I could explain details, you will be able to use it for your tests and we will have more testers for it