-
Notifications
You must be signed in to change notification settings - Fork 0
519 - User Authentication #552
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
Conversation
LexTruong
left a comment
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.
Played around with your changes and everything is functional! Before we pull into dev though, please remove any unnecessary comments, console.log()'s, whitespace, and imports.
|
|
||
| const createContext = ( | ||
| _opts: CreateAWSLambdaContextOptions<APIGatewayProxyEventV2>, | ||
| opts: CreateAWSLambdaContextOptions<APIGatewayProxyEventV2>, |
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 can remove opts since it's not being used.
| // Pass through to tRPC handler | ||
| const trpcHandler = awsLambdaRequestHandler({ | ||
| router: appRouter, | ||
| createContext, | ||
| }); |
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 can be moved outside the handler function definition.
| createContext, | ||
| }); | ||
|
|
||
| return trpcHandler(event); |
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 trpcHandler(event); | |
| return trpcHandler(event, {} as any); |
Quick fix to get rid of the warning about the context argument missing.
packages/db/src/env_test.ts
Outdated
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 file can be deleted after confirming it works.
packages/db/src/index.ts
Outdated
| /* FOR THIS FILE TO WORK, YOU MUST INCLUDE A .ENV FILE IN THE apps/next DIRECTORY | ||
| YOU CAN COPY THE ROOT .ENV FILE WITH DATABASE_URL, GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, & BETTER_AUTH_SECRET AND PASTE IT IN apps/next | ||
| */ |
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 will incorporate this into our documentation in a separate issue, so this can be removed.
packages/db/src/index.ts
Outdated
|
|
||
| // if (!process.env.DATABASE_URL) throw new Error("DATABASE_URL is not set"); | ||
|
|
||
| console.log("db/src/index.ts: DATABASE_URL:", process.env.DATABASE_URL); |
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.
Remove!
…atements, general cleanup
LexTruong
left a comment
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.
A few more tidying tasks. You can approve the suggestions if you'd like.
| // console.log("Starting sign in..."); | ||
| // console.log("Base URL:", process.env.NEXT_PUBLIC_BASE_URL); |
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.
| // console.log("Starting sign in..."); | |
| // console.log("Base URL:", process.env.NEXT_PUBLIC_BASE_URL); |
| @@ -1,13 +1,14 @@ | |||
| import { | |||
| awsLambdaRequestHandler, | |||
| CreateAWSLambdaContextOptions, | |||
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.
| CreateAWSLambdaContextOptions, |
|
|
||
| // Pass through to tRPC handler | ||
|
|
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.
| // Pass through to tRPC handler |
packages/api/src/auth/index.ts
Outdated
|
|
||
|
|
||
|
|
||
|
|
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.
packages/api/src/auth/index.ts
Outdated
|
|
||
| // console.log("DATABASE_URL:", process.env.DATABASE_URL); |
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.
| // console.log("DATABASE_URL:", process.env.DATABASE_URL); |
packages/db/drizzle.config.ts
Outdated
| if (!process.env.DATABASE_URL) throw new Error("DATABASE_URL is not set"); | ||
|
|
||
| console.log("drizzle.config.ts: DATABASE_URL:", process.env.DATABASE_URL); | ||
| // console.log("drizzle.config.ts: DATABASE_URL:", process.env.DATABASE_URL); |
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.
| // console.log("drizzle.config.ts: DATABASE_URL:", process.env.DATABASE_URL); |
LexTruong
left a comment
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.
You accidentally deleted the export for the trpc handler, so I had to add it back. Otherwise LGTM!
fix(frontend): sign in button at bottom of sidebar
Summary
Incorporated authentication with better-auth
Changes
Important - Local Replication
Closes #519