Scrimmaging record API refactor #888
Conversation
There was a problem hiding this comment.
Left some thoughts! Great work overall. I noticed that it ran a bit slowly on my local, so I on a whim threw it into Perplexity and had it transform the logic to run using Django ORM and it is super speedy now (funny, I love AI). Pls address review comments and then re request review. Again, great work on this 😎
| wins = serializers.IntegerField() | ||
| losses = serializers.IntegerField() | ||
| ties = serializers.IntegerField() | ||
| Ranked = ScrimmageRecordSerializerHelper() |
There was a problem hiding this comment.
yknow I think i made this name and it was kinda lazy of me... what if renamed to ScrimmageRecordVariantSerializer or similar?
| // scrimmageType={ | ||
| // CompeteMatchScrimmagingRecordRetrieveScrimmageTypeEnum.All | ||
| // } | ||
| scrimmageType="All" |
There was a problem hiding this comment.
hmm i think it could be good to refactor these string constants to a typed constant! is there anywhere in autogen that provides such an enum? if not, make one and put it in /api/apiTypes.ts 😄
| interface WinLossTieProps { | ||
| scrimmageType: CompeteMatchScrimmagingRecordRetrieveScrimmageTypeEnum; | ||
| scrimmageType: "All" | "Ranked" | "Unranked"; // Adjust to match the new schema |
There was a problem hiding this comment.
good comment for me the reviewer to understand but future onlookers would have no idea what schema it refers to haha. probably best to remove comment (and make this field refer to your new typed constant)
|
oh btw @beshrislambouli before committing run |
lowtorola
force-pushed
the
beshr/scri_api
branch
from
0748072 to
e818c69
Compare
rebased cleanup
lowtorola
force-pushed
the
beshr/scri_api
branch
from
e818c69 to
93e6035
Compare
great work
solves #848