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

Formularfeld Zähler & Formularverknüpfungen #72

Merged
merged 15 commits into from Dec 11, 2023

Conversation

dippeal
Copy link
Member

@dippeal dippeal commented Nov 5, 2023

  • Neues Formularfeld "zaehler" eingefügt
  • Feld "zaehler" wird in Formularübersicht als "Fortlaufende Nr."
    ausgegeben
  • Beim generieren von Formularen vom Typ Rechnung und Mahnung wird die Fortlaufende Nr.
    automatisch um den Wert 1 hochgezählt
  • Formulare können verknüpft werden
  • Bei verknüpften Formularen werden die zaehler gleichgesetzt und
    untereinander aktualisiert
  • Eine Vererbung der Verknüpfung ist nicht implementiert
  • In den Einstellungen > Rechnungen kann man die Mindestlänge der Zähler definieren
  • Bis die Mindestlänge erreicht wird werden "0" als Präfix angefügt

Zählerlänge Einstellung:
image

Übersicht der Formulare mit Spalte "Fortlaufende Nr.":
image

Ein Formular auf das mindestens eine Verknüpfung liegt:
image

Ein Formulare welches zu einem anderen eine Verknüpfung aufbaut:
image

image

Alexander Dippe added 2 commits November 5, 2023 18:43
- Neues Formularfeld "zaehler" eingefügt
- Feld "zaehler" wird in Formularübersicht als "Fortlaufende Nr."
ausgegeben
- Bei Formularen vom Typ Rechnung und Mahnung wird die Fortlaufende Nr.
automatisch hochgezählt
- Formulare können verknüpft werden
- Bei verknüpften Formularen werden die zaehler gleichgesetzt und
untereinander aktualisiert
- Eine Vererbung der Verknüpfung ist nicht implementiert
@willuhn
Copy link
Member

willuhn commented Nov 6, 2023

Gutes Patch und gut beschrieben. Ich würde hier mein Approval geben. Wir haben hier nur ein kleines Problem mit der Nummerierung der DB-Updates. Das hier trägt die Nummer 419. Du hast aber noch einige vorhergehende PRs, in denen ebenfalls diese Update-Nummer verwendet wurde. Die müssen numerisch streng aufeinander folgen.

Bei deinen bisherigen PRs blicke ich aber leider nicht mehr ganz durch. Sind da inzwischen einige redundant, weil du sie in kleineren Happen nochmal neu eingereicht hast?

Bevor wir hier #72 übernehmen, würde ich gern Ordnung in die bisherigen bringen:

Dein enormes Engagement in allen Ehren! Aber es ist wirklich schwer, das zu übernehmen, wenn sich die PRs so überschneiden. Können von den o.g. welche gelöscht werden? Kannst du bei den PRs bitte sicherstellen, dass sich die DB-Update-Nummern nicht überschneiden?

@dippeal
Copy link
Member Author

dippeal commented Nov 6, 2023

Bzgl. der Update Nummerierung würde ich wie von Nico in #65 (comment) beschrieben vorgehen. Es muss nur mal ein PR anfangen mit der 0419 - alles andere ergibt sich dann.

#58 kann #24 ablösen.

@willuhn
Copy link
Member

willuhn commented Nov 6, 2023

Bzgl. der Update Nummerierung würde ich wie von Nico in #65 (comment) beschrieben vorgehen. Es muss nur mal ein PR anfangen mit der 0419 - alles andere ergibt sich dann.

OK. Das können wir so machen.

#58 kann #24 ablösen.

Das heisst, zumindest #24 kann gelöscht werden?

@willuhn
Copy link
Member

willuhn commented Nov 6, 2023

Dann würde ich diesen PR hier als erstes übernehmen, da er gut zu überschauen ist.

willuhn
willuhn previously approved these changes Nov 6, 2023
@willuhn willuhn enabled auto-merge (squash) November 6, 2023 10:25
@dippeal
Copy link
Member Author

dippeal commented Nov 6, 2023

Bzgl. der Update Nummerierung würde ich wie von Nico in #65 (comment) beschrieben vorgehen. Es muss nur mal ein PR anfangen mit der 0419 - alles andere ergibt sich dann.

OK. Das können wir so machen.

#58 kann #24 ablösen.

Das heisst, zumindest #24 kann gelöscht werden?

Ja - vielleicht noch mal mit kpatzwald Rücksprache halten?

Copy link

@NicoB77 NicoB77 left a comment

Choose a reason for hiding this comment

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

