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

Abweichenden Zahlungsweg bei Zusatzbeträgen #445

Merged
merged 15 commits into from
Nov 24, 2024

Conversation

lenilsas
Copy link

@lenilsas lenilsas commented Nov 8, 2024

Jetz kann bei Zusatzbeträgen ein abweichender Zahlungsweg festgelegt werden. Wenn Standard ausgewählt ist wird der beim Mitglied hinterlege Zahlungsweg verwendet

Resolves #420, resolves #421

@JohannMaierhofer
Copy link

In der Lasche Zusatzbeträge in den Mitglied Details fehlt die Spalte für Zahlungsweg.

@JohannMaierhofer
Copy link

Sollte der Zahlungsweg auch in den Zusatzbetrag Vorlagen gespeichert und angezeigt werden?

@tolot27
Copy link
Member

tolot27 commented Nov 11, 2024

Sollte der Zahlungsweg auch in den Zusatzbetrag Vorlagen gespeichert und angezeigt werden?

Ja, dass wäre gut. So habe ich es auch in #420 vorgeschlagen.

@lenilsas
Copy link
Author

Sollte der Zahlungsweg auch in den Zusatzbetrag Vorlagen gespeichert und angezeigt werden?

Ich dachte, dass das nicht nötig ist. wenn ihr meint mache ich das aber noch

@lenilsas
Copy link
Author

Habe alle Kommentare eingearbeitet

@JohannMaierhofer
Copy link

JohannMaierhofer commented Nov 12, 2024

Ich sehe gerade, dass es in der Zusatzbeträge Tabelle im Mitglied in der Überschrift eine Kleinschreibung gibt.
Das "nächste..." und "letzte..." sollte groß geschrieben werden.
Kannst du das noch machen weil die Datei sowieso geändert wurde.

@lenilsas
Copy link
Author

Habe Standard zum Zahlungsweg hinzugefügt so dass er auch in den Listen angezeigt wird

@JohannMaierhofer
Copy link

Habe Standard zum Zahlungsweg hinzugefügt so dass er auch in den Listen angezeigt wird

Super, danke

@JohannMaierhofer
Copy link

Jetzt hätte ich noch einen Vorschlag. Könntest du in ZusatzbetragImpl den insertCheck erweitern so dass er bei Lastschrift prüft ob das Mitglied eine IBAN gesetzt hat und ein Mandat Datum. Da könnte man den entsprechenden Teil aus MitgliedImpl übernehmen.

Ich finde es besser das hier schon zu prüfen als dass es später zur Fehlermeldung im Abrechnungslauf kommt.

Eigentlich könnte man dann noch beim MitgliedImpl prüfen ob es einen Zusatzbeitrag mit Basislastschrift hat. Dann müsste man auch den Check machen. Aber vielleicht ist das etwas zuviel. Dann läuft man halt in die Meldung beim Abrechnungslauf.

@lenilsas
Copy link
Author

Hab den Test eingebaut

@JohannMaierhofer
Copy link

Hab den Test eingebaut

Wie ist das denn bei Vollzahler Konfiguration? Wird dann bei Zusatzbeträgen auch das Konto des Vollzahler genommen? Wenn ja müsste der Check hier noch erweitert werden.

@lenilsas
Copy link
Author

Wie ist das denn bei Vollzahler Konfiguration? Wird dann bei Zusatzbeträgen auch das Konto des Vollzahler genommen? Wenn ja müsste der Check hier noch erweitert werden.

Ja, das stimmt. Es nur die Frage ob das wirklich so sein soll, dass auch bei abweichendem Zahlungsweg der Vollzahler zahlt, oder soll es in dem Fall das mitglied selbst sein?

@tolot27
Copy link
Member

tolot27 commented Nov 15, 2024

Wie ist das denn bei Vollzahler Konfiguration? Wird dann bei Zusatzbeträgen auch das Konto des Vollzahler genommen? Wenn ja müsste der Check hier noch erweitert werden.

