-
Notifications
You must be signed in to change notification settings - Fork 7
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
Require Login for group and event creation page #120
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
One comment, rest looks great! Thanks!
export async function requireUserSession(request: Request) { | ||
const session = await getUserSession(request); | ||
if (!session) { | ||
return redirect('/login'); |
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.
return redirect('/login'); | |
throw redirect('/login'); |
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.
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.
This is a pretty useful. Not having to check that session
exists every time saves a lot of typing :)
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.
LGTM
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.
LGTM!
@@ -7,6 +7,11 @@ import { Card } from '~/components/ui/containers'; | |||
import { Input, TextArea } from '~/components/ui/forms'; | |||
import { Button } from '~/components/ui/button'; | |||
import { H1 } from '~/components/ui/headers'; | |||
import { requireUserSession } from '~/modules/session/session.server'; | |||
|
|||
export async function loader({ request }: LoaderFunctionArgs) { |
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.
As a next step, we could also check here that the user has "admin" access to the group before we allow them to create a new event for that group!
Summary (1-2 sentences)
require login for pages that would need access control in the future
Details (reason and description of the changes)
requires login in New group page and New event page