From 03d3fb6a5f2d57b81698c4f0efc5f93e49e1d157 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 13 Aug 2025 12:09:56 -0700 Subject: [PATCH 01/30] Add qty to checkout_acceptances table --- ..._add_qty_to_checkout_acceptances_table.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php diff --git a/database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php b/database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php new file mode 100644 index 0000000000..2bb32b0e90 --- /dev/null +++ b/database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php @@ -0,0 +1,27 @@ +unsignedInteger('qty')->nullable()->after('assigned_to_id')->default(null); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('checkout_acceptances', function (Blueprint $table) { + $table->dropColumn('qty'); + }); + } +}; From d2ee8de9acd0ec8126f5e055f9fc84738b7e56c7 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 13 Aug 2025 12:14:10 -0700 Subject: [PATCH 02/30] Attach qty to CheckoutAcceptance --- app/Listeners/CheckoutableListener.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 7e289b19e6..e9920702a9 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -240,6 +240,12 @@ class CheckoutableListener $acceptance->checkoutable()->associate($event->checkoutable); $acceptance->assignedTo()->associate($event->checkedOutTo); + $acceptance->qty = 1; + + if (isset($event->checkoutable->checkout_qty)) { + $acceptance->qty = $event->checkoutable->checkout_qty; + } + $category = $this->getCategoryFromCheckoutable($event->checkoutable); if ($category?->alert_on_response) { From 22fe9a786ea0dd2daac98942e4a2e167a1b682cd Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 14 Aug 2025 12:36:46 -0700 Subject: [PATCH 03/30] Display number of items being accepted --- resources/lang/en-US/general.php | 2 ++ .../views/account/accept/create.blade.php | 21 +++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/resources/lang/en-US/general.php b/resources/lang/en-US/general.php index b06d6899fd..387f50e9b6 100644 --- a/resources/lang/en-US/general.php +++ b/resources/lang/en-US/general.php @@ -351,9 +351,11 @@ return [ 'audit_overdue' => 'Overdue for Audit', 'accept' => 'Accept :asset', 'i_accept' => 'I accept', + 'i_accept_with_count' => 'I accept :count item|I accept :count items', 'i_decline_item' => 'Decline this item', 'i_accept_item' => 'Accept this item', 'i_decline' => 'I decline', + 'i_decline_with_count' => 'I decline :count item|I decline :count items', 'accept_decline' => 'Accept/Decline', 'sign_tos' => 'Sign below to indicate that you agree to the terms of service:', 'clear_signature' => 'Clear Signature', diff --git a/resources/views/account/accept/create.blade.php b/resources/views/account/accept/create.blade.php index 54a4371f1f..33dfb5f38a 100644 --- a/resources/views/account/accept/create.blade.php +++ b/resources/views/account/accept/create.blade.php @@ -38,8 +38,13 @@

- {{$acceptance->checkoutable->present()->name()}} - {{ (($acceptance->checkoutable) && ($acceptance->checkoutable->serial)) ? ' - '.trans('general.serial_number').': '.$acceptance->checkoutable->serial : '' }} +
+ {{$acceptance->checkoutable->present()->name()}} + @if ($acceptance->qty > 1) + ×{{ $acceptance->qty }} + @endif +
+
{{ (($acceptance->checkoutable) && ($acceptance->checkoutable->serial)) ? trans('general.serial_number').': '.$acceptance->checkoutable->serial : '' }}

@@ -53,11 +58,19 @@
From 59bd6ca36037a3baf180d4b1eafda9f9c4387104 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 14 Aug 2025 13:21:11 -0700 Subject: [PATCH 04/30] Update button text --- resources/lang/en-US/general.php | 4 ++-- resources/views/account/accept/create.blade.php | 13 +++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/resources/lang/en-US/general.php b/resources/lang/en-US/general.php index 387f50e9b6..b6114d618d 100644 --- a/resources/lang/en-US/general.php +++ b/resources/lang/en-US/general.php @@ -352,8 +352,8 @@ return [ 'accept' => 'Accept :asset', 'i_accept' => 'I accept', 'i_accept_with_count' => 'I accept :count item|I accept :count items', - 'i_decline_item' => 'Decline this item', - 'i_accept_item' => 'Accept this item', + 'i_decline_item' => 'Decline this item|Decline these items', + 'i_accept_item' => 'Accept this item|Accept these items', 'i_decline' => 'I decline', 'i_decline_with_count' => 'I decline :count item|I decline :count items', 'accept_decline' => 'Accept/Decline', diff --git a/resources/views/account/accept/create.blade.php b/resources/views/account/accept/create.blade.php index 33dfb5f38a..7bf98d7cad 100644 --- a/resources/views/account/accept/create.blade.php +++ b/resources/views/account/accept/create.blade.php @@ -111,7 +111,7 @@
@@ -174,7 +174,7 @@ $("#showSubmit").show(); $("#submit-button").removeClass("btn-success").addClass("btn-danger").show(); $("#submitIcon").removeClass("fa-check").addClass("fa-times"); - $("#buttonText").text('{{ trans('general.i_decline_item') }}'); + $("#buttonText").text('{{ trans_choice('general.i_decline_item', $acceptance->qty ?? 1) }}'); $("#note").prop('required', true); } else if ($(this).is(':checked') && $(this).attr('id') === 'accepted') { @@ -182,16 +182,9 @@ $("#showSubmit").show(); $("#submit-button").removeClass("btn-danger").addClass("btn-success").show(); $("#submitIcon").removeClass("fa-check").addClass("fa-check"); - $("#buttonText").text('{{ trans('general.i_accept_item') }}'); + $("#buttonText").text('{{ trans_choice('general.i_accept_item', $acceptance->qty ?? 1) }}'); $("#note").prop('required', false); - - - } - }); - - - @stop From 3f0ac103a136396c3ed56781636cbada0e8fa02d Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 14 Aug 2025 13:29:18 -0700 Subject: [PATCH 05/30] Scaffold test --- .../Ui/AccessoryAcceptanceTest.php | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index fef38623f8..ae179462a4 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -96,4 +96,27 @@ class AccessoryAcceptanceTest extends TestCase $this->assertNull($acceptance->fresh()->accepted_at); } + + /** + * @link https://github.com/grokability/snipe-it/issues/17589 + */ + public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance() + { + $this->markTestIncomplete(); + + // create accessory that requires acceptance + + // check out the accessory to a user with qty of 2 using the legacy behavior: `checkout_acceptances.qty` is null + + // check out the accessory to a user with qty of 2 using the new behavior: `checkout_acceptances.qty` is 2 + + // decline the first checkout + // ❓ + + // decline the second checkout + // both rows from `accessories_checkout` should be removed + + // ensure existing checkouts for the user are not affected. + // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. + } } From 3101212c49a6768f98885da4e54fd11ff1e2c203 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 14 Aug 2025 17:24:36 -0700 Subject: [PATCH 06/30] Continue to scaffold test --- .../Ui/AccessoryAcceptanceTest.php | 63 +++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index ae179462a4..57fb6b0c43 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -3,6 +3,7 @@ namespace Tests\Feature\CheckoutAcceptances\Ui; use App\Models\Accessory; +use App\Models\AccessoryCheckout; use App\Models\Asset; use App\Models\CheckoutAcceptance; use App\Models\User; @@ -102,19 +103,73 @@ class AccessoryAcceptanceTest extends TestCase */ public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance() { - $this->markTestIncomplete(); + // $this->markTestIncomplete(); + + $user = User::factory()->create(); + + $this->actingAs(User::factory()->checkoutAccessories()->create()); // create accessory that requires acceptance + $accessoryA = Accessory::factory()->requiringAcceptance()->create(['qty' => 4]); + $accessoryB = Accessory::factory()->requiringAcceptance()->create(['qty' => 4]); // check out the accessory to a user with qty of 2 using the legacy behavior: `checkout_acceptances.qty` is null + $this->post(route('accessories.checkout.store', $accessoryA), [ + 'assigned_user' => $user->id, + 'checkout_qty' => 2, + ]); + + $this->assertEquals(2, AccessoryCheckout::count()); + + $legacyCheckoutAcceptance = CheckoutAcceptance::query() + ->where([ + 'assigned_to_id' => $user->id, + 'qty' => 2, + ]) + ->whereNull(['accepted_at', 'declined_at']) + ->whereHasMorph( + 'checkoutable', + [Accessory::class], + ) + ->sole(); + + $legacyCheckoutAcceptance->qty = null; + $legacyCheckoutAcceptance->save(); // check out the accessory to a user with qty of 2 using the new behavior: `checkout_acceptances.qty` is 2 + $this->post(route('accessories.checkout.store', $accessoryB), [ + 'assigned_user' => $user->id, + 'checkout_qty' => 2, + ]); - // decline the first checkout - // ❓ + $this->assertEquals(4, AccessoryCheckout::count()); + + $checkoutAcceptance = CheckoutAcceptance::query() + ->where([ + 'assigned_to_id' => $user->id, + 'qty' => 2, + ]) + ->whereNull(['accepted_at', 'declined_at']) + ->whereHasMorph( + 'checkoutable', + [Accessory::class], + ) + ->sole(); + + // decline the "legacy" checkout + $this->actingAs($user); + + $this->post(route('account.store-acceptance', $legacyCheckoutAcceptance), [ + 'asset_acceptance' => 'declined', + ]); + + // decline the checkout + $this->post(route('account.store-acceptance', $checkoutAcceptance), [ + 'asset_acceptance' => 'declined', + ]); - // decline the second checkout // both rows from `accessories_checkout` should be removed + $this->assertEquals(0, AccessoryCheckout::count()); // ensure existing checkouts for the user are not affected. // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. From d8eaf2676fe303828fc1affb0bdbe14a142e883f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 19 Aug 2025 15:54:46 -0700 Subject: [PATCH 07/30] Add a couple of notes --- .../CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index 57fb6b0c43..7ec92fb4f8 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -119,6 +119,7 @@ class AccessoryAcceptanceTest extends TestCase 'checkout_qty' => 2, ]); + // @todo: this is flaky $this->assertEquals(2, AccessoryCheckout::count()); $legacyCheckoutAcceptance = CheckoutAcceptance::query() @@ -142,6 +143,7 @@ class AccessoryAcceptanceTest extends TestCase 'checkout_qty' => 2, ]); + // @todo: this is flaky $this->assertEquals(4, AccessoryCheckout::count()); $checkoutAcceptance = CheckoutAcceptance::query() @@ -168,10 +170,12 @@ class AccessoryAcceptanceTest extends TestCase 'asset_acceptance' => 'declined', ]); - // both rows from `accessories_checkout` should be removed + // four rows from `accessories_checkout` should be removed + // @todo: this is flaky $this->assertEquals(0, AccessoryCheckout::count()); // ensure existing checkouts for the user are not affected. // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. + // ie...if a user accepted previous accessories then those should not be touched. } } From 1c31f126ef792b0d819f0d228905b924966bce6e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 19 Aug 2025 16:46:41 -0700 Subject: [PATCH 08/30] Clear some flakiness --- .../Ui/AccessoryAcceptanceTest.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index 7ec92fb4f8..720b6c94a2 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -119,8 +119,11 @@ class AccessoryAcceptanceTest extends TestCase 'checkout_qty' => 2, ]); - // @todo: this is flaky - $this->assertEquals(2, AccessoryCheckout::count()); + $this->assertEquals(2, AccessoryCheckout::where([ + 'accessory_id' => $accessoryA->id, + 'assigned_to' => $user->id, + 'assigned_type' => User::class, + ])->count()); $legacyCheckoutAcceptance = CheckoutAcceptance::query() ->where([ @@ -143,8 +146,11 @@ class AccessoryAcceptanceTest extends TestCase 'checkout_qty' => 2, ]); - // @todo: this is flaky - $this->assertEquals(4, AccessoryCheckout::count()); + $this->assertEquals(2, AccessoryCheckout::where([ + 'accessory_id' => $accessoryB->id, + 'assigned_to' => $user->id, + 'assigned_type' => User::class, + ])->count()); $checkoutAcceptance = CheckoutAcceptance::query() ->where([ From cc1132be87f06ecd3d6dffde83e91540acc5c1bf Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 19 Aug 2025 16:50:36 -0700 Subject: [PATCH 09/30] Remove flakiness --- .../CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index 720b6c94a2..c94af12664 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -152,6 +152,8 @@ class AccessoryAcceptanceTest extends TestCase 'assigned_type' => User::class, ])->count()); + $originalAccessoryCheckoutCount = AccessoryCheckout::count(); + $checkoutAcceptance = CheckoutAcceptance::query() ->where([ 'assigned_to_id' => $user->id, @@ -177,8 +179,7 @@ class AccessoryAcceptanceTest extends TestCase ]); // four rows from `accessories_checkout` should be removed - // @todo: this is flaky - $this->assertEquals(0, AccessoryCheckout::count()); + $this->assertEquals($originalAccessoryCheckoutCount - 4, AccessoryCheckout::count()); // ensure existing checkouts for the user are not affected. // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. From 3ffb73a5160613d4042041710ea81f888a5050cc Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 19 Aug 2025 17:04:13 -0700 Subject: [PATCH 10/30] Remove total qty of accessory checkouts --- app/Models/CheckoutAcceptance.php | 12 ++++++++---- .../Ui/AccessoryAcceptanceTest.php | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/Models/CheckoutAcceptance.php b/app/Models/CheckoutAcceptance.php index 0599286f85..72f7accc52 100644 --- a/app/Models/CheckoutAcceptance.php +++ b/app/Models/CheckoutAcceptance.php @@ -108,10 +108,14 @@ class CheckoutAcceptance extends Model $this->signature_filename = $signature_filename; $this->save(); - /** - * Update state for the checked out item - */ - $this->checkoutable->declinedCheckout($this->assignedTo, $signature_filename); + $qty = $this->qty ?? 1; + + foreach (range(0, $qty) as $count) { + /** + * Update state for the checked out item + */ + $this->checkoutable->declinedCheckout($this->assignedTo, $signature_filename); + } } /** diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index c94af12664..8e59b5489c 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -103,7 +103,6 @@ class AccessoryAcceptanceTest extends TestCase */ public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance() { - // $this->markTestIncomplete(); $user = User::factory()->create(); @@ -181,6 +180,7 @@ class AccessoryAcceptanceTest extends TestCase // four rows from `accessories_checkout` should be removed $this->assertEquals($originalAccessoryCheckoutCount - 4, AccessoryCheckout::count()); + // @todo: // ensure existing checkouts for the user are not affected. // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. // ie...if a user accepted previous accessories then those should not be touched. From 9caa240fdb24ac3136295abc3c65f4832b660157 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 19 Aug 2025 17:17:46 -0700 Subject: [PATCH 11/30] Revert "Remove total qty of accessory checkouts" This reverts commit 3ffb73a5160613d4042041710ea81f888a5050cc. --- app/Models/CheckoutAcceptance.php | 12 ++++-------- .../Ui/AccessoryAcceptanceTest.php | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/app/Models/CheckoutAcceptance.php b/app/Models/CheckoutAcceptance.php index 72f7accc52..0599286f85 100644 --- a/app/Models/CheckoutAcceptance.php +++ b/app/Models/CheckoutAcceptance.php @@ -108,14 +108,10 @@ class CheckoutAcceptance extends Model $this->signature_filename = $signature_filename; $this->save(); - $qty = $this->qty ?? 1; - - foreach (range(0, $qty) as $count) { - /** - * Update state for the checked out item - */ - $this->checkoutable->declinedCheckout($this->assignedTo, $signature_filename); - } + /** + * Update state for the checked out item + */ + $this->checkoutable->declinedCheckout($this->assignedTo, $signature_filename); } /** diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index 8e59b5489c..c94af12664 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -103,6 +103,7 @@ class AccessoryAcceptanceTest extends TestCase */ public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance() { + // $this->markTestIncomplete(); $user = User::factory()->create(); @@ -180,7 +181,6 @@ class AccessoryAcceptanceTest extends TestCase // four rows from `accessories_checkout` should be removed $this->assertEquals($originalAccessoryCheckoutCount - 4, AccessoryCheckout::count()); - // @todo: // ensure existing checkouts for the user are not affected. // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. // ie...if a user accepted previous accessories then those should not be touched. From 48ba7eed3e1782ff9f427a6f4f994cfb3d4bbd29 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 11:45:36 -0700 Subject: [PATCH 12/30] Remove legacy checks from test --- .../Ui/AccessoryAcceptanceTest.php | 62 +++++-------------- 1 file changed, 14 insertions(+), 48 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index c94af12664..cacfd55515 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -103,51 +103,21 @@ class AccessoryAcceptanceTest extends TestCase */ public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance() { - // $this->markTestIncomplete(); - $user = User::factory()->create(); $this->actingAs(User::factory()->checkoutAccessories()->create()); // create accessory that requires acceptance - $accessoryA = Accessory::factory()->requiringAcceptance()->create(['qty' => 4]); - $accessoryB = Accessory::factory()->requiringAcceptance()->create(['qty' => 4]); - - // check out the accessory to a user with qty of 2 using the legacy behavior: `checkout_acceptances.qty` is null - $this->post(route('accessories.checkout.store', $accessoryA), [ - 'assigned_user' => $user->id, - 'checkout_qty' => 2, - ]); - - $this->assertEquals(2, AccessoryCheckout::where([ - 'accessory_id' => $accessoryA->id, - 'assigned_to' => $user->id, - 'assigned_type' => User::class, - ])->count()); - - $legacyCheckoutAcceptance = CheckoutAcceptance::query() - ->where([ - 'assigned_to_id' => $user->id, - 'qty' => 2, - ]) - ->whereNull(['accepted_at', 'declined_at']) - ->whereHasMorph( - 'checkoutable', - [Accessory::class], - ) - ->sole(); - - $legacyCheckoutAcceptance->qty = null; - $legacyCheckoutAcceptance->save(); + $accessory = Accessory::factory()->requiringAcceptance()->create(['qty' => 5]); // check out the accessory to a user with qty of 2 using the new behavior: `checkout_acceptances.qty` is 2 - $this->post(route('accessories.checkout.store', $accessoryB), [ + $this->post(route('accessories.checkout.store', $accessory), [ 'assigned_user' => $user->id, - 'checkout_qty' => 2, + 'checkout_qty' => 3, ]); - $this->assertEquals(2, AccessoryCheckout::where([ - 'accessory_id' => $accessoryB->id, + $this->assertEquals(3, AccessoryCheckout::where([ + 'accessory_id' => $accessory->id, 'assigned_to' => $user->id, 'assigned_type' => User::class, ])->count()); @@ -157,30 +127,26 @@ class AccessoryAcceptanceTest extends TestCase $checkoutAcceptance = CheckoutAcceptance::query() ->where([ 'assigned_to_id' => $user->id, - 'qty' => 2, + 'qty' => 3, ]) - ->whereNull(['accepted_at', 'declined_at']) + ->whereNull('accepted_at') + ->whereNull('declined_at') ->whereHasMorph( 'checkoutable', [Accessory::class], ) ->sole(); - // decline the "legacy" checkout - $this->actingAs($user); - - $this->post(route('account.store-acceptance', $legacyCheckoutAcceptance), [ - 'asset_acceptance' => 'declined', - ]); - // decline the checkout - $this->post(route('account.store-acceptance', $checkoutAcceptance), [ - 'asset_acceptance' => 'declined', - ]); + $this->actingAs($user) + ->post(route('account.store-acceptance', $checkoutAcceptance), [ + 'asset_acceptance' => 'declined', + ]); // four rows from `accessories_checkout` should be removed - $this->assertEquals($originalAccessoryCheckoutCount - 4, AccessoryCheckout::count()); + $this->assertEquals($originalAccessoryCheckoutCount - 3, AccessoryCheckout::count()); + // @todo: // ensure existing checkouts for the user are not affected. // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. // ie...if a user accepted previous accessories then those should not be touched. From 39d5ffeceb114aa9dc22e1d6db1f4e9b318c54e5 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 12:05:58 -0700 Subject: [PATCH 13/30] Implement fix --- app/Http/Controllers/Account/AcceptanceController.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Account/AcceptanceController.php b/app/Http/Controllers/Account/AcceptanceController.php index c273d36e3a..02b5ef99f8 100644 --- a/app/Http/Controllers/Account/AcceptanceController.php +++ b/app/Http/Controllers/Account/AcceptanceController.php @@ -345,7 +345,10 @@ class AcceptanceController extends Controller Storage::put('private_uploads/eula-pdfs/' .$pdf_filename, $pdf->output()); } - $acceptance->decline($sig_filename, $request->input('note')); + for ($i = 0; $i < $acceptance->qty; $i++) { + $acceptance->decline($sig_filename, $request->input('note')); + } + $acceptance->notify(new AcceptanceAssetDeclinedNotification($data)); Log::debug('New event acceptance.'); event(new CheckoutDeclined($acceptance)); From f80f1acaa79a14992ee925e17e43a4990df46b0f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 12:15:58 -0700 Subject: [PATCH 14/30] Improve variable name --- .../CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index cacfd55515..5baac2dab7 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -103,7 +103,7 @@ class AccessoryAcceptanceTest extends TestCase */ public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance() { - $user = User::factory()->create(); + $assignee = User::factory()->create(); $this->actingAs(User::factory()->checkoutAccessories()->create()); @@ -112,13 +112,13 @@ class AccessoryAcceptanceTest extends TestCase // check out the accessory to a user with qty of 2 using the new behavior: `checkout_acceptances.qty` is 2 $this->post(route('accessories.checkout.store', $accessory), [ - 'assigned_user' => $user->id, + 'assigned_user' => $assignee->id, 'checkout_qty' => 3, ]); $this->assertEquals(3, AccessoryCheckout::where([ 'accessory_id' => $accessory->id, - 'assigned_to' => $user->id, + 'assigned_to' => $assignee->id, 'assigned_type' => User::class, ])->count()); @@ -126,7 +126,7 @@ class AccessoryAcceptanceTest extends TestCase $checkoutAcceptance = CheckoutAcceptance::query() ->where([ - 'assigned_to_id' => $user->id, + 'assigned_to_id' => $assignee->id, 'qty' => 3, ]) ->whereNull('accepted_at') @@ -138,7 +138,7 @@ class AccessoryAcceptanceTest extends TestCase ->sole(); // decline the checkout - $this->actingAs($user) + $this->actingAs($assignee) ->post(route('account.store-acceptance', $checkoutAcceptance), [ 'asset_acceptance' => 'declined', ]); From 27f02014ca5230e9b1141b142cc3660567c95778 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 12:26:55 -0700 Subject: [PATCH 15/30] Add failing test --- .../Account/AcceptanceController.php | 2 +- .../Ui/AccessoryAcceptanceTest.php | 51 ++++++++++++++++--- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/app/Http/Controllers/Account/AcceptanceController.php b/app/Http/Controllers/Account/AcceptanceController.php index 02b5ef99f8..129351c555 100644 --- a/app/Http/Controllers/Account/AcceptanceController.php +++ b/app/Http/Controllers/Account/AcceptanceController.php @@ -345,7 +345,7 @@ class AcceptanceController extends Controller Storage::put('private_uploads/eula-pdfs/' .$pdf_filename, $pdf->output()); } - for ($i = 0; $i < $acceptance->qty; $i++) { + for ($i = 0; $i < ($acceptance->qty ?? 1); $i++) { $acceptance->decline($sig_filename, $request->input('note')); } diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index 5baac2dab7..da2f7fe346 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -110,18 +110,12 @@ class AccessoryAcceptanceTest extends TestCase // create accessory that requires acceptance $accessory = Accessory::factory()->requiringAcceptance()->create(['qty' => 5]); - // check out the accessory to a user with qty of 2 using the new behavior: `checkout_acceptances.qty` is 2 + // check out the accessory to a user with qty of 3 using the new behavior: `checkout_acceptances.qty` is 3 $this->post(route('accessories.checkout.store', $accessory), [ 'assigned_user' => $assignee->id, 'checkout_qty' => 3, ]); - $this->assertEquals(3, AccessoryCheckout::where([ - 'accessory_id' => $accessory->id, - 'assigned_to' => $assignee->id, - 'assigned_type' => User::class, - ])->count()); - $originalAccessoryCheckoutCount = AccessoryCheckout::count(); $checkoutAcceptance = CheckoutAcceptance::query() @@ -143,7 +137,6 @@ class AccessoryAcceptanceTest extends TestCase 'asset_acceptance' => 'declined', ]); - // four rows from `accessories_checkout` should be removed $this->assertEquals($originalAccessoryCheckoutCount - 3, AccessoryCheckout::count()); // @todo: @@ -151,4 +144,46 @@ class AccessoryAcceptanceTest extends TestCase // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. // ie...if a user accepted previous accessories then those should not be touched. } + + public function test_all_previous_accessory_checkouts_are_removed_when_user_declines_acceptance() + { + $assignee = User::factory()->create(); + + $this->actingAs(User::factory()->checkoutAccessories()->create()); + + // create accessory that requires acceptance + $accessory = Accessory::factory()->requiringAcceptance()->create(['qty' => 5]); + + $this->post(route('accessories.checkout.store', $accessory), [ + 'assigned_user' => $assignee->id, + 'checkout_qty' => 3, + ]); + + $originalAccessoryCheckoutCount = AccessoryCheckout::count(); + + $legacyCheckoutAcceptance = CheckoutAcceptance::query() + ->where([ + 'assigned_to_id' => $assignee->id, + 'qty' => 3, + ]) + ->whereNull('accepted_at') + ->whereNull('declined_at') + ->whereHasMorph( + 'checkoutable', + [Accessory::class], + ) + ->sole(); + + // previous behavior did not set `qty`. + $legacyCheckoutAcceptance->qty = null; + $legacyCheckoutAcceptance->save(); + + // decline the checkout + $this->actingAs($assignee) + ->post(route('account.store-acceptance', $legacyCheckoutAcceptance), [ + 'asset_acceptance' => 'declined', + ]); + + $this->assertEquals($originalAccessoryCheckoutCount - 3, AccessoryCheckout::count()); + } } From 726116574df45bac41530572eb37bdaf58dfc8f8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 12:40:03 -0700 Subject: [PATCH 16/30] Improve readability? --- .../Ui/AccessoryAcceptanceTest.php | 121 +++++++++--------- 1 file changed, 60 insertions(+), 61 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index da2f7fe346..3856185dd9 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -10,6 +10,7 @@ use App\Models\User; use App\Notifications\AcceptanceAssetAcceptedNotification; use App\Notifications\AcceptanceAssetDeclinedNotification; use Illuminate\Support\Facades\Notification; +use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; class AccessoryAcceptanceTest extends TestCase @@ -98,11 +99,66 @@ class AccessoryAcceptanceTest extends TestCase $this->assertNull($acceptance->fresh()->accepted_at); } + public static function data() + { + yield 'Current behavior' => [ + function () { + return function (User $assignee) { + return CheckoutAcceptance::query() + ->where([ + 'assigned_to_id' => $assignee->id, + 'qty' => 3, + ]) + ->whereNull('accepted_at') + ->whereNull('declined_at') + ->whereHasMorph( + 'checkoutable', + [Accessory::class], + ) + ->sole(); + }; + } + ]; + + yield 'Previous behavior' => [ + function () { + return function (User $assignee) { + $checkoutAcceptance = CheckoutAcceptance::query() + ->where([ + 'assigned_to_id' => $assignee->id, + 'qty' => 3, + ]) + ->whereNull('accepted_at') + ->whereNull('declined_at') + ->whereHasMorph( + 'checkoutable', + [Accessory::class], + ) + ->sole(); + + // previous behavior did not set `qty`. + $checkoutAcceptance->qty = null; + $checkoutAcceptance->save(); + + return $checkoutAcceptance; + }; + } + ]; + + // @todo: + // ensure existing checkouts for the user are not affected. + // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. + // ie...if a user accepted previous accessories then those should not be touched. + } + /** * @link https://github.com/grokability/snipe-it/issues/17589 */ - public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance() + #[DataProvider('data')] + public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance($provided) { + $getCheckoutAcceptance = $provided(); + $assignee = User::factory()->create(); $this->actingAs(User::factory()->checkoutAccessories()->create()); @@ -110,7 +166,7 @@ class AccessoryAcceptanceTest extends TestCase // create accessory that requires acceptance $accessory = Accessory::factory()->requiringAcceptance()->create(['qty' => 5]); - // check out the accessory to a user with qty of 3 using the new behavior: `checkout_acceptances.qty` is 3 + // checkout 3 accessories to the user $this->post(route('accessories.checkout.store', $accessory), [ 'assigned_user' => $assignee->id, 'checkout_qty' => 3, @@ -118,18 +174,8 @@ class AccessoryAcceptanceTest extends TestCase $originalAccessoryCheckoutCount = AccessoryCheckout::count(); - $checkoutAcceptance = CheckoutAcceptance::query() - ->where([ - 'assigned_to_id' => $assignee->id, - 'qty' => 3, - ]) - ->whereNull('accepted_at') - ->whereNull('declined_at') - ->whereHasMorph( - 'checkoutable', - [Accessory::class], - ) - ->sole(); + // get the checkout acceptance via the function that will put it in a state ready for testing + $checkoutAcceptance = $getCheckoutAcceptance($assignee); // decline the checkout $this->actingAs($assignee) @@ -138,52 +184,5 @@ class AccessoryAcceptanceTest extends TestCase ]); $this->assertEquals($originalAccessoryCheckoutCount - 3, AccessoryCheckout::count()); - - // @todo: - // ensure existing checkouts for the user are not affected. - // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. - // ie...if a user accepted previous accessories then those should not be touched. - } - - public function test_all_previous_accessory_checkouts_are_removed_when_user_declines_acceptance() - { - $assignee = User::factory()->create(); - - $this->actingAs(User::factory()->checkoutAccessories()->create()); - - // create accessory that requires acceptance - $accessory = Accessory::factory()->requiringAcceptance()->create(['qty' => 5]); - - $this->post(route('accessories.checkout.store', $accessory), [ - 'assigned_user' => $assignee->id, - 'checkout_qty' => 3, - ]); - - $originalAccessoryCheckoutCount = AccessoryCheckout::count(); - - $legacyCheckoutAcceptance = CheckoutAcceptance::query() - ->where([ - 'assigned_to_id' => $assignee->id, - 'qty' => 3, - ]) - ->whereNull('accepted_at') - ->whereNull('declined_at') - ->whereHasMorph( - 'checkoutable', - [Accessory::class], - ) - ->sole(); - - // previous behavior did not set `qty`. - $legacyCheckoutAcceptance->qty = null; - $legacyCheckoutAcceptance->save(); - - // decline the checkout - $this->actingAs($assignee) - ->post(route('account.store-acceptance', $legacyCheckoutAcceptance), [ - 'asset_acceptance' => 'declined', - ]); - - $this->assertEquals($originalAccessoryCheckoutCount - 3, AccessoryCheckout::count()); } } From 4dcfd8b353b13a6b9ea26e549fa85b03afeb15d1 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 13:19:00 -0700 Subject: [PATCH 17/30] Improve method name --- .../CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index 3856185dd9..d3b3e7d99f 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -99,7 +99,7 @@ class AccessoryAcceptanceTest extends TestCase $this->assertNull($acceptance->fresh()->accepted_at); } - public static function data() + public static function provider() { yield 'Current behavior' => [ function () { @@ -154,7 +154,7 @@ class AccessoryAcceptanceTest extends TestCase /** * @link https://github.com/grokability/snipe-it/issues/17589 */ - #[DataProvider('data')] + #[DataProvider('provider')] public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance($provided) { $getCheckoutAcceptance = $provided(); From 18c2508d2f35ed50ad6e7723ea008d4bf5666e1b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 13:49:19 -0700 Subject: [PATCH 18/30] Scaffold command for cleaning accessory_checkout --- .../CleanDeclinedAccessoryCheckouts.php | 51 +++++++++++++++++++ app/Models/CheckoutAcceptance.php | 5 ++ 2 files changed, 56 insertions(+) create mode 100644 app/Console/Commands/CleanDeclinedAccessoryCheckouts.php diff --git a/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php b/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php new file mode 100644 index 0000000000..9825d615be --- /dev/null +++ b/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php @@ -0,0 +1,51 @@ +get(); + // + // dd($accessoryCheckouts); + + $declinedCheckoutAcceptances = CheckoutAcceptance::query() + ->where([ + 'checkoutable_type' => Accessory::class, + ]) + // if it was declined and the qty is null that means it potentially left + // some entries in the `accessories_checkout` table behind. + ->whereNull('qty') + ->declined() + ->get(); + + + + return 0; + } +} diff --git a/app/Models/CheckoutAcceptance.php b/app/Models/CheckoutAcceptance.php index 0599286f85..876135c3f6 100644 --- a/app/Models/CheckoutAcceptance.php +++ b/app/Models/CheckoutAcceptance.php @@ -136,4 +136,9 @@ class CheckoutAcceptance extends Model { return $query->whereNull('accepted_at')->whereNull('declined_at'); } + + public function scopeDeclined(Builder $query) + { + return $query->whereNull('accepted_at')->whereNotNull('declined_at'); + } } From c7bdad649a81442d4b8cdb150147fbd61c45b6af Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Aug 2025 16:06:11 -0700 Subject: [PATCH 19/30] Build out command --- .../CleanDeclinedAccessoryCheckouts.php | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php b/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php index 9825d615be..bf2e420595 100644 --- a/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php +++ b/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php @@ -30,9 +30,7 @@ class CleanDeclinedAccessoryCheckouts extends Command */ public function handle() { - // $accessoryCheckouts = AccessoryCheckout::where('assigned_type', User::class)->get(); - // - // dd($accessoryCheckouts); + $accessoryCheckouts = AccessoryCheckout::where('assigned_type', User::class)->get(); $declinedCheckoutAcceptances = CheckoutAcceptance::query() ->where([ @@ -44,7 +42,23 @@ class CleanDeclinedAccessoryCheckouts extends Command ->declined() ->get(); - + $declinedCheckoutAcceptances->map(function (CheckoutAcceptance $acceptance) use ($accessoryCheckouts) { + $matchedCheckouts = $accessoryCheckouts->filter(function ($checkout) use ($acceptance) { + return $checkout->accessory_id == $acceptance->checkoutable_id + && $checkout->assigned_to == $acceptance->assigned_to_id + && $checkout->created_at == $acceptance->created_at; + }); + + $acceptance->setRelation('accessoryCheckouts', $matchedCheckouts); + + return $acceptance; + }); + + $checkoutsToDelete = $declinedCheckoutAcceptances->pluck('accessoryCheckouts')->flatten(); + + if ($this->confirm("Delete {$checkoutsToDelete->count()} entries from the accessory_checkout table?")) { + $checkoutsToDelete->each(fn($checkout) => $checkout->delete()); + } return 0; } From 98e23ff92ea28d3603014b50f5dda211a457f7eb Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 21 Aug 2025 14:34:16 -0700 Subject: [PATCH 20/30] Remove legacy testing from test --- .../Ui/AccessoryAcceptanceTest.php | 72 ++++--------------- 1 file changed, 14 insertions(+), 58 deletions(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index d3b3e7d99f..6070ff165b 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -99,66 +99,11 @@ class AccessoryAcceptanceTest extends TestCase $this->assertNull($acceptance->fresh()->accepted_at); } - public static function provider() - { - yield 'Current behavior' => [ - function () { - return function (User $assignee) { - return CheckoutAcceptance::query() - ->where([ - 'assigned_to_id' => $assignee->id, - 'qty' => 3, - ]) - ->whereNull('accepted_at') - ->whereNull('declined_at') - ->whereHasMorph( - 'checkoutable', - [Accessory::class], - ) - ->sole(); - }; - } - ]; - - yield 'Previous behavior' => [ - function () { - return function (User $assignee) { - $checkoutAcceptance = CheckoutAcceptance::query() - ->where([ - 'assigned_to_id' => $assignee->id, - 'qty' => 3, - ]) - ->whereNull('accepted_at') - ->whereNull('declined_at') - ->whereHasMorph( - 'checkoutable', - [Accessory::class], - ) - ->sole(); - - // previous behavior did not set `qty`. - $checkoutAcceptance->qty = null; - $checkoutAcceptance->save(); - - return $checkoutAcceptance; - }; - } - ]; - - // @todo: - // ensure existing checkouts for the user are not affected. - // in other words, make sure the removal of rows from `accessories_checkout` is not too eager, especially around legacy behavior. - // ie...if a user accepted previous accessories then those should not be touched. - } - /** * @link https://github.com/grokability/snipe-it/issues/17589 */ - #[DataProvider('provider')] - public function test_all_accessory_checkouts_are_removed_when_user_declines_acceptance($provided) + public function test_all_accessory_checkout_entries_are_removed_when_user_declines_acceptance() { - $getCheckoutAcceptance = $provided(); - $assignee = User::factory()->create(); $this->actingAs(User::factory()->checkoutAccessories()->create()); @@ -174,8 +119,19 @@ class AccessoryAcceptanceTest extends TestCase $originalAccessoryCheckoutCount = AccessoryCheckout::count(); - // get the checkout acceptance via the function that will put it in a state ready for testing - $checkoutAcceptance = $getCheckoutAcceptance($assignee); + // find the acceptance to be declined + $checkoutAcceptance = CheckoutAcceptance::query() + ->where([ + 'assigned_to_id' => $assignee->id, + 'qty' => 3, + ]) + ->whereNull('accepted_at') + ->whereNull('declined_at') + ->whereHasMorph( + 'checkoutable', + [Accessory::class], + ) + ->sole(); // decline the checkout $this->actingAs($assignee) From 0ede4da8163772f41e477a036a152b0b6d11672a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 21 Aug 2025 14:34:31 -0700 Subject: [PATCH 21/30] Remove CleanDeclinedAccessoryCheckouts command --- .../CleanDeclinedAccessoryCheckouts.php | 65 ------------------- 1 file changed, 65 deletions(-) delete mode 100644 app/Console/Commands/CleanDeclinedAccessoryCheckouts.php diff --git a/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php b/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php deleted file mode 100644 index bf2e420595..0000000000 --- a/app/Console/Commands/CleanDeclinedAccessoryCheckouts.php +++ /dev/null @@ -1,65 +0,0 @@ -get(); - - $declinedCheckoutAcceptances = CheckoutAcceptance::query() - ->where([ - 'checkoutable_type' => Accessory::class, - ]) - // if it was declined and the qty is null that means it potentially left - // some entries in the `accessories_checkout` table behind. - ->whereNull('qty') - ->declined() - ->get(); - - $declinedCheckoutAcceptances->map(function (CheckoutAcceptance $acceptance) use ($accessoryCheckouts) { - $matchedCheckouts = $accessoryCheckouts->filter(function ($checkout) use ($acceptance) { - return $checkout->accessory_id == $acceptance->checkoutable_id - && $checkout->assigned_to == $acceptance->assigned_to_id - && $checkout->created_at == $acceptance->created_at; - }); - - $acceptance->setRelation('accessoryCheckouts', $matchedCheckouts); - - return $acceptance; - }); - - $checkoutsToDelete = $declinedCheckoutAcceptances->pluck('accessoryCheckouts')->flatten(); - - if ($this->confirm("Delete {$checkoutsToDelete->count()} entries from the accessory_checkout table?")) { - $checkoutsToDelete->each(fn($checkout) => $checkout->delete()); - } - - return 0; - } -} From c22575812d9f420e966eef63936abcdbd0bcc909 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 21 Aug 2025 14:53:00 -0700 Subject: [PATCH 22/30] Add qty to AcceptanceAssetAcceptedToUserNotification --- app/Http/Controllers/Account/AcceptanceController.php | 1 + .../AcceptanceAssetAcceptedToUserNotification.php | 2 ++ .../views/notifications/markdown/asset-acceptance.blade.php | 3 +++ 3 files changed, 6 insertions(+) diff --git a/app/Http/Controllers/Account/AcceptanceController.php b/app/Http/Controllers/Account/AcceptanceController.php index 129351c555..e63afe832a 100644 --- a/app/Http/Controllers/Account/AcceptanceController.php +++ b/app/Http/Controllers/Account/AcceptanceController.php @@ -232,6 +232,7 @@ class AcceptanceController extends Controller 'signature' => ($sig_filename) ? storage_path() . '/private_uploads/signatures/' . $sig_filename : null, 'logo' => $path_logo, 'date_settings' => $branding_settings->date_display_format, + 'qty' => $acceptance->qty ?? 1, ]; if ($pdf_view_route!='') { diff --git a/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php b/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php index 22a97b709f..e39e6e40db 100644 --- a/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php +++ b/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php @@ -29,6 +29,7 @@ class AcceptanceAssetAcceptedToUserNotification extends Notification $this->company_name = $params['company_name']; $this->settings = Setting::getSettings(); $this->file = $params['file'] ?? null; + $this->qty = $params['qty'] ?? null; } @@ -66,6 +67,7 @@ class AcceptanceAssetAcceptedToUserNotification extends Notification 'accepted_date' => $this->accepted_date, 'assigned_to' => $this->assigned_to, 'company_name' => $this->company_name, + 'qty' => $this->qty, 'intro_text' => trans('mail.acceptance_asset_accepted_to_user', ['site_name' => $this->company_name ?? $this->settings->site_name]), ]) ->attach($pdf_path) diff --git a/resources/views/notifications/markdown/asset-acceptance.blade.php b/resources/views/notifications/markdown/asset-acceptance.blade.php index 1a9569609e..51d53a4405 100644 --- a/resources/views/notifications/markdown/asset-acceptance.blade.php +++ b/resources/views/notifications/markdown/asset-acceptance.blade.php @@ -37,6 +37,9 @@ @if (isset($item_serial)) | **{{ trans('mail.serial') }}** | {{ $item_serial }} | @endif +@if (isset($qty)) + | **{{ trans('general.qty') }}** | {{ $qty }} | +@endif @endcomponent {{ trans('mail.best_regards') }} From 0caaba156dac2e94d7ad9877994505756e7536dd Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 21 Aug 2025 14:54:43 -0700 Subject: [PATCH 23/30] Add qty to AcceptanceAssetAcceptedNotification --- app/Notifications/AcceptanceAssetAcceptedNotification.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Notifications/AcceptanceAssetAcceptedNotification.php b/app/Notifications/AcceptanceAssetAcceptedNotification.php index 7798dbc0d5..0e1a226dfc 100644 --- a/app/Notifications/AcceptanceAssetAcceptedNotification.php +++ b/app/Notifications/AcceptanceAssetAcceptedNotification.php @@ -30,6 +30,7 @@ class AcceptanceAssetAcceptedNotification extends Notification $this->note = $params['note']; $this->company_name = $params['company_name']; $this->settings = Setting::getSettings(); + $this->qty = $params['qty'] ?? null; } @@ -71,6 +72,7 @@ class AcceptanceAssetAcceptedNotification extends Notification 'accepted_date' => $this->accepted_date, 'assigned_to' => $this->assigned_to, 'company_name' => $this->company_name, + 'qty' => $this->qty, 'intro_text' => trans('mail.acceptance_asset_accepted'), ]) ->subject(trans('mail.acceptance_asset_accepted')); From 7c58bfa282a977c773f7837a789bf6358642f6f4 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 21 Aug 2025 14:56:24 -0700 Subject: [PATCH 24/30] Add qty to AcceptanceAssetDeclinedNotification --- app/Http/Controllers/Account/AcceptanceController.php | 1 + app/Notifications/AcceptanceAssetAcceptedToUserNotification.php | 1 - app/Notifications/AcceptanceAssetDeclinedNotification.php | 2 ++ 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Account/AcceptanceController.php b/app/Http/Controllers/Account/AcceptanceController.php index e63afe832a..8e389f951f 100644 --- a/app/Http/Controllers/Account/AcceptanceController.php +++ b/app/Http/Controllers/Account/AcceptanceController.php @@ -338,6 +338,7 @@ class AcceptanceController extends Controller 'assigned_to' => $assigned_to, 'company_name' => $branding_settings->site_name, 'date_settings' => $branding_settings->date_display_format, + 'qty' => $acceptance->qty ?? 1, ]; if ($pdf_view_route!='') { diff --git a/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php b/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php index e39e6e40db..57e7b3fad4 100644 --- a/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php +++ b/app/Notifications/AcceptanceAssetAcceptedToUserNotification.php @@ -30,7 +30,6 @@ class AcceptanceAssetAcceptedToUserNotification extends Notification $this->settings = Setting::getSettings(); $this->file = $params['file'] ?? null; $this->qty = $params['qty'] ?? null; - } /** diff --git a/app/Notifications/AcceptanceAssetDeclinedNotification.php b/app/Notifications/AcceptanceAssetDeclinedNotification.php index 0a9d6c211f..6ff06c7094 100644 --- a/app/Notifications/AcceptanceAssetDeclinedNotification.php +++ b/app/Notifications/AcceptanceAssetDeclinedNotification.php @@ -30,6 +30,7 @@ class AcceptanceAssetDeclinedNotification extends Notification $this->assigned_to = $params['assigned_to']; $this->company_name = $params['company_name']; $this->settings = Setting::getSettings(); + $this->qty = $params['qty'] ?? null; } /** @@ -69,6 +70,7 @@ class AcceptanceAssetDeclinedNotification extends Notification 'declined_date' => $this->declined_date, 'assigned_to' => $this->assigned_to, 'company_name' => $this->company_name, + 'qty' => $this->qty, 'intro_text' => trans('mail.acceptance_asset_declined'), ]) ->subject(trans('mail.acceptance_asset_declined')); From c076b37c9b06c3988b677887d55f6e3abddd5216 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 21 Aug 2025 15:05:24 -0700 Subject: [PATCH 25/30] Add qty to accessory eula pdf --- resources/views/account/accept/accept-accessory-eula.blade.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/resources/views/account/accept/accept-accessory-eula.blade.php b/resources/views/account/accept/accept-accessory-eula.blade.php index 6109196c6e..da8410ccd1 100644 --- a/resources/views/account/accept/accept-accessory-eula.blade.php +++ b/resources/views/account/accept/accept-accessory-eula.blade.php @@ -24,6 +24,7 @@

{{ trans('general.date') }}: {{ date($date_settings) }}
{{ trans('general.asset_model') }}: {{ $item_model }}
+ {{ trans('general.quantity') }}: {{ $qty }}

@if ($eula) @@ -44,4 +45,4 @@ @endif - \ No newline at end of file + From 918426a2fa70fd9940438f99a1364e3656965580 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 21 Aug 2025 15:49:36 -0700 Subject: [PATCH 26/30] Add qty to accept assets table --- resources/views/account/accept/index.blade.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/resources/views/account/accept/index.blade.php b/resources/views/account/accept/index.blade.php index 0d36ef18ca..0ccaa7c0cf 100755 --- a/resources/views/account/accept/index.blade.php +++ b/resources/views/account/accept/index.blade.php @@ -31,6 +31,7 @@ {{ trans('general.name')}} + {{ trans('general.qty') }} {{ trans('general.serial_number')}} {{ trans('table.actions')}} @@ -40,7 +41,8 @@ @if ($acceptance->checkoutable) {{ ($acceptance->checkoutable) ? $acceptance->checkoutable->present()->name : '' }} - {{ ($acceptance->checkoutable) ? $acceptance->checkoutable->serial : '' }} + {{ $acceptance->qty }} + {{ ($acceptance->checkoutable) ? $acceptance->checkoutable->serial : '' }} {{ trans('general.accept_decline') }} @else ----- From 3160d1064dd85840d6f55a82430c7fe2bb3a96b8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 26 Aug 2025 11:32:38 -0700 Subject: [PATCH 27/30] Remove unused import --- tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php index 6070ff165b..9e20b52475 100644 --- a/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php +++ b/tests/Feature/CheckoutAcceptances/Ui/AccessoryAcceptanceTest.php @@ -10,7 +10,6 @@ use App\Models\User; use App\Notifications\AcceptanceAssetAcceptedNotification; use App\Notifications\AcceptanceAssetDeclinedNotification; use Illuminate\Support\Facades\Notification; -use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; class AccessoryAcceptanceTest extends TestCase From 823c67400d16c7a913473b2e108e93409d8f390b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 26 Aug 2025 11:34:10 -0700 Subject: [PATCH 28/30] Fix indent --- .../views/notifications/markdown/asset-acceptance.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/notifications/markdown/asset-acceptance.blade.php b/resources/views/notifications/markdown/asset-acceptance.blade.php index 51d53a4405..776ba1c355 100644 --- a/resources/views/notifications/markdown/asset-acceptance.blade.php +++ b/resources/views/notifications/markdown/asset-acceptance.blade.php @@ -38,7 +38,7 @@ | **{{ trans('mail.serial') }}** | {{ $item_serial }} | @endif @if (isset($qty)) - | **{{ trans('general.qty') }}** | {{ $qty }} | +| **{{ trans('general.qty') }}** | {{ $qty }} | @endif @endcomponent From e164595a0fbf18def0b94ada8ac9c6b8450c096b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 3 Sep 2025 12:35:18 -0700 Subject: [PATCH 29/30] Fall back to displaying 1 --- resources/views/account/accept/index.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/account/accept/index.blade.php b/resources/views/account/accept/index.blade.php index 8f216b3ad3..1957bec675 100755 --- a/resources/views/account/accept/index.blade.php +++ b/resources/views/account/accept/index.blade.php @@ -43,7 +43,7 @@ @if ($acceptance->checkoutable) {{ ($acceptance->checkoutable) ? $acceptance->checkoutable->present()->name : '' }} {{ $acceptance->checkoutable_item_type }} - {{ $acceptance->qty }} + {{ $acceptance->qty ?? '1' }} {{ ($acceptance->checkoutable) ? $acceptance->checkoutable->serial : '' }} {{ trans('general.accept_decline') }} @else From 039564e74caf98e5fc4a3ebf12088e7ed5be7963 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 3 Sep 2025 12:52:46 -0700 Subject: [PATCH 30/30] Wrap migration in check --- ..._225214_add_qty_to_checkout_acceptances_table.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php b/database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php index 2bb32b0e90..c5c7726213 100644 --- a/database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php +++ b/database/migrations/2025_08_12_225214_add_qty_to_checkout_acceptances_table.php @@ -10,8 +10,10 @@ return new class extends Migration { */ public function up(): void { - Schema::table('checkout_acceptances', function (Blueprint $table) { - $table->unsignedInteger('qty')->nullable()->after('assigned_to_id')->default(null); + Schema::whenTableDoesntHaveColumn('checkout_acceptances', 'qty', function () { + Schema::table('checkout_acceptances', function (Blueprint $table) { + $table->unsignedInteger('qty')->nullable()->after('assigned_to_id')->default(null); + }); }); } @@ -20,8 +22,10 @@ return new class extends Migration { */ public function down(): void { - Schema::table('checkout_acceptances', function (Blueprint $table) { - $table->dropColumn('qty'); + Schema::whenTableHasColumn('checkout_acceptances', 'qty', function () { + Schema::table('checkout_acceptances', function (Blueprint $table) { + $table->dropColumn('qty'); + }); }); } };