-
Notifications
You must be signed in to change notification settings - Fork 1
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
Nollningar + Äventyrsuppdrag + Grupper #97
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sjukt bra pull request! Märks att du har tänkt noggrant och bra! Jag har mest kommenterat på smågrejer samt:
for var, value in vars(data).items():
setattr(nollning, var, value) if value else None
som jag tycker är superspännande. Fixa kommentarerna sen är detta en dunderrequest!
class AdventureMissionCreate(BaseSchema): | ||
title: str | ||
description: str | ||
points: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Borde vara en int eller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alla dem här som du kommenterade om de skulle vara ints funderade jag mycket över. Asså det beror lite på hur vi vill göra...
Det brukar liksom vara så att folk lägger 1-5 poäng som beskrivningen över hur många poäng för att uppdraget ska kunna ha dynamisk poäng.
Hur vill vi göra här? Vi hade kunnat göra en many-many där man kopplar grupper till uppdraget och så innehåller den modellen som är imellan hur många poäng grupper har på det uppdraget.
Eller har du någon idé?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dynamisk via max och min poäng, gör sedan many-many med grupper som klarat det och hur mkt poäng de fick.
id: int | ||
title: str | ||
description: str | ||
points: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
samma sak här
|
||
def create_adventure_mission(db: Session, data: AdventureMissionCreate): | ||
|
||
if len(data.title) > MAX_ADVENTURE_MISSION_NAME: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tror detta checkas automatiskt mha schemas om du har satt Annotated[str, MAX_ADVENT....] men detta fungerar också
if not adventure_mission: | ||
raise HTTPException(404, detail="Mission not found") | ||
|
||
for var, value in vars(data).items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crazy, fungerar detta för isf är det ny standard på hur vi implementerar sånt här!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tror jag snodde detta någonstans ifrån lol, men från min (limited) testning så hittade jag inget problem med det.
group = Group_DB(name=data.name, group_type=data.group_type) | ||
|
||
db.add(group) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
här använder du try catch men inte där uppe, varför? vi måste bestämma oss för ett sätt att göra det på!
) | ||
|
||
db.add(group_user) | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
samma sak här, vi måste bestämma oss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.