Ja, das stimmt. Es nur die Frage ob das wirklich so sein soll, dass auch bei abweichendem Zahlungsweg der Vollzahler zahlt, oder soll es in dem Fall das mitglied selbst sein?

Wenn bei einem Mitglied ein Vollzahler konfiguriert ist, sollte das meiner Meinung nach für alle Zahlungen gelten. Bisher gibt es da ja noch keine Differenzierung und wenn ich es richtig sehe, läuft das bis jetzt auch schon so.
Wenn das gewollt ist, wäre das vielleicht ein anderer PR. Ursprünglich ging es bei dem PR ja um den Zahlungsweg und nicht wer zahlt - obgleich das natürlich zusammenhängt.

@lenilsas
Copy link
Author

Test hinzugefügt und Merge Konflikte behoben

@JohannMaierhofer
Copy link

JohannMaierhofer commented Nov 17, 2024

Jetzt habe ich noch einen Kommentar zu RechnungControl wenn das damit gemerged ist.
In RechnungControl Zeile 309 wird der Filter ohneabbucher behandelt. Er fragt nach dem Zahlungsweg des Mitglieds. Hier müsste bei Vollzahler aber dieser geprüft werden und bei Zusatzbeträgen jetzt auch was dort steht. Allerdings kann sich das dort ja auch seit der Erstellung der Rechnung geändert haben.
Man müsste hier eigentlich den Zahlungsweg der Sollbuchungen der Rechnung prüfen, also über die Sollbuchungen iterieren.
ohneabbucher wäre dann wenn mindestens eine Sollbuchungen nicht Basislastschrift ist.

Evtl. wäre es einfacher man würde bei Rechnungen nur Sollbuchungen zusammen fassen die die gleiche Zahlungsart haben. Dann könnte man die Zahlungsart in der Rechnung speichern und hat es hier mit dem Filter einfacher.
Das würde vielleicht sowieso Sinn machen. Oft schickt man eine Rechnung oder Mahnung nur für Rechnungen bei denen nicht per Lastschrift bezahlt wird. Wenn in der Rechnung aber Lastschrift und Überweisung gemischt ist bekommt man beides gemahnt.

PS: Es wäre vielleicht schön wenn man im Rechnung View bei klick auf eine Sollbuchung zur Sollbuchung springen könnte.

Der Kommentar wäre auch etwas für einen eigenen PR, siehe #468

@JohannMaierhofer JohannMaierhofer added the enhancement New feature or request label Nov 19, 2024
@tolot27
Copy link
Member

tolot27 commented Nov 19, 2024

Ich teste gerade das Feature. Sieht von der Umsetzung und Visualisierung ganz gut aus. Was jedoch nicht wie erwartet funktioniert, ist die kompakte Abbuchung wenn der Zahlungsweg auf Überweisung steht. Dann werden die Zusatzbeträge nicht zu einer Summe zusammengefasst, zumindest keine negativen und positiven (bei positiver Summe).
Da ich die E-Mails mit der Zahlungsaufforderung immer vor dem Abrechnungslauf versende und da die Überweisungssumme ausgegeben wird, haben die Leute schon überwiesen. Die beim Abrechnungslauf gebildeten mehreren Sollbuchungen lassen sich dann nicht automatisch der einen Überweisung zuordnen.
Hier wäre es schön, wenn "kompakte Abbuchung" funktionieren würde. Das könnte jedoch auch in einem separaten PR gemacht werden, da dieser eigentlich seinen gewünschten Zweck erfüllt. Dann müsste man vermutlich auch "kompakte Abbuchung/Überweisung" oder besser "kompakte Sollbuchung" schreiben.

@lenilsas
Copy link
Author

Hier wäre es schön, wenn "kompakte Abbuchung" funktionieren würde. Das könnte jedoch auch in einem separaten PR gemacht werden, da dieser eigentlich seinen gewünschten Zweck erfüllt. Dann müsste man vermutlich auch "kompakte Abbuchung/Überweisung" oder besser "kompakte Sollbuchung" schreiben.

