bkd-mba-fbi / webapp-schulverwaltung

JavaScript Web-Modul, das mit SLH.Evento Backend (REST-API) die Prozesse der Schulverwaltung online sicherstellt
https://bkd-mba-fbi.github.io/webapp-schulverwaltung/
MIT License
9 stars 0 forks source link

Tests erfassen oder bearbeiten (2PT) #335

Open fbufbi opened 2 years ago

fbufbi commented 2 years ago

Test erfassen nach Klick auf Plus: image image

Gewichtung (Faktor):

Punkte (maximal und korrigiert):

Test bearbeiten oder löschen nach Klick auf Stift: image

Validierung:

Commands: PUT /Courses/{id}/Tests/New body:

{
  "Tests": [
    {
            "Date": ,
            "Designation": ,
            "Weight": ,
            "IsPointGrading": ,
            "MaxPoints": ,
            "MaxPointsAdjusted": ,
    }
  ]
}

PUT /Courses/{id}/Tests/Update body (Eigenschaften liefern die angepasst werden müssen inkl. TestId):

{
  "Tests": [
    {
            "id": ,
            "Designation": ,
            "Weight": ,
    }
  ]
}

PUT /Courses/2511/Tests/Delete body:

{
  "TestIds": [{id}]
}
caebr commented 2 years ago

@schefbi Ich habe noch etwas Mühe mit dem Absetzen der Requests (via Postman). Ich erhalte immer 405 - Method Not Allowed. Ich bin nun nicht sicher, ob ich den Request noch falsch aufrufe oder ob generell noch was nicht geht. Könntest du das kurz verifizieren? Z.B. /Courses/9248/Tests/Update mit Body

{
  "Tests": [
    {
            "Id": 1,
            "Designation": "Updated",
            "Weight": 33
    }
  ]
}
schefbi commented 2 years ago

@caebr Bei mir funktioniert das. Welcher User benutzt du? Die Rollen gemäss Payload im Token sind bei mir wie folgt: "roles": "LessonTeacherRole;TeacherRole". Wie sieht es bei dir aus?

caebr commented 2 years ago

@schefbi Ich hatte User l1 verwendet. Habe nun direkt via Browser/Applikation probiert und da geht es. Dann scheint es nur ein Problem im Postman bei mir zu sein. Danke fürs Verifizieren.

fbufbi commented 2 years ago

@caebr image

  1. [x] Die unterstrichenen Überschriften (Bezeichnung, Gewichtung, Typ) sind als h6 formatiert. Sie sollten aber als Label formatiert sein und aussehen wie die Labels "Bezeichnung" und "Datum" (vgl. Mockup).
  2. [x] Labels wie Faktor, Maximal und Korrigiert entsprechen nicht dem Mockup, sie dürften keinen Einzug nach rechts haben.
  3. [x] Wir streichen "Verhältnis zu allen Tests" inkl. der Prozentangabe - bitte löschen. Grund: Bei der Erfassung eines neuen Tests wird das Verhältnis immer mit 100% ausgegeben. Erst nach dem Speichern und beim erneute Editieren stimmt die Angabe. So bringt sie nichts.
  4. [x] Die maximale Punktezahl ist auf 10 Punkte begrenzt. Die Punkte dürfen nicht begrenzt werden.
  5. [x] In der korrigierten Punktezahl ist es zwar möglich, eine höhere Zahl als beim Maximum einzugeben. Der Wert kann aber nicht übernommen werden, es passiert nichts. => Bitte eine Validierungsmeldung ausgeben.
  6. [x] Wenn ich eine maximale und eine korrigierte Punktezahl eingeben, speichere und den Test bearbeite, dann hat es die korrigierte Punktezahl in die maximale Punktezahl geschrieben. Die vorherige maximale Punktezahl ist nicht mehr ersichtlich. Dies dürfte nicht passieren.
  7. [x] Die Darstellung der Formel ist ungenügend. "Note =" und "+1" müssten mittig stehen (vgl. Mockup) - ist eine bessere Darstellung möglich?
caebr commented 2 years ago

@fbufbi In welchem Browser hast du getestet?

Bei uns ist die Darstellung in Chrome, Firefox und Safari ok:

bkd-test-add

Kann es sein, dass das die Styles von Evento reinspielen?

caebr commented 2 years ago

@fbufbi "Die vorherige maximale Punktezahl ist nicht mehr ersichtlich": Das ist so, weil das Backend MaxPointsAdjusted: null zurückliefert. Ist das noch ein Bug?

caebr commented 2 years ago

@fbufbi "In der korrigierten Punktezahl ist es zwar möglich, eine höhere Zahl als beim Maximum einzugeben. Der Wert kann aber nicht übernommen werden, es passiert nichts. => Bitte eine Validierungsmeldung ausgeben."

