Neue Datenbank Struktur #156
3 Participants
Notifications
Due Date
No due date set.
Blocks
#171 WIP: Bearbeiten und Löschen von Matches
liquid-development/game-tracker
Reference: liquid-development/game-tracker#156
Reference in New Issue
Block a user
Delete Branch "feature/88-neue-datenbank-struktur"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Zugehörige Issue(s)
Closes #88
Beschreibung
Änderungen
Zusätzliche Anmerkungen
@flixcoo @sneeex
also die main datenbank methoden sind jetzt alle ready.
Ich muss jetzt nurnoch
wenn euch das zu lange dauert, können das auch zwei neue issues werden.
(vorallem das mit dem daten export/import)
Ich würd das beides lieber noch hier drin machen, weil vor allem der import halt essentiell ist um mit reelen daten zu testen und nicht tausend matches, groups und player zu erstellen
okay top. Dann werde ich aber entweder heute oder morgen Abend mit dem ganzen fertig.
(Außer ihr habt noch viele Änderungen im Review)
bzgl review: wir werden deine code zerbersten
Ja macht das :)
das ist ja der leichte teil das dann zu fixen
Du darfst nicht so nett sein
Okay, dann bin ich jetzt nicht mehr nett.
Du Fettsack
:(
Freunde...
Ich glaube das ist es erstmal 😅
Ich freu mich auf 100000 Anmerkungen die ich fixen muss :D
Aber der wichtige Part ist fertig.
WIP: Neue Datenbank Strukturto Neue Datenbank StrukturReview
enumin den DTOs speichern und in der Datenbank dann glaub ich lieber als int oder als String enum (kommt dann wahrscheinlich sowas raus wieColors.blue.@@ -17,1 +17,4 @@"type": "string"},"description": {"type": ["string", "null"]Ich glaube es würde mehr Sinn machen, die
descriptionnicht nullable zu machen, sondern einfach durch leeren String zu ersetzen, weil wir uns dann viel null checken sparen@@ -18,0 +23,4 @@"required": ["id","createdAt","name"Bei
requireddanndescriptionhinzufügen@@ -18,0 +42,4 @@"type": "string"},"ruleset": {"type": ["string", "null"]Ruleset hier auch nicht null, da Game immer ein Ruleset brauch.
@@ -18,0 +45,4 @@"type": ["string", "null"]},"description": {"type": ["string", "null"]Hier auch wieder description nicht
nullsondernempty@@ -18,0 +48,4 @@"type": ["string", "null"]},"color": {"type": ["integer", "null"]Warum machst du die color als integer aber die rulesets als string? Ich würde entweder beides als
Stringoder beiden alsint(bevorzugt), und beim retrieven dann alsenumin den jeweiligen Klassen speichern.war ausversehen 😅😅
@@ -18,0 +51,4 @@"type": ["integer", "null"]},"icon": {"type": ["string", "null"]Ich find Icons prinzipell keine schlechte Idee, aber finde sie dann eher bei z.B. Gruppen sinnvoller, weil was für Icons will man bei Games machen? Also bei Gruppen könnte man halt je nach sozialen Aspekten n icon setzen, bei Games fällt mir dazu nicht viel ein
hätte sowas gedacht wie poker karten oder andere icons für die games.
Gruppen icons würde ich auch adden
Das Problem ist, wenn du sagst Pokerkarten als Icon, musst du ja total viele Icons irgendwo her bekommen, die sehr speziell sind (Spielkarten, Brettspiel, etc ..), da würde ich dann vllt eher auf sowas wie emojis setzen und da ist die auswahl die thematisch dazu passt ja begrenzt. Aber sonst kann @sneeex nochmal sagen was er dazu denkt
weiß nicht ob ich emojis geil finde, sehe aber das problem mit den icons
lass das sonst demnächst mal besprechen, was man für icons bräuchte
@@ -67,6 +136,12 @@"createdAt": {"type": "string"},"gameId": {Meinst du hier
matchId?bin gerade am handy, gucke später
ja meinte ich. Ist aber jetzt komplett aus dem Team array entfernt
@@ -68,2 +137,4 @@"type": "string"},"gameId": {"anyOf": [Warum hier nicht
Würde das konsequent machen, eins von beiden und dann das gesamte Schema durch.
@@ -70,0 +139,4 @@"gameId": {"anyOf": [{"type": "string"},{"type": "null"}Wenn es hier um ein
Matchund nicht um einGamegeht, darfMatch(Game) nichtnullsein, weil ein Spiel immer eine Spielvorlage haben muss@@ -326,0 +378,4 @@required String matchId,required String winnerId,}) async {// TODO: Implement winner persistenceImplementierung vergessen
Wir speichern keine winner mehr. Der code ist ein placeholder bis der aktuelle code nicht mehr auf das winner attribut referenced
Aber das funktioniert doch trotzdem. Du gibst je nach Spielmodus und Score in diesem Spielmodus ein
PlayerObjekt zurück. Da wir aktuell sowieso nur Spiele mit winner only haben, gibst du einfach allen den Score 0 und dem winner den score 1. Und danach wird der dann retrieved bzw gesetztJa aber dafür müsste ich doch erst komplett die rulesets implementieren
@@ -326,0 +385,4 @@/// TEMPORARY: Removes the winner of a match./// Currently does nothing - winner is not persisted.Future<bool> removeWinner({required String matchId}) async {// TODO: Implement winner persistenceImplementierung vergessen
@@ -39,3 +47,3 @@@overrideint get schemaVersion => 1;int get schemaVersion => 2;Habe beim ersten Starten der App diese Meldung bekommen:
Ich glaube das ist für Datenbankmigrationen gedacht, also wenn man das Schema nachträglich verändert, deswegen lass das mal auf 1.
ups das war claude dann 😅
@@ -0,0 +3,4 @@class GameTable extends Table {TextColumn get id => text()();TextColumn get name => text()();TextColumn get ruleset => text()();Gleiche anmerkung wie oben, würde ich über einen
intlösen@@ -0,0 +4,4 @@TextColumn get id => text()();TextColumn get name => text()();TextColumn get ruleset => text()();TextColumn get description => text().nullable()();Gleiches wie oben, nicht nullable machen sondern einfach als Standard n leeren String
@@ -3,6 +3,7 @@ import 'package:drift/drift.dart';class GroupTable extends Table {TextColumn get id => text()();TextColumn get name => text()();TextColumn get description => text().nullable()();Hier auch wieder nicht nullable machen sondern als leeren String setzen
@@ -7,0 +7,4 @@TextColumn get gameId =>text().references(GameTable, #id, onDelete: KeyAction.cascade)();TextColumn get groupId =>text().references(GroupTable, #id, onDelete: KeyAction.cascade).nullable()(); // Nullable if not part of a groupKommentar drüber setzten, damit die Zeile nicht so lang ist
@@ -7,0 +8,4 @@text().references(GameTable, #id, onDelete: KeyAction.cascade)();TextColumn get groupId =>text().references(GroupTable, #id, onDelete: KeyAction.cascade).nullable()(); // Nullable if not part of a groupTextColumn get name => text().nullable()();Name ist sollte nicht nullable sein, der wird im Frontend doch gesetzt wenn man keinen eigenen eingibt
@@ -7,0 +9,4 @@TextColumn get groupId =>text().references(GroupTable, #id, onDelete: KeyAction.cascade).nullable()(); // Nullable if not part of a groupTextColumn get name => text().nullable()();TextColumn get notes => text().nullable()();Hier auch nicht nullable, sondern empty string
@@ -3,6 +3,7 @@ import 'package:drift/drift.dart';class PlayerTable extends Table {TextColumn get id => text()();TextColumn get name => text()();TextColumn get description => text().nullable()();Hier auch nicht nullable sondern empty string
@@ -0,0 +5,4 @@final String id;final DateTime createdAt;final String name;final String? ruleset;Ruleset sollte nicht optional, weil das immer gebraucht wird
Ruleset als enum
@@ -0,0 +6,4 @@final DateTime createdAt;final String name;final String? ruleset;final String? description;Description als leeren string setzen, wenn nicht übergeben
warum findest du leeren string besser als nullable?
Finde null sollte man nur dann verwenden, wenns halt nicht anders geht, wie z.B. bei objekten
@@ -0,0 +7,4 @@final String name;final String? ruleset;final String? description;final int? color;Hier lieber enum, gerne auch mit einem
Color.nonewarum aber nicht hex codes?
oder willst du die farben vorgeben?
Ja man könnte auch hexcodes machen. Würde die Farben so oder so vorgeben unabhängig davon wie man sie speichert
würde enums maybe machen, weil man sonst im export einfach den hex code ändern könnte. Also idk ob das juckt aber 🤷♂️
@@ -0,0 +8,4 @@final String? ruleset;final String? description;final int? color;final String? icon;Hier auch mit enum arbeiten?
@@ -6,3 +6,3 @@final String id;final DateTime createdAt;final String name;final String? description;Nicht nullable sondern empty string
@@ -8,3 +9,3 @@final DateTime createdAt;final String name;final List<Player>? players;final Game? game;Game nicht optional sondern required
@@ -10,2 +11,3 @@final List<Player>? players;final Game? game;final Group? group;final List<Player>? players;Spielerliste auch nicht optional
@@ -0,0 +1,16 @@import 'package:game_tracker/data/dto/team.dart';class Pair extends Team {Ich weiß nicht, was wir besprochen hatten, aber macht nicht mehr sinn
Damit wir in normalen mathces trz eine
List<Player>haben könnnelass da freitag nochmal drüber reden
Äh ne ich glaub das ist irrellevant, das hatte ich vor 5 Tagen schonmal kommentiert aber das review nicht abgeschickt
ah okay
@@ -5,26 +5,33 @@ class Player {final String id;final DateTime createdAt;final String name;final String? description;Leerer String statt nullable
@@ -320,2 +153,2 @@expect(newWinner.createdAt, testPlayer5.createdAt);}final allGames = await database.gameDao.getAllGames();expect(allGames.length, 3);Hier bitte ergänzen das alle Parameter (id, name, ...) auch den ursprünglichen parametern entsprechen
Ja geb ich dir recht :) ich guck da morgen nochmal drüber.
immer noch keine beschreibung?
@@ -0,0 +67,4 @@/// Adds multiple [games] to the database in a batch operation./// Uses insertOrIgnore to avoid overwriting existing games.Future<bool> addGamesAsList({required List<Game> games}) async {(kommentar stelle ist irrelevant)
Warum gibt es die Funktion nicht zum updaten von Membern einer Gruppe? Siehe #88
Außerdem gibt es afaik auch keine Funktion zum updaten von Member eines Matches.
Also es geht nicht darum, dass man einzelne entfernen/hinzufügen kann, sondern eine Funktion die Member nimmt und dann diese Member als Member einer Group bzw. Player eines Matches setzt
(vielleicht hab ich auch übersehen?)
@@ -28,3 +28,3 @@if (!await db.playerDao.playerExists(playerId: player.id)) {db.playerDao.addPlayer(player: player);await db.playerDao.addPlayer(player: player);Der Kommentar bezieht sich nur auf die files
player_group_daoundgroup_dao: Warum gibts hier keine generierte g.dart Datei?Das gleiche gilt auch für group_dao
gibt es doch?
wo denn?

konnte sonst jetzt gerade nichts weiteres finden
Bugs
Review
Ich hab jetzt nochmal ein paar kleine Sachen angemerkt, die mir aufgefallen sind, @sneeex soll da auf jeden Fall auch nochmal drüber gucken, weil der PR ist ja jetzt schon echt riesig.
Eine Sache, wo ich mir jetzt immer noch nicht sicher bin wie und ob wir das brauchen sind die description bzw. icons. Ich finde descriptions bei Groups und Matches in Ordnung, aber für Players sehe ich irgendwie keinen Anwendungszweck. Außerdem ist das mit den icons auch ne sache, wo ich noch nicht ganz weiß, wie das dann am ende aussieht. @gelbeinhalb falls du da n konkreten Plan auch hast, wie das UI später aussehen soll, können wir die drin lassen.
@@ -67,11 +142,14 @@"createdAt": {"type": "string"},"endedAt": {Was soll
endedAtfür einen Sinn haben? Also wozu brauche ich den Endzeitpunkt eines Spiels? Und vor allem wie lege ich den Fest? Wenn ich denn Winner setze?Ich dachte, dass man den Zeitpunkt später braucht, wenn das Winner Attribut entfernt wird. Später soll das ja nur calculated werden basierend auf den scores der Spieler und nicht extra gespeichert werden. Dann braucht man ja einen Weg zu sagen, ob das Spiel fertig ist oder nicht. Einen
endedAtTimestamp fand ich besser als einen einfachenfinishedbooleanah okay, ja fair
@@ -0,0 +36,4 @@return Game(id: result.id,name: result.name,ruleset: Ruleset.values.firstWhere((e) => e.name == result.ruleset),Funktioniert das tatsächlich? Ist das getestet, dass dann auch ein ruleset (color) gefunden wird wenn du den enum speicherst?
Ja glaube das hat funktioniert
@@ -6,1 +7,3 @@late final winnerId = text().nullable()();TextColumn get gameId =>text().references(GameTable, #id, onDelete: KeyAction.cascade)();// Nullable if not part of a groupKommentar mach keinen Sinn, sollte eher heißen
Nullable if not group takes part in the matcho.ä.@@ -0,0 +16,4 @@DateTime? createdAt,required this.name,required this.ruleset,required this.description,Description sollte nicht obligatorisch sein, sondern optional. Wenn nicht gesetzt soll es ein leerer String werden.
Dachte man soll den als leeren String selber setzen müssen
würde das lieber so machen, weil man ja sonst immer unnötig die description setzten muss
finde auch optional
@@ -12,3 +13,4 @@String? id,DateTime? createdAt,required this.name,required this.description,Auch hier Beschreibung optional
.
@@ -11,1 +13,4 @@final Game game;final Group? group;final List<Player> players;final String notes;Notes optional
Dachte leerer String
s.o.
@@ -12,0 +11,4 @@String? id,DateTime? createdAt,required this.name,required this.description,Description optional
Wieso? Ich dachte wir machen alles als leeren String?
s.o.
Nochmal kurz hierzu, habe mit mathis kurz drüber gesprochen, wir lassen das drin.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.