From e1fd87d174a06f362af25e70b4a33d1ef44b4d24 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Thu, 18 Aug 2022 13:29:18 +1000 Subject: [PATCH 01/10] Add validation for task time logs --- app/Http/Requests/Task/StoreTaskRequest.php | 33 +++++- app/Http/Requests/Task/UpdateTaskRequest.php | 31 ++++++ tests/Feature/TaskApiTest.php | 110 +++++++++++++++++++ 3 files changed, 173 insertions(+), 1 deletion(-) diff --git a/app/Http/Requests/Task/StoreTaskRequest.php b/app/Http/Requests/Task/StoreTaskRequest.php index a1de7bec3f..8d0d7af7b8 100644 --- a/app/Http/Requests/Task/StoreTaskRequest.php +++ b/app/Http/Requests/Task/StoreTaskRequest.php @@ -12,6 +12,7 @@ namespace App\Http\Requests\Task; use App\Http\Requests\Request; +use App\Models\Project; use App\Models\Task; use App\Utils\Traits\MakesHash; use Illuminate\Validation\Rule; @@ -38,19 +39,49 @@ class StoreTaskRequest extends Request $rules['number'] = Rule::unique('tasks')->where('company_id', auth()->user()->company()->id); } + if(isset($this->client_id)) + $rules['client_id'] = 'bail|required|exists:clients,id,company_id,'.auth()->user()->company()->id.',is_deleted,0'; + + if(isset($this->project_id)) + $rules['project_id'] = 'bail|required|exists:projects,id,company_id,'.auth()->user()->company()->id.',is_deleted,0'; + + $rules['timelog'] = ['bail','array',function ($attribute, $values, $fail) { + + foreach($values as $k) + { + if(!is_int($k[0]) || !is_int($k[1])) + $fail('The '.$attribute.' - '.print_r($k,1).' is invalid. Unix timestamps only.'); + } + + }]; + + return $this->globalRules($rules); } public function prepareForValidation() { $input = $this->all(); - $input = $this->decodePrimaryKeys($this->all()); if (array_key_exists('status_id', $input) && is_string($input['status_id'])) { $input['status_id'] = $this->decodePrimaryKey($input['status_id']); } + /* Ensure the project is related */ + if (array_key_exists('project_id', $input) && isset($input['project_id'])) { + $project = Project::withTrashed()->find($input['project_id'])->company()->first(); + + if($project){ + $input['client_id'] = $project->client_id; + } + else + { + unset($input['project_id']); + } + + } + $this->replace($input); } } diff --git a/app/Http/Requests/Task/UpdateTaskRequest.php b/app/Http/Requests/Task/UpdateTaskRequest.php index 1389f5ce41..c60924df0b 100644 --- a/app/Http/Requests/Task/UpdateTaskRequest.php +++ b/app/Http/Requests/Task/UpdateTaskRequest.php @@ -12,6 +12,7 @@ namespace App\Http\Requests\Task; use App\Http\Requests\Request; +use App\Models\Project; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\MakesHash; use Illuminate\Validation\Rule; @@ -39,6 +40,22 @@ class UpdateTaskRequest extends Request $rules['number'] = Rule::unique('tasks')->where('company_id', auth()->user()->company()->id)->ignore($this->task->id); } + if(isset($this->client_id)) + $rules['client_id'] = 'bail|required|exists:clients,id,company_id,'.auth()->user()->company()->id.',is_deleted,0'; + + if(isset($this->project_id)) + $rules['project_id'] = 'bail|required|exists:projects,id,company_id,'.auth()->user()->company()->id.',is_deleted,0'; + + $rules['timelog'] = ['bail','array',function ($attribute, $values, $fail) { + + foreach($values as $k) + { + if(!is_int($k[0]) || !is_int($k[1])) + $fail('The '.$attribute.' - '.print_r($k,1).' is invalid. Unix timestamps only.'); + } + + }]; + return $this->globalRules($rules); } @@ -50,6 +67,20 @@ class UpdateTaskRequest extends Request $input['status_id'] = $this->decodePrimaryKey($input['status_id']); } + /* Ensure the project is related */ + if (array_key_exists('project_id', $input) && isset($input['project_id'])) { + $project = Project::withTrashed()->find($input['project_id'])->company()->first(); + + if($project){ + $input['client_id'] = $project->client_id; + } + else + { + unset($input['project_id']); + } + + } + if (array_key_exists('color', $input) && is_null($input['color'])) { $input['color'] = ''; } diff --git a/tests/Feature/TaskApiTest.php b/tests/Feature/TaskApiTest.php index 2343a5d562..0a7cd00545 100644 --- a/tests/Feature/TaskApiTest.php +++ b/tests/Feature/TaskApiTest.php @@ -42,6 +42,97 @@ class TaskApiTest extends TestCase Model::reguard(); } + public function testTimeLogValidation() + { + $data = [ + 'timelog' => $this->faker->firstName(), + ]; + + try { + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/tasks', $data); + + $arr = $response->json(); + } catch (ValidationException $e) { + $response->assertStatus(302); + } + + } + + public function testTimeLogValidation1() + { + $data = [ + 'timelog' => [[1,2],[3,4]], + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/tasks', $data); + + $arr = $response->json(); + $response->assertStatus(200); + + + } + + public function testTimeLogValidation2() + { + $data = [ + 'timelog' => [], + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/tasks', $data); + + $arr = $response->json(); + $response->assertStatus(200); + + + } + + public function testTimeLogValidation3() + { + $data = [ + 'timelog' => [["a","b"],["c","d"]], + ]; + + try { + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/tasks', $data); + + $arr = $response->json(); + } catch (ValidationException $e) { + $response->assertStatus(302); + } + + } + + public function testTimeLogValidation4() + { + $data = [ + 'timelog' => [[1,2],[3,0]], + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/tasks', $data); + + $arr = $response->json(); + $response->assertStatus(200); + + + } + + + public function testStartTask() { $log = [ @@ -76,6 +167,7 @@ class TaskApiTest extends TestCase $data = [ 'description' => $this->faker->firstName(), 'number' => 'taskynumber', + 'client_id' => $this->client->id, ]; $response = $this->withHeaders([ @@ -126,6 +218,24 @@ class TaskApiTest extends TestCase $this->assertNotEmpty($arr['data']['number']); } + public function testTaskWithBadClientId() + { + $data = [ + 'client_id' => $this->faker->firstName(), + ]; + + try { + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/tasks', $data); + $arr = $response->json(); + } catch (ValidationException $e) { + $response->assertStatus(302); + } + + } + public function testTaskPostWithActionStart() { $data = [ From b7a5c055a89ae98a963d27dfba00cfc62a29e012 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Thu, 18 Aug 2022 14:08:50 +1000 Subject: [PATCH 02/10] Fixes for single route actions --- app/Http/Controllers/CreditController.php | 6 +++--- app/Http/Controllers/InvoiceController.php | 6 +++--- app/Http/Controllers/PurchaseOrderController.php | 8 ++++---- app/Http/Controllers/QuoteController.php | 6 +++--- .../Controllers/RecurringExpenseController.php | 6 +++--- .../Controllers/RecurringInvoiceController.php | 6 +++--- tests/Feature/InvoiceTest.php | 14 ++++++++++++++ 7 files changed, 33 insertions(+), 19 deletions(-) diff --git a/app/Http/Controllers/CreditController.php b/app/Http/Controllers/CreditController.php index e0c72b90d5..55a1d03aad 100644 --- a/app/Http/Controllers/CreditController.php +++ b/app/Http/Controllers/CreditController.php @@ -587,21 +587,21 @@ class CreditController extends BaseController $this->credit_repository->archive($credit); if (! $bulk) { - return $this->listResponse($credit); + return $this->itemResponse($credit); } break; case 'restore': $this->credit_repository->restore($credit); if (! $bulk) { - return $this->listResponse($credit); + return $this->itemResponse($credit); } break; case 'delete': $this->credit_repository->delete($credit); if (! $bulk) { - return $this->listResponse($credit); + return $this->itemResponse($credit); } break; case 'email': diff --git a/app/Http/Controllers/InvoiceController.php b/app/Http/Controllers/InvoiceController.php index dd91e5e6b5..051b6d2c5f 100644 --- a/app/Http/Controllers/InvoiceController.php +++ b/app/Http/Controllers/InvoiceController.php @@ -722,14 +722,14 @@ class InvoiceController extends BaseController $this->invoice_repo->restore($invoice); if (! $bulk) { - return $this->listResponse($invoice); + return $this->itemResponse($invoice); } break; case 'archive': $this->invoice_repo->archive($invoice); if (! $bulk) { - return $this->listResponse($invoice); + return $this->itemResponse($invoice); } break; case 'delete': @@ -737,7 +737,7 @@ class InvoiceController extends BaseController $this->invoice_repo->delete($invoice); if (! $bulk) { - return $this->listResponse($invoice); + return $this->itemResponse($invoice); } break; case 'cancel': diff --git a/app/Http/Controllers/PurchaseOrderController.php b/app/Http/Controllers/PurchaseOrderController.php index 8fc56b8fc7..83d8c9cd55 100644 --- a/app/Http/Controllers/PurchaseOrderController.php +++ b/app/Http/Controllers/PurchaseOrderController.php @@ -623,14 +623,14 @@ class PurchaseOrderController extends BaseController $this->purchase_order_repository->restore($purchase_order); if (! $bulk) { - return $this->listResponse($purchase_order); + return $this->itemResponse($purchase_order); } break; case 'archive': $this->purchase_order_repository->archive($purchase_order); if (! $bulk) { - return $this->listResponse($purchase_order); + return $this->itemResponse($purchase_order); } break; case 'delete': @@ -638,7 +638,7 @@ class PurchaseOrderController extends BaseController $this->purchase_order_repository->delete($purchase_order); if (! $bulk) { - return $this->listResponse($purchase_order); + return $this->itemResponse($purchase_order); } break; @@ -684,7 +684,7 @@ class PurchaseOrderController extends BaseController } if (! $bulk) { - return $this->listResponse($purchase_order); + return $this->itemResponse($purchase_order); } break; diff --git a/app/Http/Controllers/QuoteController.php b/app/Http/Controllers/QuoteController.php index de549bdccc..a42c9c09c8 100644 --- a/app/Http/Controllers/QuoteController.php +++ b/app/Http/Controllers/QuoteController.php @@ -728,7 +728,7 @@ class QuoteController extends BaseController $this->quote_repo->restore($quote); if (! $bulk) { - return $this->listResponse($quote); + return $this->itemResponse($quote); } break; @@ -736,7 +736,7 @@ class QuoteController extends BaseController $this->quote_repo->archive($quote); if (! $bulk) { - return $this->listResponse($quote); + return $this->itemResponse($quote); } break; @@ -744,7 +744,7 @@ class QuoteController extends BaseController $this->quote_repo->delete($quote); if (! $bulk) { - return $this->listResponse($quote); + return $this->itemResponse($quote); } break; diff --git a/app/Http/Controllers/RecurringExpenseController.php b/app/Http/Controllers/RecurringExpenseController.php index 46d3ba315b..98524be5ff 100644 --- a/app/Http/Controllers/RecurringExpenseController.php +++ b/app/Http/Controllers/RecurringExpenseController.php @@ -511,21 +511,21 @@ class RecurringExpenseController extends BaseController $this->recurring_expense_repo->archive($recurring_expense); if (! $bulk) { - return $this->listResponse($recurring_expense); + return $this->itemResponse($recurring_expense); } break; case 'restore': $this->recurring_expense_repo->restore($recurring_expense); if (! $bulk) { - return $this->listResponse($recurring_expense); + return $this->itemResponse($recurring_expense); } break; case 'delete': $this->recurring_expense_repo->delete($recurring_expense); if (! $bulk) { - return $this->listResponse($recurring_expense); + return $this->itemResponse($recurring_expense); } break; case 'email': diff --git a/app/Http/Controllers/RecurringInvoiceController.php b/app/Http/Controllers/RecurringInvoiceController.php index ec3ebe1c65..6eb6d12d20 100644 --- a/app/Http/Controllers/RecurringInvoiceController.php +++ b/app/Http/Controllers/RecurringInvoiceController.php @@ -662,21 +662,21 @@ class RecurringInvoiceController extends BaseController $this->recurring_invoice_repo->archive($recurring_invoice); if (! $bulk) { - return $this->listResponse($recurring_invoice); + return $this->itemResponse($recurring_invoice); } break; case 'restore': $this->recurring_invoice_repo->restore($recurring_invoice); if (! $bulk) { - return $this->listResponse($recurring_invoice); + return $this->itemResponse($recurring_invoice); } break; case 'delete': $this->recurring_invoice_repo->delete($recurring_invoice); if (! $bulk) { - return $this->listResponse($recurring_invoice); + return $this->itemResponse($recurring_invoice); } break; case 'email': diff --git a/tests/Feature/InvoiceTest.php b/tests/Feature/InvoiceTest.php index 5e582bf5ce..18a3618510 100644 --- a/tests/Feature/InvoiceTest.php +++ b/tests/Feature/InvoiceTest.php @@ -46,6 +46,18 @@ class InvoiceTest extends TestCase $this->makeTestData(); } + + public function testInvoiceArchiveAction() + { + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->get('/api/v1/invoices/'.$this->invoice->hashed_id.'/archive',) + ->assertStatus(200); + } + + public function testMarkingDeletedInvoiceAsSent() { Client::factory()->create(['user_id' => $this->user->id, 'company_id' => $this->company->id])->each(function ($c) { @@ -290,4 +302,6 @@ class InvoiceTest extends TestCase ])->post('/api/v1/invoices/', $data) ->assertStatus(200); } + + } From 14f8541e4d7da37b0ad633364f2de540d356872c Mon Sep 17 00:00:00 2001 From: David Bomba Date: Thu, 18 Aug 2022 17:10:47 +1000 Subject: [PATCH 03/10] Fixes for base path for canvas kit --- resources/views/index/index.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/index/index.blade.php b/resources/views/index/index.blade.php index f66926c193..2c26b35c7b 100644 --- a/resources/views/index/index.blade.php +++ b/resources/views/index/index.blade.php @@ -21,7 +21,7 @@ From e716bb5a02172fbad4f26aa03685a4370fc9c99a Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 19 Aug 2022 08:41:50 +1000 Subject: [PATCH 04/10] Fixes for showing fees in both the product and tax tables --- app/Http/Requests/Login/LoginRequest.php | 11 ------ .../ValidationRules/Account/BlackListRule.php | 3 +- app/Services/PdfMaker/Design.php | 2 +- app/Utils/Traits/MakesInvoiceValues.php | 4 +- tests/Unit/InvoiceTest.php | 39 ++++++++++++++++++- 5 files changed, 43 insertions(+), 16 deletions(-) diff --git a/app/Http/Requests/Login/LoginRequest.php b/app/Http/Requests/Login/LoginRequest.php index f02169180c..102a3e6764 100644 --- a/app/Http/Requests/Login/LoginRequest.php +++ b/app/Http/Requests/Login/LoginRequest.php @@ -47,15 +47,4 @@ class LoginRequest extends Request ]; } - // public function prepareForValidation() - // { - // $input = $this->all(); - - // // if(base64_decode(base64_encode($input['password'])) === $input['password']) - // // $input['password'] = base64_decode($input['password']); - - // // nlog($input['password']); - - // $this->replace($input); - // } } diff --git a/app/Http/ValidationRules/Account/BlackListRule.php b/app/Http/ValidationRules/Account/BlackListRule.php index 51fab56af4..0a2a681f0f 100644 --- a/app/Http/ValidationRules/Account/BlackListRule.php +++ b/app/Http/ValidationRules/Account/BlackListRule.php @@ -24,7 +24,8 @@ class BlackListRule implements Rule 'vusra.com', 'fourthgenet.com', 'arxxwalls.com', - 'superhostforumla.com' + 'superhostforumla.com', + 'wnpop.com', ]; /** diff --git a/app/Services/PdfMaker/Design.php b/app/Services/PdfMaker/Design.php index dec4e6518d..f627a0a007 100644 --- a/app/Services/PdfMaker/Design.php +++ b/app/Services/PdfMaker/Design.php @@ -394,7 +394,7 @@ class Design extends BaseDesign public function productTable(): array { $product_items = collect($this->entity->line_items)->filter(function ($item) { - return $item->type_id == 1 || $item->type_id == 6; + return $item->type_id == 1 || $item->type_id == 6 || $item->type_id == 5; }); if (count($product_items) == 0) { diff --git a/app/Utils/Traits/MakesInvoiceValues.php b/app/Utils/Traits/MakesInvoiceValues.php index 735d436c29..9a4bd3b6c2 100644 --- a/app/Utils/Traits/MakesInvoiceValues.php +++ b/app/Utils/Traits/MakesInvoiceValues.php @@ -282,9 +282,9 @@ trait MakesInvoiceValues } if ($table_type == '$task' && $item->type_id != 2) { - if ($item->type_id != 4 && $item->type_id != 5) { + // if ($item->type_id != 4 && $item->type_id != 5) { continue; - } + // } } $helpers = new Helpers(); diff --git a/tests/Unit/InvoiceTest.php b/tests/Unit/InvoiceTest.php index aff6437a53..0ed0ebb670 100644 --- a/tests/Unit/InvoiceTest.php +++ b/tests/Unit/InvoiceTest.php @@ -13,6 +13,7 @@ namespace Tests\Unit; use App\Factory\InvoiceItemFactory; use App\Helpers\Invoice\InvoiceSum; +use App\Helpers\Invoice\InvoiceSumInclusive; use App\Models\Invoice; use Illuminate\Foundation\Testing\DatabaseTransactions; use Tests\MockAccountData; @@ -41,11 +42,47 @@ class InvoiceTest extends TestCase $this->invoice->line_items = $this->buildLineItems(); - $this->invoice->usesinclusive_taxes = true; + $this->invoice->uses_inclusive_taxes = true; $this->invoice_calc = new InvoiceSum($this->invoice); } + public function testInclusiveRounding() + { + $this->invoice->line_items = []; + $this->invoice->discount = 0; + $this->invoice->uses_inclusive_taxes = true; + $this->invoice->save(); + + + $item = InvoiceItemFactory::create(); + $item->quantity = 1; + $item->cost = 50; + $item->tax_name1 = "taxy"; + $item->tax_rate1 = 19; + + $line_items[] = $item; + + $item = InvoiceItemFactory::create(); + $item->quantity = 1; + $item->cost = 50; + $item->tax_name1 = "taxy"; + $item->tax_rate1 = 19; + + $line_items[] = $item; + + $this->invoice->line_items = $line_items; + $this->invoice->save(); + + $invoice_calc = new InvoiceSumInclusive($this->invoice); + + $invoice_calc->build(); + // $this->invoice->save(); + + $this->assertEquals($invoice_calc->getTotalTaxes(), 15.96); + + } + private function buildLineItems() { $line_items = []; From 66291e69ab854334a96d12f356d1122f8fd1343f Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 19 Aug 2022 10:36:13 +1000 Subject: [PATCH 05/10] Change the order of email filters --- app/Jobs/Mail/NinjaMailerJob.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/Jobs/Mail/NinjaMailerJob.php b/app/Jobs/Mail/NinjaMailerJob.php index b157176b02..f59f09c8fa 100644 --- a/app/Jobs/Mail/NinjaMailerJob.php +++ b/app/Jobs/Mail/NinjaMailerJob.php @@ -332,14 +332,14 @@ class NinjaMailerJob implements ShouldQueue if(Ninja::isHosted() && ($this->nmo->settings->email_sending_method == 'gmail' || $this->nmo->settings->email_sending_method == 'office365')) return false; - /* On the hosted platform, if the user is over the email quotas, we do not send the email. */ - if(Ninja::isHosted() && $this->company->account && $this->company->account->emailQuotaExceeded()) - return true; - /* To handle spam users we drop all emails from flagged accounts */ if(Ninja::isHosted() && $this->company->account && $this->company->account->is_flagged) return true; + /* On the hosted platform, if the user is over the email quotas, we do not send the email. */ + if(Ninja::isHosted() && $this->company->account && $this->company->account->emailQuotaExceeded()) + return true; + /* If the account is verified, we allow emails to flow */ if(Ninja::isHosted() && $this->company->account && $this->company->account->is_verified_account) { From 6e134098b53f85db72e0beadac0114ccc116b93c Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 19 Aug 2022 12:09:50 +1000 Subject: [PATCH 06/10] tests for deleting an invoice --- app/Jobs/Mail/NinjaMailerJob.php | 8 +- app/Services/Invoice/HandleRestore.php | 32 +++++++ tests/Feature/DeleteInvoiceTest.php | 124 +++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 4 deletions(-) diff --git a/app/Jobs/Mail/NinjaMailerJob.php b/app/Jobs/Mail/NinjaMailerJob.php index f59f09c8fa..0f291e22d0 100644 --- a/app/Jobs/Mail/NinjaMailerJob.php +++ b/app/Jobs/Mail/NinjaMailerJob.php @@ -324,6 +324,10 @@ class NinjaMailerJob implements ShouldQueue if($this->company->is_disabled && !$this->override) return true; + /* To handle spam users we drop all emails from flagged accounts */ + if(Ninja::isHosted() && $this->company->account && $this->company->account->is_flagged) + return true; + /* On the hosted platform we set default contacts a @example.com email address - we shouldn't send emails to these types of addresses */ if(Ninja::isHosted() && $this->nmo->to_user && strpos($this->nmo->to_user->email, '@example.com') !== false) return true; @@ -332,10 +336,6 @@ class NinjaMailerJob implements ShouldQueue if(Ninja::isHosted() && ($this->nmo->settings->email_sending_method == 'gmail' || $this->nmo->settings->email_sending_method == 'office365')) return false; - /* To handle spam users we drop all emails from flagged accounts */ - if(Ninja::isHosted() && $this->company->account && $this->company->account->is_flagged) - return true; - /* On the hosted platform, if the user is over the email quotas, we do not send the email. */ if(Ninja::isHosted() && $this->company->account && $this->company->account->emailQuotaExceeded()) return true; diff --git a/app/Services/Invoice/HandleRestore.php b/app/Services/Invoice/HandleRestore.php index 8001c51556..71e5579b29 100644 --- a/app/Services/Invoice/HandleRestore.php +++ b/app/Services/Invoice/HandleRestore.php @@ -15,6 +15,7 @@ use App\Models\Invoice; use App\Services\AbstractService; use App\Utils\Ninja; use App\Utils\Traits\GeneratesCounter; +use Illuminate\Support\Facades\DB; class HandleRestore extends AbstractService { @@ -57,6 +58,37 @@ class HandleRestore extends AbstractService return $this->invoice; } + private function adjustPayments() + { + //if total payments = adjustment amount - that means we need to delete the payments as well. + + if ($this->adjustment_amount == $this->total_payments) { + $this->invoice->payments()->update(['payments.deleted_at' => now(), 'payments.is_deleted' => true]); + } else { + + //adjust payments down by the amount applied to the invoice payment. + + $this->invoice->payments->each(function ($payment) { + $payment_adjustment = $payment->paymentables + ->where('paymentable_type', '=', 'invoices') + ->where('paymentable_id', $this->invoice->id) + ->sum(DB::raw('amount')); + + $payment_adjustment -= $payment->paymentables + ->where('paymentable_type', '=', 'invoices') + ->where('paymentable_id', $this->invoice->id) + ->sum(DB::raw('refunded')); + + $payment->amount -= $payment_adjustment; + $payment->applied -= $payment_adjustment; + $payment->save(); + }); + } + + return $this; + } + + private function windBackInvoiceNumber() { $findme = '_'.ctrans('texts.deleted'); diff --git a/tests/Feature/DeleteInvoiceTest.php b/tests/Feature/DeleteInvoiceTest.php index 4ee19d3778..c3204718f5 100644 --- a/tests/Feature/DeleteInvoiceTest.php +++ b/tests/Feature/DeleteInvoiceTest.php @@ -15,6 +15,7 @@ use App\Factory\InvoiceItemFactory; use App\Models\Client; use App\Models\Invoice; use App\Models\Payment; +use App\Repositories\InvoiceRepository; use App\Utils\Traits\MakesHash; use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Routing\Middleware\ThrottleRequests; @@ -41,6 +42,129 @@ class DeleteInvoiceTest extends TestCase ); } + public function testDeleteAndRestoreInvoice() + { + //create an invoice for 36000 with a partial of 6000 + + $data = [ + 'name' => 'A Nice Client - About to be deleted', + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/clients', $data); + + $response->assertStatus(200); + + $arr = $response->json(); + + $client_hash_id = $arr['data']['id']; + $client = Client::find($this->decodePrimaryKey($client_hash_id)); + + $this->assertEquals($client->balance, 0); + $this->assertEquals($client->paid_to_date, 0); + + $line_items = []; + + $item = InvoiceItemFactory::create(); + $item->quantity = 1; + $item->cost = 36000; + + $line_items[] = (array) $item; + + $invoice = [ + 'status_id' => 1, + 'number' => '', + 'discount' => 0, + 'is_amount_discount' => 1, + 'po_number' => '3434343', + 'public_notes' => 'notes', + 'is_deleted' => 0, + 'partial' => 6000, + 'custom_value1' => 0, + 'custom_value2' => 0, + 'custom_value3' => 0, + 'custom_value4' => 0, + 'client_id' => $client_hash_id, + 'line_items' => (array) $line_items, + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/invoices/', $invoice) + ->assertStatus(200); + + $arr = $response->json(); + + $invoice_one_hashed_id = $arr['data']['id']; + + $invoice = Invoice::find($this->decodePrimaryKey($invoice_one_hashed_id)); + + $invoice = $invoice->service()->markSent()->save(); + + $this->assertEquals(6000, $invoice->partial); + $this->assertEquals(36000, $invoice->amount); + + + // apply a payment of 6000 + + $data = [ + 'amount' => 6000, + 'client_id' => $client->hashed_id, + 'invoices' => [ + [ + 'invoice_id' => $invoice->hashed_id, + 'amount' => 6000, + ], + ], + 'date' => '2019/12/12', + ]; + + try { + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/payments?include=invoices', $data); + } catch (ValidationException $e) { + $message = json_decode($e->validator->getMessageBag(), 1); + $this->assertNotNull($message); + } + + $response->assertStatus(200); + + $arr = $response->json(); + + $payment_id = $arr['data']['id']; + + $payment = Payment::withTrashed()->whereId($this->decodePrimaryKey($payment_id))->first(); + + $this->assertEquals(6000, $payment->amount); + $this->assertEquals(6000, $payment->applied); + + $this->assertEquals(6000, $payment->client->paid_to_date); + + $invoice = $invoice->fresh(); + + $this->assertEquals(30000, $invoice->balance); + $this->assertEquals(6000, $invoice->paid_to_date); + + $invoice_repo = new InvoiceRepository(); + + $invoice = $invoice_repo->delete($invoice); + $invoice = $invoice->fresh(); + + $this->assertTrue($invoice->is_deleted); + + $payment = $payment->fresh(); + + $this->assertTrue($payment->is_deleted); + $this->assertEquals(4, $payment->status_id); + + + } + public function testInvoiceDeletionAfterCancellation() { $data = [ From 87e3f12920b5da1e4eabfd0e4c8ac1559c7885ad Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 19 Aug 2022 12:48:58 +1000 Subject: [PATCH 07/10] Fixes for edge case when deleting an invoice with a partial payment --- app/Services/Invoice/HandleRestore.php | 55 ++++++++++++++++++++++++-- tests/Feature/DeleteInvoiceTest.php | 20 ++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/app/Services/Invoice/HandleRestore.php b/app/Services/Invoice/HandleRestore.php index 71e5579b29..a2d0177be8 100644 --- a/app/Services/Invoice/HandleRestore.php +++ b/app/Services/Invoice/HandleRestore.php @@ -25,6 +25,10 @@ class HandleRestore extends AbstractService private $payment_total = 0; + private $total_payments = 0; + + private $adjustment_amount = 0; + public function __construct(Invoice $invoice) { $this->invoice = $invoice; @@ -48,22 +52,63 @@ class HandleRestore extends AbstractService //adjust ledger balance $this->invoice->ledger()->updateInvoiceBalance($this->invoice->balance, "Restored invoice {$this->invoice->number}")->save(); - $this->invoice->client->service()->updateBalance($this->invoice->balance)->save(); + $this->invoice->client + ->service() + ->updateBalance($this->invoice->balance) + ->updatePaidToDate($this->invoice->paid_to_date) + ->save(); $this->windBackInvoiceNumber(); $this->invoice->is_deleted = false; $this->invoice->save(); + $this->restorePaymentables() + ->setAdjustmentAmount() + ->adjustPayments(); + return $this->invoice; } + /* Touches all paymentables as deleted */ + private function restorePaymentables() + { + $this->invoice->payments->each(function ($payment) { + $payment->paymentables() + ->where('paymentable_type', '=', 'invoices') + ->where('paymentable_id', $this->invoice->id) + ->update(['deleted_at' => false]); + }); + + return $this; + } + + + private function setAdjustmentAmount() + { + foreach ($this->invoice->payments as $payment) { + $this->adjustment_amount += $payment->paymentables + ->where('paymentable_type', '=', 'invoices') + ->where('paymentable_id', $this->invoice->id) + ->sum(DB::raw('amount')); + + $this->adjustment_amount += $payment->paymentables + ->where('paymentable_type', '=', 'invoices') + ->where('paymentable_id', $this->invoice->id) + ->sum(DB::raw('refunded')); + } + + $this->total_payments = $this->invoice->payments->sum('amount') - $this->invoice->payments->sum('refunded'); + + return $this; + } + private function adjustPayments() { //if total payments = adjustment amount - that means we need to delete the payments as well. if ($this->adjustment_amount == $this->total_payments) { - $this->invoice->payments()->update(['payments.deleted_at' => now(), 'payments.is_deleted' => true]); + $this->invoice->payments()->update(['payments.deleted_at' => null, 'payments.is_deleted' => false]); } else { //adjust payments down by the amount applied to the invoice payment. @@ -79,8 +124,10 @@ class HandleRestore extends AbstractService ->where('paymentable_id', $this->invoice->id) ->sum(DB::raw('refunded')); - $payment->amount -= $payment_adjustment; - $payment->applied -= $payment_adjustment; + $payment->amount += $payment_adjustment; + $payment->applied += $payment_adjustment; + $payment->is_deleted = false; + $payment->restore(); $payment->save(); }); } diff --git a/tests/Feature/DeleteInvoiceTest.php b/tests/Feature/DeleteInvoiceTest.php index c3204718f5..4d48351272 100644 --- a/tests/Feature/DeleteInvoiceTest.php +++ b/tests/Feature/DeleteInvoiceTest.php @@ -150,6 +150,8 @@ class DeleteInvoiceTest extends TestCase $this->assertEquals(30000, $invoice->balance); $this->assertEquals(6000, $invoice->paid_to_date); + //delete the invoice an inspect the balances + $invoice_repo = new InvoiceRepository(); $invoice = $invoice_repo->delete($invoice); @@ -162,6 +164,24 @@ class DeleteInvoiceTest extends TestCase $this->assertTrue($payment->is_deleted); $this->assertEquals(4, $payment->status_id); + $client->fresh(); + + $this->assertEquals(0, $client->balance); + $this->assertEquals(0, $client->paid_to_date); + + //restore the invoice. this should also rehydrate the payments and restore the correct paid to dates on the client record + + $invoice_repo->restore($invoice); + $invoice = $invoice->fresh(); + $client = $client->fresh(); + $payment = $payment->fresh(); + + $this->assertEquals(30000, $invoice->balance); + $this->assertEquals(6000, $invoice->paid_to_date); + $this->assertEquals(6000, $client->paid_to_date); + $this->assertEquals(30000, $client->balance); + $this->assertEquals(6000, $payment->amount); + $this->assertFalse($payment->is_deleted); } From 9781fc2fbc87adb7d44ac722cefd81ae3a20a719 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 19 Aug 2022 12:49:35 +1000 Subject: [PATCH 08/10] Clean up for tests --- tests/Feature/DeleteInvoiceTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Feature/DeleteInvoiceTest.php b/tests/Feature/DeleteInvoiceTest.php index 4d48351272..4d378b54a0 100644 --- a/tests/Feature/DeleteInvoiceTest.php +++ b/tests/Feature/DeleteInvoiceTest.php @@ -182,6 +182,7 @@ class DeleteInvoiceTest extends TestCase $this->assertEquals(30000, $client->balance); $this->assertEquals(6000, $payment->amount); $this->assertFalse($payment->is_deleted); + $this->assertNull($payment->deleted_at); } From 5c13668a37af5f6c910db3867af80a995c3be879 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 19 Aug 2022 14:07:33 +1000 Subject: [PATCH 09/10] Remove snappdf download from composer script --- app/Models/Client.php | 10 +++++----- composer.json | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/Models/Client.php b/app/Models/Client.php index 0d55cce344..5c414e978a 100644 --- a/app/Models/Client.php +++ b/app/Models/Client.php @@ -236,6 +236,11 @@ class Client extends BaseModel implements HasLocalePreference return $this->hasMany(Task::class)->withTrashed(); } + public function payments() + { + return $this->hasMany(Payment::class)->withTrashed(); + } + public function recurring_invoices() { return $this->hasMany(RecurringInvoice::class)->withTrashed(); @@ -627,11 +632,6 @@ class Client extends BaseModel implements HasLocalePreference return $defaults; } - public function payments() - { - return $this->hasMany(Payment::class)->withTrashed(); - } - public function timezone_offset() { $offset = 0; diff --git a/composer.json b/composer.json index 4e8727c7b3..b8b92990cc 100644 --- a/composer.json +++ b/composer.json @@ -136,7 +136,6 @@ "if [ \"${IS_DOCKER:-false}\" != \"true\" ]; then vendor/bin/snappdf download; fi" ], "post-update-cmd": [ - "vendor/bin/snappdf download", "@php artisan vendor:publish --tag=laravel-assets --ansi --force" ], "post-root-package-install": [ From 66d40692ce134fe1a3aa7ee843186a3d2ec69270 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 19 Aug 2022 14:12:40 +1000 Subject: [PATCH 10/10] v5.5.12 --- VERSION.txt | 2 +- config/ninja.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/VERSION.txt b/VERSION.txt index 9a532528f2..4edeecad54 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -5.5.11 \ No newline at end of file +5.5.12 \ No newline at end of file diff --git a/config/ninja.php b/config/ninja.php index 286bbdc05d..999134172d 100644 --- a/config/ninja.php +++ b/config/ninja.php @@ -14,8 +14,8 @@ return [ 'require_https' => env('REQUIRE_HTTPS', true), 'app_url' => rtrim(env('APP_URL', ''), '/'), 'app_domain' => env('APP_DOMAIN', 'invoicing.co'), - 'app_version' => '5.5.11', - 'app_tag' => '5.5.11', + 'app_version' => '5.5.12', + 'app_tag' => '5.5.12', 'minimum_client_version' => '5.0.16', 'terms_version' => '1.0.1', 'api_secret' => env('API_SECRET', ''),