diff --git a/app/Models/ReportTemplate.php b/app/Models/ReportTemplate.php index fc08f0fd43..85fd5fda0e 100644 --- a/app/Models/ReportTemplate.php +++ b/app/Models/ReportTemplate.php @@ -65,7 +65,7 @@ class ReportTemplate extends Model return $this->options[$property] ?? null; } - public function selectValues(string $property) + public function selectValues(string $property, string $model = null) { if (!isset($this->options[$property])) { return null; @@ -75,6 +75,12 @@ class ReportTemplate extends Model return null; } + // If a model is provided then we should ensure we only return + // the ids of models that exist and are not deleted. + if ($model) { + return $model::findMany($this->options[$property])->pluck('id'); + } + return $this->options[$property]; } diff --git a/resources/views/reports/custom.blade.php b/resources/views/reports/custom.blade.php index 4e27d52595..dab856d647 100644 --- a/resources/views/reports/custom.blade.php +++ b/resources/views/reports/custom.blade.php @@ -287,14 +287,14 @@ 'multiple' => 'true', 'fieldname' => 'by_location_id[]', 'hide_new' => 'true', - 'selected' => $reportTemplate->selectValues('by_location_id') + 'selected' => $reportTemplate->selectValues('by_location_id', \App\Models\Location::class) ]) @include ('partials.forms.edit.location-select', [ 'translated_name' => trans('admin/hardware/form.default_location'), 'multiple' => 'true', 'fieldname' => 'by_rtd_location_id[]', 'hide_new' => 'true', - 'selected' => $reportTemplate->selectValues('by_rtd_location_id') + 'selected' => $reportTemplate->selectValues('by_rtd_location_id', \App\Models\Location::class) ]) @include ('partials.forms.edit.department-select', [ 'translated_name' => trans('general.department'), diff --git a/tests/Unit/ReportTemplateTest.php b/tests/Unit/ReportTemplateTest.php index 7af41710c7..5a745b4ba3 100644 --- a/tests/Unit/ReportTemplateTest.php +++ b/tests/Unit/ReportTemplateTest.php @@ -2,6 +2,7 @@ namespace Tests\Unit; +use App\Models\Location; use App\Models\ReportTemplate; use Tests\TestCase; @@ -85,11 +86,34 @@ class ReportTemplateTest extends TestCase public function testSelectValuesDoNotIncludeDeletedOrNonExistentModels() { - $this->markTestIncomplete(); + [$locationA, $locationB] = Location::factory()->count(2)->create(); - // report saved with select option for a company (or whatever) - // company is deleted - // ensure company's id is not returned + $savedReport = ReportTemplate::factory()->create([ + 'options' => [ + 'by_location_id' => [ + $locationA->id, + $locationB->id, + 10000 + ], + ], + ]); + + $locationB->delete(); + + $this->assertContains( + $locationA->id, + $savedReport->selectValues('by_location_id', Location::class) + ); + + $this->assertNotContains( + $locationB->id, + $savedReport->selectValues('by_location_id', Location::class) + ); + + $this->assertNotContains( + 10000, + $savedReport->selectValues('by_location_id', Location::class) + ); } public function testGracefullyHandlesSingleSelectBecomingMultiSelect()