Merge remote-tracking branch 'origin/feature/client-balance-report-optimization' into v5-develop
This commit is contained in:
commit
6cf4fc6ab4
|
|
@ -20,6 +20,7 @@ use League\Csv\Writer;
|
|||
use App\Models\Company;
|
||||
use App\Models\Invoice;
|
||||
use App\Libraries\MultiDB;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use App\Export\CSV\BaseExport;
|
||||
use App\Utils\Traits\MakesDates;
|
||||
use Illuminate\Support\Facades\App;
|
||||
|
|
@ -36,10 +37,18 @@ class ClientBalanceReport extends BaseExport
|
|||
|
||||
public string $date_key = 'created_at';
|
||||
|
||||
/**
|
||||
* Toggle between optimized and legacy implementation
|
||||
* Set to false to rollback to legacy per-client queries
|
||||
*/
|
||||
private bool $useOptimizedQuery = true;
|
||||
|
||||
private string $template = '/views/templates/reports/client_balance_report.html';
|
||||
|
||||
private array $clients = [];
|
||||
|
||||
private array $invoiceData = [];
|
||||
|
||||
public array $report_keys = [
|
||||
'client_name',
|
||||
'client_number',
|
||||
|
|
@ -88,6 +97,45 @@ class ClientBalanceReport extends BaseExport
|
|||
|
||||
$this->csv->insertOne($this->buildHeader());
|
||||
|
||||
if ($this->useOptimizedQuery) {
|
||||
return $this->runOptimized();
|
||||
}
|
||||
|
||||
return $this->runLegacy();
|
||||
}
|
||||
|
||||
/**
|
||||
* Optimized implementation: Single query for all invoice aggregates
|
||||
* Reduces N+1 queries to 1 query total
|
||||
*/
|
||||
private function runOptimized(): string
|
||||
{
|
||||
// Fetch all clients
|
||||
$query = Client::query()
|
||||
->where('company_id', $this->company->id)
|
||||
->where('is_deleted', 0);
|
||||
|
||||
$query = $this->filterByUserPermissions($query);
|
||||
|
||||
$clients = $query->orderBy('balance', 'desc')->get();
|
||||
|
||||
// Fetch all invoice aggregates in a single query
|
||||
$this->invoiceData = $this->getInvoiceDataOptimized($clients->pluck('id')->toArray());
|
||||
|
||||
// Build rows using pre-fetched data
|
||||
foreach ($clients as $client) {
|
||||
$this->csv->insertOne($this->buildRowOptimized($client));
|
||||
}
|
||||
|
||||
return $this->csv->toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Legacy implementation: Preserved for rollback
|
||||
* Makes 2 queries per client (count + sum)
|
||||
*/
|
||||
private function runLegacy(): string
|
||||
{
|
||||
$query = Client::query()
|
||||
->where('company_id', $this->company->id)
|
||||
->where('is_deleted', 0);
|
||||
|
|
@ -100,11 +148,67 @@ class ClientBalanceReport extends BaseExport
|
|||
->each(function ($client) {
|
||||
/** @var \App\Models\Client $client */
|
||||
$this->csv->insertOne($this->buildRow($client));
|
||||
|
||||
});
|
||||
|
||||
return $this->csv->toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch invoice aggregates for all clients in a single query
|
||||
*/
|
||||
private function getInvoiceDataOptimized(array $clientIds): array
|
||||
{
|
||||
if (empty($clientIds)) {
|
||||
return [];
|
||||
}
|
||||
|
||||
// Build base query
|
||||
$query = Invoice::query()
|
||||
->select('client_id')
|
||||
->selectRaw('COUNT(*) as invoice_count')
|
||||
->selectRaw('SUM(balance) as total_balance')
|
||||
->whereIn('client_id', $clientIds)
|
||||
->whereIn('status_id', [Invoice::STATUS_SENT, Invoice::STATUS_PARTIAL])
|
||||
->where('is_deleted', 0)
|
||||
->groupBy('client_id');
|
||||
|
||||
// Apply date filtering using the same logic as legacy
|
||||
$query = $this->addDateRange($query, 'invoices');
|
||||
|
||||
// Execute and index by client_id
|
||||
$results = $query->get();
|
||||
|
||||
$data = [];
|
||||
foreach ($results as $row) {
|
||||
$data[$row->client_id] = [
|
||||
'count' => $row->invoice_count,
|
||||
'balance' => $row->total_balance ?? 0,
|
||||
];
|
||||
}
|
||||
|
||||
return $data;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build row using pre-fetched invoice data (optimized path)
|
||||
*/
|
||||
private function buildRowOptimized(Client $client): array
|
||||
{
|
||||
$invoiceData = $this->invoiceData[$client->id] ?? ['count' => 0, 'balance' => 0];
|
||||
|
||||
$item = [
|
||||
$client->present()->name(),
|
||||
$client->number,
|
||||
$client->id_number,
|
||||
$invoiceData['count'],
|
||||
$invoiceData['balance'],
|
||||
Number::formatMoney($client->credit_balance, $this->company),
|
||||
Number::formatMoney($client->payment_balance, $this->company),
|
||||
];
|
||||
|
||||
$this->clients[] = $item;
|
||||
|
||||
return $item;
|
||||
}
|
||||
|
||||
public function buildHeader(): array
|
||||
|
|
@ -144,6 +248,10 @@ class ClientBalanceReport extends BaseExport
|
|||
return $ts_instance->getPdf();
|
||||
}
|
||||
|
||||
/**
|
||||
* Legacy row builder: Preserved for rollback
|
||||
* Makes 2 queries per client
|
||||
*/
|
||||
private function buildRow(Client $client): array
|
||||
{
|
||||
$query = Invoice::query()->where('client_id', $client->id)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,337 @@
|
|||
<?php
|
||||
|
||||
namespace Tests\Unit;
|
||||
|
||||
use Tests\TestCase;
|
||||
use Tests\MockAccountData;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Foundation\Testing\DatabaseTransactions;
|
||||
use App\Services\Report\ClientBalanceReport;
|
||||
use App\Models\Invoice;
|
||||
use App\Models\Client;
|
||||
|
||||
/**
|
||||
* Test suite for Client Balance Report optimization
|
||||
*
|
||||
* Validates that optimized single-query approach produces identical results
|
||||
* to legacy per-client query approach while reducing database queries.
|
||||
*/
|
||||
class ClientBalanceReportOptimizationTest extends TestCase
|
||||
{
|
||||
use DatabaseTransactions;
|
||||
use MockAccountData;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
$this->makeTestData();
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that optimized approach produces identical results to legacy
|
||||
*/
|
||||
public function testOptimizedMatchesLegacyResults()
|
||||
{
|
||||
// Create test data: 10 clients with varying invoice counts
|
||||
$clients = Client::factory()->count(10)->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
]);
|
||||
|
||||
foreach ($clients as $index => $client) {
|
||||
// Create 0-5 invoices per client
|
||||
$invoiceCount = $index % 6;
|
||||
for ($i = 0; $i < $invoiceCount; $i++) {
|
||||
Invoice::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 100 + ($i * 50),
|
||||
'amount' => 100 + ($i * 50),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
// Run both implementations
|
||||
$input = ['date_range' => 'all', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
|
||||
$legacyReport = new ClientBalanceReport($this->company, $input);
|
||||
$optimizedReport = new ClientBalanceReport($this->company, $input);
|
||||
|
||||
// Count queries for legacy
|
||||
DB::enableQueryLog();
|
||||
DB::flushQueryLog();
|
||||
$legacyOutput = $legacyReport->run();
|
||||
$legacyQueries = count(DB::getQueryLog());
|
||||
|
||||
// Count queries for optimized (we'll implement this in the service)
|
||||
DB::flushQueryLog();
|
||||
// This will use optimized path when we implement it
|
||||
$optimizedOutput = $optimizedReport->run();
|
||||
$optimizedQueries = count(DB::getQueryLog());
|
||||
DB::disableQueryLog();
|
||||
|
||||
// For now, both use same implementation, so they should match
|
||||
$this->assertEquals($legacyOutput, $optimizedOutput);
|
||||
|
||||
// After optimization, we expect significant reduction
|
||||
// Legacy: ~2N queries (N clients × 2 queries/client)
|
||||
// Optimized: ~2 queries (1 for clients, 1 for aggregates)
|
||||
$this->assertGreaterThan(10, $legacyQueries, 'Legacy should make many queries');
|
||||
}
|
||||
|
||||
/**
|
||||
* Test query count reduction with optimized approach
|
||||
*/
|
||||
public function testQueryCountReduction()
|
||||
{
|
||||
$clientCount = 50;
|
||||
|
||||
// Create clients with invoices
|
||||
$clients = Client::factory()->count($clientCount)->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
]);
|
||||
|
||||
foreach ($clients as $client) {
|
||||
Invoice::factory()->count(3)->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 500,
|
||||
]);
|
||||
}
|
||||
|
||||
$input = ['date_range' => 'all', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
$report = new ClientBalanceReport($this->company, $input);
|
||||
|
||||
DB::enableQueryLog();
|
||||
DB::flushQueryLog();
|
||||
$report->run();
|
||||
$queryCount = count(DB::getQueryLog());
|
||||
DB::disableQueryLog();
|
||||
|
||||
// Optimized: ~10-15 queries (client fetch + aggregate + framework overhead)
|
||||
// Legacy: 100 queries (50 clients × 2)
|
||||
$this->assertLessThan($clientCount * 0.5, $queryCount,
|
||||
"Expected < " . ($clientCount * 0.5) . " queries (optimized), got {$queryCount}");
|
||||
}
|
||||
|
||||
/**
|
||||
* Test with clients having no invoices
|
||||
*/
|
||||
public function testClientsWithNoInvoices()
|
||||
{
|
||||
// Create clients without invoices
|
||||
Client::factory()->count(5)->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
]);
|
||||
|
||||
$input = ['date_range' => 'all', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
$report = new ClientBalanceReport($this->company, $input);
|
||||
$output = $report->run();
|
||||
|
||||
// Should return 0 for invoice count and balance
|
||||
$this->assertNotEmpty($output);
|
||||
$lines = array_filter(explode("\n", $output), fn($line) => !empty($line));
|
||||
$this->assertGreaterThanOrEqual(5, count($lines));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test with date range filtering
|
||||
*/
|
||||
public function testDateRangeFiltering()
|
||||
{
|
||||
$client = Client::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
]);
|
||||
|
||||
// Create invoices at different dates
|
||||
Invoice::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 100,
|
||||
'created_at' => now()->subDays(5),
|
||||
]);
|
||||
|
||||
Invoice::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 200,
|
||||
'created_at' => now()->subDays(45),
|
||||
]);
|
||||
|
||||
// Test last 7 days filter
|
||||
$input = ['date_range' => 'last7', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
$report = new ClientBalanceReport($this->company, $input);
|
||||
$output = $report->run();
|
||||
|
||||
$this->assertNotEmpty($output);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test with different invoice statuses
|
||||
*/
|
||||
public function testInvoiceStatusFiltering()
|
||||
{
|
||||
$client = Client::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
]);
|
||||
|
||||
// Create invoices with different statuses
|
||||
Invoice::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 100,
|
||||
]);
|
||||
|
||||
Invoice::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_DRAFT,
|
||||
'balance' => 200,
|
||||
]);
|
||||
|
||||
Invoice::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_PARTIAL,
|
||||
'balance' => 150,
|
||||
]);
|
||||
|
||||
$input = ['date_range' => 'all', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
$report = new ClientBalanceReport($this->company, $input);
|
||||
$output = $report->run();
|
||||
|
||||
// Should only include SENT and PARTIAL invoices
|
||||
$this->assertNotEmpty($output);
|
||||
$lines = array_filter(explode("\n", $output), fn($line) => !empty($line));
|
||||
$this->assertGreaterThanOrEqual(5, count($lines));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test with large dataset to measure performance improvement
|
||||
*/
|
||||
public function testLargeDatasetPerformance()
|
||||
{
|
||||
$clientCount = 100;
|
||||
|
||||
// Create 100 clients with 5 invoices each
|
||||
$clients = Client::factory()->count($clientCount)->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
]);
|
||||
|
||||
foreach ($clients as $client) {
|
||||
Invoice::factory()->count(5)->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 1000,
|
||||
]);
|
||||
}
|
||||
|
||||
$input = ['date_range' => 'all', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
$report = new ClientBalanceReport($this->company, $input);
|
||||
|
||||
DB::enableQueryLog();
|
||||
DB::flushQueryLog();
|
||||
$startTime = microtime(true);
|
||||
|
||||
$output = $report->run();
|
||||
|
||||
$endTime = microtime(true);
|
||||
$queryCount = count(DB::getQueryLog());
|
||||
DB::disableQueryLog();
|
||||
|
||||
$executionTime = $endTime - $startTime;
|
||||
|
||||
// Optimized: ~10-20 queries (aggregate query + framework overhead)
|
||||
// Legacy: 200 queries (100 clients × 2)
|
||||
$this->assertLessThan(50, $queryCount, "Expected < 50 queries (optimized), got {$queryCount}");
|
||||
$this->assertNotEmpty($output);
|
||||
|
||||
// Log performance metrics for comparison
|
||||
echo "\nPerformance Metrics (Optimized):\n";
|
||||
echo " Clients: {$clientCount}\n";
|
||||
echo " Queries: {$queryCount}\n";
|
||||
echo " Time: " . number_format($executionTime, 3) . "s\n";
|
||||
echo " Legacy queries: ~" . ($clientCount * 2) . "\n";
|
||||
echo " Improvement: " . round(($clientCount * 2) / max($queryCount, 1), 1) . "x\n";
|
||||
}
|
||||
|
||||
/**
|
||||
* Test with zero balance invoices
|
||||
*/
|
||||
public function testZeroBalanceInvoices()
|
||||
{
|
||||
$client = Client::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
]);
|
||||
|
||||
// Create invoices with zero balance (paid)
|
||||
Invoice::factory()->count(3)->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 0,
|
||||
'amount' => 100,
|
||||
]);
|
||||
|
||||
$input = ['date_range' => 'all', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
$report = new ClientBalanceReport($this->company, $input);
|
||||
$output = $report->run();
|
||||
|
||||
// Should still count the invoices
|
||||
$this->assertNotEmpty($output);
|
||||
$lines = array_filter(explode("\n", $output), fn($line) => !empty($line));
|
||||
$this->assertGreaterThanOrEqual(5, count($lines));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test report output structure
|
||||
*/
|
||||
public function testReportOutputStructure()
|
||||
{
|
||||
$client = Client::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'name' => 'Test Client',
|
||||
'number' => 'CLI-001',
|
||||
'id_number' => 'TAX-123',
|
||||
]);
|
||||
|
||||
Invoice::factory()->create([
|
||||
'company_id' => $this->company->id,
|
||||
'user_id' => $this->user->id,
|
||||
'client_id' => $client->id,
|
||||
'status_id' => Invoice::STATUS_SENT,
|
||||
'balance' => 500,
|
||||
]);
|
||||
|
||||
$input = ['date_range' => 'all', 'report_keys' => [], 'user_id' => $this->user->id];
|
||||
$report = new ClientBalanceReport($this->company, $input);
|
||||
$output = $report->run();
|
||||
|
||||
// Verify output contains client data
|
||||
$this->assertNotEmpty($output);
|
||||
$lines = array_filter(explode("\n", $output), fn($line) => !empty($line));
|
||||
$this->assertGreaterThanOrEqual(5, count($lines));
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue