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

Fehler SpendenbescheinigungAction #75

Closed
JohannMaierhofer opened this issue Nov 7, 2023 · 8 comments
Closed

Fehler SpendenbescheinigungAction #75

JohannMaierhofer opened this issue Nov 7, 2023 · 8 comments

Comments

@JohannMaierhofer
Copy link

Beim Testen von #70 ist mir ein Fehler aufgefallen.
Wenn man in der Liste der Spendenbescheinigungen eine Bescheinigung doppelt clicked sollte die Bescheinigung in einem View dargestellt werden. Das passiert aber nicht und es kommt die Meldung "Kein Mitgliedskonto ausgewählt".
Das gleiche passiert wenn man den Button "neu (manuell)" klicked. Hier sollte eine leere Bescheinigung im View angezeigt werden.
Ich habe das nach verfolgt und der Grund ist in den Zeilen 45..48 in der Datei SpendenbescheinigungAction.java.
Diese Zeilen sind im Originalrepository nicht enthalten. Wenn ich sie auskommentiere funktioniert es wieder wie es eigentlich gewünscht ist.
Ich weiß jetzt nicht warum dieser Check eingefügt wurde, jedenfalls führt das zu dem beschriebenen Problem.

@JohannMaierhofer JohannMaierhofer changed the title Fehler SpendenbescheinigungAktion Fehler SpendenbescheinigungAction Nov 7, 2023
@dippeal
Copy link
Member

dippeal commented Nov 7, 2023

Problem besteht seit openjverein Build 2.8.20. Siehe #20 und #21 und im Code e0b2c87

@JohannMaierhofer
Copy link
Author

Also wenn ich mir den Code anschaue dann bearbeitet er mehrere mögliche Eingangsparameter und erlaubt auch NULL.
Falls man also eine Prüfung vorab einbaut sollte sie wie folgt aussehen (evtl. bessere Fehlermeldung):

if (!(context == null || context instanceof Spendenbescheinigung || context instanceof MitgliedskontoNode || context instanceof Mitglied))
{
throw new ApplicationException("Falsches Element ausgewählt");
}

dippeal pushed a commit to dippeal/jverein that referenced this issue Nov 8, 2023
- Change context check in SpendenbescheinigungAction
- Remove context check in MitgliedskontoDetailAction
@dippeal
Copy link
Member

dippeal commented Nov 8, 2023

Also wenn ich mir den Code anschaue dann bearbeitet er mehrere mögliche Eingangsparameter und erlaubt auch NULL. Falls man also eine Prüfung vorab einbaut sollte sie wie folgt aussehen (evtl. bessere Fehlermeldung):

if (!(context == null || context instanceof Spendenbescheinigung || context instanceof MitgliedskontoNode || context instanceof Mitglied)) { throw new ApplicationException("Falsches Element ausgewählt"); }

Dein Code würde, meiner Meinung nach, nicht zum gewünschten Ergebnis führen. Habe einen PR #76 erstellt und die Fehler korrigiert.

@JohannMaierhofer
Copy link
Author

JohannMaierhofer commented Nov 8, 2023

Ich bin immer noch der Meinung, dass mein Vorschlag stimmt. Ich habe deine Änderung ausprobiert und damit funktioniert der Button "manuell(neu)" in dem View der Spendenbescheinigungen nicht. Der Button übergibt "null" an die Funktion. Sie erstellt dann eine leere Spendenbescheinigung. Also sollte das auch erlaubt sein.

@JohannMaierhofer
Copy link
Author

Natürlich könnte null auch bei einem Fehlerfall übergeben werden, was man dann damit nicht abfängt.
Eine ganz saubere Lösung wäre wohl beim Button neu(manuell) etwas anderes als nichts zu übergeben z.B. einen Text und den dann explizit zu akzeptieren. Damit bliebe die null ein Fehler.

@dippeal
Copy link
Member

dippeal commented Nov 8, 2023

Wird die if Abfrage dann überhaupt benötigt? Es wird doch in den anschließenden Zeilen nach != null und dem instanceof geprüft.

@JohannMaierhofer
Copy link
Author

Eigentlich ist die Abfrage nicht nötig.

@dippeal
Copy link
Member

dippeal commented Nov 8, 2023

PR #76 aktualisiert.
Wäre gut, wenn du der Organisation beitrittst (Beispiel #64) und die Code-Anpassungen validieren könntest.

MSchmalzl pushed a commit that referenced this issue Nov 8, 2023
* Fehler SpendenbescheinigungAction #75

- Change context check in SpendenbescheinigungAction
- Remove context check in MitgliedskontoDetailAction

* Fix: Remove context instance of validation

---------

Co-authored-by: Alexander Dippe <info@dippe-it.de>
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

No branches or pull requests

3 participants