From 7852f4df1f819088ab1925f22fd0882f67bfe127 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 26 Nov 2017 09:54:09 +0100 Subject: [PATCH] Expand test coverage. --- app/Helpers/Attachments/AttachmentHelper.php | 7 + app/Http/Controllers/AccountController.php | 4 +- app/Http/Controllers/Admin/LinkController.php | 4 +- app/Import/Object/ImportJournal.php | 109 ++++---- resources/stubs/binary.bin | Bin 0 -> 100 bytes .../Controllers/AccountControllerTest.php | 5 + .../Controllers/Admin/LinkControllerTest.php | 235 ++++++++++++++++++ tests/Unit/Helpers/AttachmentHelperTest.php | 6 +- 8 files changed, 321 insertions(+), 49 deletions(-) create mode 100644 resources/stubs/binary.bin create mode 100644 tests/Feature/Controllers/Admin/LinkControllerTest.php diff --git a/app/Helpers/Attachments/AttachmentHelper.php b/app/Helpers/Attachments/AttachmentHelper.php index d36a840063..e71ea6fde3 100644 --- a/app/Helpers/Attachments/AttachmentHelper.php +++ b/app/Helpers/Attachments/AttachmentHelper.php @@ -107,7 +107,9 @@ class AttachmentHelper implements AttachmentHelperInterface */ public function saveAttachmentsForModel(Model $model, ?array $files): bool { + Log::debug(sprintf('Now in saveAttachmentsForModel for model %s', get_class($model))); if (is_array($files)) { + Log::debug('$files is an array.'); /** @var UploadedFile $entry */ foreach ($files as $entry) { if (null !== $entry) { @@ -155,6 +157,7 @@ class AttachmentHelper implements AttachmentHelperInterface */ protected function processFile(UploadedFile $file, Model $model): Attachment { + Log::debug('Now in processFile()'); $validation = $this->validateUpload($file, $model); if (false === $validation) { return new Attachment; @@ -199,8 +202,11 @@ class AttachmentHelper implements AttachmentHelperInterface */ protected function validMime(UploadedFile $file): bool { + Log::debug('Now in validMime()'); $mime = e($file->getMimeType()); $name = e($file->getClientOriginalName()); + Log::debug(sprintf('Name is %, and mime is %s', $name, $mime)); + Log::debug('Valid mimes are', $this->allowedMimes); if (!in_array($mime, $this->allowedMimes)) { $msg = (string)trans('validation.file_invalid_mime', ['name' => $name, 'mime' => $mime]); @@ -243,6 +249,7 @@ class AttachmentHelper implements AttachmentHelperInterface */ protected function validateUpload(UploadedFile $file, Model $model): bool { + Log::debug('Now in validateUpload()'); if (!$this->validMime($file)) { return false; } diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index ea1c6ba692..e8dd4be714 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -284,7 +284,7 @@ class AccountController extends Controller $currencyId = intval($account->getMeta('currency_id')); $currency = $currencyRepos->find($currencyId); if (0 === $currencyId) { - $currency = app('amount')->getDefaultCurrency(); + $currency = app('amount')->getDefaultCurrency(); // @codeCoverageIgnore } // prep for "all" view. @@ -348,8 +348,10 @@ class AccountController extends Controller // update preferences if necessary: $frontPage = Preferences::get('frontPageAccounts', [])->data; if (count($frontPage) > 0 && AccountType::ASSET === $account->accountType->type) { + // @codeCoverageIgnoreStart $frontPage[] = $account->id; Preferences::set('frontPageAccounts', $frontPage); + // @codeCoverageIgnoreEnd } if (1 === intval($request->get('create_another'))) { diff --git a/app/Http/Controllers/Admin/LinkController.php b/app/Http/Controllers/Admin/LinkController.php index 06c8146e37..38bf575b7d 100644 --- a/app/Http/Controllers/Admin/LinkController.php +++ b/app/Http/Controllers/Admin/LinkController.php @@ -137,7 +137,7 @@ class LinkController extends Controller // put previous url in session if not redirect from store (not "return_to_edit"). if (true !== session('link_types.edit.fromUpdate')) { - $this->rememberPreviousUri('link_types.edit.uri'); + $this->rememberPreviousUri('link_types.edit.uri'); // @codeCoverageIgnore } $request->session()->forget('link_types.edit.fromUpdate'); @@ -197,7 +197,7 @@ class LinkController extends Controller // set value so create routine will not overwrite URL: $request->session()->put('link_types.create.fromStore', true); - return redirect(route('link_types.create', [$request->input('what')]))->withInput(); + return redirect(route('admin.links.create'))->withInput(); } // redirect to previous URL. diff --git a/app/Import/Object/ImportJournal.php b/app/Import/Object/ImportJournal.php index 299315b47f..54f194c481 100644 --- a/app/Import/Object/ImportJournal.php +++ b/app/Import/Object/ImportJournal.php @@ -109,49 +109,7 @@ class ImportJournal Log::debug(sprintf('credit amount is %s', var_export($this->amountCredit, true))); if (null === $this->convertedAmount) { - // first check if the amount is set: - Log::debug('convertedAmount is NULL'); - - $info = []; - $converterClass = ''; - - if (!is_null($this->amount)) { - Log::debug('Amount value is not NULL, assume this is the correct value.'); - $converterClass = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $this->amount['role']))); - $info = $this->amount; - } - if (!is_null($this->amountDebet)) { - Log::debug('Amount DEBET value is not NULL, assume this is the correct value (overrules Amount).'); - $converterClass = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $this->amountDebet['role']))); - $info = $this->amountDebet; - } - if (!is_null($this->amountCredit)) { - Log::debug('Amount CREDIT value is not NULL, assume this is the correct value (overrules Amount and AmountDebet).'); - $converterClass = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $this->amountCredit['role']))); - $info = $this->amountCredit; - } - if (0 === count($info)) { - throw new FireflyException('No amount information for this row.'); - } - - Log::debug(sprintf('Converter class is %s', $converterClass)); - /** @var ConverterInterface $amountConverter */ - $amountConverter = app($converterClass); - $this->convertedAmount = $amountConverter->convert($info['value']); - Log::debug(sprintf('First attempt to convert gives "%s"', $this->convertedAmount)); - // modify - foreach ($this->modifiers as $modifier) { - $class = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $modifier['role']))); - /** @var ConverterInterface $converter */ - $converter = app($class); - Log::debug(sprintf('Now launching converter %s', $class)); - if ($converter->convert($modifier['value']) === -1) { - $this->convertedAmount = Steam::negative($this->convertedAmount); - } - Log::debug(sprintf('convertedAmount after conversion is %s', $this->convertedAmount)); - } - - Log::debug(sprintf('After modifiers the result is: "%s"', $this->convertedAmount)); + $this->calculateAmount(); } Log::debug(sprintf('convertedAmount is: "%s"', $this->convertedAmount)); if (0 === bccomp($this->convertedAmount, '0')) { @@ -324,6 +282,71 @@ class ImportJournal } } + /** + * If convertedAmount is NULL, this method will try to calculate the correct amount. + * It starts with amount, but can be overruled by debet and credit amounts. + * + * @throws FireflyException + */ + private function calculateAmount() + { + // first check if the amount is set: + Log::debug('convertedAmount is NULL'); + + $info = $this->selectAmountInput(); + + if (0 === count($info)) { + throw new FireflyException('No amount information for this row.'); + } + + Log::debug(sprintf('Converter class is %s', $info['class'])); + /** @var ConverterInterface $amountConverter */ + $amountConverter = app($info['class']); + $this->convertedAmount = $amountConverter->convert($info['value']); + Log::debug(sprintf('First attempt to convert gives "%s"', $this->convertedAmount)); + // modify + foreach ($this->modifiers as $modifier) { + $class = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $modifier['role']))); + /** @var ConverterInterface $converter */ + $converter = app($class); + Log::debug(sprintf('Now launching converter %s', $class)); + if ($converter->convert($modifier['value']) === -1) { + $this->convertedAmount = Steam::negative($this->convertedAmount); + } + Log::debug(sprintf('convertedAmount after conversion is %s', $this->convertedAmount)); + } + + Log::debug(sprintf('After modifiers the result is: "%s"', $this->convertedAmount)); + } + + /** + * This methods decides which input to use for the amount calculation. + * + * @return array + */ + private function selectAmountInput() + { + $converterClass = ''; + if (!is_null($this->amount)) { + Log::debug('Amount value is not NULL, assume this is the correct value.'); + $converterClass = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $this->amount['role']))); + $info = $this->amount; + } + if (!is_null($this->amountDebet)) { + Log::debug('Amount DEBET value is not NULL, assume this is the correct value (overrules Amount).'); + $converterClass = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $this->amountDebet['role']))); + $info = $this->amountDebet; + } + if (!is_null($this->amountCredit)) { + Log::debug('Amount CREDIT value is not NULL, assume this is the correct value (overrules Amount and AmountDebet).'); + $converterClass = sprintf('FireflyIII\Import\Converter\%s', config(sprintf('csv.import_roles.%s.converter', $this->amountCredit['role']))); + $info = $this->amountCredit; + } + $info['class'] = $converterClass; + + return $info; + } + /** * @param array $array */ diff --git a/resources/stubs/binary.bin b/resources/stubs/binary.bin new file mode 100644 index 0000000000000000000000000000000000000000..25602c8fbcdd6a3b3c558827e592858189c073b8 GIT binary patch literal 100 zcmbshouldReceive('store')->once()->andReturn(factory(Account::class)->make()); $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); + // change the preference: + Preferences::setForUser($this->user(), 'frontPageAccounts', [1]); + $this->session(['accounts.create.uri' => 'http://localhost']); $this->be($this->user()); $data = [ diff --git a/tests/Feature/Controllers/Admin/LinkControllerTest.php b/tests/Feature/Controllers/Admin/LinkControllerTest.php new file mode 100644 index 0000000000..590f380fb7 --- /dev/null +++ b/tests/Feature/Controllers/Admin/LinkControllerTest.php @@ -0,0 +1,235 @@ +. + */ +declare(strict_types=1); + +namespace Tests\Feature\Controllers; + +use FireflyIII\Models\LinkType; +use Tests\TestCase; + +/** + * Class LinkControllerTest + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ +class LinkControllerTest extends TestCase +{ + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::__construct + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::create + */ + public function testCreate() + { + + $this->be($this->user()); + $response = $this->get(route('admin.links.create')); + $response->assertStatus(200); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::delete + */ + public function testDeleteEditable() + { + // create editable link type just in case: + LinkType::create(['editable' => 1, 'inward' => 'hello', 'outward' => 'bye', 'name' => 'Test type']); + + $linkType = LinkType::where('editable', 1)->first(); + $this->be($this->user()); + $response = $this->get(route('admin.links.delete', [$linkType->id])); + $response->assertStatus(200); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::delete + */ + public function testDeleteNonEditable() + { + $linkType = LinkType::where('editable', 0)->first(); + $this->be($this->user()); + $response = $this->get(route('admin.links.delete', [$linkType->id])); + $response->assertStatus(302); + $response->assertSessionHas('error'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::destroy + */ + public function testDestroy() + { + // create editable link type just in case: + LinkType::create(['editable' => 1, 'inward' => 'hellox', 'outward' => 'byex', 'name' => 'Test typeX']); + + $linkType = LinkType::where('editable', 1)->first(); + $this->be($this->user()); + $this->session(['link_types.delete.uri' => 'http://localhost']); + $response = $this->post(route('admin.links.destroy', [$linkType->id])); + $response->assertStatus(302); + $response->assertSessionHas('success'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::edit + */ + public function testEditEditable() + { + // create editable link type just in case: + LinkType::create(['editable' => 1, 'inward' => 'hello Y', 'outward' => 'bye Y', 'name' => 'Test type Y']); + + $linkType = LinkType::where('editable', 1)->first(); + $this->be($this->user()); + $response = $this->get(route('admin.links.edit', [$linkType->id])); + $response->assertStatus(200); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::edit + */ + public function testEditNonEditable() + { + $linkType = LinkType::where('editable', 0)->first(); + $this->be($this->user()); + $response = $this->get(route('admin.links.edit', [$linkType->id])); + $response->assertStatus(302); + $response->assertSessionHas('error'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::index + */ + public function testIndex() + { + $this->be($this->user()); + $response = $this->get(route('admin.links.index')); + $response->assertStatus(200); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::show + */ + public function testShow() + { + $linkType = LinkType::first(); + $this->be($this->user()); + $response = $this->get(route('admin.links.show', [$linkType->id])); + $response->assertStatus(200); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::store + */ + public function testStore() + { + $data = [ + 'name' => 'test ' . rand(1, 1000), + 'inward' => 'test inward' . rand(1, 1000), + 'outward' => 'test outward' . rand(1, 1000), + ]; + $this->session(['link_types.create.uri' => 'http://localhost']); + $this->be($this->user()); + $response = $this->post(route('admin.links.store'), $data); + $response->assertStatus(302); + $response->assertSessionHas('success'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::store + */ + public function testStoreRedirect() + { + $data = [ + 'name' => 'test ' . rand(1, 1000), + 'inward' => 'test inward' . rand(1, 1000), + 'outward' => 'test outward' . rand(1, 1000), + 'create_another' => '1', + ]; + $this->session(['link_types.create.uri' => 'http://localhost']); + $this->be($this->user()); + $response = $this->post(route('admin.links.store'), $data); + $response->assertStatus(302); + $response->assertSessionHas('success'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::update + */ + public function testUpdate() + { + // create editable link type just in case: + $linKType = LinkType::create(['editable' => 1, 'inward' => 'helloxz', 'outward' => 'bzyex', 'name' => 'Test tyzpeX']); + + + $data = [ + 'name' => 'test ' . rand(1, 1000), + 'inward' => 'test inward' . rand(1, 1000), + 'outward' => 'test outward' . rand(1, 1000), + ]; + $this->session(['link_types.edit.uri' => 'http://localhost']); + $this->be($this->user()); + $response = $this->post(route('admin.links.update', [$linKType->id]), $data); + $response->assertStatus(302); + $response->assertSessionHas('success'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::update + */ + public function testUpdateNonEditable() + { + // create editable link type just in case: + $linkType = LinkType::where('editable', 0)->first(); + + $data = [ + 'name' => 'test ' . rand(1, 1000), + 'inward' => 'test inward' . rand(1, 1000), + 'outward' => 'test outward' . rand(1, 1000), + 'return_to_edit' => '1', + ]; + $this->session(['link_types.edit.uri' => 'http://localhost']); + $this->be($this->user()); + $response = $this->post(route('admin.links.update', [$linkType->id]), $data); + $response->assertStatus(302); + $response->assertSessionHas('error'); + } + + /** + * @covers \FireflyIII\Http\Controllers\Admin\LinkController::update + */ + public function testUpdateRedirect() + { + // create editable link type just in case: + $linkType = LinkType::create(['editable' => 1, 'inward' => 'healox', 'outward' => 'byaex', 'name' => 'Test tyapeX']); + + $data = [ + 'name' => 'test ' . rand(1, 1000), + 'inward' => 'test inward' . rand(1, 1000), + 'outward' => 'test outward' . rand(1, 1000), + 'return_to_edit' => '1', + ]; + $this->session(['link_types.edit.uri' => 'http://localhost']); + $this->be($this->user()); + $response = $this->post(route('admin.links.update', [$linkType->id]), $data); + $response->assertStatus(302); + $response->assertSessionHas('success'); + } +} diff --git a/tests/Unit/Helpers/AttachmentHelperTest.php b/tests/Unit/Helpers/AttachmentHelperTest.php index 428abcb83b..41a74815c9 100644 --- a/tests/Unit/Helpers/AttachmentHelperTest.php +++ b/tests/Unit/Helpers/AttachmentHelperTest.php @@ -65,8 +65,8 @@ class AttachmentHelperTest extends TestCase { $journal = TransactionJournal::first(); $helper = new AttachmentHelper; - $path = resource_path('stubs/csv.csv'); - $file = new UploadedFile($path, 'csv.csv', 'text/plain', filesize($path), null, true); + $path = resource_path('stubs/binary.bin'); + $file = new UploadedFile($path, 'binary.bin', 'application/octet-stream', filesize($path), null, true); $helper->saveAttachmentsForModel($journal, [$file]); $errors = $helper->getErrors(); @@ -74,7 +74,7 @@ class AttachmentHelperTest extends TestCase $this->assertCount(1, $errors); $this->assertCount(0, $messages); - $this->assertEquals('File "csv.csv" is of type "text/plain" which is not accepted as a new upload.', $errors->first()); + $this->assertEquals('File "binary.bin" is of type "application/octet-stream" which is not accepted as a new upload.', $errors->first()); } /**