-
Notifications
You must be signed in to change notification settings - Fork 9
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
KB-225-Block-campus-functionality-if-student-not-signed-Code-of-Honor #355
KB-225-Block-campus-functionality-if-student-not-signed-Code-of-Honor #355
Conversation
const t = await getTranslations('global.menu'); | ||
const user = await getUserDetails(); | ||
|
||
if (!user?.codeOfHonorSignDate && !!user?.studentProfile) { |
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.
Let's try to move this logic to middleware and create a separate page with this dialog.
</AlertDialogHeader> | ||
<AlertDialogFooter> | ||
<Link href="/"> | ||
<AlertDialogCancel>{t('button.go-to-home')}</AlertDialogCancel> |
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.
No cancel option. Only sign.
…not-signed-Code-of-Honor # Conflicts: # src/app/[locale]/(private)/profile/profile.tsx
…not-signed-Code-of-Honor # Conflicts: # src/app/[locale]/(private)/profile/profile.tsx
…y-if-student-not-signed-Code-of-Honor' into KB-225-Block-campus-functionality-if-student-not-signed-Code-of-Honor
src/middleware.ts
Outdated
@@ -90,5 +112,10 @@ export async function middleware(request: NextRequest) { | |||
return intlMiddleware(request); | |||
} | |||
|
|||
const honorRedirect = await CoHMiddleware(request); |
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.
Why are you checking code of honor before checking that user is authenticated?
src/middleware.ts
Outdated
@@ -76,8 +80,26 @@ const authMiddleware = (request: NextRequest) => { | |||
return intlMiddleware(request); | |||
}; | |||
|
|||
const CoHMiddleware = async (request: NextRequest) => { |
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.
Inconsistent naming
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.
Why shortened page path? accept-code-of-honor
is ok IMO
const [, setUser] = useLocalStorage<User>('user'); | ||
|
||
const handleAcceptCodeOfHonor = async () => { | ||
const res = await acceptCodeOfHonor(); |
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.
We do not utilize sign date stored in localStorage, and besides that we're planning to get rid of localStorage usage anyways. So I think we don't need to re-request user details here and store user in localStorage after signing code of honor.
src/middleware.ts
Outdated
@@ -32,6 +35,7 @@ const publicPathRegExp = composePathsRegExp([ | |||
const isRoot = (request: NextRequest) => rootRegExp.test(request.nextUrl.pathname); | |||
const isPublicPath = (request: NextRequest) => publicPathRegExp.test(request.nextUrl.pathname); | |||
const isAuthPath = (request: NextRequest) => authPathRegExp.test(request.nextUrl.pathname); | |||
const isAcceptHonorPath = (request: NextRequest) => honorPageRegExp.test(request.nextUrl.pathname); |
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.
Also a bit inconsistent naming. isAcceptCodeOfHonorPath
No description provided.