Conversation
Co-authored-by: alebiosuf0802 <alebiosuf0802@students.bowiestate.edu>
|
Cursor Agent can help with this pull request. Just |
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a Supabase-backed Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/App
participant StreakService as Streak Service
participant LocalStore as Local Storage
participant Supabase as Supabase API
participant DB as user_streaks Table
note over Client,StreakService: Load Streak Data
Client->>StreakService: loadStreakData()
StreakService->>LocalStore: check cached streak
alt cache hit
LocalStore-->>StreakService: cached StreakData
StreakService-->>Client: return cached StreakData
else cache miss
StreakService->>Supabase: fetch user_streaks by user_id
Supabase->>DB: SELECT row
DB-->>Supabase: row or null
Supabase-->>StreakService: remote data
alt remote available
StreakService->>LocalStore: cache remote data
LocalStore-->>StreakService: cached
StreakService-->>Client: return remote StreakData
else none
StreakService-->>Client: return defaults
end
end
note over Client,StreakService: Save Streak Data
Client->>StreakService: saveStreakData(streak)
StreakService->>LocalStore: save locally
LocalStore-->>StreakService: saved
StreakService-->>Client: immediate return
par non-blocking remote sync
StreakService->>Supabase: upsert user_streaks
Supabase->>DB: INSERT/UPDATE (RLS)
DB-->>Supabase: result
Supabase-->>StreakService: success/error (handled silently)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
Co-authored-by: alebiosuf0802 <alebiosuf0802@students.bowiestate.edu>
![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
🤖 Fix all issues with AI agents
In @frontend/supabase/migrations/20260111000000_streaks.sql:
- Around line 57-62: The CREATE POLICY "Users can update own streak" for table
user_streaks is split by an accidental semicolon after the USING clause, causing
the subsequent WITH CHECK to be a syntax error; fix it by removing the semicolon
so the statement reads as a single CREATE POLICY ... FOR UPDATE ... TO
authenticated USING (auth.uid() = user_id) WITH CHECK (auth.uid() = user_id)
ensuring the USING and WITH CHECK clauses are part of the same statement.
🧹 Nitpick comments (3)
frontend/supabase/migrations/20260111000000_streaks.sql (1)
28-39: Remove redundant backfill and ALTER statements.Lines 24-25 already define
created_atandupdated_atasNOT NULL DEFAULT now(), so the backfill (lines 29-35) and subsequentALTER TABLE ... SET NOT NULL(lines 37-39) are unnecessary. These operations only make sense when migrating an existing table with nullable columns.♻️ Simplify by removing redundant code
); --- Backfill and enforce NOT NULL constraints for existing rows (if any) -UPDATE user_streaks -SET created_at = now() -WHERE created_at IS NULL; - -UPDATE user_streaks -SET updated_at = now() -WHERE updated_at IS NULL; - -ALTER TABLE user_streaks - ALTER COLUMN created_at SET NOT NULL, - ALTER COLUMN updated_at SET NOT NULL; - -- Enable Row Level Security ALTER TABLE user_streaks ENABLE ROW LEVEL SECURITY;frontend/services/streak-service.ts (2)
99-114: Refactor to avoid unsafe type assertions.The double
as neverassertions on line 107 suppress TypeScript's type checking. This suggests a mismatch between the upsert payload and the expected types.♻️ Use proper typing for upsert
+ type UserStreakInsert = Database['public']['Tables']['user_streaks']['Insert']; + try { const { error } = await supabase .from('user_streaks') - .upsert({ + .upsert<UserStreakInsert>({ user_id: userId, current_streak: data.currentStreak, max_streak: data.maxStreak, - } as never, { onConflict: 'user_id' } as never); + }, { onConflict: 'user_id' });Add this import at the top:
import { supabase } from '@/lib/supabase'; +import type { Database } from '@/types/database';
90-115: Consider conflict resolution for concurrent updates.The current implementation has a race condition: if a user updates their streak on device A and device B simultaneously, the last write wins without merging. For example,
max_streakcould be lost if device A hasmax_streak: 10and device B (with stale data) writesmax_streak: 5.Consider implementing optimistic locking or read-before-write logic:
// Pseudo-code for safer upsert const { data: current } = await supabase .from('user_streaks') .select('max_streak') .eq('user_id', userId) .single(); const safeMaxStreak = Math.max( data.maxStreak, current?.max_streak ?? 0 ); await supabase.from('user_streaks').upsert({ user_id: userId, current_streak: data.currentStreak, max_streak: safeMaxStreak, });Would you like me to generate a complete implementation with conflict resolution?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/constants/supabase.tsfrontend/services/streak-service.tsfrontend/supabase/migrations/20260111000000_streaks.sqlfrontend/types/database.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/services/streak-service.ts (3)
frontend/lib/supabase.ts (1)
supabase(14-30)frontend/lib/logger.ts (1)
error(32-35)frontend/services/device-storage.ts (1)
deviceStorage(170-170)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (4)
frontend/constants/supabase.ts (1)
15-15: LGTM! Clean addition of USER_STREAKS constant and schema.The new table constant and schema definition follow the existing patterns and correctly match the migration structure.
Also applies to: 125-132
frontend/supabase/migrations/20260111000000_streaks.sql (2)
45-68: LGTM! Comprehensive RLS policies for user isolation.The policies correctly ensure that authenticated users can only access their own streak records via
auth.uid() = user_idchecks across all operations.
70-78: LGTM! Proper indexing and auto-timestamp trigger.The unique index on
user_idenforces one-row-per-user and the trigger keepsupdated_atin sync.frontend/types/database.ts (1)
320-345: LGTM! Type definitions correctly model the user_streaks table.The Row, Insert, and Update types accurately reflect the schema defined in the migration, with appropriate optionality for auto-generated and defaulted fields.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Co-authored-by: alebiosuf0802 <alebiosuf0802@students.bowiestate.edu>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/services/streak-service.ts:
- Around line 100-116: Remove the unsafe "as never" casts on the Supabase upsert
call and use the generated Database types instead: call
supabase.from<'user_streaks'>(...) and pass the correctly typed payload (e.g.,
the insert type for the user_streaks table from your Database types) to upsert,
and supply the appropriate options type for onConflict; update the function
(saveStreakData or the surrounding method) to import and use the Database types
so TypeScript validates the shape of user_id, current_streak, max_streak,
last_entry_date, and last_access_time rather than bypassing checks.
🧹 Nitpick comments (1)
frontend/supabase/migrations/20260111000000_streaks.sql (1)
32-43: Backfill is redundant for a new table.Since
created_atandupdated_atare already defined withNOT NULL DEFAULT now()in the CREATE TABLE statement (lines 28-29), the backfill and ALTER statements are unnecessary for a fresh table. This only matters if the migration could run against an existing table with a different schema.If this migration is always for a new table, consider removing these statements for clarity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/constants/supabase.tsfrontend/services/streak-service.tsfrontend/supabase/migrations/20260111000000_streaks.sqlfrontend/types/database.ts
🔇 Additional comments (7)
frontend/supabase/migrations/20260111000000_streaks.sql (2)
45-76: LGTM! RLS policies and unique index are well-designed.The RLS policies correctly restrict access to the authenticated user's own row using
auth.uid() = user_id. The unique index onuser_idproperly supports the upsert operation used in the streak service.
78-82: No action required for the trigger function dependency.The
update_updated_at_column()function is already defined in migration20250825200450_misty_boat.sql, so the trigger dependency is properly satisfied.frontend/constants/supabase.ts (1)
15-15: LGTM!The
USER_STREAKStable constant and schema definition are consistent with the SQL migration and TypeScript types.Also applies to: 125-134
frontend/services/streak-service.ts (3)
34-42: Cache-first strategy may return stale data on cross-device sync.The current implementation returns cached data immediately without checking if newer data exists remotely. For cross-device sync (the stated goal of this PR), a user switching devices may see outdated streak data until the local cache is cleared or expires.
Consider:
- Remote-first with local fallback for offline support, or
- A background refresh that updates cache from remote after returning cached data
Is this cache-first behavior intentional? If offline-first is a priority, this is fine, but it may surprise users expecting immediate cross-device sync.
44-78: LGTM on Supabase fallback implementation.The use of
maybeSingle()is appropriate for the single-row-per-user pattern, and the error handling correctly logs without throwing (best-effort sync). The data mapping toStreakDatais accurate.
135-185: LGTM!The streak calculation logic is sound, and the Supabase sync is properly integrated via
saveStreakData.frontend/types/database.ts (1)
320-351: LGTM!The
user_streakstype definitions are well-structured and consistent with the SQL migration schema. The nullable fields (last_entry_date,last_access_time) and optional insert fields are correctly typed.
2 participants
Sync streak data to Supabase to enable cross-device synchronization and persistence.
Linear Issue: DEF-81
Note
Enables cross-device persistence of streak stats.
user_streakstable (migration) with RLS policies, unique index onuser_id, andupdated_attriggerStreakServicenow falls back to Supabase on load and upsertscurrent_streak/max_streakon save; local storage kept as cacheUSER_STREAKS; TypeScriptDatabasetypes extended foruser_streaksWritten by Cursor Bugbot for commit b3e96f9. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.