[feat]: refactor supabase database service with user linking to github #81
Conversation
WalkthroughThis update introduces a comprehensive GitHub verification flow integrated with Supabase and Discord. It adds new FastAPI endpoints for OAuth callbacks and session status, refactors Supabase authentication to support asynchronous operations and better error handling, updates database models and schema, and enhances the Discord bot with verification commands, periodic cleanup, and improved user feedback. Changes
Sequence Diagram(s)sequenceDiagram
participant DiscordUser
participant DiscordBot
participant BackendAPI
participant Supabase
participant GitHub
DiscordUser->>DiscordBot: !verify_github command
DiscordBot->>BackendAPI: Create verification session
BackendAPI->>Supabase: Get or create user/session
BackendAPI-->>DiscordBot: Session ID
DiscordBot->>DiscordUser: DM with OAuth link (includes session ID)
DiscordUser->>GitHub: Authorize via OAuth
GitHub->>BackendAPI: Redirect with code & session ID (/callback)
BackendAPI->>Supabase: Exchange code for user info
BackendAPI->>Supabase: Link GitHub to user, verify session
BackendAPI-->>DiscordUser: HTML success/failure page
DiscordUser->>DiscordBot: !verification_status command
DiscordBot->>BackendAPI: Query session/user status
BackendAPI-->>DiscordBot: Status info
DiscordBot-->>DiscordUser: Status embed
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
backend/.env.example (1)
5-6: Consider reordering environment variables alphabeticallyThe static analysis tool correctly identifies that
SUPABASE_SERVICE_ROLE_KEYshould come beforeSUPABASE_URLfor alphabetical ordering.-SUPABASE_URL= -SUPABASE_SERVICE_ROLE_KEY= +SUPABASE_SERVICE_ROLE_KEY= +SUPABASE_URL=backend/app/api/v1/auth.py (1)
16-17: Consider masking sensitive data in logsWhile session IDs are temporary, consider partially masking them in logs for better security hygiene.
- logger.info( - f"OAuth callback received with code: {'[PRESENT]' if code else '[MISSING]'}, session: {'[PRESENT]' if session else '[MISSING]'}") + logger.info( + f"OAuth callback received with code: {'[PRESENT]' if code else '[MISSING]'}, session: {'[PRESENT]' if session else '[MISSING]'}")For sensitive session logging elsewhere in the file, consider:
masked_session = f"{session[:8]}..." if session and len(session) > 8 else session logger.info(f"Verifying user with session ID: {masked_session}")backend/app/db/supabase/users_service.py (3)
10-11: Consider using persistent storage for verification sessions in production.The in-memory
_verification_sessionsdictionary won't be shared across multiple worker processes, which could lead to session validation failures in a multi-worker deployment. Consider using Redis or storing sessions in the database for better scalability.
22-40: Remove unnecessary else block after return statement.The else block is redundant since the function returns early when a user is found.
if existing_user_res.data: logger.info(f"Found existing user for Discord ID: {discord_id}") return User(**existing_user_res.data[0]) - else: - logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.") - new_user_data = { - "id": str(uuid.uuid4()), - "discord_id": discord_id, - "display_name": display_name, - "discord_username": discord_username, - "avatar_url": avatar_url, - "preferred_languages": [], - "created_at": datetime.now().isoformat(), - "updated_at": datetime.now().isoformat() - } - insert_res = await supabase.table("users").insert(new_user_data).execute() - if not insert_res.data: - raise Exception("Failed to create new user in database.") - return User(**insert_res.data[0]) + + logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.") + new_user_data = { + "id": str(uuid.uuid4()), + "discord_id": discord_id, + "display_name": display_name, + "discord_username": discord_username, + "avatar_url": avatar_url, + "preferred_languages": [], + "created_at": datetime.now().isoformat(), + "updated_at": datetime.now().isoformat() + } + insert_res = await supabase.table("users").insert(new_user_data).execute() + if not insert_res.data: + raise Exception("Failed to create new user in database.") + return User(**insert_res.data[0])
79-86: Remove unnecessary else block after return statement.Similar to the previous function, the else block is redundant.
if update_res.data: _verification_sessions[session_id] = (discord_id, expiry_time) logger.info( f"Created verification session {session_id} for Discord user {discord_id}, expires at {expiry_time}") return session_id - else: - logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.") - return None + + logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.") + return Nonebackend/app/scripts/supabase/populate_db.sql (2)
108-113: Consider using clearly fake Discord IDs in test data.The static analysis detected what appears to be Discord client IDs. While these seem to be test data, consider using clearly fake IDs (e.g., 'test-discord-thread-1') to avoid any confusion.
122-127: Consider using clearly fake Discord IDs in test data.Same as above - consider using clearly fake IDs for test data.
backend/bots/discord/discord_cogs.py (1)
186-191: Consider adding more context to OAuth URL generation errors.When the OAuth URL generation fails, it might be helpful to log the response data for debugging.
auth_url_data = await login_with_github(redirect_to=callback_url) auth_url = auth_url_data.get("url") if not auth_url: + logger.error(f"OAuth URL generation failed. Response: {auth_url_data}") raise Exception("Failed to generate OAuth URL.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
backend/.env.example(2 hunks)backend/app/api/v1/auth.py(1 hunks)backend/app/core/config.py(1 hunks)backend/app/db/supabase/auth.py(1 hunks)backend/app/db/supabase/supabase_client.py(1 hunks)backend/app/db/supabase/users_service.py(1 hunks)backend/app/model/supabase/models.py(4 hunks)backend/app/scripts/supabase/create_db.sql(1 hunks)backend/app/scripts/supabase/populate_db.sql(1 hunks)backend/bots/discord/discord_bot.py(1 hunks)backend/bots/discord/discord_cogs.py(2 hunks)backend/main.py(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
backend/.env.example
[warning] 6-6: [UnorderedKey] The SUPABASE_SERVICE_ROLE_KEY key should go before the SUPABASE_URL key
🪛 Pylint (3.3.7)
backend/app/db/supabase/users_service.py
[refactor] 22-40: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 79-86: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
backend/app/api/v1/auth.py
[refactor] 12-12: Too many return statements (9/6)
(R0911)
backend/main.py
[error] 74-74: Instance of 'DevRAIApplication' has no 'weaviate_client' member
(E1101)
[error] 75-75: Instance of 'DevRAIApplication' has no 'weaviate_client' member
(E1101)
backend/app/model/supabase/models.py
[refactor] 159-159: Too few public methods (0/2)
(R0903)
🪛 Gitleaks (8.26.0)
backend/app/scripts/supabase/populate_db.sql
109-109: Identified a Discord client ID, which may lead to unauthorized integrations and data exposure in Discord applications.
(discord-client-id)
123-123: Identified a Discord client ID, which may lead to unauthorized integrations and data exposure in Discord applications.
(discord-client-id)
🔇 Additional comments (24)
backend/bots/discord/discord_bot.py (1)
18-18: LGTM!The addition of
dm_messagesintent is necessary for the verification flow to send OAuth links and status updates via DMs.backend/app/core/config.py (1)
35-37: LGTM!The backend URL configuration follows the existing pattern for optional settings.
backend/app/db/supabase/supabase_client.py (1)
2-13: LGTM!The transition to AsyncClient is well-implemented with proper type annotations and clear documentation.
backend/app/api/v1/auth.py (3)
11-81: Multiple return statements are justified for OAuth flowThe static analysis flagged 9 return statements, but each handles a specific error case with appropriate user messaging. This is acceptable for an OAuth callback handler.
183-188: Good UX with auto-close functionalityThe success response includes a 5-second auto-close feature with clear user notification, providing excellent user experience.
36-44: Verify Supabase async client response structureThe code expects
session_response.user, but the Supabase async client might return the user data in a different structure.#!/bin/bash # Search for other usages of exchange_code_for_session to verify the response structure ast-grep --pattern 'exchange_code_for_session($_)'#!/bin/bash # Look for Supabase auth response handling patterns rg -A 5 "exchange_code_for_session|auth\.exchange" --type pybackend/app/db/supabase/users_service.py (1)
176-196: Well-implemented session info retrieval.The function properly handles session cleanup and expiry checks before returning session information.
backend/app/scripts/supabase/create_db.sql (1)
46-54: Well-designed indexes for verification queries.The indexes on
verification_token_expires_atand the composite index on(discord_id, verification_token)will significantly improve query performance for the verification flow.backend/app/db/supabase/auth.py (1)
7-26: Excellent OAuth flow enhancement.The addition of
redirect_toandstateparameters properly supports the GitHub verification flow, and the error handling with stack traces will help with debugging.backend/main.py (2)
88-96: Clean implementation of FastAPI lifespan management.The use of
asynccontextmanagerfor lifespan events properly handles startup and shutdown, ensuring background tasks are managed correctly.
109-117: Comprehensive environment variable validation.Good addition of
BACKEND_URLto the required variables list, which is essential for the OAuth callback flow.backend/bots/discord/discord_cogs.py (6)
2-2: LGTM! Well-structured imports for the new verification features.The imports are properly organized and include all necessary components for the GitHub verification flow.
Also applies to: 6-10, 13-13
21-26: Excellent implementation of task lifecycle management.The cleanup task is properly started on initialization and cancelled on unload, preventing resource leaks and ensuring clean shutdown.
27-38: Well-implemented background cleanup task.The task properly handles errors and waits for the bot to be ready before starting. The 5-minute interval aligns perfectly with the verification token expiry time mentioned in the user flow.
54-82: Clear and comprehensive help command update.The help command now properly lists all available commands including the new verification commands, and provides a clear features overview.
84-132: Excellent implementation of the verification status command.The command provides clear feedback about the user's verification status with proper error handling. The use of Discord's relative timestamp formatting enhances the user experience.
133-268: Comprehensive refactoring with excellent error handling and user feedback.The command now handles multiple edge cases gracefully:
- Already verified users
- Pending verifications
- DM permission issues
- Backend configuration problems
The fallback from DM to channel with privacy notice is a thoughtful UX improvement.
backend/app/model/supabase/models.py (7)
15-15: Appropriate change to make email optional.Making the email field optional aligns with the GitHub-first verification flow where users might not have an email initially.
Also applies to: 42-42
27-28: Good addition of verification token expiry tracking.The
verification_token_expires_atfield properly supports the token expiry management implemented in the cleanup task.Also applies to: 58-58
80-81: Good change to make github_id required.Making
github_ida required field ensures data integrity for GitHub repositories.Also applies to: 99-99
88-89: Appropriate change from single language to languages list.The change from
languagetolanguages_usedas a list better represents repositories that use multiple programming languages.Also applies to: 109-109
159-189: Well-designed ConversationContext model.The new model properly captures conversation history with all necessary fields for tracking user interactions across platforms.
Ignoring the static analysis hint about too few public methods as Pydantic models don't require methods
126-127: Verify the impact of making repository_id optional.Making
repository_idoptional in the Interaction model might affect queries that expect this field to always be present.#!/bin/bash # Description: Check for code that might assume repository_id is always present # Search for interaction queries that might expect repository_id rg "interaction.*repository_id" --type py -A 3 # Search for joins or filters on repository_id ast-grep --pattern 'interaction.repository_id'Also applies to: 142-142
30-31: I didn’t find any list‐specific usage ofskillswith the previous search. Let’s enumerate all occurrences and specifically look for list operations or iterations:#!/bin/bash # 1. List every reference to 'skills' in Python files rg -n "skills" --type py # 2. Look for dot‐access list methods on skills rg -n "skills\." --type py | grep -E "(append|extend|insert|pop|remove)" # 3. Search for iteration patterns over skills rg -n "for .* in .*skills" --type py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/app/db/supabase/users_service.py (1)
93-164: Address the race condition issue flagged in previous reviews.This is a duplicate of the previously identified race condition. The function checks session expiry in-memory (line 109) but then queries the database with a separate expiry check (lines 116-118). This can lead to inconsistent behavior.
Remove the in-memory expiry check and rely solely on the database query for consistency:
- session_data = _verification_sessions.get(session_id) - if not session_data: - logger.warning(f"No verification session found for session ID: {session_id}") - return None - - discord_id, expiry_time = session_data + session_data = _verification_sessions.get(session_id) + if not session_data: + logger.warning(f"No verification session found for session ID: {session_id}") + return None + + discord_id, _ = session_data # Don't use expiry_time for validationAdditionally, consider making the Exception more specific on line 140:
- raise Exception(f"GitHub account {github_username} is already linked to another Discord user") + raise ValueError(f"GitHub account {github_username} is already linked to another Discord user")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/db/supabase/users_service.py(1 hunks)backend/app/scripts/supabase/create_db.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/scripts/supabase/create_db.sql
🧰 Additional context used
🪛 Pylint (3.3.7)
backend/app/db/supabase/users_service.py
[refactor] 24-42: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 81-88: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🔇 Additional comments (4)
backend/app/db/supabase/users_service.py (4)
1-14: LGTM! Clean module setup with appropriate imports.The imports are well-organized and the global session storage approach is reasonable for this use case. The session expiry constant is clearly defined.
44-61: LGTM! Efficient in-memory cleanup implementation.The cleanup logic is well-implemented with proper logging and handles expired sessions correctly.
166-183: LGTM! Effective database cleanup implementation.The cleanup function properly handles expired tokens in the database with appropriate error handling and logging.
185-205: LGTM! Well-implemented session info retrieval.The function correctly handles session validation and provides useful information about verification sessions.
| async def create_verification_session(discord_id: str) -> Optional[str]: | ||
| """ | ||
| Create a verification session with expiry and return session ID. | ||
| """ | ||
| supabase = get_supabase_client() | ||
|
|
||
| _cleanup_expired_sessions() | ||
|
|
||
| token = str(uuid.uuid4()) | ||
| session_id = str(uuid.uuid4()) | ||
| expiry_time = datetime.now() + timedelta(minutes=SESSION_EXPIRY_MINUTES) | ||
|
|
||
| try: | ||
| update_res = await supabase.table("users").update({ | ||
| "verification_token": token, | ||
| "verification_token_expires_at": expiry_time.isoformat(), | ||
| "updated_at": datetime.now().isoformat() | ||
| }).eq("discord_id", discord_id).execute() | ||
|
|
||
| if update_res.data: | ||
| _verification_sessions[session_id] = (discord_id, expiry_time) | ||
| logger.info( | ||
| f"Created verification session {session_id} for Discord user {discord_id}, expires at {expiry_time}") | ||
| return session_id | ||
| else: | ||
| logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.") | ||
| return None | ||
| except Exception as e: | ||
| logger.error(f"Error creating verification session for Discord ID {discord_id}: {str(e)}") | ||
| return None |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary else clause and improve error specificity.
The function logic is correct but needs the same style improvements as the previous function.
Apply this diff:
- if update_res.data:
+ if update_res.data:
_verification_sessions[session_id] = (discord_id, expiry_time)
logger.info(
f"Created verification session {session_id} for Discord user {discord_id}, expires at {expiry_time}")
return session_id
- else:
- logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.")
- return None
+
+ logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.")
+ return None📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def create_verification_session(discord_id: str) -> Optional[str]: | |
| """ | |
| Create a verification session with expiry and return session ID. | |
| """ | |
| supabase = get_supabase_client() | |
| _cleanup_expired_sessions() | |
| token = str(uuid.uuid4()) | |
| session_id = str(uuid.uuid4()) | |
| expiry_time = datetime.now() + timedelta(minutes=SESSION_EXPIRY_MINUTES) | |
| try: | |
| update_res = await supabase.table("users").update({ | |
| "verification_token": token, | |
| "verification_token_expires_at": expiry_time.isoformat(), | |
| "updated_at": datetime.now().isoformat() | |
| }).eq("discord_id", discord_id).execute() | |
| if update_res.data: | |
| _verification_sessions[session_id] = (discord_id, expiry_time) | |
| logger.info( | |
| f"Created verification session {session_id} for Discord user {discord_id}, expires at {expiry_time}") | |
| return session_id | |
| else: | |
| logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.") | |
| return None | |
| except Exception as e: | |
| logger.error(f"Error creating verification session for Discord ID {discord_id}: {str(e)}") | |
| return None | |
| async def create_verification_session(discord_id: str) -> Optional[str]: | |
| """ | |
| Create a verification session with expiry and return session ID. | |
| """ | |
| supabase = get_supabase_client() | |
| _cleanup_expired_sessions() | |
| token = str(uuid.uuid4()) | |
| session_id = str(uuid.uuid4()) | |
| expiry_time = datetime.now() + timedelta(minutes=SESSION_EXPIRY_MINUTES) | |
| try: | |
| update_res = await supabase.table("users").update({ | |
| "verification_token": token, | |
| "verification_token_expires_at": expiry_time.isoformat(), | |
| "updated_at": datetime.now().isoformat() | |
| }).eq("discord_id", discord_id).execute() | |
| if update_res.data: | |
| _verification_sessions[session_id] = (discord_id, expiry_time) | |
| logger.info( | |
| f"Created verification session {session_id} for Discord user {discord_id}, expires at {expiry_time}") | |
| return session_id | |
| logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.") | |
| return None | |
| except Exception as e: | |
| logger.error(f"Error creating verification session for Discord ID {discord_id}: {str(e)}") | |
| return None |
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 81-88: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🤖 Prompt for AI Agents
In backend/app/db/supabase/users_service.py between lines 62 and 91, remove the
unnecessary else clause after checking update_res.data by returning None
directly after logging the error for user not found. Also, improve error
handling by catching more specific exceptions instead of a general Exception to
provide clearer error context. Adjust the try-except block accordingly to
reflect these changes.
| async def get_or_create_user_by_discord( | ||
| discord_id: str, display_name: str, discord_username: str, avatar_url: Optional[str] | ||
| ) -> User: | ||
| """ | ||
| Get or create a user by Discord ID. | ||
| """ | ||
| supabase = get_supabase_client() | ||
| existing_user_res = await supabase.table("users").select("*").eq("discord_id", discord_id).limit(1).execute() | ||
|
|
||
| if existing_user_res.data: | ||
| logger.info(f"Found existing user for Discord ID: {discord_id}") | ||
| return User(**existing_user_res.data[0]) | ||
| else: | ||
| logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.") | ||
| new_user_data = { | ||
| "id": str(uuid.uuid4()), | ||
| "discord_id": discord_id, | ||
| "display_name": display_name, | ||
| "discord_username": discord_username, | ||
| "avatar_url": avatar_url, | ||
| "preferred_languages": [], | ||
| "created_at": datetime.now().isoformat(), | ||
| "updated_at": datetime.now().isoformat() | ||
| } | ||
| insert_res = await supabase.table("users").insert(new_user_data).execute() | ||
| if not insert_res.data: | ||
| raise Exception("Failed to create new user in database.") | ||
| return User(**insert_res.data[0]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary else clause and improve error handling.
The logic is sound but there are two improvements needed:
- Static analysis correctly identifies an unnecessary else clause (pylint R1705)
- The generic Exception should be more specific
Apply this diff to address the issues:
- if existing_user_res.data:
+ if existing_user_res.data:
logger.info(f"Found existing user for Discord ID: {discord_id}")
return User(**existing_user_res.data[0])
- else:
- logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.")
- new_user_data = {
- "id": str(uuid.uuid4()),
- "discord_id": discord_id,
- "display_name": display_name,
- "discord_username": discord_username,
- "avatar_url": avatar_url,
- "preferred_languages": [],
- "created_at": datetime.now().isoformat(),
- "updated_at": datetime.now().isoformat()
- }
- insert_res = await supabase.table("users").insert(new_user_data).execute()
- if not insert_res.data:
- raise Exception("Failed to create new user in database.")
- return User(**insert_res.data[0])
+
+ logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.")
+ new_user_data = {
+ "id": str(uuid.uuid4()),
+ "discord_id": discord_id,
+ "display_name": display_name,
+ "discord_username": discord_username,
+ "avatar_url": avatar_url,
+ "preferred_languages": [],
+ "created_at": datetime.now().isoformat(),
+ "updated_at": datetime.now().isoformat()
+ }
+ insert_res = await supabase.table("users").insert(new_user_data).execute()
+ if not insert_res.data:
+ raise RuntimeError("Failed to create new user in database.")
+ return User(**insert_res.data[0])📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def get_or_create_user_by_discord( | |
| discord_id: str, display_name: str, discord_username: str, avatar_url: Optional[str] | |
| ) -> User: | |
| """ | |
| Get or create a user by Discord ID. | |
| """ | |
| supabase = get_supabase_client() | |
| existing_user_res = await supabase.table("users").select("*").eq("discord_id", discord_id).limit(1).execute() | |
| if existing_user_res.data: | |
| logger.info(f"Found existing user for Discord ID: {discord_id}") | |
| return User(**existing_user_res.data[0]) | |
| else: | |
| logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.") | |
| new_user_data = { | |
| "id": str(uuid.uuid4()), | |
| "discord_id": discord_id, | |
| "display_name": display_name, | |
| "discord_username": discord_username, | |
| "avatar_url": avatar_url, | |
| "preferred_languages": [], | |
| "created_at": datetime.now().isoformat(), | |
| "updated_at": datetime.now().isoformat() | |
| } | |
| insert_res = await supabase.table("users").insert(new_user_data).execute() | |
| if not insert_res.data: | |
| raise Exception("Failed to create new user in database.") | |
| return User(**insert_res.data[0]) | |
| async def get_or_create_user_by_discord( | |
| discord_id: str, display_name: str, discord_username: str, avatar_url: Optional[str] | |
| ) -> User: | |
| """ | |
| Get or create a user by Discord ID. | |
| """ | |
| supabase = get_supabase_client() | |
| existing_user_res = await supabase.table("users") \ | |
| .select("*") \ | |
| .eq("discord_id", discord_id) \ | |
| .limit(1) \ | |
| .execute() | |
| if existing_user_res.data: | |
| logger.info(f"Found existing user for Discord ID: {discord_id}") | |
| return User(**existing_user_res.data[0]) | |
| logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.") | |
| new_user_data = { | |
| "id": str(uuid.uuid4()), | |
| "discord_id": discord_id, | |
| "display_name": display_name, | |
| "discord_username": discord_username, | |
| "avatar_url": avatar_url, | |
| "preferred_languages": [], | |
| "created_at": datetime.now().isoformat(), | |
| "updated_at": datetime.now().isoformat() | |
| } | |
| insert_res = await supabase.table("users").insert(new_user_data).execute() | |
| if not insert_res.data: | |
| raise RuntimeError("Failed to create new user in database.") | |
| return User(**insert_res.data[0]) |
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 24-42: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🤖 Prompt for AI Agents
In backend/app/db/supabase/users_service.py around lines 15 to 42, remove the
unnecessary else clause after checking if existing_user_res.data, by returning
early when the user is found. Also, replace the generic Exception raised on
insert failure with a more specific exception type, such as a custom
DatabaseError or a built-in exception like RuntimeError, to improve error
clarity and handling.
smokeyScraper
force-pushed
the
supabase_config
branch
from
0a35c8a to
4a05d78
Compare
2 participants
Interactions
Discord
GitHub
Callback
Supabase
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores