From b5d8499ac743fcd2b0e33ff7ea5c2ad8b7818217 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=BCneyt=20=C5=9Eent=C3=BCrk?= Date: Sun, 9 Nov 2025 15:25:46 +0000 Subject: [PATCH] Contact n+1 issue solved.. --- app/Http/Controllers/Purchases/Vendors.php | 15 +- app/Http/Controllers/Sales/Customers.php | 15 +- app/Models/Common/Contact.php | 6 +- app/Models/Document/Document.php | 12 +- .../Feature/Performance/VendorN1QueryTest.php | 255 ++++++++++++++++++ 5 files changed, 298 insertions(+), 5 deletions(-) create mode 100644 tests/Feature/Performance/VendorN1QueryTest.php diff --git a/app/Http/Controllers/Purchases/Vendors.php b/app/Http/Controllers/Purchases/Vendors.php index 194e84992..7b2f1359a 100644 --- a/app/Http/Controllers/Purchases/Vendors.php +++ b/app/Http/Controllers/Purchases/Vendors.php @@ -30,7 +30,20 @@ class Vendors extends Controller */ public function index() { - $vendors = Contact::with('media', 'bills.histories', 'bills.totals', 'bills.transactions', 'bills.media')->vendor()->collect(); + $vendors = Contact::with([ + 'media', + 'bills.histories', + 'bills.totals', + 'bills.transactions', + 'bills.media' + ]) + ->withCount([ + 'contact_persons as contact_persons_with_email_count' => function ($query) { + $query->whereNotNull('email'); + } + ]) + ->vendor() + ->collect(); return $this->response('purchases.vendors.index', compact('vendors')); } diff --git a/app/Http/Controllers/Sales/Customers.php b/app/Http/Controllers/Sales/Customers.php index 87fe6eb8a..cffe307a9 100644 --- a/app/Http/Controllers/Sales/Customers.php +++ b/app/Http/Controllers/Sales/Customers.php @@ -30,7 +30,20 @@ class Customers extends Controller */ public function index() { - $customers = Contact::customer()->with('media', 'invoices.histories', 'invoices.totals', 'invoices.transactions', 'invoices.media')->collect(); + $customers = Contact::customer() + ->with([ + 'media', + 'invoices.histories', + 'invoices.totals', + 'invoices.transactions', + 'invoices.media' + ]) + ->withCount([ + 'contact_persons as contact_persons_with_email_count' => function ($query) { + $query->whereNotNull('email'); + } + ]) + ->collect(); return $this->response('sales.customers.index', compact('customers')); } diff --git a/app/Models/Common/Contact.php b/app/Models/Common/Contact.php index 982b250f2..768357822 100644 --- a/app/Models/Common/Contact.php +++ b/app/Models/Common/Contact.php @@ -303,7 +303,11 @@ class Contact extends Model { if (! empty($this->email)) { return true; - } + } + + if (isset($this->contact_persons_with_email_count)) { + return $this->contact_persons_with_email_count > 0; + } if ($this->contact_persons()->whereNotNull('email')->count()) { return true; diff --git a/app/Models/Document/Document.php b/app/Models/Document/Document.php index 040bc2cd1..fc5c25d3d 100644 --- a/app/Models/Document/Document.php +++ b/app/Models/Document/Document.php @@ -274,14 +274,22 @@ class Document extends Model public function getSentAtAttribute(string $value = null) { - $sent = $this->histories()->where('document_histories.status', 'sent')->first(); + if ($this->relationLoaded('histories')) { + $sent = $this->histories->where('status', 'sent')->first(); + } else { + $sent = $this->histories()->where('document_histories.status', 'sent')->first(); + } return $sent->created_at ?? null; } public function getReceivedAtAttribute(string $value = null) { - $received = $this->histories()->where('document_histories.status', 'received')->first(); + if ($this->relationLoaded('histories')) { + $received = $this->histories->where('status', 'received')->first(); + } else { + $received = $this->histories()->where('document_histories.status', 'received')->first(); + } return $received->created_at ?? null; } diff --git a/tests/Feature/Performance/VendorN1QueryTest.php b/tests/Feature/Performance/VendorN1QueryTest.php new file mode 100644 index 000000000..e80b21b5a --- /dev/null +++ b/tests/Feature/Performance/VendorN1QueryTest.php @@ -0,0 +1,255 @@ +loginAs(); + + // Create vendors using factory + $vendors = Contact::factory()->vendor()->count(10)->create([ + 'company_id' => $this->company->id + ]); + + // Add contact persons to vendors (with required type field) + foreach ($vendors as $vendor) { + for ($j = 0; $j < rand(1, 3); $j++) { + ContactPerson::create([ + 'company_id' => $this->company->id, + 'type' => 'vendor', // Required field + 'contact_id' => $vendor->id, + 'name' => 'Person ' . $j, + 'email' => 'person' . $vendor->id . '_' . $j . '@example.com', + 'phone' => '123-456-7890' + ]); + } + } + + // Test optimized query with withCount + DB::enableQueryLog(); + + $optimizedVendors = Contact::vendor() + ->withCount([ + 'contact_persons as contact_persons_with_email_count' => function ($query) { + $query->whereNotNull('email'); + } + ]) + ->limit(10) + ->get(); + + // Access has_email attribute (uses cached count) + foreach ($optimizedVendors as $vendor) { + $hasEmail = $vendor->has_email; + } + + $queryCount = count(DB::getQueryLog()); + DB::disableQueryLog(); + + // Assertions + $this->assertEquals(10, $optimizedVendors->count()); + $this->assertLessThan(5, $queryCount, 'Should use less than 5 queries'); + + // Verify count attribute is loaded + foreach ($optimizedVendors as $vendor) { + $this->assertTrue(isset($vendor->contact_persons_with_email_count)); + } + } + + /** + * Test document histories eager loading prevents N+1 + */ + public function testDocumentHistoriesEagerLoadingOptimization() + { + $this->loginAs(); + + // Create vendor with bills + $vendor = Contact::factory()->vendor()->create([ + 'company_id' => $this->company->id + ]); + + // Create bills with histories + for ($i = 0; $i < 5; $i++) { + $bill = Document::factory()->bill()->create([ + 'company_id' => $this->company->id, + 'contact_id' => $vendor->id + ]); + + // Add sent and received histories (with required type field) + DocumentHistory::create([ + 'company_id' => $this->company->id, + 'type' => Document::BILL_TYPE, // Required field + 'document_id' => $bill->id, + 'status' => 'sent', + 'notify' => 0, + 'description' => 'Sent to vendor' + ]); + + DocumentHistory::create([ + 'company_id' => $this->company->id, + 'type' => Document::BILL_TYPE, // Required field + 'document_id' => $bill->id, + 'status' => 'received', + 'notify' => 0, + 'description' => 'Received from vendor' + ]); + } + + // Test optimized query with eager loading + DB::enableQueryLog(); + + $optimizedBills = Document::bill() + ->where('contact_id', $vendor->id) + ->with('histories') + ->get(); + + // Access sent_at and received_at attributes (uses eager loaded data) + foreach ($optimizedBills as $bill) { + $sentAt = $bill->sent_at; + $receivedAt = $bill->received_at; + } + + $queryCount = count(DB::getQueryLog()); + DB::disableQueryLog(); + + // Assertions + $this->assertEquals(5, $optimizedBills->count()); + $this->assertLessThan(5, $queryCount, 'Should use less than 5 queries'); + + // Verify histories relationship is loaded + foreach ($optimizedBills as $bill) { + $this->assertTrue($bill->relationLoaded('histories')); + } + } + + /** + * Test vendors index page uses optimized queries + */ + public function testVendorsIndexPageOptimization() + { + $this->loginAs(); + + // Create realistic test data + $vendors = Contact::factory()->vendor()->count(10)->create([ + 'company_id' => $this->company->id + ]); + + foreach ($vendors->take(5) as $vendor) { + // Add contact persons (with required type field) + ContactPerson::create([ + 'company_id' => $this->company->id, + 'type' => 'vendor', // Required field + 'contact_id' => $vendor->id, + 'name' => 'Test Person', + 'email' => 'person@example.com', + 'phone' => '1234567890' + ]); + + // Add bills with histories + for ($i = 0; $i < 2; $i++) { + $bill = Document::factory()->bill()->create([ + 'company_id' => $this->company->id, + 'contact_id' => $vendor->id + ]); + + DocumentHistory::create([ + 'company_id' => $this->company->id, + 'type' => Document::BILL_TYPE, // Required field + 'document_id' => $bill->id, + 'status' => 'sent', + 'notify' => 0, + 'description' => 'Sent' + ]); + } + } + + // Test optimized controller query + DB::enableQueryLog(); + + $optimizedVendors = Contact::with([ + 'media', + 'bills.histories', + 'bills.totals', + 'bills.transactions', + 'bills.media' + ]) + ->withCount([ + 'contact_persons as contact_persons_with_email_count' => function ($query) { + $query->whereNotNull('email'); + } + ]) + ->vendor() + ->collect(); + + // Simulate view access + foreach ($optimizedVendors as $vendor) { + $hasEmail = $vendor->has_email; + $open = $vendor->open; + + foreach ($vendor->bills as $bill) { + $sentAt = $bill->sent_at; + } + } + + $queryCount = count(DB::getQueryLog()); + DB::disableQueryLog(); + + // Assertions + $this->assertGreaterThan(0, $optimizedVendors->count()); + $this->assertLessThan(15, $queryCount, 'Should use less than 15 queries'); + + // Verify relationships are loaded + foreach ($optimizedVendors as $vendor) { + $this->assertTrue(isset($vendor->contact_persons_with_email_count)); + + if ($vendor->bills->count() > 0) { + $this->assertTrue($vendor->bills->first()->relationLoaded('histories')); + } + } + } + + /** + * Test actual HTTP request to vendors index + */ + public function testVendorsIndexHttpRequest() + { + $this->loginAs(); + + // Create test data + Contact::factory()->vendor()->count(5)->create([ + 'company_id' => $this->company->id + ]); + + DB::enableQueryLog(); + + $response = $this->get(route('vendors.index')); + + $queryCount = count(DB::getQueryLog()); + DB::disableQueryLog(); + + $response->assertStatus(200); + $response->assertViewIs('purchases.vendors.index'); + + // Should not use excessive queries + $this->assertLessThan(30, $queryCount, 'Vendors index should use reasonable number of queries'); + } +}