-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
chore: github Oauth #276
chore: github Oauth #276
Conversation
@Girishbari is attempting to deploy a commit to the Sanchit Bajaj's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe recent updates revolve around transitioning from Google OAuth to GitHub OAuth for user authentication. These changes involve adapting backend API functions to support GitHub login, updating frontend components for GitHub integration, refining user lookup mechanisms by email, and introducing a new function for managing OAuth data storage. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Great job, @Girishbari! 🎉 Thank you for submitting your pull request. Your contribution is valuable and we appreciate your efforts to improve our project. Keep being the part of the community!
'We will promptly review your changes and offer feedback. Keep up the excellent work!
Kindly remember to check our Contributing Guidelines'
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.
Actionable comments posted: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/backend/auth.api.ts (5 hunks)
- src/components/pages/auth/login/index.tsx (2 hunks)
Additional comments not posted (1)
src/backend/auth.api.ts (1)
223-229
: RefactoredgetUserSession
to enhance clarity by fetching user details directly.Verification successful
The search results indicate that the only instance of
getUserSession
being called is in a commented-out line within the same file as its definition. This suggests that the function might not be widely used yet, or other usages are not captured in the provided output. To ensure thorough verification, I recommend conducting a more extensive search across the entire codebase to confirm that all instances ofgetUserSession
are updated to handle the new return type correctly.
The comprehensive search across all TypeScript files confirms that the only reference to
getUserSession
is within a commented-out line insrc/backend/auth.api.ts
. There are no active calls to this function in the codebase, which indicates that there are no issues with how the function is being used, as it is not currently in use.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `getUserSession` are updated to handle the new return type. # Test: Search for the function usage. Expect: Proper handling of the new return type. rg --type ts $'getUserSession'Length of output: 167
Script:
#!/bin/bash # Description: Perform a comprehensive search for all occurrences of `getUserSession` across the entire codebase to ensure they handle the new return type correctly. # Test: Search for the function usage in all TypeScript files. Expect: Proper handling of the new return type. rg --type ts -g '*.ts' 'await getUserSession()'Length of output: 126
src/backend/auth.api.ts
Outdated
const getSingleUser = async (email: string) => { | ||
try { | ||
const resp = await db.listDocuments(palettegramDB, usersCollection, [ | ||
Query.search("accountId", id), | ||
Query.search("Email", email), |
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.
Change in getSingleUser
to use email instead of ID aligns with OAuth standards but verify the use of Query.search
.
- Query.search("Email", email),
+ Query.equal("email", email),
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
const getSingleUser = async (email: string) => { | |
try { | |
const resp = await db.listDocuments(palettegramDB, usersCollection, [ | |
Query.search("accountId", id), | |
Query.search("Email", email), | |
const getSingleUser = async (email: string) => { | |
try { | |
const resp = await db.listDocuments(palettegramDB, usersCollection, [ | |
Query.equal("email", email), |
src/backend/auth.api.ts
Outdated
const loginWithGithub = async () => { | ||
try { | ||
account.createOAuth2Session( | ||
"github", | ||
`${process.env.NEXT_PUBLIC_BASE_URL}/feed`, | ||
`${process.env.NEXT_PUBLIC_BASE_URL}/login`, | ||
["repo", "user"], | ||
); | ||
/* const userSession = await getUserSession(); | ||
|
||
const resp = saveDataToDatabaseForOauth(userSession); */ | ||
} catch (error) { | ||
console.log(error); | ||
} |
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.
Introduced loginWithGithub
for GitHub OAuth. Consider removing or implementing the commented-out session handling and database saving code.
- /* const userSession = await getUserSession();
- const resp = saveDataToDatabaseForOauth(userSession); */
+ const userSession = await getUserSession();
+ const resp = saveDataToDatabaseForOauth(userSession);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
const loginWithGithub = async () => { | |
try { | |
account.createOAuth2Session( | |
"github", | |
`${process.env.NEXT_PUBLIC_BASE_URL}/feed`, | |
`${process.env.NEXT_PUBLIC_BASE_URL}/login`, | |
["repo", "user"], | |
); | |
/* const userSession = await getUserSession(); | |
const resp = saveDataToDatabaseForOauth(userSession); */ | |
} catch (error) { | |
console.log(error); | |
} | |
const loginWithGithub = async () => { | |
try { | |
account.createOAuth2Session( | |
"github", | |
`${process.env.NEXT_PUBLIC_BASE_URL}/feed`, | |
`${process.env.NEXT_PUBLIC_BASE_URL}/login`, | |
["repo", "user"], | |
); | |
const userSession = await getUserSession(); | |
const resp = saveDataToDatabaseForOauth(userSession); | |
} catch (error) { | |
console.log(error); | |
} |
</details>
<!-- suggestion_end -->
<!-- This is an auto-generated comment by CodeRabbit -->
src/backend/auth.api.ts
Outdated
const saveDataToDatabaseForOauth = async (session: Models.User<Models.Preferences>) => { | ||
try { | ||
const currentUser = await db.listDocuments(palettegramDB, usersCollection, [ | ||
Query.equal("email", session.email), | ||
]); | ||
|
||
let resp; | ||
|
||
if (currentUser.documents.length === 0) { | ||
const avatar = generateAvatar(session.name); | ||
resp = await db.createDocument(palettegramDB, usersCollection, ID.unique(), { | ||
email: session.email, | ||
fullName: session.name, | ||
isVerified: session.emailVerification, | ||
accountId: session.$id, | ||
username: session?.prefs?.username, | ||
avatarURL: avatar, | ||
}); | ||
} | ||
if (!resp) { | ||
throw new Error("Database not working"); | ||
} | ||
console.log(resp); | ||
return resp; | ||
} catch (error: any) { | ||
console.log(error.message); | ||
throw new Error(error.message); | ||
} |
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.
Tip
Codebase Verification
The additional verification aimed to identify active usages of the saveDataToDatabaseForOauth
function in the codebase, excluding comments. The results confirm that the only mentions of this function are in its definition and within a commented-out line. There are no active usages of this function in the non-commented code.
Given the absence of active usages of the saveDataToDatabaseForOauth
function in the codebase, it appears that the function is not currently integrated into the system. This situation raises concerns about the testing and error handling capabilities of the function as mentioned in the original review comment. It is crucial to address this issue to ensure the robustness of the OAuth data storage functionality.
- The function
saveDataToDatabaseForOauth
is defined but not actively used in the codebase. - This lack of integration might affect the testing and error handling of the function.
Analysis chain
New function saveDataToDatabaseForOauth
correctly handles OAuth data storage. Ensure robust testing for this critical functionality.
The verification process aimed to confirm the integration and error handling of the saveDataToDatabaseForOauth
function within the codebase. The search results from the rg
command indicate that the function is defined in src/backend/auth.api.ts
and is mentioned in a commented-out line where it seems to be used.
Given that the function appears in a commented-out line, it's unclear whether it is actively used in the codebase. This raises concerns about the actual integration and testing of the function. To ensure a thorough verification, it would be prudent to search for any active references or invocations of this function in non-commented code.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration and error handling of `saveDataToDatabaseForOauth`.
# Test: Search for the function usage and error handling patterns.
rg --type ts $'saveDataToDatabaseForOauth'
Length of output: 239
Script:
#!/bin/bash
# Description: Search for active usages of `saveDataToDatabaseForOauth` excluding comments.
rg --type ts 'saveDataToDatabaseForOauth' --glob '!*.spec.ts' | grep -v '^\s*//'
Length of output: 278
@@ -257,7 +257,7 @@ export default function LoginComponent() { | |||
className="px-4 py-2 flex flex-row text-sm md:text-base rounded border text-black bg-white transition duration-300 ease hover:bg-secondary hover:text-white items-center" | |||
onClick={(e) => { | |||
e.preventDefault(); | |||
loginWithGoogle(); | |||
loginWithGithub(); |
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.
Button text should reflect the actual functionality. Update "Login with Google" to "Login with GitHub".
- <span>Login with Google</span>
+ <span>Login with GitHub</span>
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
loginWithGithub(); | |
loginWithGithub(); |
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.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- README.md (1 hunks)
Additional Context Used
LanguageTool (9)
README.md (9)
Near line 58: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... ## 🤔 Why open source Contributing in open source increases your opportunities to work wi...
Near line 66: Unpaired symbol: ‘[’ seems to be missing
Context: ...? ## 💻 Tech Stack - Next.js - Next.js is an ope...
Near line 67: Unpaired symbol: ‘[’ seems to be missing
Context: ...web development framework. - TypeScript - TypeS...
Near line 68: Unpaired symbol: ‘[’ seems to be missing
Context: ...el programming language. - Tailwind CSS - Tailwind CSS...
Near line 69: Unpaired symbol: ‘[’ seems to be missing
Context: ...hout ever leaving your HTML. - Appwrite - Appwrite is an ...
Near line 85: Consider a different adverb to strengthen your wording.
Context: ...# 🔨 What does it do? UI designers are always looking for new inspiration, and a soci...
Near line 93: The phrase “A variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...ing their work and getting feedback. - A variety of ways to share work: Designers would be ...
Near line 93: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...rs would be able to share their work in a variety of ways, including by uploading images, cr...
Near line 139: Using many exclamation marks might seem excessive (in this case: 8 exclamation marks for a text that’s 4984 characters long)
Context: ... your contributions are truly invaluable! 🌟😄 [![Contributors](https://contrib....
Additional comments not posted (1)
README.md (1)
125-125
: Update the URL for creating issues to reflect the new repository.
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.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- README.md (1 hunks)
Additional Context Used
LanguageTool (9)
README.md (9)
Near line 74: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... ## 🤔 Why open source Contributing to open source increases your opportunities to work wi...
Near line 82: Unpaired symbol: ‘[’ seems to be missing
Context: ...? ## 💻 Tech Stack - Next.js - Next.js is an ope...
Near line 83: Unpaired symbol: ‘[’ seems to be missing
Context: ...web development framework. - TypeScript - TypeS...
Near line 84: Unpaired symbol: ‘[’ seems to be missing
Context: ...el programming language. - Tailwind CSS - Tailwind CSS...
Near line 85: Unpaired symbol: ‘[’ seems to be missing
Context: ...hout ever leaving your HTML. - Appwrite - Appwrite is an ...
Near line 101: Consider a different adverb to strengthen your wording.
Context: ...# 🔨 What does it do? UI designers are always looking for new inspiration, and a soci...
Near line 107: The phrase “A variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...ring their work and getting feedback. - A variety of ways to share work: Designers would be ...
Near line 107: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...rs would be able to share their work in a variety of ways, including by uploading images, cr...
Near line 151: Using many exclamation marks might seem excessive (in this case: 8 exclamation marks for a text that’s 5130 characters long)
Context: ... your contributions are truly invaluable! 🌟😄 [![Contributors](https://contrib....
Additional comments not posted (1)
README.md (1)
137-137
: Update the URL for creating issues to align with the new repository.
Related Issue
#141 : added github 0auth
Summary by CodeRabbit