Skip to content

fix: resolve N+1 query pattern in SupabaseService#236

Open
g-k-s-03 wants to merge 5 commits intoAOSSIE-Org:mainfrom
g-k-s-03:fix/n-plus-one-supabase-queries
Open

fix: resolve N+1 query pattern in SupabaseService#236
g-k-s-03 wants to merge 5 commits intoAOSSIE-Org:mainfrom
g-k-s-03:fix/n-plus-one-supabase-queries

Conversation

@g-k-s-03
Copy link
Copy Markdown
Contributor

@g-k-s-03 g-k-s-03 commented Mar 22, 2026

Closes #149

Description

Several methods in SupabaseService were making O(n) database calls due to calling _getUserInfo() in a loop for each item fetched. This caused serious performance degradation as team data grows — for example, loading 50 tickets triggered 101 separate database round-trips. This PR fixes all affected methods by replacing the loop-based user lookups with Supabase PostgREST inline joins, reducing every affected method to O(1) queries.

Changes Made

  • Refactored getTickets() to use a single joined query instead of calling _getUserInfo() per ticket for creator and assignee
  • Refactored getTasks() with the same join-based approach
  • Refactored getTicketDetails() to fetch creator, assignee, and all comment users via joins instead of a per-comment loop
  • Refactored getTaskDetails() with the same fix
  • Refactored getMeetings() to join creator info inline instead of looping
  • Refactored getMeetingDetails() to use a single joined query
  • Refactored addTicketComment() and addTaskComment() to return user info via .select() join instead of a follow-up query
  • Removed _getUserInfo() helper entirely as it is no longer needed anywhere

Before the fix, methods like getTickets(), getTasks(), getMeetings(), and their detail methods were calling _getUserInfo() inside a loop for every item fetched. This meant loading 50 tickets triggered 101 database queries, and loading ticket details with 20 comments triggered 22 queries. Performance got worse as data grew.

After the fix, all user data is fetched inline using Supabase PostgREST joins directly in the select() call. Loading 50 tickets now takes 1 query. Loading ticket details with 20 comments now takes 2 queries. The _getUserInfo() helper has been removed entirely as it is no longer needed.

Summary by CodeRabbit

  • Chores
    • Optimized database query operations to enhance application performance and reduce load times when retrieving tasks, tickets, and meetings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 22, 2026

Walkthrough

The change refactors database query methods in SupabaseService to eliminate N+1 query patterns by replacing sequential user lookups with PostgREST foreign key joins. Methods for fetching tasks, tickets, meetings, and comments now inline related user data (creator, assignee, comment author) via joined selects instead of per-item _getUserInfo calls, consolidating multiple queries into single database roundtrips.

Changes

Cohort / File(s) Summary
Database Query Optimization
lib/services/supabase_service.dart
Replaced N+1 user enrichment pattern with PostgREST joins: getTasks/_doGetTasks and getTickets now inline creator and assignee via joins; getTaskDetails and getTicketDetails additionally inline comment user data; addTaskComment and addTicketComment fetch user info via select joins; getMeetings and getMeetingDetails inline creator joins. Removed _getUserInfo helper method entirely. Consolidated filter logic and simplified object assembly by returning raw joined responses. Updated import formatting and removed trailing EOF newline.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

Dart/Flutter

Poem

🐰 Once queries multiplied like spring carrots in rows,
N+1 patterns making the database groans!
But joins came to rescue with elegant grace,
Now one query fetches the whole user space!

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: resolve N+1 query pattern in SupabaseService' clearly and concisely summarizes the primary change: eliminating N+1 queries through PostgREST joins.
Linked Issues check ✅ Passed All coding requirements from issue #149 are met: N+1 patterns eliminated in getTickets, getTasks, getTicketDetails, getTaskDetails using joins; user data fetched via single queries; database calls reduced from O(n) to O(1).
Out of Scope Changes check ✅ Passed All changes directly address N+1 query elimination as specified in issue #149; import formatting and EOF newline removal are minor cleanup changes within scope of the refactor.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 1209-1213: Extract the repeated embed projection "id, full_name,
role" into a single reusable constant (e.g., USER_EMBED_PROJECTION) or helper
and use it wherever you call select with embedded users (for example in the
_client.from('tasks').select(...) call that builds creator:created_by(...) and
assignee:assigned_to(...)); replace the inline fragments in all locations (the
select calls for tasks, tickets, meetings, comments referenced in the review) to
interpolate or concatenate that constant instead of duplicating the literal so
future schema changes only require updating the single symbol.
- Around line 1574-1595: The method that fetches tickets currently returns the
rows but does not publish them to ticketsStream, so callers like createTicket(),
updateTicketStatus(), updateTicketPriority(), updateTicketApproval(), and
assignTicket() (which call getTickets() and ignore its return) won't notify
subscribers; after building response and converting it to List<Map<String,
dynamic>> (the existing List.from(response)), push that list into ticketsStream
(e.g., ticketsStream.add(fetchedList) or ticketsStream.addStream/ sink.add
depending on your StreamController usage) before returning it so subscribers
receive the refreshed ticket list.
- Line 1: Remove the stray backslash before the import statement at the top of
lib/services/supabase_service.dart so the line reads a valid Dart import; locate
the line containing "\import 'dart:convert';" and change it to "import
'dart:convert';" (no leading backslash) ensuring the file parses correctly.
🪄 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: 6a6dc264-201f-40b1-bb02-c2e474371167

📥 Commits

Reviewing files that changed from the base of the PR and between 5afe656 and 141d1ba.

📒 Files selected for processing (1)
  • lib/services/supabase_service.dart

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: N+1 query pattern in getTickets/getTasks/getTicketDetails causes performance issues

1 participant