From 358609720a071160586af1a20d9195f47713d39c Mon Sep 17 00:00:00 2001 From: Daniel Meltzer Date: Wed, 29 Apr 2020 12:50:09 -0400 Subject: [PATCH 1/6] Component checkout/checkin fixes. - Provide proper translated messages for checkin vs checkout - Pass appropriate methods to the Checkout event, fixes an error on checkin. - Default to a value of 1 on checkin in UI to save a click sometimes. --- .../Controllers/Components/ComponentCheckinController.php | 7 ++++--- resources/views/components/checkout.blade.php | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Components/ComponentCheckinController.php b/app/Http/Controllers/Components/ComponentCheckinController.php index b42cdca9eb..eee0f15827 100644 --- a/app/Http/Controllers/Components/ComponentCheckinController.php +++ b/app/Http/Controllers/Components/ComponentCheckinController.php @@ -8,6 +8,7 @@ use App\Http\Controllers\Controller; use App\Models\Asset; use App\Models\Component; use Illuminate\Http\Request; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Validator; @@ -96,12 +97,12 @@ class ComponentCheckinController extends Controller $asset = Asset::find($component_assets->asset_id); - event(new CheckoutableCheckedIn($component, $asset, Auth::user(), $request->input('note'))); + event(new CheckoutableCheckedIn($component, $asset, Auth::user(), $request->input('note'), Carbon::now())); return redirect()->route('components.index')->with('success', - trans('admin/components/message.checkout.success')); + trans('admin/components/message.checkin.success')); } - return redirect()->route('components.index')->with('error', trans('admin/components/message.not_found')); + return redirect()->route('components.index')->with('error', trans('admin/components/message.does_not_exist')); } } diff --git a/resources/views/components/checkout.blade.php b/resources/views/components/checkout.blade.php index d1338f4f8c..2bebc06d6a 100644 --- a/resources/views/components/checkout.blade.php +++ b/resources/views/components/checkout.blade.php @@ -42,7 +42,7 @@
- + {!! $errors->first('assigned_qty', '
') !!}
From 68224757f4b60576cb6224553f1514b544a30b97 Mon Sep 17 00:00:00 2001 From: Daniel Meltzer Date: Wed, 29 Apr 2020 13:25:04 -0400 Subject: [PATCH 2/6] Validate when editing the quantity of a component that the new quantity is > the amount checked out --- .../Components/ComponentsController.php | 11 +++++++ app/Models/Component.php | 30 ++++++++++++------- resources/lang/en/validation.php | 3 ++ 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/app/Http/Controllers/Components/ComponentsController.php b/app/Http/Controllers/Components/ComponentsController.php index 2118b37b42..9fde48df41 100644 --- a/app/Http/Controllers/Components/ComponentsController.php +++ b/app/Http/Controllers/Components/ComponentsController.php @@ -8,6 +8,7 @@ use App\Models\Component; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Input; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Facades\Validator; /** * This class controls all actions related to Components for @@ -121,6 +122,16 @@ class ComponentsController extends Controller if (is_null($component = Component::find($componentId))) { return redirect()->route('components.index')->with('error', trans('admin/components/message.does_not_exist')); } + $min = $component->numCHeckedOut(); + $validator = Validator::make($request->all(), [ + "qty" => "required|numeric|gt:$min" + ]); + + if ($validator->fails()) { + return redirect()->back() + ->withErrors($validator) + ->withInput(); + } $this->authorize('update', $component); diff --git a/app/Models/Component.php b/app/Models/Component.php index e880234ba7..f24f3250e7 100644 --- a/app/Models/Component.php +++ b/app/Models/Component.php @@ -155,6 +155,23 @@ class Component extends SnipeModel return $this->hasMany('\App\Models\Actionlog', 'item_id')->where('item_type', Component::class)->orderBy('created_at', 'desc')->withTrashed(); } + /** + * Check how many items within a component are checked out + * + * @author [A. Gianotto] [] + * @since [v5.0] + * @return int + */ + public function numCheckedOut() + { + $checkedout = 0; + foreach ($this->assets as $checkout) { + $checkedout += $checkout->pivot->assigned_qty; + } + + return $checkedout; + } + /** * Check how many items within a component are remaining * @@ -164,17 +181,8 @@ class Component extends SnipeModel */ public function numRemaining() { - $checkedout = 0; - - foreach ($this->assets as $checkout) { - $checkedout += $checkout->pivot->assigned_qty; - } - - - $total = $this->qty; - $remaining = $total - $checkedout; - return $remaining; - } + return $this->qty - $this->numCheckedOut(); + } /** * Query builder scope to order on company diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index 47f4377ad0..e399504d08 100644 --- a/resources/lang/en/validation.php +++ b/resources/lang/en/validation.php @@ -44,6 +44,9 @@ return array( 'exists' => 'The selected :attribute is invalid.', 'file' => 'The :attribute must be a file.', 'filled' => 'The :attribute field must have a value.', + 'gt' => [ + 'numeric' => 'The :attribute field must be greater than :value.' + ], 'hashed_pass' => 'Your password is incorrect.', 'image' => 'The :attribute must be an image.', 'in' => 'The selected :attribute is invalid.', From b28a4f310884ca3790a0c2c1d2bc28c12a2e8cb8 Mon Sep 17 00:00:00 2001 From: Daniel Meltzer Date: Thu, 30 Apr 2020 09:50:12 -0400 Subject: [PATCH 3/6] Add component unit test and unify some validation. --- app/Models/Component.php | 13 ++--- tests/unit/ComponentTest.php | 100 +++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 tests/unit/ComponentTest.php diff --git a/app/Models/Component.php b/app/Models/Component.php index f24f3250e7..3e027cc4ee 100644 --- a/app/Models/Component.php +++ b/app/Models/Component.php @@ -20,17 +20,18 @@ class Component extends SnipeModel protected $dates = ['deleted_at', 'purchase_date']; protected $table = 'components'; - + /** * Category validation rules */ public $rules = array( - 'name' => 'required|min:3|max:255', - 'qty' => 'required|integer|min:1', - 'category_id' => 'required|integer', - 'company_id' => 'integer|nullable', + 'name' => 'required|min:3|max:255', + 'qty' => 'required|integer|min:1', + 'category_id' => 'required|integer|exists:categories,id', + 'company_id' => 'integer|nullable', + 'min_amt' => 'integer|min:0|nullable', 'purchase_date' => 'date|nullable', - 'purchase_cost' => 'numeric|nullable', + 'purchase_cost' => 'numeric|nullable', ); /** diff --git a/tests/unit/ComponentTest.php b/tests/unit/ComponentTest.php new file mode 100644 index 0000000000..1e167955a6 --- /dev/null +++ b/tests/unit/ComponentTest.php @@ -0,0 +1,100 @@ +assertFalse($a->isValid()); + $fields = [ + 'name' => 'name', + 'qty' => 'qty', + 'category_id' => 'category id' + ]; + $errors = $a->getErrors(); + foreach ($fields as $field => $fieldTitle) { + $this->assertEquals($errors->get($field)[0], "The ${fieldTitle} field is required."); + } + } + + public function testFailsMinValidation() + { + // An Component name has a min length of 3 + // An Component has a min qty of 1 + // An Component has a min amount of 0 + $a = factory(Component::class)->make([ + 'name' => 'a', + 'qty' => 0, + 'min_amt' => -1 + ]); + $fields = [ + 'name' => 'name', + 'qty' => 'qty', + 'min_amt' => 'min amt' + ]; + $this->assertFalse($a->isValid()); + $errors = $a->getErrors(); + foreach ($fields as $field => $fieldTitle) { + $this->assertStringContainsString("The ${fieldTitle} must be at least", $errors->get($field)[0]); + } + } + + public function testCategoryIdMustExist() + { + $category = $this->createValidCategory('component-hdd-category', + ['category_type' => 'component']); + $component = factory(Component::class) + ->states('ssd-crucial240') + ->make(['category_id' => $category->id]); + $this->createValidManufacturer('apple'); + + $component->save(); + $this->assertTrue($component->isValid()); + $newId = $category->id + 1; + $component = factory(Component::class)->states('ssd-crucial240')->make(['category_id' => $newId]); + $component->save(); + + $this->assertFalse($component->isValid()); + $this->assertStringContainsString("The selected category id is invalid.", $component->getErrors()->get('category_id')[0]); + } + + public function testAnComponentBelongsToACompany() + { + $component = factory(Component::class) + ->create(['company_id' => factory(Company::class)->create()->id]); + $this->assertInstanceOf(Company::class, $component->company); + } + + public function testAnComponentHasALocation() + { + $component = factory(Component::class) + ->create(['location_id' => factory(Location::class)->create()->id]); + $this->assertInstanceOf(Location::class, $component->location); + } + + public function testAnComponentBelongsToACategory() + { + $component = factory(Component::class)->states('ssd-crucial240') + ->create([ + 'category_id' => factory(Category::class) + ->states('component-hdd-category') + ->create(['category_type' => 'component'])->id + ]); + $this->assertInstanceOf(Category::class, $component->category); + $this->assertEquals('component', $component->category->category_type); + } +} From 441049d8accda4293d222ecbf7b868985abc3c95 Mon Sep 17 00:00:00 2001 From: Daniel Meltzer Date: Thu, 30 Apr 2020 10:34:30 -0400 Subject: [PATCH 4/6] Sort license seats by their number only, making the sort order more logical --- app/Presenters/LicensePresenter.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Presenters/LicensePresenter.php b/app/Presenters/LicensePresenter.php index 1b3272c6ad..32053dd79c 100644 --- a/app/Presenters/LicensePresenter.php +++ b/app/Presenters/LicensePresenter.php @@ -161,6 +161,7 @@ class LicensePresenter extends Presenter "field" => "name", "searchable" => false, "sortable" => false, + "sorter" => "numericOnly", "switchable" => true, "title" => trans('admin/licenses/general.seat'), "visible" => true, From debdccf5f16a708894da2a82eab6178e7f43b7d0 Mon Sep 17 00:00:00 2001 From: Daniel Meltzer Date: Thu, 30 Apr 2020 10:35:11 -0400 Subject: [PATCH 5/6] Remove old help text translation that doesn't exist any more --- resources/views/accessories/view.blade.php | 6 ------ resources/views/consumables/view.blade.php | 11 ++++++----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/resources/views/accessories/view.blade.php b/resources/views/accessories/view.blade.php index 66e8466742..f8ec45c31f 100644 --- a/resources/views/accessories/view.blade.php +++ b/resources/views/accessories/view.blade.php @@ -101,12 +101,6 @@ numRemaining() > 0 ) ? '' : ' disabled') }}>{{ trans('general.checkout') }} @endcan - - -

{{ trans('admin/accessories/general.about_accessories_title') }}

-

{{ trans('admin/accessories/general.about_accessories_text') }}

- - @stop diff --git a/resources/views/consumables/view.blade.php b/resources/views/consumables/view.blade.php index 62d82b2ff9..a6271a2f87 100644 --- a/resources/views/consumables/view.blade.php +++ b/resources/views/consumables/view.blade.php @@ -67,7 +67,6 @@
- @if ($consumable->image!='')
{{ $consumable->name }} @@ -116,10 +115,12 @@ {{ $consumable->order_number }}
@endif -
-

{{ trans('admin/consumables/general.about_consumables_title') }}

-

{{ trans('admin/consumables/general.about_consumables_text') }}

-
+ + @can('checkout', \App\Models\Accessory::class) +
+ numRemaining() > 0 ) ? '' : ' disabled') }}>{{ trans('general.checkout') }} +
+ @endcan
From bfba30058c59382987aaa657b6d55768e39ab977 Mon Sep 17 00:00:00 2001 From: Daniel Meltzer Date: Thu, 30 Apr 2020 10:35:26 -0400 Subject: [PATCH 6/6] Fix incorrect routing. --- resources/views/licenses/view.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/licenses/view.blade.php b/resources/views/licenses/view.blade.php index ce85e42d4d..369718b31f 100755 --- a/resources/views/licenses/view.blade.php +++ b/resources/views/licenses/view.blade.php @@ -338,7 +338,7 @@ data-sort-order="asc" data-sort-name="name" class="table table-striped snipe-table" - data-url="{{ route('api.license.seats',['license_id' => $license->id]) }}" + data-url="{{ route('api.license.seats', $license->id) }}" data-export-options='{ "fileName": "export-seats-{{ str_slug($license->name) }}-{{ date('Y-m-d') }}", "ignoreColumn": ["actions","image","change","checkbox","checkincheckout","icon"]