Fix N+1 query when viewing people custom fields#146
Fix N+1 query when viewing people custom fields#146NeloNew wants to merge 1 commit intorelaticle:mainfrom
Conversation
Eager load custom field values and their relationships when querying People resources. This eliminates repeated database queries that were occurring when displaying custom field data on the view page. The withCustomFieldValues() scope loads: - customFieldValues - customFieldValues.customField - customFieldValues.customField.options Fixes RELATICLE-CRM-39
There was a problem hiding this comment.
Pull request overview
This pull request addresses an N+1 query performance issue when viewing people records by adding eager loading of custom field relationships. The fix prevents up to 10-20 separate database queries per page load by batch-loading all custom field data in a single query.
Changes:
- Added
->withCustomFieldValues()scope toPeopleResource::getEloquentQuery()to eagerly load custom field relationships (customFieldValues, customFieldValues.customField, and customFieldValues.customField.options)
| { | ||
| return parent::getEloquentQuery() | ||
| ->with(['team']) | ||
| ->withCustomFieldValues() |
There was a problem hiding this comment.
This eager loading optimization should be applied consistently to other resources that use custom fields. The CompanyResource, OpportunityResource, TaskResource, and NoteResource all implement HasCustomFields but don't include withCustomFieldValues() in their getEloquentQuery() methods, which means they likely suffer from the same N+1 query issue that this PR fixes for PeopleResource. Consider applying the same optimization to maintain consistency and performance across all entity types.
Addresses Copilot's feedback on PR relaticle#146 by applying eager loading optimization consistently across all resources that implement HasCustomFields. Changes: - Added ->withCustomFieldValues() to PeopleResource - Added ->withCustomFieldValues() to CompanyResource - Added ->withCustomFieldValues() to OpportunityResource - Added ->withCustomFieldValues() to TaskResource - Added ->withCustomFieldValues() to NoteResource Performance impact: - Before: 10-20+ queries per page load (one per record) - After: 1-3 queries total (batch loaded) Related: - Supersedes PR relaticle#146 (PeopleResource only) - Supersedes PR relaticle#150 (OpportunityResource only) - Resolves RELATICLE-CRM-39 - Resolves RELATICLE-CRM-38 Tests: 722 passed, 5 skipped
|
Superseded by comprehensive fix in PR #151 which applies the N+1 optimization across all 5 resources (People, Company, Opportunity, Task, Note) as suggested by Copilot. This ensures consistent performance optimization across all entity types that implement HasCustomFields. |
Addresses Copilot's feedback on PR #146 by applying eager loading optimization consistently across all resources that implement HasCustomFields. Changes: - Added ->withCustomFieldValues() to PeopleResource - Added ->withCustomFieldValues() to CompanyResource - Added ->withCustomFieldValues() to OpportunityResource - Added ->withCustomFieldValues() to TaskResource - Added ->withCustomFieldValues() to NoteResource Performance impact: - Before: 10-20+ queries per page load (one per record) - After: 1-3 queries total (batch loaded) Related: - Supersedes PR #146 (PeopleResource only) - Supersedes PR #150 (OpportunityResource only) - Resolves RELATICLE-CRM-39 - Resolves RELATICLE-CRM-38 Tests: 722 passed, 5 skipped
Issue
Resolves RELATICLE-CRM-39 - N+1 query detected in Sentry when viewing people records.
Problem
When displaying the People view page, custom field values were being loaded individually in a loop, causing up to 10 separate database queries per page load:
Solution
Added eager loading to the
PeopleResource::getEloquentQuery()method using the existingwithCustomFieldValues()scope. This loads all custom field relationships in a single query batch:Testing
✅ All 723 tests passing (296s duration)
✅ No regressions detected
✅ Eager loading applied to all People queries (list, view)
Impact