Das ist nur eine kompakte Abbuchung, das heißt es wird eine Lastschrift für mehrere Sollbuchungen erstellt. Daher bleiben bei jedem Zahlungsweg mehrere Sollbuchungen bestehen. Ich würde empfehlen erst denAbrechnungslauf durchzuführen, anschließend die Rechnungen erstellen und versenden (in der Nigthly Version ist hier das Vorgehen anders als bisher)
Hierfür würde eher ein Featur benötigt um einer Buchung mehrere Sollbuchungen zuzuordnen.
In diesem PR wurde an dem Vorgehen nichts geändert.

@tolot27
Copy link
Member

tolot27 commented Nov 20, 2024

Ich würde empfehlen erst den Abrechnungslauf durchzuführen, anschließend die Rechnungen erstellen und versenden.

Das werde ich mir ansehen. Bisher habe ich nicht mit Rechnungen gearbeitet, da man da (bisher) immer ein PDF hatte, ich jedoch direkt im Nachrichtentext die Informationen haben wollte. Aber das ist OT hier.

Also von mir aus kann der PR merged werden.

@JohannMaierhofer
Copy link

JohannMaierhofer commented Nov 20, 2024

Also von mir aus an der PR merged werden.

Dann musst du nur noch das Review approven. Dann kann man mergen. Es geht nur mit zwei Appovals.

Ich sehe gerade, dass du kein Mitglied bist, dann kannst du das wohl nicht.

Vielleicht solltest du ein Issue mit der Bitte um Aufnahme stellen.

@tolot27
Copy link
Member

tolot27 commented Nov 20, 2024

Also von mir aus an der PR merged werden.
Vielleicht solltest du ein Issue mit der Bitte um Aufnahme stellen.

Kann ich machen. Allerdings kenne ich das bei anderen Projekt so, dass man dafür erstmal bisl was contributed haben muss und auch qualifizierte Reviews gemacht haben muss, bevor man die approval permission bekommt. Wenn ihr das anders handhabt, dann gern (#479).

@tolot27 tolot27 self-requested a review November 24, 2024 11:40
Copy link
Member

@tolot27 tolot27 left a comment

Choose a reason for hiding this comment

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

Da ich schon alles getestet habe und es soweit funktioniert, habe ich mich nun auf den Code Review beschränkt. Teilweise hast du Code-Formattierungen korrigiert, oft jedoch bei ifs selbst keine angewendet. Mir scheint, dass bei dir der code formatter nicht automatisch aktiv ist.
Das sieht jetzt irgendwie pingelig aus. Allerdings bräuchten wir sonst die Diskussion um Code Style (#384) nicht führen.

@lenilsas
Copy link
Author

Ich habe die Formatierung angepasst.
Die {} bei ifs sind nicht im Formatter enthalten, ich habe jetzt die "Clean Up" Funktion über den Code laufen lassen, das hat die fehlenden {} hinzugefügt, es sieht dadurch nach vielen Änderungen aus, ist alles aber nur Formatierung.

Copy link
Member

@tolot27 tolot27 left a comment

Choose a reason for hiding this comment

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

Für mich passt es so. Ich schlage jedoch vor, dass wir in Zukunft Formatierungen nur bei den geänderten Kodezeilen durchführen (Menüeintrag "Format Element"). Ansonsten könnte es viel schneller zu Merge Conflicts kommen, wenn jeder von uns bei seinen verschiedenen Branches ein Rebase macht. Das war auch meine ursprüngliche Intention. Ich wollte nicht gleich die ganzen Dateien "korrekt" formatiert sehen.

@JohannMaierhofer JohannMaierhofer merged commit 9127a4f into openjverein:master Nov 24, 2024
@lenilsas lenilsas deleted the zusatzbetrag_zahlungsweg branch November 28, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zahlungsweg als zusätzliche Spalte bei Sollbuchungen Zusatzbetrag mit anderem Zahlungsweg
3 participants