Skip to content

Commit 4a05d78

Browse files
committed
[fixes]: coderabbit suggested fixes
1 parent 09d7027 commit 4a05d78

File tree

2 files changed

+39
-32
lines changed

2 files changed

+39
-32
lines changed

backend/app/db/supabase/users_service.py

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212

1313
SESSION_EXPIRY_MINUTES = 5
1414

15-
async def get_or_create_user_by_discord(discord_id: str, display_name: str, discord_username: str, avatar_url: Optional[str]) -> User:
15+
async def get_or_create_user_by_discord(
16+
discord_id: str, display_name: str, discord_username: str, avatar_url: Optional[str]
17+
) -> User:
1618
"""
1719
Get or create a user by Discord ID.
1820
"""
@@ -22,22 +24,21 @@ async def get_or_create_user_by_discord(discord_id: str, display_name: str, disc
2224
if existing_user_res.data:
2325
logger.info(f"Found existing user for Discord ID: {discord_id}")
2426
return User(**existing_user_res.data[0])
25-
else:
26-
logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.")
27-
new_user_data = {
28-
"id": str(uuid.uuid4()),
29-
"discord_id": discord_id,
30-
"display_name": display_name,
31-
"discord_username": discord_username,
32-
"avatar_url": avatar_url,
33-
"preferred_languages": [],
34-
"created_at": datetime.now().isoformat(),
35-
"updated_at": datetime.now().isoformat()
36-
}
37-
insert_res = await supabase.table("users").insert(new_user_data).execute()
38-
if not insert_res.data:
39-
raise Exception("Failed to create new user in database.")
40-
return User(**insert_res.data[0])
27+
logger.info(f"No user found for Discord ID: {discord_id}. Creating new user.")
28+
new_user_data = {
29+
"id": str(uuid.uuid4()),
30+
"discord_id": discord_id,
31+
"display_name": display_name,
32+
"discord_username": discord_username,
33+
"avatar_url": avatar_url,
34+
"preferred_languages": [],
35+
"created_at": datetime.now().isoformat(),
36+
"updated_at": datetime.now().isoformat()
37+
}
38+
insert_res = await supabase.table("users").insert(new_user_data).execute()
39+
if not insert_res.data:
40+
raise Exception("Failed to create new user in database.")
41+
return User(**insert_res.data[0])
4142

4243
def _cleanup_expired_sessions():
4344
"""
@@ -81,14 +82,15 @@ async def create_verification_session(discord_id: str) -> Optional[str]:
8182
logger.info(
8283
f"Created verification session {session_id} for Discord user {discord_id}, expires at {expiry_time}")
8384
return session_id
84-
else:
85-
logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.")
86-
return None
85+
logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.")
86+
return None
8787
except Exception as e:
8888
logger.error(f"Error creating verification session for Discord ID {discord_id}: {str(e)}")
8989
return None
9090

91-
async def find_user_by_session_and_verify(session_id: str, github_id: str, github_username: str, email: Optional[str]) -> Optional[User]:
91+
async def find_user_by_session_and_verify(
92+
session_id: str, github_id: str, github_username: str, email: Optional[str]
93+
) -> Optional[User]:
9294
"""
9395
Find and verify user using session ID with expiry validation.
9496
"""
@@ -104,23 +106,28 @@ async def find_user_by_session_and_verify(session_id: str, github_id: str, githu
104106

105107
discord_id, expiry_time = session_data
106108

107-
if datetime.now() > expiry_time:
108-
logger.warning(f"Verification session {session_id} has expired")
109-
del _verification_sessions[session_id]
110-
return None
111-
112-
del _verification_sessions[session_id]
113-
114109
current_time = datetime.now().isoformat()
115-
user_res = await supabase.table("users").select("*").eq("discord_id", discord_id).neq("verification_token", None).gt("verification_token_expires_at", current_time).limit(1).execute()
110+
user_res = await supabase.table("users").select("*").eq(
111+
"discord_id", discord_id
112+
).neq(
113+
"verification_token", None
114+
).gt(
115+
"verification_token_expires_at", current_time
116+
).limit(1).execute()
116117

117118
if not user_res.data:
118-
logger.warning(f"No valid pending verification found for Discord ID: {discord_id}")
119+
logger.warning(f"No valid pending verification found for Discord ID: {discord_id} (token may have expired)")
120+
del _verification_sessions[session_id]
119121
return None
120122

123+
# Delete the session after successful validation
124+
del _verification_sessions[session_id]
125+
121126
user_to_verify = user_res.data[0]
122127

123-
existing_github_user = await supabase.table("users").select("*").eq("github_id", github_id).neq("id", user_to_verify['id']).limit(1).execute()
128+
existing_github_user = await supabase.table("users").select("*").eq(
129+
"github_id", github_id
130+
).neq("id", user_to_verify['id']).limit(1).execute()
124131
if existing_github_user.data:
125132
logger.warning(f"GitHub account {github_username} is already linked to another user")
126133
await supabase.table("users").update({

backend/app/scripts/supabase/create_db.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ CREATE TABLE users (
4040
last_active_slack TIMESTAMPTZ,
4141

4242
total_interactions_count INTEGER NOT NULL DEFAULT 0,
43-
preferred_languages TEXT[],
43+
preferred_languages TEXT[]
4444
);
4545

4646
-- Create index for efficient cleanup queries

0 commit comments

Comments
 (0)