-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add new load graph by relation trait #384
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds new functionality to load entity graphs with relations efficiently to address N+1 query problems when fetching summit data. The main goal is to improve performance by implementing a graph loading trait that can eagerly load related entities in optimized queries.
Key changes:
- Added a new
GraphLoaderTraitfor efficient entity graph loading with relation support - Enhanced the Summit repository with new methods that utilize the graph loading functionality
- Updated the Summit finder strategy to support relation preloading
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
app/libs/Utils/Doctrine/GraphLoaderTrait.php |
New trait implementing graph loading logic for entities with relations |
app/Repositories/Summit/DoctrineSummitRepository.php |
Added graph loading trait usage and new methods for loading summits with relations |
app/Models/Foundation/Summit/Repositories/ISummitRepository.php |
Added interface methods for relation-based summit retrieval |
app/Http/Controllers/Apis/Protected/Summit/Strategies/ISummitFinderStrategy.php |
Updated interface to support relations parameter |
app/Http/Controllers/Apis/Protected/Summit/Strategies/CurrentSummitFinderStrategy.php |
Implemented relation support in summit finder strategy |
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitApiController.php |
Updated summit retrieval to use relation preloading |
tests/OAuth2SummitApiTest.php |
Added test case for summit retrieval with relations parameter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| public function find($summit_id) | ||
| public function find($summit_id, array $relations = []) | ||
| { | ||
| if(count($relations) > 0){ |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after 'if' keyword. Should be 'if (count($relations) > 0) {' to follow PHP coding standards.
| if(count($relations) > 0){ | |
| if (count($relations) > 0){ |
| { | ||
| if(count($relations) > 0){ | ||
| $summit = $summit_id === 'current' ? $this->repository->getCurrentAndRelations($relations) : $this->repository->getByIdAndRelations(intval($summit_id), $relations); | ||
| if(is_null($summit)) |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after 'if' keyword. Should be 'if (is_null($summit))' to follow PHP coding standards.
| if(is_null($summit)) | |
| if (is_null($summit)) |
chore: improvee performance of get summit by id ( N + 1 ) problem
f6ae70b to
347251f
Compare
chore: improvee performance of get summit by id ( N + 1 ) problem
ref: https://app.clickup.com/t/86b6z5t03
feat: add new load graph by relation trait
chore: improve performance of get summit by id ( N + 1 ) problem