Ist das der Soll-Zustand? Also anders gesagt, es soll verhindert werden, dass der korrigierte Wert grösser ist als der maximale Wert?

fbufbi commented 2 years ago

@caebr Ich teste immer mit Edge, aber auch im Firefox ist die Darstellung bei mir gleich und nicht so wie in deinem Screenshot. @schefbi: Spielen da styles von unseren css mit rein, können wir das nächste Woche zusammen anschauen?

fbufbi commented 2 years ago

@fbufbi "In der korrigierten Punktezahl ist es zwar möglich, eine höhere Zahl als beim Maximum einzugeben. Der Wert kann aber nicht übernommen werden, es passiert nichts. => Bitte eine Validierungsmeldung ausgeben."

Ist das der Soll-Zustand? Also anders gesagt, es soll verhindert werden, dass der korrigierte Wert grösser ist als der maximale Wert?

@caebr IST-Zustand: Ist der korrigierte Wert höher als der maximale Wert, ist keine Speicherung möglich - es wird also bereits verhindert, dass der korrigierte Wert höher als der maximale Wert sein kann. Ich bin mir nicht mehr sicher, ob wir das mal mündlich besprochen haben, denn im Task steht es nirgends.

SOLL-Zustand: zu besprechen im nächsten Daily

caebr commented 2 years ago

@fbufbi "Ist der korrigierte Wert höher als der maximale Wert, ist keine Speicherung möglich"

Hmm, also ich kann momentan problemlos einen Test speichern bei dem MaxPointsAdjusted grösser ist als MaxPoints. Das wird meiner Meinung nach weder vom Frontend noch vom Backend verhindert.

Ja - besprechen wir das doch am Daily.

caebr commented 2 years ago

@fbufbi @schefbi

Ich habe gerade auch noch bemerkt, dass sich das Backend anders verhaltet, als ich bisher angenommen habe. Wenn ich z.B. ein PUT auf New oder Update mache mit folgenden Punkten

MaxPoints   5
MaxPointsAdjusted   4

dann liefert das Backend den neuen/upgedateten Test folgendermassen zurück:

MaxPoints   4
MaxPointsAdjusted null

Ist das die Idee?

fbufbi commented 2 years ago

@caebr Das erklärt das Verhalten, ist aber natürlich nicht die Idee. @schefbi Bug im Backend?

caebr commented 2 years ago

@caebr Das erklärt das Verhalten, ist aber natürlich nicht die Idee. @schefbi Bug im Backend?

IsPointGrading wird auch false zurückgeliefert obwohl ich true mitschicke. Jetzt bin ich grad etwas verwirrt.

mfehlmann commented 2 years ago

@schefbi Wir haben das Test erfassen/editieren Formular noch einmal getestet und folgendes gefunden.

IsPointGrading kann nur initial gesetzt werden, wenn der Test neu erfasst wird. Danach ist es nicht mehr möglich dies übers editieren zu ändern (Auch wenn noch keine Resultate erfasst sind).

Werden die MaxPoints und MaxPointsAdjusted eingegeben, wird MaxPoints von MaxPointsAdjusted überschreiben und die MaxPointsAdjusted werden dabei auf null gesetzt.

Siehe als Beispiel den Test mit der ID 8.

schefbi commented 2 years ago

@schefbi Wir haben das Test erfassen/editieren Formular noch einmal getestet und folgendes gefunden.

IsPointGrading kann nur initial gesetzt werden, wenn der Test neu erfasst wird. Danach ist es nicht mehr möglich dies übers editieren zu ändern (Auch wenn noch keine Resultate erfasst sind).

Werden die MaxPoints und MaxPointsAdjusted eingegeben, wird MaxPoints von MaxPointsAdjusted überschreiben und die MaxPointsAdjusted werden dabei auf null gesetzt.

Siehe als Beispiel den Test mit der ID 8.

@mfehlmann Ich konnte es auch Nachstellen und habe gleich 2 Bugs bei SLH eingegeben.

mfehlmann commented 2 years ago

@fbufbi Die Punkte welche in der Checkliste als abgeschlossen markiert sind, befinden sich bereits auf dem Master Branch.

fbufbi commented 2 years ago

@caebr Das Modal beim Löschen wird in Edge und auch Firefox nicht korrekt dargestellt - es ist einfach die Browser-Meldung, es sollte aber wie in der Absenzenverwaltung aussehen. Siehe Mockup oben mit der Box und dem grauem Layover: image

mfehlmann commented 2 years ago

@fbufbi Für das löschen der Tests wird jetzt ein Modal verwendet (wie bei der Absenzenverwaltung und bim publizieren der Tests). Die Änderungen sind auf dem Master Branch und bereit zum testen.