Gefällt mir insgesamt gut. Zwei Kleinigkeiten habe ich angemerkt. Ergänzt Du das Handbuch? Meine Nachfolge als Schatzmeister (und sicher viele andere) schauen bestenfalls dort, aber nicht auf GitHub.

src/de/jost_net/JVerein/server/FormularImpl.java Outdated Show resolved Hide resolved
src/de/jost_net/JVerein/server/FormularImpl.java Outdated Show resolved Hide resolved
@dippeal
Copy link
Member Author

dippeal commented Nov 6, 2023

Gefällt mir insgesamt gut. Zwei Kleinigkeiten habe ich angemerkt. Ergänzt Du das Handbuch? Meine Nachfolge als Schatzmeister (und sicher viele andere) schauen bestenfalls dort, aber nicht auf GitHub.

Meinst du das GitBook? Wenn mir jemand sagt wie dort etwas hinzugefügt werden kann und wenn die Funktion in einem Build hinzugefügt wurde, ja.

@NicoB77
Copy link

NicoB77 commented Nov 7, 2023

Gefällt mir insgesamt gut. Zwei Kleinigkeiten habe ich angemerkt. Ergänzt Du das Handbuch? Meine Nachfolge als Schatzmeister (und sicher viele andere) schauen bestenfalls dort, aber nicht auf GitHub.

Meinst du das GitBook? Wenn mir jemand sagt wie dort etwas hinzugefügt werden kann und wenn die Funktion in einem Build hinzugefügt wurde, ja.

Das meine ich. Es wird automatisch mit dem master-Branch von https://github.com/openjverein/jverein-Book synchronisiert.

willuhn
willuhn previously approved these changes Nov 7, 2023
@dippeal
Copy link
Member Author

dippeal commented Nov 7, 2023

Neuer commit 14592f8

…ounter-and-linking

- Add new Update0420 database update file
@MSchmalzl
Copy link

Du lagst mit deinem Verdacht richtig. Nach Änderungen in Update0422 und FormularImpl lässt sich ein Formular wieder erstellen.

@dippeal
Copy link
Member Author

dippeal commented Dec 4, 2023

Bitte neu laden. Ich habe mit 187c04c die Anpassungen am formlink umgesetzt. Auch ist die Update0422 jetzt die 0424 um den merge-Konflikt aufzulösen.

@MSchmalzl
Copy link

Sind eigentlich die vielen neuen Felder durch diesen PR hinzugekommen oder durch einen anderen?

NeueFormularfelder

@dippeal
Copy link
Member Author

dippeal commented Dec 4, 2023

Sind eigentlich die vielen neuen Felder durch diesen PR hinzugekommen oder durch einen anderen?

NeueFormularfelder

Dieser PR bringt lediglich das Feld "zaehler" mit.

@MSchmalzl
Copy link

MSchmalzl commented Dec 4, 2023

Hatte ich so auch in Erinnerung dass das mal ein eigener PR war. War mir nur nicht mehr ganz sicher. Da scheint auch etwas nicht so zu passen. Ist aber ein anderes Thema.

@MSchmalzl
Copy link

MSchmalzl commented Dec 4, 2023

Ich habe mal DAU gespielt und es jetzt hinbekommen, dass ich ein Formular drin habe welches ich nicht mehr löschen kann.

de.willuhn.datasource.rmi.ObjectNotFoundException: object [id: 5, type: de.jost_net.JVerein.server.FormularImpl] not found at de.willuhn.datasource.db.AbstractDBObject.load(AbstractDBObject.java:270) at de.willuhn.datasource.db.DBServiceImpl.createObject(DBServiceImpl.java:394) at de.jost_net.JVerein.gui.input.FormularInput.initdefault(FormularInput.java:66) at de.jost_net.JVerein.gui.input.FormularInput.<init>(FormularInput.java:37) at de.jost_net.JVerein.gui.control.FormularControl.getFormlink(FormularControl.java:143) at de.jost_net.JVerein.gui.view.FormularDetailView.bind(FormularDetailView.java:42) at de.willuhn.jameica.gui.GUI$5.run(GUI.java:724) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67) at de.willuhn.jameica.gui.GUI$6.run(GUI.java:921) at org.eclipse.swt.widgets.Synchronizer.syncExec(Synchronizer.java:183) at org.eclipse.swt.widgets.Display.syncExec(Display.java:4785) at de.willuhn.jameica.gui.GUI.startSync(GUI.java:917) at de.willuhn.jameica.gui.GUI.startView(GUI.java:601) at de.willuhn.jameica.gui.GUI.startView(GUI.java:536) at de.willuhn.jameica.gui.GUI.startView(GUI.java:573) at de.jost_net.JVerein.gui.action.FormularAction.handleAction(FormularAction.java:53) at de.willuhn.jameica.gui.parts.AbstractTablePart.open(AbstractTablePart.java:382) at de.willuhn.jameica.gui.parts.TablePart$5.mouseDoubleClick(TablePart.java:683) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:200) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4256) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1066) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4054) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3642) at de.willuhn.jameica.gui.GUI.loop(GUI.java:938) at de.willuhn.jameica.gui.GUI.init(GUI.java:335) at de.willuhn.jameica.system.Application.init(Application.java:145) at de.willuhn.jameica.system.Application.newInstance(Application.java:87) at de.willuhn.jameica.Main.main(Main.java:78)

