Skip to content
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

Conversation

Markusplay
Copy link
Collaborator

No description provided.

@Markusplay Markusplay self-assigned this Jan 28, 2025
@Markusplay Markusplay changed the title add code of honor alert KB-225-Block-campus-functionality-if-student-not-signed-Code-of-Honor Jan 28, 2025
@Markusplay Markusplay requested a review from niravzi January 30, 2025 22:17
const t = await getTranslations('global.menu');
const user = await getUserDetails();

if (!user?.codeOfHonorSignDate && !!user?.studentProfile) {
Copy link
Collaborator

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>
Copy link
Collaborator

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
@Markusplay Markusplay requested a review from niravzi January 31, 2025 23:11
@@ -90,5 +112,10 @@ export async function middleware(request: NextRequest) {
return intlMiddleware(request);
}

const honorRedirect = await CoHMiddleware(request);
Copy link
Collaborator

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?

@@ -76,8 +80,26 @@ const authMiddleware = (request: NextRequest) => {
return intlMiddleware(request);
};

const CoHMiddleware = async (request: NextRequest) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent naming

Copy link
Collaborator

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();
Copy link
Collaborator

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.

@@ -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);
Copy link
Collaborator

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

@niravzi niravzi merged commit 8f57d20 into dev Feb 7, 2025
1 check passed
@niravzi niravzi deleted the KB-225-Block-campus-functionality-if-student-not-signed-Code-of-Honor branch February 7, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants