From 374812f8fe966b520ed7e3f693a7cf9af35c6d90 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 6 Aug 2024 10:48:38 -0700 Subject: [PATCH 1/4] Add failing test --- .../Checkouts/Ui/ComponentsCheckoutTest.php | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php b/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php index da285bebb..601eeb0b5 100644 --- a/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php +++ b/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php @@ -1,10 +1,13 @@ assertForbidden(); } + public function testCannotCheckoutAcrossCompaniesWhenFullCompanySupportEnabled() + { + Event::fake([CheckoutableCheckedOut::class]); + + $this->settings->enableMultipleFullCompanySupport(); + + [$assetCompany, $componentCompany] = Company::factory()->count(2)->create(); + + $asset = Asset::factory()->for($assetCompany)->create(); + $component = Component::factory()->for($componentCompany)->create(); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('components.checkout.store', $component), [ + 'asset_id' => $asset->id, + 'assigned_qty' => '1', + // @todo: + 'note' => null, + // @todo: + 'redirect_option' => 'index', + ]); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); + } + public function testComponentCheckoutPagePostIsRedirectedIfRedirectSelectionIsIndex() { $component = Component::factory()->create(); @@ -63,6 +90,4 @@ class ComponentsCheckoutTest extends TestCase ->assertStatus(302) ->assertRedirect(route('hardware.show', ['hardware' => $asset])); } - - } From 0aff35b622538eaf99657013e9bbd1cc29239e11 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 6 Aug 2024 12:07:34 -0700 Subject: [PATCH 2/4] Scaffold additional failed tests --- .../Checkouts/Ui/ComponentsCheckoutTest.php | 5 +- tests/Unit/ComponentTest.php | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php b/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php index 601eeb0b5..e2e91823c 100644 --- a/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php +++ b/tests/Feature/Checkouts/Ui/ComponentsCheckoutTest.php @@ -21,7 +21,7 @@ class ComponentsCheckoutTest extends TestCase ->assertForbidden(); } - public function testCannotCheckoutAcrossCompaniesWhenFullCompanySupportEnabled() + public function test_cannot_checkout_across_companies_when_full_company_support_enabled() { Event::fake([CheckoutableCheckedOut::class]); @@ -36,9 +36,6 @@ class ComponentsCheckoutTest extends TestCase ->post(route('components.checkout.store', $component), [ 'asset_id' => $asset->id, 'assigned_qty' => '1', - // @todo: - 'note' => null, - // @todo: 'redirect_option' => 'index', ]); diff --git a/tests/Unit/ComponentTest.php b/tests/Unit/ComponentTest.php index df8f64771..6abbffd82 100644 --- a/tests/Unit/ComponentTest.php +++ b/tests/Unit/ComponentTest.php @@ -1,10 +1,12 @@ assertInstanceOf(Category::class, $component->category); $this->assertEquals('component', $component->category->category_type); } + + public function test_num_checked_out_takes_does_not_scope_by_company() + { + $this->settings->enableMultipleFullCompanySupport(); + + [$companyA, $companyB] = Company::factory()->count(2)->create(); + + $componentForCompanyA = Component::factory()->for($companyA)->create(['qty' => 5]); + $assetForCompanyB = Asset::factory()->for($companyB)->create(); + + // Ideally, we shouldn't have a component attached to an + // asset from a different company but alas... + $componentForCompanyA->assets()->attach($componentForCompanyA->id, [ + 'component_id' => $componentForCompanyA->id, + 'assigned_qty' => 4, + 'asset_id' => $assetForCompanyB->id, + ]); + + $this->actingAs(User::factory()->superuser()->create()); + $this->assertEquals(4, $componentForCompanyA->fresh()->numCheckedOut()); + + $this->actingAs(User::factory()->admin()->create()); + $this->assertEquals(4, $componentForCompanyA->fresh()->numCheckedOut()); + + $this->actingAs(User::factory()->for($companyA)->create()); + $this->assertEquals(4, $componentForCompanyA->fresh()->numCheckedOut()); + } + + public function test_num_remaining_takes_company_scoping_into_account() + { + $this->settings->enableMultipleFullCompanySupport(); + + [$companyA, $companyB] = Company::factory()->count(2)->create(); + + $componentForCompanyA = Component::factory()->for($companyA)->create(['qty' => 5]); + $assetForCompanyB = Asset::factory()->for($companyB)->create(); + + // Ideally, we shouldn't have a component attached to an + // asset from a different company but alas... + $componentForCompanyA->assets()->attach($componentForCompanyA->id, [ + 'component_id' => $componentForCompanyA->id, + 'assigned_qty' => 4, + 'asset_id' => $assetForCompanyB->id, + ]); + + $this->actingAs(User::factory()->superuser()->create()); + $this->assertEquals(1, $componentForCompanyA->fresh()->numRemaining()); + + $this->actingAs(User::factory()->admin()->create()); + $this->assertEquals(1, $componentForCompanyA->fresh()->numRemaining()); + + $this->actingAs(User::factory()->for($companyA)->create()); + $this->assertEquals(1, $componentForCompanyA->fresh()->numRemaining()); + } } From bee80fcf8a42590deca8d74a28b66545add345c0 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 6 Aug 2024 12:16:06 -0700 Subject: [PATCH 3/4] Remove global scope when counting check outs --- app/Models/Component.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/Models/Component.php b/app/Models/Component.php index 671b0101c..536e06d0a 100644 --- a/app/Models/Component.php +++ b/app/Models/Component.php @@ -205,7 +205,11 @@ class Component extends SnipeModel public function numCheckedOut() { $checkedout = 0; - foreach ($this->assets as $checkout) { + + // In case there are elements checked out to assets that belong to a different company + // than this asset and full multiple company support is on we'll remove the global scope, + // so they are included in the count. + foreach ($this->assets()->withoutGlobalScope(new CompanyableScope)->get() as $checkout) { $checkedout += $checkout->pivot->assigned_qty; } From c7ddabcc8bff1a3d48c462cb292cee4f193d61a8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 6 Aug 2024 12:27:15 -0700 Subject: [PATCH 4/4] Check for FMCS when checking out component --- .../Controllers/Components/ComponentCheckoutController.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/Http/Controllers/Components/ComponentCheckoutController.php b/app/Http/Controllers/Components/ComponentCheckoutController.php index 5547b7170..e9db70811 100644 --- a/app/Http/Controllers/Components/ComponentCheckoutController.php +++ b/app/Http/Controllers/Components/ComponentCheckoutController.php @@ -8,6 +8,7 @@ use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Models\Asset; use App\Models\Component; +use App\Models\Setting; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Input; @@ -97,6 +98,10 @@ class ComponentCheckoutController extends Controller // Check if the asset exists $asset = Asset::find($request->input('asset_id')); + if ((Setting::getSettings()->full_multiple_companies_support) && $component->company_id !== $asset->company_id) { + return redirect()->route('components.checkout.show', $componentId)->with('error', trans('general.error_user_company')); + } + // Update the component data $component->asset_id = $request->input('asset_id'); $component->assets()->attach($component->id, [