MasterSlave

Wenn ich jetzt das "Slave" Formular lösche kommt obige Situation zustande.

@dippeal
Copy link
Member Author

dippeal commented Dec 4, 2023

Was wäre denn deine Lösung für das Problem?

@MSchmalzl
Copy link

Man könnte bspw. beim löschen eines Formulares überprüfen ob es sich um ein verknüpftes Formular handelt. Wenn ja, dann löschen verhindern, ansonsten löschen.

@dippeal
Copy link
Member Author

dippeal commented Dec 5, 2023

Mit dem 3a9b529 habe ich das Löschen für verknüpfte Formulare verhindert.

Bildschirmfoto 2023-12-05 um 20 36 14 Bildschirmfoto 2023-12-05 um 20 39 09

@MSchmalzl
Copy link

Sieht sehr gut aus. Danke für die schnelle Implementierung.

@MSchmalzl
Copy link

Derzeit können bei mir Formulare mit sich selbst verlinkt werden. Wie ist hierzu die Meinung? Also beibehalten, korrigieren oder braucht man in bestimmten Situationen?

@willuhn
Copy link
Member

willuhn commented Dec 6, 2023

Fachlich macht es aus meiner Sicht keinen Sinn, ein Formular mit sich selbst zu verlinken. Ich sehe hier eher sogar die Gefahr von Rekursionen. Allerdings dachte ich, dass der Code ursprünglich genau dafür Prüfungen enthielt. In der Liste der verlinkbaren Formulare wurde die ID des aktuellen Formulars explizit ausgefiltert. Finde die Stelle aber nicht mehr. Kann es sein, dass das im Rahmen der Reviews wieder verloren gegangen ist?

@MSchmalzl
Copy link

Okay. Dann sollten wir das bei Gelegenheit ändern.

@dippeal
Copy link
Member Author

dippeal commented Dec 10, 2023

Mit ef296da habe ich das aktuelle Formular aus der Auswahl "Formularverknüpfung" entfernt.

image

@dippeal dippeal requested a review from NicoB77 December 10, 2023 14:11
Copy link

@MSchmalzl MSchmalzl left a comment

Choose a reason for hiding this comment

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

Von meiner Seite her wäre das Feature so jetzt in Ordnung.

@willuhn
Copy link
Member

willuhn commented Dec 10, 2023

Zu der letzten Änderung mit dem Entfernen des Formulars aus der Selectbox, welches gerade bearbeitet wird: Wäre es nicht sauberer, es schon beim Erstellen der Liste per WHERE-Bedingung auszuschließen, damit es gar nicht erst in der Selectbox landet? Dann muss es im Anschluß nicht wieder umständlich entfernt werden.

@dippeal
Copy link
Member Author

dippeal commented Dec 10, 2023

Zu der letzten Änderung mit dem Entfernen des Formulars aus der Selectbox, welches gerade bearbeitet wird: Wäre es nicht sauberer, es schon beim Erstellen der Liste per WHERE-Bedingung auszuschließen, damit es gar nicht erst in der Selectbox landet? Dann muss es im Anschluß nicht wieder umständlich entfernt werden.

FormularInput abstrahiert von SelectInput. Ich wollte SelectInput nicht umbauen nur für diesen einen Anwendungsfall.

@dippeal dippeal dismissed NicoB77’s stale review December 11, 2023 09:13

Code ist getestet und funktioniert. Für neue Anpassungen bitte einen issue oder PR eröffnen.

@willuhn willuhn merged commit 1feeee8 into openjverein:master Dec 11, 2023
@dippeal dippeal deleted the feature/form-counter-and-linking branch December 11, 2023 09:14
@dippeal
Copy link
Member Author

dippeal commented Dec 11, 2023

Als nächstes werde ich die Dokumentation erstellen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants