From ccbcaefbc3f2e96d23d503fafd29a43c665cf274 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sun, 3 May 2026 20:32:16 +1000 Subject: [PATCH] Fixes for PUT on /projects/ unsetting client_id --- .../Elastic/ImportElasticSearchableModels.php | 2 +- .../Requests/Project/UpdateProjectRequest.php | 4 +- app/Import/Transformer/BaseTransformer.php | 17 +- app/Jobs/Util/Import.php | 6 + .../Invoice/InvoiceTransactionEventEntry.php | 3 +- app/Livewire/Flow2/ProcessPayment.php | 11 -- app/Services/Quickbooks/Models/QbInvoice.php | 4 + app/Services/Report/TaxPeriodReport.php | 60 ++++--- app/Utils/Traits/GeneratesCounter.php | 11 ++ tests/Feature/ProjectClientIdApiTest.php | 151 ++++++++++++++++++ 10 files changed, 221 insertions(+), 48 deletions(-) create mode 100644 tests/Feature/ProjectClientIdApiTest.php diff --git a/app/Console/Commands/Elastic/ImportElasticSearchableModels.php b/app/Console/Commands/Elastic/ImportElasticSearchableModels.php index 1fb9b04b33..fda2108378 100644 --- a/app/Console/Commands/Elastic/ImportElasticSearchableModels.php +++ b/app/Console/Commands/Elastic/ImportElasticSearchableModels.php @@ -57,7 +57,7 @@ use ReflectionMethod; */ class ImportElasticSearchableModels extends Command { - private const LARGE_IMPORT_JOB_THRESHOLD = 10000; + private const LARGE_IMPORT_JOB_THRESHOLD = 20000; private const OPTION_DATABASE = 'database'; diff --git a/app/Http/Requests/Project/UpdateProjectRequest.php b/app/Http/Requests/Project/UpdateProjectRequest.php index 784645d6bb..40e067d5a8 100644 --- a/app/Http/Requests/Project/UpdateProjectRequest.php +++ b/app/Http/Requests/Project/UpdateProjectRequest.php @@ -65,9 +65,7 @@ class UpdateProjectRequest extends Request $this->files->set('file', [$this->file('file')]); } - if (isset($input['client_id'])) { - unset($input['client_id']); - } + $input['client_id'] = $this->project->client_id; if (array_key_exists('color', $input) && is_null($input['color'])) { $input['color'] = ''; diff --git a/app/Import/Transformer/BaseTransformer.php b/app/Import/Transformer/BaseTransformer.php index 5423d5c57a..fc1d5b9b9f 100644 --- a/app/Import/Transformer/BaseTransformer.php +++ b/app/Import/Transformer/BaseTransformer.php @@ -801,17 +801,22 @@ class BaseTransformer ]) ->first(); - return $project ? $project->id : $this->createProject($name, $clientId); + if ($project) { + return $project->id; + } + + if (! $clientId) { + return null; + } + + return $this->createProject($name, $clientId); } - private function createProject($name, $clientId) + private function createProject(string $name, int $clientId): int { $project = ProjectFactory::create($this->company->id, $this->company->owner()->id); $project->name = $name; - - if ($clientId) { - $project->client_id = $clientId; - } + $project->client_id = $clientId; $project->saveQuietly(); diff --git a/app/Jobs/Util/Import.php b/app/Jobs/Util/Import.php index c46de722fa..31733e74a9 100644 --- a/app/Jobs/Util/Import.php +++ b/app/Jobs/Util/Import.php @@ -1878,6 +1878,12 @@ class Import implements ShouldQueue $modified['client_id'] = $this->transformId('clients', $resource['client_id']); } + if (! isset($modified['client_id'])) { + nlog('Skipping project import row without client_id (legacy id: ' . ($resource['id'] ?? '') . ')'); + + continue; + } + /** @var \App\Models\Project $project **/ $project = Project::create($modified); diff --git a/app/Listeners/Invoice/InvoiceTransactionEventEntry.php b/app/Listeners/Invoice/InvoiceTransactionEventEntry.php index f96b234642..50f05cd68b 100644 --- a/app/Listeners/Invoice/InvoiceTransactionEventEntry.php +++ b/app/Listeners/Invoice/InvoiceTransactionEventEntry.php @@ -85,7 +85,8 @@ class InvoiceTransactionEventEntry } elseif ($invoice->is_deleted) { } - /** If the invoice hasn't changed its state... return early!! */ elseif (BcMath::comp($invoice->amount, $event->invoice_amount) == 0 || $event->period->format('Y-m-d') == $period) { + /** If the invoice hasn't changed its state... return early!! */ + elseif (BcMath::comp($invoice->amount, $event->invoice_amount) == 0 || $event->period->format('Y-m-d') == $period) { nlog("event period => {$period} => " . $event->period->format('Y-m-d')); nlog("invoice amount => {$invoice->amount} => " . $event->invoice_amount); nlog("apparently no change in amount or period"); diff --git a/app/Livewire/Flow2/ProcessPayment.php b/app/Livewire/Flow2/ProcessPayment.php index 2d9472bf76..d447477581 100644 --- a/app/Livewire/Flow2/ProcessPayment.php +++ b/app/Livewire/Flow2/ProcessPayment.php @@ -39,12 +39,6 @@ class ProcessPayment extends Component $_context = $this->getContext($this->_key); - if (isset($_context['payment_processed'])) { - $this->payment_view = $_context['payment_processed']['payment_view']; - $this->payment_data_payload = $_context['payment_processed']['payment_data_payload']; - return; - } - $data = [ 'company_gateway_id' => $_context['company_gateway_id'], 'payment_method_id' => $_context['gateway_type_id'], @@ -100,11 +94,6 @@ class ProcessPayment extends Component ); } - $this->setContext($this->_key, 'payment_processed', [ - 'payment_view' => $this->payment_view, - 'payment_data_payload' => $this->payment_data_payload, - ]); - } public function placeholder(): string diff --git a/app/Services/Quickbooks/Models/QbInvoice.php b/app/Services/Quickbooks/Models/QbInvoice.php index b770c0bd31..30da819025 100644 --- a/app/Services/Quickbooks/Models/QbInvoice.php +++ b/app/Services/Quickbooks/Models/QbInvoice.php @@ -700,6 +700,10 @@ class QbInvoice implements SyncInterface foreach ($payment_ids as $payment_id) { + if(!$payment_id) { + continue; + } + $payment = $this->service->sdk->FindById('Payment', $payment_id); $payment_transformer = new PaymentTransformer($this->service->company); diff --git a/app/Services/Report/TaxPeriodReport.php b/app/Services/Report/TaxPeriodReport.php index e38bcf9593..472310cba6 100644 --- a/app/Services/Report/TaxPeriodReport.php +++ b/app/Services/Report/TaxPeriodReport.php @@ -133,11 +133,17 @@ class TaxPeriodReport extends BaseExport private function initializeData(): self { + // First sweep: only active statuses (SENT/PARTIAL/PAID). + // CANCELLED/REVERSED/deleted invoices early-return without writing an + // event in InvoiceTransactionEventEntry::run, so including them here + // causes them to be reprocessed on every report run forever. The + // second sweep below handles their state transitions instead. $q = Invoice::withTrashed() + ->with('payments') ->where('company_id', $this->company->id) - ->whereIn('status_id', [2,3,4,5,6]) + ->whereIn('status_id', [Invoice::STATUS_SENT, Invoice::STATUS_PARTIAL, Invoice::STATUS_PAID]) + ->where('is_deleted', false) ->whereBetween('date', ['1970-01-01', $this->end_date]) - // ->whereDoesntHave('transaction_events'); //filter by no transaction events for THIS month. ->whereDoesntHave('transaction_events', function ($query) { $query->where('period', $this->end_date); }); @@ -152,6 +158,7 @@ class TaxPeriodReport extends BaseExport //Harvest point in time records for cash payments. \App\Models\Paymentable::where('paymentable_type', 'invoices') + ->where('paymentable_id', $invoice->id) ->whereIn('payment_id', $invoice->payments->pluck('id')) ->get() ->groupBy(function ($paymentable) { @@ -159,8 +166,9 @@ class TaxPeriodReport extends BaseExport }) ->map(function ($group) { return $group->first(); - })->each(function (\App\Models\Paymentable $pp) { - (new InvoiceTransactionEventEntryCash())->run($pp->paymentable, \Carbon\Carbon::parse($pp->created_at)->startOfMonth()->format('Y-m-d'), \Carbon\Carbon::parse($pp->created_at)->endOfMonth()->format('Y-m-d')); + })->each(function (\App\Models\Paymentable $pp) use ($invoice) { + // Pass $invoice directly to avoid morph lazy-load on $pp->paymentable. + (new InvoiceTransactionEventEntryCash())->run($invoice, \Carbon\Carbon::parse($pp->created_at)->startOfMonth()->format('Y-m-d'), \Carbon\Carbon::parse($pp->created_at)->endOfMonth()->format('Y-m-d')); }); } @@ -196,7 +204,7 @@ class TaxPeriodReport extends BaseExport }); - $this->backfillClassificationBreakdown(); + $this->backfillClassificationBreakdown(); // TEMP disabled to test perf impact return $this; } @@ -282,9 +290,25 @@ class TaxPeriodReport extends BaseExport private function resolveQuery(): Builder { + $start_date = $this->start_date; + $end_date = $this->end_date; + $cash_accounting = $this->cash_accounting; + $query = Invoice::query() ->withTrashed() - ->with('transaction_events') + ->with(['transaction_events' => function ($q) use ($start_date, $end_date, $cash_accounting) { + $q->whereBetween('period', [$start_date, $end_date]) + ->orderBy('timestamp', 'desc'); + + if ($cash_accounting) { + $q->where(function ($sub_q) { + $sub_q->where('event_id', '!=', TransactionEvent::INVOICE_UPDATED) + ->orWhere('metadata->tax_report->tax_summary->status', 'reversed'); + }); + } else { + $q->where('event_id', TransactionEvent::INVOICE_UPDATED); + } + }]) ->where('company_id', $this->company->id); if ($this->cash_accounting) { //cash @@ -530,28 +554,12 @@ class TaxPeriodReport extends BaseExport $query->cursor()->each(function ($invoice) { - $invoice->transaction_events() - ->when(!$this->cash_accounting, function ($query) { - $query->where('event_id', TransactionEvent::INVOICE_UPDATED); - }) - ->when($this->cash_accounting, function ($query) { - $query->where(function ($sub_q) { - $sub_q->where('event_id', '!=', TransactionEvent::INVOICE_UPDATED) - ->orWhere('metadata->tax_report->tax_summary->status', 'reversed'); - - }); - - // $query->where('event_id', '!=', TransactionEvent::INVOICE_UPDATED); - }) - ->whereBetween('period', [$this->start_date, $this->end_date]) - ->orderBy('timestamp', 'desc') - ->cursor() - ->each(function ($event) use ($invoice) { - + // transaction_events were eager-loaded with the same filters in resolveQuery(); + // iterate the loaded collection rather than firing a fresh query per invoice. + foreach ($invoice->transaction_events as $event) { /** @var Invoice $invoice */ $this->processTransactionEvent($event, $invoice); - - }); + } }); return $this; diff --git a/app/Utils/Traits/GeneratesCounter.php b/app/Utils/Traits/GeneratesCounter.php index 96f17f0e1c..8c1434f61f 100644 --- a/app/Utils/Traits/GeneratesCounter.php +++ b/app/Utils/Traits/GeneratesCounter.php @@ -296,6 +296,17 @@ trait GeneratesCounter */ public function getNextProjectNumber(Project $project): string { + if (! $project->client_id) { + $this->resetCompanyCounters($project->company); + + $counter = $project->company->settings->project_number_counter; + $project_number = $this->checkEntityNumber(Project::class, $project, $counter, $project->company->settings->counter_padding, $project->company->settings->project_number_pattern); + + $this->incrementCounter($project->company, 'project_number_counter'); + + return $this->replaceUserVars($project, $project_number); + } + $entity_number = $this->getNextEntityNumber(Project::class, $project->client, false); return $this->replaceUserVars($project, $entity_number); diff --git a/tests/Feature/ProjectClientIdApiTest.php b/tests/Feature/ProjectClientIdApiTest.php new file mode 100644 index 0000000000..cdbb46c112 --- /dev/null +++ b/tests/Feature/ProjectClientIdApiTest.php @@ -0,0 +1,151 @@ +makeTestData(); + + Session::start(); + Model::reguard(); + } + + /** + * @return array + */ + private function apiHeaders(): array + { + return [ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ]; + } + + /** + * @param array $payload + * @return array + */ + private function assertCreatedProjectResponse(array $payload): array + { + $response = $this->withHeaders($this->apiHeaders())->postJson('/api/v1/projects', $payload); + + $response->assertStatus(200); + + return $response->json(); + } + + public function test_store_project_with_full_api_payload_succeeds(): void + { + $body = $this->assertCreatedProjectResponse([ + 'client_id' => $this->client->hashed_id, + 'name' => 'Payload Created Project', + 'task_rate' => 75.5, + 'budgeted_hours' => 12, + 'public_notes' => 'notes', + ]); + + $this->assertNotEmpty($body['data']['id']); + $this->assertSame('Payload Created Project', $body['data']['name']); + $this->assertSame(75.5, $body['data']['task_rate']); + $this->assertSame($this->encodePrimaryKey($this->client->id), $body['data']['client_id']); + } + + public function test_store_project_without_client_id_returns_422(): void + { + $response = $this->withHeaders($this->apiHeaders())->postJson('/api/v1/projects', [ + 'name' => 'Missing Client', + 'task_rate' => 10, + ]); + + $response->assertStatus(422); + } + + /** + * Regression: JSON null must not clear client_id (prepareForValidation must strip by key, not isset). + */ + public function test_put_payload_with_explicit_null_client_id_does_not_clear_client(): void + { + $created = $this->assertCreatedProjectResponse([ + 'client_id' => $this->client->hashed_id, + 'name' => 'Before Put Null Client Key', + 'task_rate' => 22, + ]); + + $hashedProjectId = $created['data']['id']; + $expectedClientHash = $this->encodePrimaryKey($this->client->id); + + $response = $this->withHeaders($this->apiHeaders())->putJson("/api/v1/projects/{$hashedProjectId}", [ + 'client_id' => null, + 'name' => 'After Put Should Keep Client', + 'task_rate' => 33, + 'budgeted_hours' => 0, + ]); + + $response->assertStatus(200); + $this->assertSame($expectedClientHash, $response->json('data.client_id')); + $this->assertSame('After Put Should Keep Client', $response->json('data.name')); + + $this->assertSame( + $this->client->id, + Project::query()->find($this->decodePrimaryKey($hashedProjectId))->client_id + ); + } + + public function test_put_payload_omitting_client_id_preserves_client(): void + { + $created = $this->assertCreatedProjectResponse([ + 'client_id' => $this->client->hashed_id, + 'name' => 'Omit Client Field', + 'task_rate' => 5, + ]); + + $hashedProjectId = $created['data']['id']; + $expectedClientHash = $this->encodePrimaryKey($this->client->id); + + $response = $this->withHeaders($this->apiHeaders())->putJson("/api/v1/projects/{$hashedProjectId}", [ + 'name' => 'Renamed Only', + 'task_rate' => 6, + ]); + + $response->assertStatus(200); + $this->assertSame($expectedClientHash, $response->json('data.client_id')); + + $this->assertSame( + $this->client->id, + Project::query()->find($this->decodePrimaryKey($hashedProjectId))->client_id + ); + } +}