Skip to content

Fix N+1 tickets #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
VanshKaushal wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix N+1 tickets #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b0f70e
feat: automatically create GitHub issue when a ticket is created #212
VanshKaushal Mar 9, 2026
4fa6834
perf: refactor getTickets to use relational joins and fix N+1 problem…
VanshKaushal Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 52 additions & 22 deletions lib/services/supabase_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1665,7 +1665,14 @@ class SupabaseService {
final isAdmin = userProfile['role'] == 'admin';

// Create base query
final query = _client.from('tickets').select('*').eq('team_id', teamId);
// Using Supabase relational joins here completely eliminates the N+1 query problem.
// Instead of looping and querying `_getUserInfo` for each ticket individually,
// we fetch the creator and assignee info in a single optimized database request.
final query = _client.from('tickets').select('''
*,
creator:users!tickets_created_by_fkey(id, full_name, role),
assignee:users!tickets_assigned_to_fkey(id, full_name, role)
''').eq('team_id', teamId);

// Filter by assignment if requested and user is not admin
if (filterByAssignment && !isAdmin) {
Expand All @@ -1685,30 +1692,15 @@ class SupabaseService {
query.eq('priority', filterByPriority);
}

// Execute the optimized query and order the results
final response = await query.order('created_at', ascending: false);

// Process the response to add creator and assignee info
// Process the response into the format expected by the app
final List<Map<String, dynamic>> processedTickets = [];
for (var ticket in response) {
final Map<String, dynamic> processedTicket = {...ticket};

// Add creator info
if (ticket['created_by'] != null) {
final creatorInfo = await _getUserInfo(ticket['created_by']);
if (creatorInfo != null) {
processedTicket['creator'] = creatorInfo;
}
}

// Add assignee info
if (ticket['assigned_to'] != null) {
final assigneeInfo = await _getUserInfo(ticket['assigned_to']);
if (assigneeInfo != null) {
processedTicket['assignee'] = assigneeInfo;
}
}

processedTickets.add(processedTicket);
// The relational join already handles embedding the creator and assignee data.
// We ensure we retain compatibility with the original returned data structure.
processedTickets.add({...ticket});
}

return processedTickets;
Comment on lines +1695 to 1706
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider simplifying the processing loop.

Since the relational join already embeds creator and assignee data, the loop that spreads each ticket into a new map is redundant. You could simplify this to a direct type cast.

♻️ Optional simplification 
       // Execute the optimized query and order the results
       final response = await query.order('created_at', ascending: false);

-      // Process the response into the format expected by the app
-      final List<Map<String, dynamic>> processedTickets = [];
-      for (var ticket in response) {
-        // The relational join already handles embedding the creator and assignee data.
-        // We ensure we retain compatibility with the original returned data structure.
-        processedTickets.add({...ticket});
-      }
-
-      return processedTickets;
+      return List<Map<String, dynamic>>.from(response);
📝 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
// Execute the optimized query and order the results
final response = await query.order('created_at', ascending: false);
// Process the response to add creator and assignee info
// Process the response into the format expected by the app
final List<Map<String, dynamic>> processedTickets = [];
for (var ticket in response) {
final Map<String, dynamic> processedTicket = {...ticket};
// Add creator info
if (ticket['created_by'] != null) {
final creatorInfo = await _getUserInfo(ticket['created_by']);
if (creatorInfo != null) {
processedTicket['creator'] = creatorInfo;
}
}
// Add assignee info
if (ticket['assigned_to'] != null) {
final assigneeInfo = await _getUserInfo(ticket['assigned_to']);
if (assigneeInfo != null) {
processedTicket['assignee'] = assigneeInfo;
}
}
processedTickets.add(processedTicket);
// The relational join already handles embedding the creator and assignee data.
// We ensure we retain compatibility with the original returned data structure.
processedTickets.add({...ticket});
}
return processedTickets;
// Execute the optimized query and order the results
final response = await query.order('created_at', ascending: false);
return List<Map<String, dynamic>>.from(response);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/services/supabase_service.dart` around lines 1695 - 1706, The loop
copying each ticket into processedTickets is redundant; replace the manual
iteration that uses processedTickets and processedTickets.add({...ticket}) with
a direct cast of response to the expected type (e.g. return response as
List<Map<String, dynamic>>) after the query.order(...) call, ensuring the
returned value matches the function's expected List<Map<String, dynamic>>
signature and removing the unnecessary processedTickets variable and for-loop.

Expand Down Expand Up @@ -1818,12 +1810,50 @@ class SupabaseService {
};
}

final createdTicket = Map<String, dynamic>.from(response[0]);
final ticketId = createdTicket['id'];

try {
final functionsResponse = await _client.functions.invoke(
'create-github-issue',
body: {
'title': title,
'description': description,
'category': category,
'priority': priority,
'ticketId': ticketId,
},
);

if (functionsResponse.status == 200) {
final responseData = functionsResponse.data;
if (responseData != null && responseData['success'] == true) {
final String? issueNumber = responseData['issueNumber'];
final String? issueUrl = responseData['issueUrl'];

if (issueNumber != null && issueUrl != null) {
await _client.from('tickets').update({
'github_issue_number': issueNumber,
'github_issue_url': issueUrl,
}).eq('id', ticketId);

createdTicket['github_issue_number'] = issueNumber;
createdTicket['github_issue_url'] = issueUrl;
}
}
} else {
debugPrint('GitHub Edge Function returned status ${functionsResponse.status}');
}
} catch (e) {
debugPrint('Failed to sync ticket to GitHub: $e');
}
Comment on lines +1816 to +1849
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

supabase_flutter FunctionResponse status property

💡 Result:

In supabase_flutter (via the underlying functions_client), FunctionResponse.status is the HTTP status code returned by your Supabase Edge Function invocation (e.g., 200, 201, etc.). [1][2]

Important behavior: if the function returns a non-2xx status code, invoke() throws a FunctionException (which also includes the HTTP status) instead of returning a FunctionResponse. [2]

Example

try {
  final res = await supabase.functions.invoke('hello');
  print(res.status); // e.g. 200
  print(res.data);
} on FunctionException catch (e) {
  print(e.status);   // non-2xx status code
  print(e.details);  // response body / error details
}

Sources:
[1] Supabase Flutter Dart API: FunctionResponse class (status → int) https://pub.dev/documentation/supabase_flutter/latest/supabase_flutter/FunctionResponse-class.html
[2] supabase-flutter Edge Functions response/error handling (status is HTTP status; non-2xx throws FunctionException) https://deepwiki.com/supabase/supabase-flutter/7-edge-functions

Error handling logic is fundamentally flawed: the else branch is unreachable.

Per supabase_flutter documentation, invoke() throws a FunctionException for non-2xx status codes rather than returning a FunctionResponse. The else branch at line 1828 will never execute for errors—they're already caught by the catch block. Remove the else clause or refactor to clarify that only 2xx responses reach this point.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/services/supabase_service.dart` around lines 1816 - 1849, The else branch
after awaiting _client.functions.invoke('create-github-issue') is unreachable
because invoke throws on non-2xx; remove the else block and instead catch and
handle FunctionException in the catch clause (or add a specific on
FunctionException catch) so you can log the error status and payload; keep the
success path that checks functionsResponse.status == 200 and
responseData['success'] and update createdTicket and the tickets row as
currently done in that branch.


// Refresh tickets
await getTickets();

return {
'success': true,
'ticket': response[0],
'ticket': createdTicket,
};
} catch (e) {
debugPrint('Error creating ticket: $e');
Expand Down
72 changes: 72 additions & 0 deletions supabase/functions/create-github-issue/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { serve } from "https://deno.land/std@0.168.0/http/server.ts"

const corsHeaders = {
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Headers': 'authorization, x-client-info, apikey, content-type',
}

serve(async (req) => {
// Handle CORS preflight
if (req.method === 'OPTIONS') {
return new Response('ok', { headers: corsHeaders })
}

try {
const { title, description, category, priority, ticketId } = await req.json()
Comment on lines +14 to +15
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add input validation for required fields.

The function destructures inputs but doesn't validate that required fields (title, category, priority, ticketId) are present before use. If title is missing, the GitHub issue would be created with malformed content like [undefined] undefined.

🛡️ Proposed fix to add validation 
   try {
     const { title, description, category, priority, ticketId } = await req.json()
+
+    if (!title || !category || !priority || !ticketId) {
+      return new Response(
+        JSON.stringify({ success: false, error: 'Missing required fields: title, category, priority, ticketId' }),
+        { headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 400 }
+      )
+    }
     
     const githubToken = Deno.env.get('GITHUB_ACCESS_TOKEN')
📝 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
try {
const { title, description, category, priority, ticketId } = await req.json()
try {
const { title, description, category, priority, ticketId } = await req.json()
if (!title || !category || !priority || !ticketId) {
return new Response(
JSON.stringify({ success: false, error: 'Missing required fields: title, category, priority, ticketId' }),
{ headers: { ...corsHeaders, "Content-Type": "application/json" }, status: 400 }
)
}
const githubToken = Deno.env.get('GITHUB_ACCESS_TOKEN')
🤖 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 14 - 15, The
handler reads const { title, description, category, priority, ticketId } = await
req.json() but doesn't validate required inputs; add explicit checks after that
destructuring to ensure title, category, priority and ticketId are present
(non-empty) and return a 400/validation error response if any are missing;
update the code paths that build the GitHub issue (where title/description are
used) to only proceed when validation passes, and include clear validation error
messages referencing the title, category, priority and ticketId symbols so
malformed values (e.g. undefined) cannot be sent to create the issue.


const githubToken = Deno.env.get('GITHUB_ACCESS_TOKEN')
const repoOwner = Deno.env.get('GITHUB_REPO_OWNER')
const repoName = Deno.env.get('GITHUB_REPO_NAME')

if (!githubToken || !repoOwner || !repoName) {
throw new Error("GitHub configuration is missing in environment variables.")
}

const githubApiUrl = `https://api.github.com/repos/${repoOwner}/${repoName}/issues`

const response = await fetch(githubApiUrl, {
method: "POST",
headers: {
"Accept": "application/vnd.github+json",
"Authorization": `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
"Content-Type": "application/json"
},
body: JSON.stringify({
title: `[${category}] ${title}`,
body: `**Ticket ID:** ${ticketId}\n**Priority:** ${priority}\n\n${description || 'No description provided.'}`,
labels: [category?.toLowerCase() || 'bug', priority?.toLowerCase() || 'medium']
})
})

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}`)
}

const data = await response.json()

return new Response(
JSON.stringify({
success: true,
issueNumber: data.number.toString(),
issueUrl: data.html_url
}),
{
headers: { ...corsHeaders, "Content-Type": "application/json" },
status: 200,
}
)
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error)
console.error("Edge Function Error:", errorMessage)
return new Response(
JSON.stringify({ success: false, error: errorMessage }),
{
headers: { ...corsHeaders, "Content-Type": "application/json" },
status: 500, // Return 500 but handled gracefully by the client
}
)
}
})
4 changes: 4 additions & 0 deletions supabase/migrations/20260309152700_add_github_issue_to_tickets.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- Migration to add GitHub issue tracking to tickets
ALTER TABLE tickets
ADD COLUMN github_issue_number TEXT,
ADD COLUMN github_issue_url TEXT;