-
Notifications
You must be signed in to change notification settings - Fork 17
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
Sollbuchungen zusammenfassen, Rechnung überarbeitet #547
base: master
Are you sure you want to change the base?
Conversation
Ich finde diese Lösung gut. Das automatisch erstellen der Rechnungen aus dem RechnungView könnte man lassen, aber so abändern, dass es Rechnungen erstellt für Sollbuchungen die noch keine Rechnung haben. Also ähnlich wie das manuelle erstellen der Rechnung, nur halt für alle die die Filterkriterien erfüllen und noch keine Rechnung haben. Folgendes ist mir beim kurzen rumspielen aufgefallen:
|
entfernt
behoben
geändert
geändert
Ich habe es bisher so umgesetzt, dass die Splitbuchungen nach Buchungsart und Klasse gruppiert erstellt werden, also ggf. mehrere Sollbuchungspositonen eine Splitbuchung ergeben. Daher kann ich hier nicht den Zweck der einzelnen Sollbuchungsposition verwenden
Das ist die Frage ob man manuell Sollbuchungen und Sollbuchungspositionen erzeugen können soll, das haben wir ja auch schon wo anders diskutiert. Grundsätzlich denke ich sollten sie über den Abrechnungslauf erstellt werden und es ist kein manuelles erstellen nötig. Es kann jedoch fälle geben wo es sinnvoll wäre.
habe ich so umgesetzt das eine Sollbuchungsposition erzeugt wird.
Hier habe ich auch das automatische Spliten implementiert. Allerdings nur wenn eine Buchung einer Sollbuchung zugeordnet wird, bei allen SollbuchungsPositionen die Buchungsart gesetzt ist und die Beträge gleich sind. |
Ich bekomme jetzt eine Exception beim Abrechnungslauf: Buchungsart und Buchungsklasse sollten ein Long sein so wie in MitgliedskontoImp. |
Ja, da sollten wir uns weitere Meinungen einholen. |
src/de/jost_net/JVerein/gui/action/BuchungSollbuchungZuordnungAction.java
Show resolved
Hide resolved
Da bei der Sollbuchung die Buchungsart und Buchungsklasse noch bei alten Sollbuchungen ausgewertet wird müssen wir sie wohl belassen. Ich würde sie am GUI aber dann nur anzeigen wenn die Sollbuchung keine Sollbuchungspositionen hat. |
habe ich korrigiert, auch gleich beim JVereinZahler |
Ich überlege, ob ich per UpdateScript für alle bestehenden Sollbuchungen eine Sollbuchungsposition erstellen soll. Dan haben wir eine einheitliche Datengrundlage und müssen nicht die verschiedenen Versionen berücksichtigen. Das wäre dann allerdings ein Update das nicht rückgängig gemacht werden könnte (es sei den man lässt die Spalten vorerst in der DB bestehen ohne sie zu nutzen.) |
Ich habe das gleiche Problem mit der Anlagenkonto Checkbox beim Konto. Das boolean ist jetzt durch ein Integer ersetzt. Ich habe das boolean flag auch gelöscht. So kann man nicht mehr zurück. Ich müsste das Flag auch belassen. Ich habe das in #482 kommentiert. |
Ich habe noch einige Probleme entdeckt:
|
Mir gefällt die Art der Gliederung bei "Mitglieder Ansicht" oder "Mail" etwas besser bzw. viel mehr stört mich, dass die obere group box dynamisch ist und nicht die untere und so in der Mitte des Fensters eine große leere Fläche entsteht. |
execute(this.createForeignKey("fk_sollbuchungposition1", | ||
"sollbuchungposition", "sollbuchung", "mitgliedskonto", "id", | ||
"CASCADE", "NO ACTION")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mir erschließt sich gerade nicht, warum die Sollbuchungsposition eine Beziehung zum Mitgliedskonto hat, statt zur übergeordneten Sollbuchung. Das finde ich irgendwie nicht intuitiv. Kannst du mir das kurz erklären?
Falls es wirklich so notwendig ist, würde ich die Spalte nicht "sollbuchung" nennen, sondern auch "mitgliedskonto", wie bei Buchungen.
Außerdem könnte die Klasse einfach Buchungsposition heißen. Dann könnte man sie später sowohl für Soll- als auch für Haben- und Korrekturbuchungen und was auch immer für Buchungen verwenden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mitgliedskonto ist die Sollbuchung. Früher wurde sie Mitgliedskonto genannt. Ich habe sie dann zumindest am GUI in Sollbuchung umbenannt und auch einige Klassen und Actions.
Ich plane noch weitere Umbenennung, aber bei den DB Attributen ist das schwierig weil man dann nicht mehr zurück kann und alle SQL Strings durchsuchen muss.
Aber neue Attribute sollten gleich richtig benannt werden. Darum finde ich das hier schon richtig.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aber gibt es in Zukunft wirklich nur Sollbuchungen? Du hast selbst auch schon den Terminus Korrekturbuchung und Gegenbuchung verwendet. Dazu könnte es auch Buchungspositionen geben. Könnte die Tabelle und Klasse durchaus einfach "buchungsposition" heißen.
Ich sehe die Mitglieder primär als Kreditoren an. Für Sponsoringrechnungen und Lieferanten (also Debitoren) bräuchte man auch Adressen. Das kann man als Nichtmitglieder abbilden.
Sinnvoller fände ich, wenn wir die Adress-/Mitgliederverwaltung von dem buchalterischen trennen würden und Buchungen dann in einer extra m:n-Tabelle erfassen würden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deinen Vorschlag verstehe ich nicht ganz, könntest du genauer beschreiben was du mit trennen und n:m meinst.
Das mit den Kreditoren und Debitoren klingt für mich etwas nach doppelter Buchführung. In der einfachen Buchführung gibt es diese nicht. Hier zählt nur das Zuflussprinzip (Buchung).
Damit JVerein die Vereinsmitgieder und andere Personen unter anderem für Forderungen verwalten kann gibt es dann getrennt die Mitgiederverwaltung. Da Forderungen in der einfachen Buchführung nicht in der Buchführung auftauchen (dürfen) sind sie bei uns in der Mitgliederverwaltung als Sollbuchung untergebracht. Und um zu sehen ob jemand seine Forderungen beglichen hat gibt es die Zuordnung von Buchung zu Sollbuchung (Forderung).
Für mich ist da die Trennung von Buchführung und Mitgiederverwaltung gegeben. Und die Sollbuchungen können damit auch keine Buchungen sein. Bei einer Splittbuchung einer Buchung sind die Buchungpositionen auch Buchungen. Bei der Sollbuchung sind die Sollbuchungposition aber weder Sollbuchungen noch Buchungen.
Das ist jedenfalls meine Vorstellung.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aber gibt es in Zukunft wirklich nur Sollbuchungen?
Wie beschrieben gibt es Sollbuchungen und natürlich auch Buchungen.
Oh, gut dass du den Fehler gefunden hast, das waren 700% oder 1900% Steuer :) |
} | ||
if (!splitten) | ||
{ | ||
// Wenn kein automatisches Spliten m�glich ist nur Buchungsart, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deine Änderung hat nicht gereicht. Ich versuche eine Buchung zwei Sollbuchungen zuzuweisen. Die erste hat Positionen ohne Buchungsart. Man landet jetzt hier und immerSplitten ist gesetzt. Es wird aber nicht gesplittet. In BuchungSollbuchungZuordnungAction wird da hier null zurückgeliefert wird eine Buchung erzeugt und angenommen, dass die Buchung gesplittet ist, was sie aber nicht ist. Dann gibts die Exception unten.
Vermutlich muss man hier wenn immerSplitten gesetzt ist auf Sollbuchungsebene splitten.
[Sun Jan 12 11:41:32 CET 2025][ERROR][main][de.jost_net.JVerein.gui.action.BuchungSollbuchungZuordnungAction.handleAction] Fehler
java.lang.NullPointerException: Cannot invoke "java.lang.Integer.equals(Object)" because the return value of "de.jost_net.JVerein.rmi.Buchung.getSplitTyp()" is null
at de.jost_net.JVerein.io.SplitbuchungsContainer.getSumme(SplitbuchungsContainer.java:146)
at de.jost_net.JVerein.io.SplitbuchungsContainer.store(SplitbuchungsContainer.java:187)
at de.jost_net.JVerein.gui.action.BuchungSollbuchungZuordnungAction.handleAction(BuchungSollbuchungZuordnungAction.java:180)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dann sehe ich noch ein paar Probleme. Man müsste dann hier auch noch abfangen ob es eine Haupt- oder Gegenbuchung ist. Die wird ja nicht nochmal gesplittet, so wie es oben geprüft wird.
Wenn eine Haupt- oder Gegenbuchung selektiert ist, dann wird hier oder in Zeile 444 oben eine null zurückgegeben. In BuchungSollbuchungZuordnungAction wird in der Schleife wegen der null nun eine Buchung erzeugt und versucht der nächsten Sollbuchung zuzuweisen. Da dies eine Splittbuchung ist passieren seltsame Dinge. Hier sollte man aber wohl ganz abbrechen.
Wahrscheinlich sollte man es gleich nicht erlauben eine Haupt- oder Gegenbuchung mehreren Sollbuchungen zuzuweisen und dies gleich in BuchungSollbuchungZuordnungAction abfangen.
Nun stimmt es fast. Im PDF wird es korrekt angezeigt, im XML jedoch nicht: Hier scheint eine Vorwärtsberechnung der MwSt. aus dem gerundeten Nettobetrag zu passieren: 63,87 * 19% = 12,1353, gerundet 12,14. Wenn man den ungerundeten Nettobetrag verwendet, passt es wieder: 63,8655 * 19% = 12,1345. Intern rechnen wir mit Double, statt BigDecimal oder MonetaryAmount. Das führt zu den Rundungsproblemen: http://www.javapractices.com/topic/TopicAction.do?Id=213. Das ist mir auch schon früher bei Splitbuchungen mit USt-Berechnung aufgefallen. |
Ich hab jetzt Steuerbetrag und Nettobetrag zu BigDecimal geändert. Bei mir stimmt es jetzt. |
{ | ||
betrag = sp.getBetrag(); | ||
} | ||
BigDecimal betrag = sp.getNettobetrag(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leider kommt im XML immer noch der falsche Umsatzsteuerbetrag heraus, da der Steuerbetrag aus der gerundeten Nettosumme errechnet wird und nicht aus dem ursprünglichen Bruttobetrag (-76 €), so wie bei der Sollbuchungsposition:
Die Berechnung sollte überall in JVerein gleich sein, d. h. bruttobetragsorientiert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich glaube, ein Problem könnte auch sein, dass wir den XML den Nettobetrag übergeben müssen. Die Mustang Library rechnet sich dann selbst den Steuerbetrag aus.
Wir müssten also wissen wie sie das machen damit wir den Nettobetrag so ausrechnen, dass sich dann mit ihrer Steuerberechnung wieder der korrekte Bruttobetrag ergibt.
Besser wäre es wahrscheinlich man könnte der Library alternativ den Bruttobetrag übergeben und sie splitten dann selbst. Dann würde zumindest der Bruttobetrag stimmen.
Oder noch besser wäre es natürlich wenn man gleich beide Werte an das XMP übergeben könnte. Dann wären die Werte in JVerein und im XML sicher gleich.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe da mal bisl recherchiert. Leider scheint es dafür nicht wirklich eine Lösung zu geben, siehe ZUGFeRD/mustangproject#641. Als Idee habe ich noch, dass wir die Werte entweder ungerundet oder auf vier Stellen nach dem Komma gerundet an Mustang übergeben. Alternativ könnte man mit z. B. einer bestimmten Anzahl an signifikanten Stellen (Vor- und Nachkommastellen) arbeiten, wie es bei der Einführung des Euros war (sechs signifikante Stellen). Vier oder sechs Nachkommastellen halte ich jedoch für sinnvoller.
Allerdings würde ich das hier komplett rausnehmen, da nicht wirklich Teil dieses PR. Das könnte man auch im Nachgang lösen. Sonst werden wir hier nie fertig.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich hab in ZUGFeRD/mustangproject#480 gefunden, dass es nun die Funktion invoce.setRoundingAmount()
gibt. Damit könnten wir den Fehlenden Cent hinzuzählen oder abziehen wenn ich das richtig verstanden habe. Dann müsste man nur selber beide Varianten Rechnen und prüfen ob das nötig ist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja, dieses Ticket habe ich auch gesehen. Das ist jedoch nicht dafür gedacht. Wichtig wäre, dass in der Mustang-Lib korrigiert wird, insbesondere ZUGFeRD/mustangproject#641 (comment).
Ich kann momentan mit diesem Problem leben, da es noch nicht relevant ist. Wir sind ja eigentlich C2C oder C2B.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das es nicht dafür gedacht ist weiß ich, aber mann könnte es dazu missbrauchen. Wir können es aber auch erstmal so lassen un später nochmal anschauen.
} | ||
return betrag / (1 + steuersatz / 100); | ||
return new BigDecimal(betrag / (1 + steuersatz / 100)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hier wird die ganze Berechnung nach wie vor mit Double durchgeführt und erst das Ergebnis in BigDecimal konvertiert. Wenn, dann müssten die einzelnen Variablen und Konstanten auch als BigDecimal deklariert werden.
Allerdings habe ich darüber nochmal nachgedacht und wir werden nicht in die Rundungsproblematik "reinlaufen", wenn wir nur auf zwei Nachkommastellen runden. Daher bitte ich dich, den Commit wieder rückgängig zu machen und nur die Rundung auf zwei Nachkommastallen zu verwenden. Entschuldige bitte!
} | ||
return betrag - betrag / (1 + steuersatz / 100); | ||
return new BigDecimal(betrag - betrag / (1 + steuersatz / 100)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hier würde ich rechnen: Double steuerbetrag = betrag * steuersatz / (100 + steuersatz)
. Das Runden dann entweder wie in der RechnungMap per CurrencyFormatter oder doch via BigDecimal
:
Double steuerbetrag = brutto * steuersatz / (100 + steuersatz);
BigDecimal bd = new BigDecimal(Double.toString(steuerbetrag));
steuerbetrag = bd.setScale(2, RoundingMode.HALF_UP).doubleValue();
Leider Java keine native Rundungsfunktion, die sauber funktioniert und eine neue Lib hinzuzufügen, ist es glaub ich nicht wert.
Auf jeden Fall würde ich numerische Werte zurückgeben und keine Strings. Da es hier gerundet ist, kann man es einfach in FormularAufbereitung
nutzen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double steuerbetrag = betrag * steuersatz / (100 + steuersatz)
Mathematisch ist das das gleiche, wenn du meinst dass es mit dem runden besser ist kann ich es aber auch so verwenden.
{ | ||
betrag = sp.getBetrag(); | ||
} | ||
BigDecimal betrag = sp.getNettobetrag(); | ||
if (sp.getBetrag() < 0) | ||
{ | ||
invoice.addItem(new Item( | ||
new Product(sp.getZweck(), "", "LS", | ||
new BigDecimal(sp.getSteuersatz()).setScale(2, | ||
RoundingMode.HALF_DOWN)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das müsste eigentlich RoundingMode.HALF_UP
sein. Da hoffentlich in SollbuchungPositionImpl
gerundet wird (siehe weiter unten), brauchen wir hier gar nicht mehr zu runden und die ganze Arithmetik bleibt zusammen und ohne Code Duplications.
This reverts commit 63d161a.
if (sp.getBetrag() < 0) | ||
{ | ||
invoice.addItem(new Item( | ||
new Product(mk.getZweck1(), "", "LS", | ||
new BigDecimal(mk.getSteuersatz()).setScale(2, | ||
new Product(sp.getZweck(), "", "LS", | ||
new BigDecimal(sp.getSteuersatz()).setScale(2, | ||
RoundingMode.HALF_DOWN)), | ||
new BigDecimal(betrag * -1).setScale(2, RoundingMode.HALF_DOWN), | ||
betrag.multiply(new BigDecimal(-1)), | ||
new BigDecimal(-1.0))); | ||
} | ||
else | ||
{ | ||
invoice.addItem(new Item(new Product(mk.getZweck1(), "", "LS", // LS = | ||
// pauschal | ||
new BigDecimal(mk.getSteuersatz()).setScale(2, | ||
invoice.addItem(new Item(new Product(sp.getZweck(), "", | ||
"LS", // LS = pauschal | ||
new BigDecimal(sp.getSteuersatz()).setScale(2, | ||
RoundingMode.HALF_DOWN)), | ||
new BigDecimal(betrag).setScale(2, RoundingMode.HALF_DOWN), | ||
betrag, | ||
new BigDecimal(1.0))); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das lässt sich auch stark reduzieren, indem man einfach BigDecimal.abs()
verwendet. Dann braucht man kein if
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kompakt sieht es so aus:
// Sollbuchungspositionen
for (SollbuchungPosition sp : re.getMitgliedskonto()
.getSollbuchungPositionList())
{
BigDecimal betrag = sp.getNettobetrag();
invoice.addItem(new Item(
new Product(sp.getZweck(), "", "LS", // LS = pauschal
new BigDecimal(sp.getSteuersatz()).setScale(2, RoundingMode.HALF_UP)),
betrag.abs().setScale(4, RoundingMode.HALF_UP), new BigDecimal(betrag.signum())));
}
Leider haben die vier Nachkommastellen auch nichts gebracht.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich komme jetzt nicht mehr ganz mit. Mit der Rundungsproblematik habe ich bisher wenig Erfahrung.
Wie meinst du soll ich es jetzt machen? getSteuerbetrag und getNettoBetrag als Double oder als BigDecimal?
Und dann den berichtigte Code von dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich meine konkret, SollbuchungPosition.java
und SollbuchungPositionImpl.java
wieder revertieren, also Double
statt BigDecimal
.
Dann 63d161a#r1919399140 umsetzen, da es die Bruttoberechnungsmethode ist. Und zum Schluss noch #547 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Habe ich umgesetzt. Damit sollten jetzt alle Kommentare eingearbeitet sein.
Wie in #513 besprochen habe ich den Workflow bei den Rechnungen überarbeitet.
Außerdem habe ein folgende Änderungen gemacht die nicht direkt mit der Rechnung zusammenhängen
Ein Paar Änderungen fehlen noch
-Die AbrechnungslaufView müssen noch die neuen Parameter angezeigt werden
Gerne würde ich von euch schon mal hören, was ihr von dem aktuellen Stand haltet