fbufbi commented 2 years ago

@mfehlmann Das Modal kommt jetzt richtig, aber der Text ist weg und es steht nur noch "tests.form.confirm": image Die Schaltflächen, v.a. jene für "Ja", sind zudem sehr schmal.

mfehlmann commented 2 years ago

@fbufbi War bei uns ein kleiner Fehler und ist jetzt behoben. Wir haben die Buttons ein wenig vergrössert.

Beide Änderungen sind bereits auf dem Master.

caebr commented 2 years ago

@fbufbi Die Grösse der Number-Input-Felder ist auch angepasst auf master

fbufbi commented 2 years ago

@caebr Das Erstellen von neuen Tests geht nicht (mehr): 404 Not Found bei PUT /Courses/0/Tests/New Antwort: Course does not exist Problem: Die mitgegebene id ist 0, dort müsste aber die id des Anlasses stehen: /Courses/{id}/Tests/New

caebr commented 2 years ago

@caebr Das Erstellen von neuen Tests geht nicht (mehr): 404 Not Found bei PUT /Courses/0/Tests/New Antwort: Course does not exist Problem: Die mitgegebene id ist 0, dort müsste aber die id des Anlasses stehen: /Courses/{id}/Tests/New

Das sollte nun wieder gehen

fbufbi commented 2 years ago

@caebr

  1. Test Typ Punkte: Ab einer maximalen / korrigierten Punktezahl >= 1000 kommt es zu einem Serverfehler => Bitte bei beiden Feldern eine Validierung für < 1000 einbauen: "Der Wert muss unter 1000 liegen."
  2. Wird eine Eigenschaft (Faktor, Punktezahl, Test gelöscht) verändert, wird die Tabelle nicht aktualisiert (erst mit F5)
  3. Wird die maximale oder korrigierte Punktezahl verändert, werden die Noten in der Tabelle nicht aktualisiert (auch nicht mit F5)
mburri commented 2 years ago

@caebr

  1. Test Typ Punkte: Ab einer maximalen / korrigierten Punktezahl >= 1000 kommt es zu einem Serverfehler => Bitte bei beiden Feldern eine Validierung für < 1000 einbauen: "Der Wert muss unter 1000 liegen."
  2. Wird eine Eigenschaft (Faktor, Punktezahl, Test gelöscht) verändert, wird die Tabelle nicht aktualisiert (erst mit F5)
  3. Wird die maximale oder korrigierte Punktezahl verändert, werden die Noten in der Tabelle nicht aktualisiert (auch nicht mit F5)

Wir haben jetzt die maximale Punktzahl auf 999 eingeschränkt.

Ebenfalls werden jetzt die Daten auf der Testübersicht aktualisiert - allerdings ist uns aufgefallen, dass das Backend die Noten nicht neu berechnet, wenn die maximale Anzahl Punkte ändert... ist das ein Fehler im Backend?

fbufbi commented 2 years ago

@mburri Testübersicht aktualisieren bei Tests mit Punkten: Wenn die Anzahl Punkte (maximal oder korrigiert) verändert wird, dann sollen für diesen Test (Achtung: nicht alle Tests) die Noten neu berechnet werden, sofern eine Punktzahl vorhanden ist. Diese Bedingung trifft in folgendem Fall nicht zu: Die Punkte leer sind, weil die Lehrperson die automatisch berechnete Note übersteuern will. In diesem Fall gibt es nichts neu zu berechnen. Request pro Schüler/in mit Punktezahl im betroffenen Test absetzen: PUT /Courses/{idCourse}/SetTestResult

mburri commented 2 years ago

@fbufbi das können wir schon machen - ich bin aber trotzdem der Ansicht, dass dies ein Fehler im Backend ist, und zwar aus folgendem Grund: Wenn das Backend zulässt, dass man die max. Punkte für einen Test - für welchen schon Punkte pro Schüler erfaasst wurden - anpasst, dann sollte das Backend auch dafür sorgen, dass die Daten im Test konsistent bleiben.

Dies ist aber aktuell offensichtlich nicht der Fall - das heisst, dass der Zustand des Backends nicht mehr valide ist.

Wie gesagt, wir können diese Request schon absetzen - schön finde ich es jetzt aber nicht wirklich

schefbi commented 2 years ago

@mburri Ich habe ein issue bei SLH erfasst.

fbufbi commented 2 years ago

@schefbi Das von dir im letzten Kommentar erwähnte Issue wurde noch nicht gelöst - welches ist es? Alles Andere Test OK.

schefbi commented 2 years ago

@schefbi Das von dir im letzten Kommentar erwähnte Issue wurde noch nicht gelöst - welches ist es? Alles Andere Test OK.

-> EVO-11435