Add Sponsor system: DB migration, service layer, and app-wide badge UI#35
Open
fuzziecoder wants to merge 1 commit intomainfrom
Open
Add Sponsor system: DB migration, service layer, and app-wide badge UI#35fuzziecoder wants to merge 1 commit intomainfrom
fuzziecoder wants to merge 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a comprehensive sponsorship system for drink spots. It adds database schema changes (new spot_sponsors table, profile and spot columns), a SQL migration with triggers, TypeScript types for sponsorship data, a new SponsorBadge UI component, a sponsorService with CRUD operations, and integrates sponsor information across multiple pages and contexts. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Admin
participant UI as HomePage UI
participant SponsorService as SponsorService
participant Database as Database
participant RealtimeAPI as Realtime API
User->>UI: Opens spot admin panel
UI->>SponsorService: getSponsor(spotId)
SponsorService->>Database: Query spot_sponsors
Database-->>SponsorService: Sponsor data
SponsorService-->>UI: Display current sponsor
User->>UI: Click "Add Sponsor"
UI->>UI: Open sponsor modal
UI->>SponsorService: getAllSponsors()
SponsorService->>Database: Query sponsors
Database-->>SponsorService: List of sponsors
SponsorService-->>UI: Populate dropdown
User->>UI: Select sponsor & submit
UI->>SponsorService: sponsorSpot({spot_id, sponsor_id, amount_covered, message})
SponsorService->>Database: Upsert spot_sponsors
Database->>Database: Execute trigger: update profiles & spots
Database-->>SponsorService: Return sponsor record
SponsorService-->>UI: Sponsor assigned
RealtimeAPI-->>UI: Subscription update (on_new_spot_sponsor)
UI->>UI: Refresh sponsor display with SponsorBadge
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+1617
to
+1640
| async removeSponsor(spotId: string): Promise<void> { | ||
| const { error } = await supabase | ||
| .from('spot_sponsors') | ||
| .delete() | ||
| .eq('spot_id', spotId); | ||
|
|
||
| if (error) { | ||
| console.error('Error removing sponsor:', error); | ||
| throw error; | ||
| } | ||
|
|
||
| const { error: spotError } = await supabase | ||
| .from('spots') | ||
| .update({ | ||
| is_sponsored: false, | ||
| sponsored_by: null, | ||
| }) | ||
| .eq('id', spotId); | ||
|
|
||
| if (spotError) { | ||
| console.error('Error resetting spot sponsorship:', spotError); | ||
| throw spotError; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🔴 removeSponsor never decrements sponsor_count or resets is_sponsor on the profile
When removeSponsor is called, it deletes the spot_sponsors row and resets the spots table flags, but it never decrements profiles.sponsor_count or resets profiles.is_sponsor. The DB trigger handle_new_sponsor only fires on INSERT (incrementing the count), with no corresponding logic for deletion.
Root Cause and Impact
The removeSponsor method at services/database.ts:1617-1640 only updates the spots table (is_sponsored = false, sponsored_by = null) but does not touch the profiles table at all. Meanwhile, the trigger at supabase_migration_sponsor.sql:40-43 only fires AFTER INSERT, so there is no mechanism to decrement sponsor_count or reset is_sponsor when a sponsorship is removed.
Impact:
sponsor_countwill only ever increase, never decrease. If an admin removes and re-adds a sponsor, the count inflates.is_sponsoris permanently set totrueafter the first sponsorship and never reverted, even if all sponsorships are removed. This means the SponsorBadge will display permanently for any user who was ever a sponsor, even after removal.- The leaderboard (
getAllSponsors) will show inflated counts.
Prompt for agents
In services/database.ts, the removeSponsor method (lines 1617-1640) needs to also update the sponsor's profile. Before deleting the spot_sponsors row, first fetch the current sponsor record to get the sponsor_id, then after deletion, decrement sponsor_count on that profile. If sponsor_count reaches 0, also set is_sponsor to false. Alternatively, add a DELETE trigger in supabase_migration_sponsor.sql (alongside the existing INSERT trigger) that decrements sponsor_count and conditionally resets is_sponsor when a spot_sponsors row is deleted.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+1569
to
+1614
| const { data: inserted, error } = await supabase | ||
| .from('spot_sponsors') | ||
| .upsert({ | ||
| spot_id: data.spot_id, | ||
| sponsor_id: data.sponsor_id, | ||
| amount_covered: data.amount_covered, | ||
| message: data.message || null, | ||
| }, { | ||
| onConflict: 'spot_id' | ||
| }) | ||
| .select(` | ||
| *, | ||
| sponsor:sponsor_id ( | ||
| id, | ||
| name, | ||
| username, | ||
| phone, | ||
| email, | ||
| role, | ||
| profile_pic_url, | ||
| location, | ||
| is_sponsor, | ||
| sponsor_count | ||
| ) | ||
| `) | ||
| .single(); | ||
|
|
||
| if (error) { | ||
| console.error('Error assigning sponsor:', error); | ||
| throw error; | ||
| } | ||
|
|
||
| const { error: spotError } = await supabase | ||
| .from('spots') | ||
| .update({ | ||
| is_sponsored: true, | ||
| sponsored_by: data.sponsor_id, | ||
| }) | ||
| .eq('id', data.spot_id); | ||
|
|
||
| if (spotError) { | ||
| console.error('Error marking spot as sponsored:', spotError); | ||
| throw spotError; | ||
| } | ||
|
|
||
| return inserted; |
There was a problem hiding this comment.
🔴 sponsorSpot upsert won't trigger sponsor_count increment when replacing an existing sponsor
The sponsorSpot method uses Supabase upsert with onConflict: 'spot_id', but the database trigger handle_new_sponsor only fires AFTER INSERT. When a spot already has a sponsor and the admin assigns a new one, the upsert resolves to an UPDATE, so the trigger never fires.
Root Cause and Impact
At services/database.ts:1569-1577, the upsert with onConflict: 'spot_id' will perform an UPDATE (not INSERT) if a spot_sponsors row already exists for that spot. The trigger at supabase_migration_sponsor.sql:40-43 is defined as AFTER INSERT only.
When replacing sponsor A with sponsor B on the same spot:
- Sponsor B's
is_sponsorflag is never set totrue - Sponsor B's
sponsor_countis never incremented - Sponsor A's
sponsor_countis never decremented - The manual code at lines 1601-1607 only updates the
spotstable, not theprofilestable
Impact: The new sponsor won't get their SponsorBadge or correct count. The old sponsor retains an inflated count.
Prompt for agents
In services/database.ts sponsorSpot method (lines 1569-1614): When the upsert resolves to an UPDATE (replacing an existing sponsor), the INSERT trigger won't fire, so the new sponsor's profile won't be updated. Fix by either: (1) adding an AFTER UPDATE trigger on spot_sponsors in supabase_migration_sponsor.sql that handles the sponsor swap (decrement old sponsor_count, increment new sponsor_count, set is_sponsor flags), or (2) changing the application code to first check if a sponsor exists, remove it (with proper profile updates), then insert a new one instead of using upsert.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+105
to
+106
| const profiles = await profileService.getAllProfiles(); | ||
| setAllProfiles(profiles || []); |
There was a problem hiding this comment.
🟡 getAllProfiles called on every fetchData for all users, not just admins
In HomePage.tsx, profileService.getAllProfiles() is called unconditionally inside fetchData for every user, even though the profiles list is only used in the admin-only sponsor modal.
Root Cause and Impact
At pages/HomePage.tsx:105-106, profileService.getAllProfiles() is called outside the if (spotData) block and without any admin check. The comment in services/database.ts:492 says this is "admin only", and the data is only used in the sponsor modal which is gated behind isAdmin.
Impact: Every non-admin user makes an unnecessary database query fetching all profiles on every page load and every real-time refresh. This is a performance issue and potentially a data exposure concern (all profile data fetched client-side for non-admin users).
Suggested change
| const profiles = await profileService.getAllProfiles(); | |
| setAllProfiles(profiles || []); | |
| if (profile?.role === UserRole.ADMIN) { | |
| const profiles = await profileService.getAllProfiles(); | |
| setAllProfiles(profiles || []); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/DrinksPage.tsx (1)
815-823:⚠️ Potential issue | 🟠 MajorSponsored spots are still blocked by the payment gate.
This branch still forces payment even when a sponsor exists, which conflicts with the “Everyone pays Rs.0” banner and the sponsor intent. Consider bypassing the payment requirement whenspotSponsoris present (or auto‑mark users as paid).✅ Suggested fix
- if (!isPaid) { + if (!isPaid && !spotSponsor) { return ( <div className="space-y-6 pb-20 max-w-6xl mx-auto px-4"> <h1 className="text-2xl md:text-3xl font-bold">Bar Menu</h1> {sponsorBanner} <Card className="p-8 text-center"> <p className="text-gray-400 mb-4">You need to complete payment first to access the bar.</p> <Button onClick={() => window.location.href = '/dashboard/payment'}>Go to Payment</Button> </Card> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/DrinksPage.tsx` around lines 815 - 823, The current payment-gate branch in the DrinksPage component blocks sponsored users; update the check that returns the payment prompt so sponsored spots bypass it by using the spotSponsor presence (e.g., change the guard from if (!isPaid) to if (!isPaid && !spotSponsor) or set isPaid = true when spotSponsor is present) so the sponsorBanner/spotSponsor logic is honored; adjust the conditional around the Card that renders the payment prompt and ensure sponsorBanner still renders for sponsored users.
🧹 Nitpick comments (3)
pages/HomePage.tsx (1)
983-988: Consider adding validation constraints to Amount Covered input.The number input lacks
minandrequiredattributes, allowing negative values or empty submission (caught in handler but better UX to validate in form).🛠️ Proposed enhancement
<Input label="Amount Covered" type="number" value={sponsorForm.amount_covered} onChange={(e) => setSponsorForm(prev => ({ ...prev, amount_covered: e.target.value }))} + min="0" + required />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/HomePage.tsx` around lines 983 - 988, Add client-side validation to the Amount Covered input by adding required and min="0" attributes on the <Input> for amount_covered and ensure the onChange handler preserves a numeric value (convert e.target.value to a number before setting sponsorForm.amount_covered) so the form doesn't accept empty or negative values; update the Input element and the onChange callback that currently references sponsorForm.amount_covered and setSponsorForm.pages/ChatPage.tsx (1)
197-205: SponsorBadge integration is correct but may need spacing.The badge is rendered inline immediately after the sender name without a gap. Consider adding a space or gap for visual separation.
💅 Proposed styling improvement
{!isMe && showAvatar && ( - <span - className="text-[10px] font-black text-zinc-500 uppercase tracking-tighter ml-1 cursor-pointer hover:text-zinc-400 transition-colors" + <span + className="text-[10px] font-black text-zinc-500 uppercase tracking-tighter ml-1 cursor-pointer hover:text-zinc-400 transition-colors inline-flex items-center gap-1" onClick={() => window.location.href = `/dashboard/profile/${msg.user_id}`} > {msg.profiles.name} - {msg.profiles.is_sponsor && <SponsorBadge size="sm" count={msg.profiles.sponsor_count || 0} />} + {msg.profiles.is_sponsor && ( + <SponsorBadge size="sm" count={msg.profiles.sponsor_count || 0} /> + )} </span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/ChatPage.tsx` around lines 197 - 205, The sender name and SponsorBadge in the ChatPage.tsx JSX are rendered immediately adjacent (inside the span that shows {msg.profiles.name} and <SponsorBadge ... />), so add visual separation by updating the markup/CSS: either render the name and badge as separate inline elements with a gap (e.g., wrap them in a flex container or give the badge a left margin) or insert a non-breaking space between {msg.profiles.name} and SponsorBadge; target the span that contains {msg.profiles.name} and the SponsorBadge component (and preserve the existing props like size="sm" and count={msg.profiles.sponsor_count || 0}) to implement the spacing.DATABASE_SCHEMA.md (1)
339-363: Documentation accurately reflects the schema changes.The
spot_sponsorstable documentation is clear. However, consider adding a note about:
- The DELETE behavior (if a trigger is added per the migration review)
- Clarifying that
is_sponsorremainstruepermanently as per the feature design ("permanent golden star badge")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DATABASE_SCHEMA.md` around lines 339 - 363, Update the spot_sponsors docs to explicitly state the lifecycle rules: note that the trigger on spot_sponsors will set profiles.is_sponsor = true and increment profiles.sponsor_count when a sponsorship is inserted, and clarify the DELETE behavior (specify whether deletions of spot_sponsors rows decrement sponsor_count and whether spots.sponsored_by and spots.is_sponsored are cleared on delete or cascade); also explicitly state that profiles.is_sponsor is intended to remain true permanently (the "golden star badge") per design, even if individual sponsorship rows are later removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/common/SponsorBadge.tsx`:
- Line 24: The title string in SponsorBadge.tsx currently always uses "spots"
which causes "Sponsored 1 spots"; update the title expression that uses the
count prop/variable (the title={`Sponsored ${count} spots`} expression) to
conditionally pluralize—e.g. choose "spot" when count === 1 and "spots"
otherwise (or use a small pluralize helper) so the title reads "Sponsored 1
spot" for a single count.
In `@services/database.ts`:
- Around line 1617-1640: The two DB calls in removeSponsor (deleting from
'spot_sponsors' then updating 'spots') must be made atomic to avoid orphaned
state; refactor removeSponsor to run both operations inside a single DB
transaction (e.g., create and call a Postgres function or use supabase.rpc to
execute a BEGIN/.../COMMIT block) so both delete and update succeed or both are
rolled back, and if transactions are not available implement a safe fallback:
update the 'spots' row first then delete the sponsor or catch update failure and
re-insert the sponsor to restore consistency using the same spotId and returned
data.
- Around line 1563-1615: sponsorSpot currently issues two separate requests
(upsert to spot_sponsors and update to spots) which can leave inconsistent
state; instead create a Postgres stored procedure (e.g.,
sponsor_spot_transaction) that performs the upsert into spot_sponsors and the
update of spots (set is_sponsored=true and sponsored_by) within a single
transaction and returns the inserted/updated row, then change the sponsorSpot
implementation to call supabase.rpc('sponsor_spot_transaction', { spot_id,
sponsor_id, amount_covered, message }) and handle the rpc result/error; remove
the separate supabase.from('spot_sponsors').upsert(...) and
supabase.from('spots').update(...) calls so the atomic logic lives in the DB and
sponsorSpot returns the RPC result.
In `@supabase_migration_sponsor.sql`:
- Around line 3-10: Enable row-level security on the spot_sponsors table and add
the proposed policies and index: run ALTER TABLE spot_sponsors ENABLE ROW LEVEL
SECURITY; create a SELECT policy named "Everyone can read sponsors" that allows
all reads (USING (true)); create an admin-only management policy named "Admins
can manage sponsors" that restricts FOR ALL actions to rows where a lookup
against profiles for auth.uid() has role = 'admin' (USE EXISTS (...) with
profiles.id = auth.uid() and profiles.role = 'admin'); and add an index
spot_sponsors_sponsor_id_idx on sponsor_id to optimize sponsor profile lookups.
- Around line 20-43: Add a new AFTER DELETE trigger and handler to keep profile
sponsor counts and flags consistent: create a function handle_remove_sponsor()
that runs on DELETE from spot_sponsors, updates profiles by decrementing
sponsor_count using GREATEST(0, COALESCE(sponsor_count,0) - 1), sets is_sponsor
= false when the resulting count is 0 (true otherwise), updates updated_at =
NOW(), and returns OLD; then add a trigger on_remove_spot_sponsor AFTER DELETE
ON spot_sponsors FOR EACH ROW EXECUTE FUNCTION handle_remove_sponsor(). Ensure
you reference the existing symbols handle_remove_sponsor and
on_remove_spot_sponsor when adding these.
---
Outside diff comments:
In `@pages/DrinksPage.tsx`:
- Around line 815-823: The current payment-gate branch in the DrinksPage
component blocks sponsored users; update the check that returns the payment
prompt so sponsored spots bypass it by using the spotSponsor presence (e.g.,
change the guard from if (!isPaid) to if (!isPaid && !spotSponsor) or set isPaid
= true when spotSponsor is present) so the sponsorBanner/spotSponsor logic is
honored; adjust the conditional around the Card that renders the payment prompt
and ensure sponsorBanner still renders for sponsored users.
---
Nitpick comments:
In `@DATABASE_SCHEMA.md`:
- Around line 339-363: Update the spot_sponsors docs to explicitly state the
lifecycle rules: note that the trigger on spot_sponsors will set
profiles.is_sponsor = true and increment profiles.sponsor_count when a
sponsorship is inserted, and clarify the DELETE behavior (specify whether
deletions of spot_sponsors rows decrement sponsor_count and whether
spots.sponsored_by and spots.is_sponsored are cleared on delete or cascade);
also explicitly state that profiles.is_sponsor is intended to remain true
permanently (the "golden star badge") per design, even if individual sponsorship
rows are later removed.
In `@pages/ChatPage.tsx`:
- Around line 197-205: The sender name and SponsorBadge in the ChatPage.tsx JSX
are rendered immediately adjacent (inside the span that shows
{msg.profiles.name} and <SponsorBadge ... />), so add visual separation by
updating the markup/CSS: either render the name and badge as separate inline
elements with a gap (e.g., wrap them in a flex container or give the badge a
left margin) or insert a non-breaking space between {msg.profiles.name} and
SponsorBadge; target the span that contains {msg.profiles.name} and the
SponsorBadge component (and preserve the existing props like size="sm" and
count={msg.profiles.sponsor_count || 0}) to implement the spacing.
In `@pages/HomePage.tsx`:
- Around line 983-988: Add client-side validation to the Amount Covered input by
adding required and min="0" attributes on the <Input> for amount_covered and
ensure the onChange handler preserves a numeric value (convert e.target.value to
a number before setting sponsorForm.amount_covered) so the form doesn't accept
empty or negative values; update the Input element and the onChange callback
that currently references sponsorForm.amount_covered and setSponsorForm.
| }) => { | ||
| return ( | ||
| <span | ||
| title={`Sponsored ${count} spots`} |
There was a problem hiding this comment.
Minor: Pluralization issue in title attribute.
When count is 1, the title reads "Sponsored 1 spots" instead of "Sponsored 1 spot".
🛠️ Proposed fix
- title={`Sponsored ${count} spots`}
+ title={`Sponsored ${count} spot${count === 1 ? '' : 's'}`}📝 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.
Suggested change
| title={`Sponsored ${count} spots`} | |
| title={`Sponsored ${count} spot${count === 1 ? '' : 's'}`} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/common/SponsorBadge.tsx` at line 24, The title string in
SponsorBadge.tsx currently always uses "spots" which causes "Sponsored 1 spots";
update the title expression that uses the count prop/variable (the
title={`Sponsored ${count} spots`} expression) to conditionally pluralize—e.g.
choose "spot" when count === 1 and "spots" otherwise (or use a small pluralize
helper) so the title reads "Sponsored 1 spot" for a single count.
Comment on lines
+1563
to
+1615
| async sponsorSpot(data: { | ||
| spot_id: string; | ||
| sponsor_id: string; | ||
| amount_covered: number; | ||
| message?: string; | ||
| }): Promise<SpotSponsor> { | ||
| const { data: inserted, error } = await supabase | ||
| .from('spot_sponsors') | ||
| .upsert({ | ||
| spot_id: data.spot_id, | ||
| sponsor_id: data.sponsor_id, | ||
| amount_covered: data.amount_covered, | ||
| message: data.message || null, | ||
| }, { | ||
| onConflict: 'spot_id' | ||
| }) | ||
| .select(` | ||
| *, | ||
| sponsor:sponsor_id ( | ||
| id, | ||
| name, | ||
| username, | ||
| phone, | ||
| email, | ||
| role, | ||
| profile_pic_url, | ||
| location, | ||
| is_sponsor, | ||
| sponsor_count | ||
| ) | ||
| `) | ||
| .single(); | ||
|
|
||
| if (error) { | ||
| console.error('Error assigning sponsor:', error); | ||
| throw error; | ||
| } | ||
|
|
||
| const { error: spotError } = await supabase | ||
| .from('spots') | ||
| .update({ | ||
| is_sponsored: true, | ||
| sponsored_by: data.sponsor_id, | ||
| }) | ||
| .eq('id', data.spot_id); | ||
|
|
||
| if (spotError) { | ||
| console.error('Error marking spot as sponsored:', spotError); | ||
| throw spotError; | ||
| } | ||
|
|
||
| return inserted; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Supabase JavaScript client transactions support 2025
💡 Result:
As of 2025, the Supabase JavaScript client (@supabase/supabase-js) does not support client-managed database transactions (i.e., you can’t BEGIN a transaction, run multiple queries, then COMMIT/ROLLBACK from the JS client). Supabase’s documented/official stance is that this isn’t planned outside of using Postgres functions (RPC), largely because the PostgREST/HTTP layer is stateless. [1]
Supported approaches instead:
- Put the multi-step logic inside a Postgres function (where you can use transactional SQL) and call it from JS with
supabase.rpc(...). [2] - If you truly need application-managed transactions, do it in a server environment by connecting directly to Postgres (e.g., in your own backend / Edge Function) using a Postgres driver/ORM that supports transactions (not via the browser client). [1]
Sources
- Supabase GitHub Discussion: “Client-side database transactions” (
#526) (includes Supabase support response + “not supported / not on roadmap” updates). [1] - Supabase Docs: JavaScript “Call a Postgres function” (
supabase.rpc). [2]
🌐 Web query:
Supabase RPC database functions transactions
💡 Result:
- Supabase
rpc()calls a Postgres function via PostgREST’s/rpcendpoint. (supabase.com) - Every PostgREST request (table CRUD or
/rpc) runs inside a single database transaction. By default, if nothing fails it COMMITs; if a constraint/error happens (or you raise an error), PostgREST ROLLBACKs that request’s transaction. (docs.postgrest.org) - You cannot have one transaction span multiple Supabase client calls (because they are separate HTTP requests / DB connections). The usual approach is: put all multi-step work into one SQL function and call it once via
rpc(). (github.com)
Pattern: multi-statement atomic operation via RPC (rollback on error)
create or replace function public.transfer(p_from uuid, p_to uuid, p_amount numeric)
returns void
language plpgsql
as $$
begin
update accounts set balance = balance - p_amount where id = p_from;
if not found then
raise exception 'from account not found';
end if;
update accounts set balance = balance + p_amount where id = p_to;
if not found then
raise exception 'to account not found';
end if;
-- any exception (including constraint violations) aborts and rolls back the whole request
end;
$$;await supabase.rpc('transfer', { p_from, p_to, p_amount })If you need manual BEGIN/COMMIT/ROLLBACK controlled from application code, you generally must connect to Postgres from your own backend (not via the Supabase JS PostgREST API). (github.com)
Citations:
- 1: https://supabase.com/docs/reference/javascript/rpc?utm_source=openai
- 2: https://docs.postgrest.org/en/v12/references/transactions.html?utm_source=openai
- 3: https://github.com/orgs/supabase/discussions/526?utm_source=openai
- 4: https://github.com/orgs/supabase/discussions/526?utm_source=openai
🌐 Web query:
Supabase upsert onConflict atomicity multiple tables
💡 Result:
upsert(..., { onConflict })is atomic only for that single table/request. Supabase’s.upsert()maps to PostgresINSERT ... ON CONFLICT DO UPDATE, whose core guarantee is an atomic “insert-or-update” decision under concurrency for the target table/unique constraint. [1], [2]- You can’t make an atomic, multi-table upsert by chaining multiple
.upsert()calls. With Supabase’s Data API (PostgREST), each HTTP request runs inside its own database transaction—so two requests are two separate transactions and can’t be committed/rolled back as a unit. [3] - How to do multi-table atomic writes: put the whole workflow (multiple inserts/updates across tables) in a Postgres function (using a transaction inside the function body as needed) and call it via
.rpc()so it executes within a single request/transaction boundary. [3], [4]
Sources:
[1] Supabase JS upsert docs (onConflict) (supabase.com)
[2] PostgreSQL wiki on UPSERT atomicity/concurrency (wiki.postgresql.org)
[3] PostgREST transactions: every request runs inside a transaction (docs.postgrest.org)
[4] Supabase JS RPC (call Postgres function) docs (supabase.com)
Citations:
- 1: https://supabase.com/docs/reference/javascript/upsert?utm_source=openai
- 2: https://wiki.postgresql.org/wiki/UPSERT?utm_source=openai
- 3: https://docs.postgrest.org/en/stable/references/transactions.html
- 4: https://supabase.com/docs/reference/javascript/rpc?utm_source=openai
Non-atomic operations in sponsorSpot can leave inconsistent state.
The method performs two separate HTTP requests:
- Upsert into
spot_sponsors(lines 1569–1594) - Update
spotstable (lines 1601–1607)
Since Supabase's PostgREST API executes each request in its own transaction, if the second update fails, the sponsor record exists but the spot isn't marked as sponsored. To ensure atomicity, create a Postgres function that handles both operations in a single transaction and call it via supabase.rpc().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/database.ts` around lines 1563 - 1615, sponsorSpot currently issues
two separate requests (upsert to spot_sponsors and update to spots) which can
leave inconsistent state; instead create a Postgres stored procedure (e.g.,
sponsor_spot_transaction) that performs the upsert into spot_sponsors and the
update of spots (set is_sponsored=true and sponsored_by) within a single
transaction and returns the inserted/updated row, then change the sponsorSpot
implementation to call supabase.rpc('sponsor_spot_transaction', { spot_id,
sponsor_id, amount_covered, message }) and handle the rpc result/error; remove
the separate supabase.from('spot_sponsors').upsert(...) and
supabase.from('spots').update(...) calls so the atomic logic lives in the DB and
sponsorSpot returns the RPC result.
Comment on lines
+1617
to
+1640
| async removeSponsor(spotId: string): Promise<void> { | ||
| const { error } = await supabase | ||
| .from('spot_sponsors') | ||
| .delete() | ||
| .eq('spot_id', spotId); | ||
|
|
||
| if (error) { | ||
| console.error('Error removing sponsor:', error); | ||
| throw error; | ||
| } | ||
|
|
||
| const { error: spotError } = await supabase | ||
| .from('spots') | ||
| .update({ | ||
| is_sponsored: false, | ||
| sponsored_by: null, | ||
| }) | ||
| .eq('id', spotId); | ||
|
|
||
| if (spotError) { | ||
| console.error('Error resetting spot sponsorship:', spotError); | ||
| throw spotError; | ||
| } | ||
| }, |
There was a problem hiding this comment.
Similar atomicity concern in removeSponsor.
If the delete succeeds but the spot update fails (lines 1628-1634), the sponsor record is gone but the spot still shows is_sponsored: true.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/database.ts` around lines 1617 - 1640, The two DB calls in
removeSponsor (deleting from 'spot_sponsors' then updating 'spots') must be made
atomic to avoid orphaned state; refactor removeSponsor to run both operations
inside a single DB transaction (e.g., create and call a Postgres function or use
supabase.rpc to execute a BEGIN/.../COMMIT block) so both delete and update
succeed or both are rolled back, and if transactions are not available implement
a safe fallback: update the 'spots' row first then delete the sponsor or catch
update failure and re-insert the sponsor to restore consistency using the same
spotId and returned data.
Comment on lines
+3
to
+10
| CREATE TABLE IF NOT EXISTS spot_sponsors ( | ||
| id UUID PRIMARY KEY DEFAULT gen_random_uuid(), | ||
| spot_id UUID NOT NULL REFERENCES spots(id) ON DELETE CASCADE UNIQUE, | ||
| sponsor_id UUID NOT NULL REFERENCES profiles(id) ON DELETE CASCADE, | ||
| amount_covered NUMERIC NOT NULL DEFAULT 0, | ||
| message TEXT, | ||
| created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() | ||
| ); |
There was a problem hiding this comment.
Missing RLS policies and index on spot_sponsors table.
The table lacks:
- Row Level Security policies (all other tables in the schema have RLS enabled)
- An index on
sponsor_idfor efficient sponsor profile lookups
🔒 Proposed RLS and index additions
-- Enable RLS
ALTER TABLE spot_sponsors ENABLE ROW LEVEL SECURITY;
-- Policy: Everyone can read sponsors
CREATE POLICY "Everyone can read sponsors" ON spot_sponsors
FOR SELECT USING (true);
-- Policy: Only admins can manage sponsors
CREATE POLICY "Admins can manage sponsors" ON spot_sponsors
FOR ALL USING (
EXISTS (
SELECT 1 FROM profiles
WHERE profiles.id = auth.uid()
AND profiles.role = 'admin'
)
);
-- Index for sponsor lookups
CREATE INDEX spot_sponsors_sponsor_id_idx ON spot_sponsors(sponsor_id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase_migration_sponsor.sql` around lines 3 - 10, Enable row-level
security on the spot_sponsors table and add the proposed policies and index: run
ALTER TABLE spot_sponsors ENABLE ROW LEVEL SECURITY; create a SELECT policy
named "Everyone can read sponsors" that allows all reads (USING (true)); create
an admin-only management policy named "Admins can manage sponsors" that
restricts FOR ALL actions to rows where a lookup against profiles for auth.uid()
has role = 'admin' (USE EXISTS (...) with profiles.id = auth.uid() and
profiles.role = 'admin'); and add an index spot_sponsors_sponsor_id_idx on
sponsor_id to optimize sponsor profile lookups.
Comment on lines
+20
to
+43
| CREATE OR REPLACE FUNCTION handle_new_sponsor() | ||
| RETURNS TRIGGER AS $$ | ||
| BEGIN | ||
| UPDATE profiles | ||
| SET is_sponsor = true, | ||
| sponsor_count = COALESCE(sponsor_count, 0) + 1, | ||
| updated_at = NOW() | ||
| WHERE id = NEW.sponsor_id; | ||
|
|
||
| UPDATE spots | ||
| SET is_sponsored = true, | ||
| sponsored_by = NEW.sponsor_id, | ||
| updated_at = NOW() | ||
| WHERE id = NEW.spot_id; | ||
|
|
||
| RETURN NEW; | ||
| END; | ||
| $$ LANGUAGE plpgsql; | ||
|
|
||
| DROP TRIGGER IF EXISTS on_new_spot_sponsor ON spot_sponsors; | ||
| CREATE TRIGGER on_new_spot_sponsor | ||
| AFTER INSERT ON spot_sponsors | ||
| FOR EACH ROW | ||
| EXECUTE FUNCTION handle_new_sponsor(); |
There was a problem hiding this comment.
Trigger only handles INSERT - DELETE leaves stale data.
The handle_new_sponsor trigger increments sponsor_count and sets is_sponsor = true on INSERT, but there's no corresponding trigger for DELETE or UPDATE. When removeSponsor() deletes a row from spot_sponsors:
- The profile's
sponsor_countwon't be decremented - The profile's
is_sponsorwill remaintrueeven if count reaches 0
This creates data inconsistency over time.
🔧 Proposed fix: Add DELETE trigger
CREATE OR REPLACE FUNCTION handle_remove_sponsor()
RETURNS TRIGGER AS $$
BEGIN
UPDATE profiles
SET sponsor_count = GREATEST(0, COALESCE(sponsor_count, 0) - 1),
is_sponsor = CASE
WHEN COALESCE(sponsor_count, 0) - 1 <= 0 THEN false
ELSE true
END,
updated_at = NOW()
WHERE id = OLD.sponsor_id;
RETURN OLD;
END;
$$ LANGUAGE plpgsql;
DROP TRIGGER IF EXISTS on_remove_spot_sponsor ON spot_sponsors;
CREATE TRIGGER on_remove_spot_sponsor
AFTER DELETE ON spot_sponsors
FOR EACH ROW
EXECUTE FUNCTION handle_remove_sponsor();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase_migration_sponsor.sql` around lines 20 - 43, Add a new AFTER DELETE
trigger and handler to keep profile sponsor counts and flags consistent: create
a function handle_remove_sponsor() that runs on DELETE from spot_sponsors,
updates profiles by decrementing sponsor_count using GREATEST(0,
COALESCE(sponsor_count,0) - 1), sets is_sponsor = false when the resulting count
is 0 (true otherwise), updates updated_at = NOW(), and returns OLD; then add a
trigger on_remove_spot_sponsor AFTER DELETE ON spot_sponsors FOR EACH ROW
EXECUTE FUNCTION handle_remove_sponsor(). Ensure you reference the existing
symbols handle_remove_sponsor and on_remove_spot_sponsor when adding these.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
supabase_migration_sponsor.sqlto createspot_sponsorsand alterprofiles/spots, plus a trigger to mark sponsor profiles and incrementsponsor_counton insert.types.tswithSpotSponsorand new fields onUserProfile(is_sponsor,sponsor_count) andSpot(is_sponsored,sponsored_by,sponsor), and expandedChatMessageprofile pick to include sponsor metadata.sponsorServicetoservices/database.tswithgetSponsor,sponsorSpot,removeSponsor,getAllSponsors, andsubscribeToSponsorsimplementations.components/common/SponsorBadge.tsx(golden shimmer star) and integrated sponsor UI across pages:HomePage(sponsored banner, admin Add/Remove Sponsor modal, realtime refresh),DrinksPage(sponsor banner and bill override making totalRs.0for sponsored spots),ProfilePage(badge next to username and sponsor stats), andChatPage(badge next to sender names). Also updatedDATABASE_SCHEMA.mddocumentation.Testing
npm run -s build, which completed successfully.npx tsc --noEmitfailed due to a pre-existing unrelated import issue (components/SpotHistory.tsxreferences missing../services/supabaseClient); sponsor changes did not introduce type errors beyond that existing problem.Codex Task
Summary by CodeRabbit