-
-
Notifications
You must be signed in to change notification settings - Fork 121
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
fix page redirects across all pages #515
base: main
Are you sure you want to change the base?
Conversation
Co-authored by: Dylan Uribe <[email protected]> Co-authored by: Ann Pan <[email protected]>
This reverts commit 8339fea.
Co-authored-by: Dylan Uribe <[email protected]> Co-authored-by: Ann Pan <[email protected]>
Co-authored-by: Dylan Uribe <[email protected]> Co-authored-by: Ann Pan <[email protected]>
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.
Good Work!! Looks like you guys manually tested these changes, hope there was a UI test framework in place to test this. Nevertheless, the code and documentation is pretty clean!!
}; | ||
}; | ||
|
||
export default redirectUser; |
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.
Tested the PR by doing the following (all checks resulted as expected):
- Initially signing in as user ‘NONE’ which is the default user when first signing on to the app -> this correctly showed the ‘Access Denied’ UI as intended
- Initially signing in as user ‘STUDENT’ -> this correctly showed the ‘Access Denied’ UI as intended
- Initially signing in as user ‘TEACHER’ -> this correctly shows the class creation UI as intended
- Initially signing in as user ‘ADMIN’ -> this correctly displays the admin dashboard
Edge case to consider:
When updating the user privileges in Prisma, the UI does not update immediately as the link is still set to https://{CODESPACES}.app.github.dev/error. This could mean that logic may need to be added which would update the URL (‘error’, ‘admin’, ‘teacher’, ‘student’) once a user privilege is updated.
Mentioning @utsab for further view.
Checklist:
Update index.md
)Co-authored by: Dylan Uribe [email protected]
Co-authored by: Ann Pan [email protected]
Thank you to SenthilKumar Ilango [email protected] for mentoring us.
Resolved Issue #497 and refactored all page redirects. Redirects were previously handled with ctx.writeHead, but we replaced it with our method called redirectUser which uses the recommended page redirect instruction provided by next.js. (https://nextjs.org/docs/pages/api-reference/functions/get-server-side-props#redirect) Tested changes locally.
Closes #497