108 m01 add basic triage #111
Conversation
| @@ -34,6 +34,7 @@ class ClosureCodes: | |||
| def __init__(self, *args, **kwargs): | |||
| super().__init__(*args, **kwargs) | |||
| self.exercise_code = "" | |||
There was a problem hiding this comment.
Wieso hier nicht auch ID statt Code benutzen?
There was a problem hiding this comment.
hatte das erstmal nicht angefasst, weil für Triage egal. Würde ich mit in den weiter unten angesprochenen Refactoring PR reinwerfen
| patient = Patient.objects.create( | ||
| name="Max Mustermann", exercise=exercise, patientId=123456 | ||
| name="Max Mustermann", exercise=self.exercise, patientId=123456 |
There was a problem hiding this comment.
An anderen Stellen im Code schreiben wir "xyID" statt "xyId". Da sollten wir uns festlegen. Und eigentlich ist keines von beiden snake_case. Sollte es nicht "xy_id" sein?
There was a problem hiding this comment.
prinzipiell, ja. Sowas überall umzubenennen wäre dann aber glaube ich eher ein Refactoring PR, oder?
| if self.authenticate(token): | ||
| success, patientId = self.authenticate(token) | ||
| if success: | ||
| self.patient = Patient.objects.filter(patientId=patientId).first() |
There was a problem hiding this comment.
Ich glaube hier würde man Patient.objects.get(pk=patientId) verwenden, schaue hier: https://docs.djangoproject.com/en/1.8/topics/db/queries/#the-pk-lookup-shortcut
There was a problem hiding this comment.
sieht gut aus. Danke für den Hinweis :)
There was a problem hiding this comment.
Guck vorher bitte ob es auf Main bereits ne 0006 migration gibt. In diesem Fall bei dir lokal main rein mergen, dann bekommst du unterstützung von Django beim mergen von migrations
There was a problem hiding this comment.
Ist erledigt. keine Probleme in diesem Fall
There was a problem hiding this comment.
Wir gehen ja davon aus, dass verschiedene Triagierungsverfahren von den Trainern genutzt werden können. Deshalb sorge ich mich, dass wir uns zu sehr einschränken. Bei der Übungskonfiguration sollte m.E. irgendwo nen dict von Name auf Farbe sein, wir speichern uns den Namen in nen einfachen CharField
There was a problem hiding this comment.
Hatte das mit den Frontend Leuten so besprochen, dass die Farbe ein reines Frontend-Problem ist. Darum muss sich das Backend nicht kümmern.
There was a problem hiding this comment.
okay, wir schränken uns aber trotzdem von den namen ein. Das sollte alles reingepack werden können
There was a problem hiding this comment.
Jo ist richtig. Alles was möglich sein soll, muss in das Choices field rein. Das ist aber denke ich kein Problem und hat den Vorteil, dass es sauber spezifiziert ist und Typos abfängt
There was a problem hiding this comment.
Okay, dann mal bitte in main und gleich nen refactoring issue anlegen
No description provided.