perf(supabase): Eliminate N+1 queries and optimize fetch payloads in SupabaseService #149#218
Open
VanshKaushal wants to merge 4 commits intoAOSSIE-Org:mainfrom
Open
perf(supabase): Eliminate N+1 queries and optimize fetch payloads in SupabaseService #149#218VanshKaushal wants to merge 4 commits intoAOSSIE-Org:mainfrom
VanshKaushal wants to merge 4 commits intoAOSSIE-Org:mainfrom
Conversation
Contributor
WalkthroughReplaces per-record user lookups with relational joins for tasks, tickets, and meetings; consolidates comment retrieval and ordering via joins. Adds GitHub integration: a Deno edge function creates GitHub issues after ticket insertion and the ticket is updated with issue metadata. Migration adds GitHub fields to tickets. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant App as Supabase Service
participant DB as Supabase DB
participant CloudFunc as create-github-issue
participant GitHub as GitHub API
Client->>App: createTicket(payload)
App->>DB: INSERT ticket
DB-->>App: ticketId
App->>CloudFunc: POST /create-github-issue (title, desc, category, priority, ticketId)
rect rgba(100, 150, 255, 0.5)
CloudFunc->>CloudFunc: validate input & env
CloudFunc->>CloudFunc: format title/body & labels
end
CloudFunc->>GitHub: POST /repos/.../issues (title, body, labels)
GitHub-->>CloudFunc: issueNumber, issueUrl
CloudFunc-->>App: {success, issueNumber, issueUrl}
rect rgba(100, 200, 100, 0.5)
App->>DB: UPDATE ticket SET github_issue_number, github_issue_url
DB-->>App: updated ticket
end
App-->>Client: ticket + github info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/services/supabase_service.dart`:
- Around line 1754-1790: When the call to
_client.functions.invoke('create-github-issue', ...) returns a non-200
functionsResponse, log the response body along with the status to aid debugging:
update the else branch that currently calls debugPrint('GitHub Edge Function
returned status ${functionsResponse.status}') to include functionsResponse.data
(or the raw body) and any error fields; also extend the catch block
debugPrint('Failed to sync ticket to GitHub: $e') to include the exception
details/stack or relevant response info where available. Target symbols:
_client.functions.invoke, functionsResponse, createdTicket, ticketId.
- Around line 1210-1216: The relational join in the Supabase query (the select
using creator:users!tasks_created_by_fkey(...) and
assignee:users!tasks_assigned_to_fkey(...), stored in the local variable query
in supabase_service.dart) fails because the current FK constraints point to
auth.users rather than public.users; update your database schema migrations to
drop and recreate the tasks_created_by_fkey and tasks_assigned_to_fkey
constraints so they reference the public.users(id) table (and apply the same
change for tickets_created_by_fkey, tickets_assigned_to_fkey, and
meetings_created_by_fkey), run the migration, and then re-run the code that
builds query so the PostgREST relational joins resolve and the N+1 optimization
works.
In `@supabase/functions/create-github-issue/index.ts`:
- Line 15: Validate the incoming payload after destructuring const { title,
description, category, priority, ticketId } = await req.json() by checking
required fields (at minimum title and description) and reject requests with a
400 response and a clear error message if they are missing or empty; update the
handler that uses these variables to perform this validation early, trimming
strings and ensuring types (e.g., title is a non-empty string) before proceeding
to create the GitHub issue so you never call the issue-creation logic with
undefined values.
In `@supabase/migrations/20260309152700_add_github_issue_to_tickets.sql`:
- Around line 1-4: Add a non-unique index on tickets.github_issue_number to
speed webhook lookups that update by that column: modify the migration to
include a CREATE INDEX (preferably CREATE INDEX CONCURRENTLY IF NOT EXISTS) on
github_issue_number for the tickets table and ensure the migration's rollback
drops that index; reference the tickets table and github_issue_number column
when adding the index so the webhook handler's lookup is accelerated without
locking the table during migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eb96ab44-8a52-4566-95bd-5309ad4c9154
📒 Files selected for processing (3)
lib/services/supabase_service.dartsupabase/functions/create-github-issue/index.tssupabase/migrations/20260309152700_add_github_issue_to_tickets.sql
Comment on lines
+1
to
+4
| -- Migration to add GitHub issue tracking to tickets | ||
| ALTER TABLE tickets | ||
| ADD COLUMN github_issue_number TEXT, | ||
| ADD COLUMN github_issue_url TEXT; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding an index on github_issue_number for webhook lookups.
Based on learnings, the GitHub webhook handler updates tickets by github_issue_number. If this lookup pattern is frequent, adding an index would improve query performance.
📈 Optional: Add index for webhook lookups
-- Migration to add GitHub issue tracking to tickets
ALTER TABLE tickets
ADD COLUMN github_issue_number TEXT,
ADD COLUMN github_issue_url TEXT;
+
+-- Index for efficient lookups by github_issue_number (used by webhook handler)
+CREATE INDEX idx_tickets_github_issue_number ON tickets(github_issue_number) WHERE github_issue_number IS NOT NULL;📝 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
| -- Migration to add GitHub issue tracking to tickets | |
| ALTER TABLE tickets | |
| ADD COLUMN github_issue_number TEXT, | |
| ADD COLUMN github_issue_url TEXT; | |
| -- Migration to add GitHub issue tracking to tickets | |
| ALTER TABLE tickets | |
| ADD COLUMN github_issue_number TEXT, | |
| ADD COLUMN github_issue_url TEXT; | |
| -- Index for efficient lookups by github_issue_number (used by webhook handler) | |
| CREATE INDEX idx_tickets_github_issue_number ON tickets(github_issue_number) WHERE github_issue_number IS NOT NULL; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260309152700_add_github_issue_to_tickets.sql` around
lines 1 - 4, Add a non-unique index on tickets.github_issue_number to speed
webhook lookups that update by that column: modify the migration to include a
CREATE INDEX (preferably CREATE INDEX CONCURRENTLY IF NOT EXISTS) on
github_issue_number for the tickets table and ensure the migration's rollback
drops that index; reference the tickets table and github_issue_number column
when adding the index so the webhook handler's lookup is accelerated without
locking the table during migration.
…ing, FK joins, and webhook index
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/create-github-issue/index.ts`:
- Around line 64-68: The code logs the raw GitHub API errorText when response.ok
is false (variables: response, errorText) which may expose sensitive repo
metadata; replace direct logging with a sanitized summary: read response.text()
into errorText, attempt JSON.parse(errorText) and extract known safe fields like
message or errors, fallback to stripping newlines and URLs and truncating to a
short length (e.g., 200 chars), then call console.error with the status and this
sanitized summary only and throw a new Error containing just the status (or the
sanitized short message) instead of the full raw response.
- Around line 17-37: Add validation for the variable category (used when
building the issue title like `"[${category}] ${title}"`) similar to
title/description: ensure category is a string and not just whitespace; if
invalid, either return a 400 response with a clear error (matching the other
validations) or coerce it to a safe default (e.g., "General") before
constructing the issue title to prevent "[undefined]" from appearing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a7833fdd-92b0-47d9-a8ab-9a0f3540b19a
📒 Files selected for processing (3)
lib/services/supabase_service.dartsupabase/functions/create-github-issue/index.tssupabase/migrations/20260309152700_add_github_issue_to_tickets.sql
Comment on lines
+17
to
+37
| // Strict Input Validation | ||
| if (!title || typeof title !== 'string' || title.trim() === '') { | ||
| return new Response( | ||
| JSON.stringify({ success: false, error: "Validation failed: 'title' must be a non-empty string." }), | ||
| { headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 400 } | ||
| ) | ||
| } | ||
|
|
||
| if (!description || typeof description !== 'string' || description.trim() === '') { | ||
| return new Response( | ||
| JSON.stringify({ success: false, error: "Validation failed: 'description' must be a non-empty string." }), | ||
| { headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 400 } | ||
| ) | ||
| } | ||
|
|
||
| if (!ticketId) { | ||
| return new Response( | ||
| JSON.stringify({ success: false, error: "Validation failed: 'ticketId' is required." }), | ||
| { headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 400 } | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Good input validation, but category is not validated and could produce "[undefined]" in issue titles.
The validation for title, description, and ticketId is comprehensive. However, category is used directly in the issue title at line 58 without validation. If category is undefined or not a string, the GitHub issue title will be formatted as "[undefined] title".
🛡️ Proposed fix to add category validation
if (!ticketId) {
return new Response(
JSON.stringify({ success: false, error: "Validation failed: 'ticketId' is required." }),
{ headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 400 }
)
}
+
+ if (!category || typeof category !== 'string' || category.trim() === '') {
+ return new Response(
+ JSON.stringify({ success: false, error: "Validation failed: 'category' must be a non-empty string." }),
+ { headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 400 }
+ )
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/create-github-issue/index.ts` around lines 17 - 37, Add
validation for the variable category (used when building the issue title like
`"[${category}] ${title}"`) similar to title/description: ensure category is a
string and not just whitespace; if invalid, either return a 400 response with a
clear error (matching the other validations) or coerce it to a safe default
(e.g., "General") before constructing the issue title to prevent "[undefined]"
from appearing.
Comment on lines
+64
to
+68
| if (!response.ok) { | ||
| const errorText = await response.text() | ||
| console.error(`GitHub API Error: ${response.status} ${errorText}`) | ||
| throw new Error(`GitHub API failed with status ${response.status}`) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider sanitizing logged error responses.
The GitHub API error text is logged directly at line 66. While this aids debugging, be aware that GitHub error responses could potentially include repository metadata. For production, consider logging only the status code and a sanitized summary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/create-github-issue/index.ts` around lines 64 - 68, The
code logs the raw GitHub API errorText when response.ok is false (variables:
response, errorText) which may expose sensitive repo metadata; replace direct
logging with a sanitized summary: read response.text() into errorText, attempt
JSON.parse(errorText) and extract known safe fields like message or errors,
fallback to stripping newlines and URLs and truncating to a short length (e.g.,
200 chars), then call console.error with the status and this sanitized summary
only and throw a new Error containing just the status (or the sanitized short
message) instead of the full raw response.
Author
|
All CodeRabbit review suggestions have been applied:
This branch is ready for re-review. |
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.
This PR improves the performance of multiple data-fetching methods in SupabaseService by removing N+1 query patterns and optimizing database payload size.
Previously, methods such as _doGetTasks(), getTaskDetails(), getTicketDetails(), getMeetings(), and getMeetingDetails() fetched base records and then executed additional asynchronous queries inside loops to retrieve related user information (creator, assignee, comment author) using _getUserInfo().
This resulted in multiple database round-trips per screen load and degraded performance as the number of records increased.
✅ What’s Changed
Replaced sequential _getUserInfo() calls with Supabase PostgREST relational joins to fetch related user data in a single query.
Removed N+1 query patterns across task, ticket, and meeting data-fetching methods.
Replaced all SELECT * queries with explicit column selection to reduce response payload size.
Ensured that existing filters (team_id, status, priority, assignments, etc.) continue to work as expected.
Verified that the returned nested JSON structure remains compatible with existing UI models, requiring no downstream changes.
⚡ Performance Impact
Reduces database round-trips from O(n) per screen load to O(1).
Improves data loading performance by fetching relational data (creator, assignee, comments, comment authors) in a single request.
Decreases network bandwidth usage due to optimized column selection.
🛠 Example Improvement
### Before (N+1 Query Pattern):
_final tasks = await _client.from('tasks').select('*');
for (var task in tasks) {
final creator = await getUserInfo(task['created_by']);
}
After (Relational Join):
_final tasks = await client.from('tasks').select('''
id, title, status, created_at,
creator:users!tasks_created_by_fkey(id, full_name, role)
''');
🧪 Validation
Tested with larger datasets to confirm reduced database calls.
Confirmed relational joins return correct nested user data.
Verified UI rendering remains unaffected.
📎 Notes
This change improves scalability and aligns the data layer with recommended Supabase query practices by avoiding unnecessary sequential requests.
Summary by CodeRabbit
New Features
Performance