Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
474ae6b to
a969991
Compare
a969991 to
606076b
Compare
3675919 to
9ea6210
Compare
2a012ce to
8eae0e1
Compare
4163a6a to
52e242f
Compare
junlarsen
left a comment
There was a problem hiding this comment.
Hard to say much about the logic but I think it makes sense?
Found a list of nits/code changes I would like though
apps/dashboard/src/app/(internal)/brukere/components/membership-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I think we can just delete these scripts? There is no OW4 API anymore or anything like that. That honestly goes for most if not all import scripts
There was a problem hiding this comment.
Feel free to delete this file then
| export function getStudyGrade(semester: number): number | ||
| export function getStudyGrade(semester: null | undefined): null | ||
| export function getStudyGrade(semester: number | null | undefined): number | null | ||
| export function getStudyGrade(semester: number | null | undefined): number | null { | ||
| if (!semester) { | ||
| return null | ||
| } |
There was a problem hiding this comment.
Why should we ever accept that semester is null or undefined here? It should be the callers responsibility.
There was a problem hiding this comment.
I think it's nice for a helper to allow inputs to be nullish. Regardless, I think it will help with avoiding cases like const grade = membership?.semester ? getStudyGrade(membership.semester) : null
There was a problem hiding this comment.
I heavily disagree that it should be the responsibility of the function to do this ¯_(ツ)_/¯
e1c0ed1 to
f590541
Compare
|
Afaik this is finished |
7c27ad7 to
0497d1b
Compare
There was a problem hiding this comment.
Feel free to delete this file then
| import { ActionIcon, Stack } from "@mantine/core" | ||
| import { IconX } from "@tabler/icons-react" | ||
|
|
||
| export function createDateInput<F extends FieldValues>({ |
There was a problem hiding this comment.
Lol did we not have a DateInput before? Only DateTime?
| const JANUARY = 0 | ||
| const AUGUST = 7 |
0497d1b to
e58be75
Compare
Merge activity
|
e58be75 to
1814774
Compare

Main changes:
BACHELOR_STUDYandMASTER_STUDYmemberships now last exactly one semester