From 06af9311fc070504429d760af34bdd114e482971 Mon Sep 17 00:00:00 2001 From: Daniel Meltzer Date: Mon, 26 Dec 2016 18:17:46 -0500 Subject: [PATCH] Move sanitization of input to the model attribute setters. This cleans up a lot of checks in the various controller methods and ensures data will be set in the model accurately regardless of where it's set. Add unit tests for these methods (#3102) --- .../Controllers/AccessoriesController.php | 39 +---- .../AssetMaintenancesController.php | 80 ++-------- .../Controllers/AssetModelsController.php | 28 +--- app/Http/Controllers/AssetsController.php | 69 ++------ app/Http/Controllers/ComponentsController.php | 30 +--- .../Controllers/ConsumablesController.php | 37 +---- app/Http/Controllers/LicensesController.php | 150 +++--------------- app/Http/Controllers/LocationsController.php | 24 +-- .../Controllers/StatuslabelsController.php | 12 +- app/Http/Controllers/UsersController.php | 41 +---- app/Models/AssetMaintenance.php | 46 +++++- app/Models/AssetModel.php | 7 + app/Models/License.php | 16 ++ app/Models/SnipeModel.php | 62 ++++++++ .../en/admin/asset_maintenances/message.php | 4 + tests/unit.suite.yml | 1 + tests/unit/AssetMaintenanceTest.php | 65 ++++++++ tests/unit/AssetModelTest.php | 11 ++ tests/unit/SnipeModelTest.php | 98 ++++++++++++ 19 files changed, 403 insertions(+), 417 deletions(-) create mode 100644 tests/unit/AssetMaintenanceTest.php create mode 100644 tests/unit/SnipeModelTest.php diff --git a/app/Http/Controllers/AccessoriesController.php b/app/Http/Controllers/AccessoriesController.php index a361fd449c..9f55487893 100755 --- a/app/Http/Controllers/AccessoriesController.php +++ b/app/Http/Controllers/AccessoriesController.php @@ -85,19 +85,8 @@ class AccessoriesController extends Controller $accessory->order_number = request('order_number'); $accessory->manufacturer_id = request('manufacturer_id'); $accessory->model_number = request('model_number'); - - if (request('purchase_date') == ''){ - $accessory->purchase_date = null; - } else { - $accessory->purchase_date = request('purchase_date'); - } - - if (request('purchase_cost') == '0.00'){ - $accessory->purchase_cost = null; - } else { - $accessory->purchase_cost = Helper::ParseFloat(request('purchase_cost')); - } - + $accessory->purchase_date = request('purchase_date'); + $accessory->purchase_cost = Helper::ParseFloat(request('purchase_cost')); $accessory->qty = request('qty'); $accessory->user_id = Auth::user()->id; @@ -153,32 +142,16 @@ class AccessoriesController extends Controller $this->authorize($accessory); // Update the accessory data - $accessory->name = e(request('name')); - - if (e(request('location_id')) == '') { - $accessory->location_id = null; - } else { - $accessory->location_id = request('location_id'); - } + $accessory->name = request('name'); + $accessory->location_id = request('location_id'); $accessory->min_amt = request('min_amt'); $accessory->category_id = request('category_id'); $accessory->company_id = Company::getIdForCurrentUser(request('company_id')); $accessory->manufacturer_id = request('manufacturer_id'); $accessory->order_number = request('order_number'); $accessory->model_number = request('model_number'); - - if (request('purchase_date') == '') { - $accessory->purchase_date = null; - } else { - $accessory->purchase_date = request('purchase_date'); - } - - if (request('purchase_cost') == '0.00') { - $accessory->purchase_cost = null; - } else { - $accessory->purchase_cost = request('purchase_cost'); - } - + $accessory->purchase_date = request('purchase_date'); + $accessory->purchase_cost = request('purchase_cost'); $accessory->qty = request('qty'); // Was the accessory updated? diff --git a/app/Http/Controllers/AssetMaintenancesController.php b/app/Http/Controllers/AssetMaintenancesController.php index bc45c5ca03..6d4f3fafe0 100644 --- a/app/Http/Controllers/AssetMaintenancesController.php +++ b/app/Http/Controllers/AssetMaintenancesController.php @@ -9,11 +9,9 @@ use Input; use Lang; use Log; use Mail; -use Redirect; use Response; use Slack; use Str; -use App\Models\Supplier; use TCPDF; use Validator; use View; @@ -183,31 +181,10 @@ class AssetMaintenancesController extends Controller { // create a new model instance $assetMaintenance = new AssetMaintenance(); - - if (e(Input::get('supplier_id')) == '') { - $assetMaintenance->supplier_id = null; - } else { - $assetMaintenance->supplier_id = e($request->input('supplier_id')); - } - - if (e(Input::get('is_warranty')) == '') { - $assetMaintenance->is_warranty = 0; - } else { - $assetMaintenance->is_warranty = e($request->input('is_warranty')); - } - - if (e(Input::get('cost')) == '') { - $assetMaintenance->cost = ''; - } else { - $assetMaintenance->cost = Helper::ParseFloat(e($request->input('cost'))); - } - - if (e(Input::get('notes')) == '') { - $assetMaintenance->notes = null; - } else { - $assetMaintenance->notes = e($request->input('notes')); - } - + $assetMaintenance->supplier_id = $request->input('supplier_id'); + $assetMaintenance->is_warranty = $request->input('is_warranty'); + $assetMaintenance->cost = e($request->input('cost')); + $assetMaintenance->notes = e($request->input('notes')); $asset = Asset::find(e($request->input('asset_id'))); if (!Company::isCurrentUserHasAccess($asset)) { @@ -222,14 +199,7 @@ class AssetMaintenancesController extends Controller $assetMaintenance->completion_date = $request->input('completion_date'); $assetMaintenance->user_id = Auth::id(); - if (( $assetMaintenance->completion_date == "" ) - || ( $assetMaintenance->completion_date == "0000-00-00" ) - ) { - $assetMaintenance->completion_date = null; - } - - if (( $assetMaintenance->completion_date !== "" ) - && ( $assetMaintenance->completion_date !== "0000-00-00" ) + if (( $assetMaintenance->completion_date !== null ) && ( $assetMaintenance->start_date !== "" ) && ( $assetMaintenance->start_date !== "0000-00-00" ) ) { @@ -240,7 +210,6 @@ class AssetMaintenancesController extends Controller // Was the asset maintenance created? if ($assetMaintenance->save()) { - // Redirect to the new asset maintenance page return redirect()->route('maintenances.index') ->with('success', trans('admin/asset_maintenances/message.create.success')); @@ -248,9 +217,6 @@ class AssetMaintenancesController extends Controller return redirect()->back()->withInput()->withErrors($assetMaintenance->getErrors()); - - - } /** @@ -324,29 +290,10 @@ class AssetMaintenancesController extends Controller return static::getInsufficientPermissionsRedirect(); } - if (request('supplier_id') == '') { - $assetMaintenance->supplier_id = null; - } else { - $assetMaintenance->supplier_id = e($request->input('supplier_id')); - } - - if (request('is_warranty') == '') { - $assetMaintenance->is_warranty = 0; - } else { - $assetMaintenance->is_warranty = e($request->input('is_warranty')); - } - - if (request('cost') == '') { - $assetMaintenance->cost = ''; - } else { - $assetMaintenance->cost = Helper::ParseFloat(e($request->input('cost'))); - } - - if (request('notes') == '') { - $assetMaintenance->notes = null; - } else { - $assetMaintenance->notes = e($request->input('notes')); - } + $assetMaintenance->supplier_id = e($request->input('supplier_id')); + $assetMaintenance->is_warranty = e($request->input('is_warranty')); + $assetMaintenance->cost = Helper::ParseFloat(e($request->input('cost'))); + $assetMaintenance->notes = e($request->input('notes')); $asset = Asset::find(request('asset_id')); @@ -361,10 +308,8 @@ class AssetMaintenancesController extends Controller $assetMaintenance->start_date = $request->input('start_date'); $assetMaintenance->completion_date = $request->input('completion_date'); - if (( $assetMaintenance->completion_date == "" ) - || ( $assetMaintenance->completion_date == "0000-00-00" ) + if (( $assetMaintenance->completion_date == null ) ) { - $assetMaintenance->completion_date = null; if (( $assetMaintenance->asset_maintenance_time !== 0 ) || ( !is_null($assetMaintenance->asset_maintenance_time) ) ) { @@ -372,8 +317,7 @@ class AssetMaintenancesController extends Controller } } - if (( $assetMaintenance->completion_date !== "" ) - && ( $assetMaintenance->completion_date !== "0000-00-00" ) + if (( $assetMaintenance->completion_date !== null ) && ( $assetMaintenance->start_date !== "" ) && ( $assetMaintenance->start_date !== "0000-00-00" ) ) { @@ -387,7 +331,7 @@ class AssetMaintenancesController extends Controller // Redirect to the new asset maintenance page return redirect()->route('maintenances.index') - ->with('success', trans('admin/asset_maintenances/message.create.success')); + ->with('success', trans('admin/asset_maintenances/message.edit.success')); } return redirect()->back()->withInput()->withErrors($assetMaintenance->getErrors()); } diff --git a/app/Http/Controllers/AssetModelsController.php b/app/Http/Controllers/AssetModelsController.php index b1852fa6b6..687742341d 100755 --- a/app/Http/Controllers/AssetModelsController.php +++ b/app/Http/Controllers/AssetModelsController.php @@ -75,19 +75,9 @@ class AssetModelsController extends Controller // Create a new asset model $model = new AssetModel; - if ($request->input('depreciation_id') == '') { - $model->depreciation_id = 0; - } else { - $model->depreciation_id = $request->input('depreciation_id'); - } - - if ($request->input('eol') == '') { - $model->eol = 0; - } else { - $model->eol = $request->input('eol'); - } - // Save the model data + $model->eol = $request->input('eol'); + $model->depreciation_id = $request->input('depreciation_id'); $model->name = $request->input('name'); $model->model_number = $request->input('model_number'); $model->manufacturer_id = $request->input('manufacturer_id'); @@ -199,18 +189,8 @@ class AssetModelsController extends Controller return redirect()->route('models.index')->with('error', trans('admin/models/message.does_not_exist')); } - if ($request->input('depreciation_id') == '') { - $model->depreciation_id = 0; - } else { - $model->depreciation_id = $request->input('depreciation_id'); - } - - if ($request->input('eol') == '') { - $model->eol = null; - } else { - $model->eol = $request->input('eol'); - } - + $model->depreciation_id = $request->input('depreciation_id'); + $model->eol = $request->input('eol'); $model->name = $request->input('name'); $model->model_number = $request->input('model_number'); $model->manufacturer_id = $request->input('manufacturer_id'); diff --git a/app/Http/Controllers/AssetsController.php b/app/Http/Controllers/AssetsController.php index 0e125f8fca..842803d260 100755 --- a/app/Http/Controllers/AssetsController.php +++ b/app/Http/Controllers/AssetsController.php @@ -153,23 +153,17 @@ class AssetsController extends Controller $asset->notes = Input::get('notes'); $asset->asset_tag = Input::get('asset_tag'); $asset->user_id = Auth::id(); - $asset->archived = '0'; - $asset->physical = '1'; - $asset->depreciate = '0'; - - $asset->status_id = request('status_id',0); - $asset->warranty_months = request('warranty_months', null); - - if (Input::get('purchase_cost') == '') { - $asset->purchase_cost = null; - } else { - $asset->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); - } - $asset->purchase_date = request('purchase_date', null); - $asset->assigned_to = request('assigned_to', null); - $asset->supplier_id = request('supplier_id', 0); - $asset->requestable = request('requestable', 0); - $asset->rtd_location_id = request('rtd_location_id', null); + $asset->archived = '0'; + $asset->physical = '1'; + $asset->depreciate = '0'; + $asset->status_id = request('status_id',0); + $asset->warranty_months = request('warranty_months', null); + $asset->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); + $asset->purchase_date = request('purchase_date', null); + $asset->assigned_to = request('assigned_to', null); + $asset->supplier_id = request('supplier_id', 0); + $asset->requestable = request('requestable', 0); + $asset->rtd_location_id = request('rtd_location_id', null); // Create the image (if one was chosen.) if (Input::has('image')) { @@ -285,44 +279,15 @@ class AssetsController extends Controller } $this->authorize($asset); - if ($request->has('status_id')) { - $asset->status_id = $request->input('status_id'); - } else { - $asset->status_id = null; - } - - if ($request->has('warranty_months')) { - $asset->warranty_months = $request->input('warranty_months'); - } else { - $asset->warranty_months = null; - } - - if ($request->has('purchase_cost')) { - $asset->purchase_cost = Helper::ParseFloat($request->input('purchase_cost')); - } else { - $asset->purchase_cost = null; - } - - if ($request->has('purchase_date')) { - $asset->purchase_date = $request->input('purchase_date'); - } else { - $asset->purchase_date = null; - } - - if ($request->has('supplier_id')) { - $asset->supplier_id = $request->input('supplier_id'); - } else { - $asset->supplier_id = null; - } + $asset->status_id = $request->input('status_id', null); + $asset->warranty_months = $request->input('warranty_months', null); + $asset->purchase_cost = Helper::ParseFloat($request->input('purchase_cost', null)); + $asset->purchase_date = $request->input('purchase_date', null); + $asset->supplier_id = $request->input('supplier_id', null); // If the box isn't checked, it's not in the request at all. $asset->requestable = $request->has('requestable'); - - if ($request->has('rtd_location_id')) { - $asset->rtd_location_id = $request->input('rtd_location_id'); - } else { - $asset->rtd_location_id = null; - } + $asset->rtd_location_id = $request->input('rtd_location_id', null); if ($request->has('image_delete')) { unlink(public_path().'/uploads/assets/'.$asset->image); diff --git a/app/Http/Controllers/ComponentsController.php b/app/Http/Controllers/ComponentsController.php index b394996f1d..f8d4b65275 100644 --- a/app/Http/Controllers/ComponentsController.php +++ b/app/Http/Controllers/ComponentsController.php @@ -90,19 +90,8 @@ class ComponentsController extends Controller $component->order_number = Input::get('order_number'); $component->min_amt = Input::get('min_amt'); $component->serial = Input::get('serial'); - - if (Input::get('purchase_date') == '') { - $component->purchase_date = null; - } else { - $component->purchase_date = Input::get('purchase_date'); - } - - if (Input::get('purchase_cost') == '0.00') { - $component->purchase_cost = null; - } else { - $component->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); - } - + $component->purchase_date = Input::get('purchase_date'); + $component->purchase_cost = request('purchase_cost'); $component->qty = Input::get('qty'); $component->user_id = Auth::id(); @@ -169,19 +158,8 @@ class ComponentsController extends Controller $component->order_number = Input::get('order_number'); $component->min_amt = Input::get('min_amt'); $component->serial = Input::get('serial'); - - if (Input::get('purchase_date') == '') { - $component->purchase_date = null; - } else { - $component->purchase_date = Input::get('purchase_date'); - } - - if (Input::get('purchase_cost') == '0.00') { - $component->purchase_cost = null; - } else { - $component->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); - } - + $component->purchase_date = Input::get('purchase_date'); + $component->purchase_cost = request('purchase_cost'); $component->qty = Input::get('qty'); if ($component->save()) { diff --git a/app/Http/Controllers/ConsumablesController.php b/app/Http/Controllers/ConsumablesController.php index 2df57c8a98..f1c2862f30 100644 --- a/app/Http/Controllers/ConsumablesController.php +++ b/app/Http/Controllers/ConsumablesController.php @@ -82,22 +82,11 @@ class ConsumablesController extends Controller $consumable->company_id = Company::getIdForCurrentUser(Input::get('company_id')); $consumable->order_number = Input::get('order_number'); $consumable->min_amt = Input::get('min_amt'); - $consumable->manufacturer_id = Input::get('manufacturer_id'); - $consumable->model_number = Input::get('model_number'); - $consumable->item_no = Input::get('item_no'); - - if (Input::get('purchase_date') == '') { - $consumable->purchase_date = null; - } else { - $consumable->purchase_date = Input::get('purchase_date'); - } - - if (Input::get('purchase_cost') == '0.00') { - $consumable->purchase_cost = null; - } else { - $consumable->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); - } - + $consumable->manufacturer_id = Input::get('manufacturer_id'); + $consumable->model_number = Input::get('model_number'); + $consumable->item_no = Input::get('item_no'); + $consumable->purchase_date = Input::get('purchase_date'); + $consumable->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); $consumable->qty = Input::get('qty'); $consumable->user_id = Auth::id(); @@ -110,7 +99,6 @@ class ConsumablesController extends Controller return redirect()->back()->withInput()->withErrors($consumable->getErrors()); - } /** @@ -166,19 +154,8 @@ class ConsumablesController extends Controller $consumable->manufacturer_id = Input::get('manufacturer_id'); $consumable->model_number = Input::get('model_number'); $consumable->item_no = Input::get('item_no'); - - if (Input::get('purchase_date') == '') { - $consumable->purchase_date = null; - } else { - $consumable->purchase_date = Input::get('purchase_date'); - } - - if (Input::get('purchase_cost') == '0.00') { - $consumable->purchase_cost = null; - } else { - $consumable->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); - } - + $consumable->purchase_date = Input::get('purchase_date'); + $consumable->purchase_cost = Helper::ParseFloat(Input::get('purchase_cost')); $consumable->qty = Helper::ParseFloat(Input::get('qty')); if ($consumable->save()) { diff --git a/app/Http/Controllers/LicensesController.php b/app/Http/Controllers/LicensesController.php index c38704aad3..19c6dd213a 100755 --- a/app/Http/Controllers/LicensesController.php +++ b/app/Http/Controllers/LicensesController.php @@ -95,72 +95,29 @@ class LicensesController extends Controller $this->authorize('create', License::class); // create a new model instance $license = new License(); - - if ($request->input('purchase_cost') == '') { - $license->purchase_cost = null; - } else { - $license->purchase_cost = Helper::ParseFloat($request->input('purchase_cost')); - } - - if ($request->input('supplier_id') == '') { - $license->supplier_id = null; - } else { - $license->supplier_id = $request->input('supplier_id'); - } - - if ($request->input('maintained') == '') { - $license->maintained = 0; - } else { - $license->maintained = $request->input('maintained'); - } - - if ($request->input('reassignable') == '') { - $license->reassignable = 0; - } else { - $license->reassignable = $request->input('reassignable'); - } - - if ($request->input('purchase_order') == '') { - $license->purchase_order = ''; - } else { - $license->purchase_order = $request->input('purchase_order'); - } - - if (empty($request->input('manufacturer_id'))) { - $license->manufacturer_id = null; - } else { - $license->manufacturer_id = $request->input('manufacturer_id'); - } - // Save the license data - $license->name = $request->input('name'); - $license->serial = $request->input('serial'); + $license->company_id = Company::getIdForCurrentUser($request->input('company_id')); + $license->depreciation_id = $request->input('depreciation_id'); + $license->expiration_date = $request->input('expiration_date'); $license->license_email = $request->input('license_email'); $license->license_name = $request->input('license_name'); + $license->maintained = $request->input('maintained', 0); + $license->manufacturer_id = $request->input('manufacturer_id'); + $license->name = $request->input('name'); $license->notes = $request->input('notes'); $license->order_number = $request->input('order_number'); - $license->seats = $request->input('seats'); + $license->purchase_cost = $request->input('purchase_cost'); $license->purchase_date = $request->input('purchase_date'); $license->purchase_order = $request->input('purchase_order'); - $license->depreciation_id = $request->input('depreciation_id'); - $license->company_id = Company::getIdForCurrentUser($request->input('company_id')); - $license->expiration_date = $request->input('expiration_date'); + $license->purchase_order = $request->input('purchase_order'); + $license->reassignable = $request->input('reassignable', 0); + $license->seats = $request->input('seats'); + $license->serial = $request->input('serial'); + $license->supplier_id = $request->input('supplier_id'); $license->termination_date = $request->input('termination_date'); $license->user_id = Auth::id(); - if (($license->purchase_date == "") || ($license->purchase_date == "0000-00-00")) { - $license->purchase_date = null; - } - - if (($license->expiration_date == "") || ($license->expiration_date == "0000-00-00")) { - $license->expiration_date = null; - } - - if (($license->purchase_cost == "") || ($license->purchase_cost == "0.00")) { - $license->purchase_cost = null; - } - - // Was the license created? + // Was the license created? if ($license->save()) { $license->logCreate(); $insertedId = $license->id; @@ -198,14 +155,6 @@ class LicensesController extends Controller $this->authorize('update', $item); - if ($item->purchase_date == "0000-00-00") { - $item->purchase_date = null; - } - - if ($item->purchase_cost == "0.00") { - $item->purchase_cost = null; - } - $maintained_list = [ '' => 'Maintained', '1' => 'Yes', @@ -243,73 +192,24 @@ class LicensesController extends Controller $this->authorize('update', $license); // Update the license data - $license->name = $request->input('name'); - $license->serial = $request->input('serial'); + $license->company_id = Company::getIdForCurrentUser($request->input('company_id')); + $license->depreciation_id = $request->input('depreciation_id'); + $license->expiration_date = $request->input('expiration_date'); $license->license_email = $request->input('license_email'); $license->license_name = $request->input('license_name'); + $license->maintained = $request->input('maintained'); + $license->maintained = $request->input('maintained', 0); + $license->name = $request->input('name'); $license->notes = $request->input('notes'); $license->order_number = $request->input('order_number'); - $license->depreciation_id = $request->input('depreciation_id'); - $license->company_id = Company::getIdForCurrentUser($request->input('company_id')); + $license->purchase_cost = $request->input('purchase_cost'); + $license->purchase_date = $request->input('purchase_date'); $license->purchase_order = $request->input('purchase_order'); - $license->maintained = $request->input('maintained'); + $license->purchase_order = $request->input('purchase_order'); $license->reassignable = $request->input('reassignable'); - - if (empty($request->input('manufacturer_id'))) { - $license->manufacturer_id = null; - } else { - $license->manufacturer_id = $request->input('manufacturer_id'); - } - - - if ($request->input('supplier_id') == '') { - $license->supplier_id = null; - } else { - $license->supplier_id = $request->input('supplier_id'); - } - - // Update the asset data - if ($request->input('purchase_date') == '') { - $license->purchase_date = null; - } else { - $license->purchase_date = $request->input('purchase_date'); - } - - if ($request->input('expiration_date') == '') { - $license->expiration_date = null; - } else { - $license->expiration_date = $request->input('expiration_date'); - } - - if ($request->input('termination_date') == '') { - $license->termination_date = null; - } else { - $license->termination_date = $request->input('termination_date'); - } - - if ($request->input('purchase_cost') == '') { - $license->purchase_cost = null; - } else { - $license->purchase_cost = Helper::ParseFloat($request->input('purchase_cost')); - } - - if ($request->input('maintained') == '') { - $license->maintained = 0; - } else { - $license->maintained = $request->input('maintained'); - } - - if ($request->input('reassignable') == '') { - $license->reassignable = 0; - } else { - $license->reassignable = $request->input('reassignable'); - } - - if ($request->input('purchase_order') == '') { - $license->purchase_order = ''; - } else { - $license->purchase_order = $request->input('purchase_order'); - } + $license->reassignable = $request->input('reassignable', 0); + $license->serial = $request->input('serial'); + $license->termination_date = $request->input('termination_date'); //Are we changing the total number of seats? if ($license->seats != $request->input('seats')) { diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index 194a12b259..f7eefc398c 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -80,11 +80,7 @@ class LocationsController extends Controller { $location = new Location(); $location->name = Input::get('name'); - if (Input::get('parent_id')=='') { - $location->parent_id = null; - } else { - $location->parent_id = Input::get('parent_id'); - } + $location->parent_id = Input::get('parent_id', null); $location->currency = Input::get('currency', '$'); $location->address = Input::get('address'); $location->address2 = Input::get('address2'); @@ -180,18 +176,14 @@ class LocationsController extends Controller // Update the location data $location->name = Input::get('name'); - if (Input::get('parent_id')=='') { - $location->parent_id = null; - } else { - $location->parent_id = Input::get('parent_id', ''); - } - $location->currency = Input::get('currency', '$'); - $location->address = Input::get('address'); - $location->address2 = Input::get('address2'); - $location->city = Input::get('city'); - $location->state = Input::get('state'); + $location->parent_id = Input::get('parent_id', null); + $location->currency = Input::get('currency', '$'); + $location->address = Input::get('address'); + $location->address2 = Input::get('address2'); + $location->city = Input::get('city'); + $location->state = Input::get('state'); $location->country = Input::get('country'); - $location->zip = Input::get('zip'); + $location->zip = Input::get('zip'); // Was the asset created? if ($location->save()) { diff --git a/app/Http/Controllers/StatuslabelsController.php b/app/Http/Controllers/StatuslabelsController.php index 1f557bdf15..e7939b95ae 100755 --- a/app/Http/Controllers/StatuslabelsController.php +++ b/app/Http/Controllers/StatuslabelsController.php @@ -112,13 +112,13 @@ class StatuslabelsController extends Controller // Save the Statuslabel data $statusLabel->name = Input::get('name'); - $statusLabel->user_id = Auth::id(); - $statusLabel->notes = Input::get('notes'); - $statusLabel->deployable = $statusType['deployable']; - $statusLabel->pending = $statusType['pending']; + $statusLabel->user_id = Auth::id(); + $statusLabel->notes = Input::get('notes'); + $statusLabel->deployable = $statusType['deployable']; + $statusLabel->pending = $statusType['pending']; $statusLabel->archived = $statusType['archived']; - $statusLabel->color = Input::get('color'); - $statusLabel->show_in_nav = Input::get('show_in_nav', 0); + $statusLabel->color = Input::get('color'); + $statusLabel->show_in_nav = Input::get('show_in_nav', 0); // Was the asset created? diff --git a/app/Http/Controllers/UsersController.php b/app/Http/Controllers/UsersController.php index 6b94c290d3..3d0a4c27c7 100755 --- a/app/Http/Controllers/UsersController.php +++ b/app/Http/Controllers/UsersController.php @@ -118,9 +118,9 @@ class UsersController extends Controller $user->activated = $request->input('activated', $user->activated); $user->jobtitle = $request->input('jobtitle'); $user->phone = $request->input('phone'); - $user->location_id = $request->input('location_id'); - $user->company_id = Company::getIdForUser($request->input('company_id')); - $user->manager_id = $request->input('manager_id'); + $user->location_id = $request->input('location_id', null); + $user->company_id = Company::getIdForUser($request->input('company_id', null)); + $user->manager_id = $request->input('manager_id', null); $user->notes = $request->input('notes'); // Strip out the superuser permission if the user isn't a superadmin @@ -129,24 +129,8 @@ class UsersController extends Controller if (!Auth::user()->isSuperUser()) { unset($permissions_array['superuser']); } - $user->permissions = json_encode($permissions_array); - - - if ($user->manager_id == "") { - $user->manager_id = null; - } - - if ($user->location_id == "") { - $user->location_id = null; - } - - if ($user->company_id == "") { - $user->company_id = null; - } - - if ($user->save()) { if ($request->has('groups')) { @@ -339,9 +323,9 @@ class UsersController extends Controller $user->activated = $request->input('activated', $user->activated); $user->jobtitle = $request->input('jobtitle'); $user->phone = $request->input('phone'); - $user->location_id = $request->input('location_id'); - $user->company_id = Company::getIdForUser($request->input('company_id')); - $user->manager_id = $request->input('manager_id'); + $user->location_id = $request->input('location_id', null); + $user->company_id = Company::getIdForUser($request->input('company_id', null)); + $user->manager_id = $request->input('manager_id', null); $user->notes = $request->input('notes'); // Strip out the superuser permission if the user isn't a superadmin @@ -354,18 +338,6 @@ class UsersController extends Controller $user->permissions = json_encode($permissions_array); - if ($user->manager_id == "") { - $user->manager_id = null; - } - - if ($user->location_id == "") { - $user->location_id = null; - } - - if ($user->company_id == "") { - $user->company_id = null; - } - // Was the user updated? if ($user->save()) { // Prepare the success message @@ -404,7 +376,6 @@ class UsersController extends Controller } if ($user->accessories()->count() > 0) { - // Redirect to the user management page return redirect()->route('users.index')->with('error', 'This user still has ' . $user->accessories()->count() . ' accessories associated with them.'); } diff --git a/app/Models/AssetMaintenance.php b/app/Models/AssetMaintenance.php index 3b977ccb78..454320240a 100644 --- a/app/Models/AssetMaintenance.php +++ b/app/Models/AssetMaintenance.php @@ -1,6 +1,7 @@ 'required|integer', @@ -29,7 +29,7 @@ class AssetMaintenance extends Model implements ICompanyableChild 'title' => 'required|max:100', 'is_warranty' => 'boolean', 'start_date' => 'required|date_format:"Y-m-d"', - 'completion_date' => 'date_format:"Y-m-d', + 'completion_date' => 'date_format:"Y-m-d"', 'notes' => 'string|nullable', 'cost' => 'numeric|nullable' ]; @@ -56,6 +56,48 @@ class AssetMaintenance extends Model implements ICompanyableChild ]; } + public function setIsWarrantyAttribute($value) + { + if($value == '') { + $value = 0; + } + $this->attributes['is_warranty'] = $value; + } + + /** + * @param $value + */ + public function setCostAttribute($value) + { + $value = Helper::ParseFloat($value); + if($value == '0.0') { + $value = null; + } + $this->attributes['cost'] = $value; + } + + /** + * @param $value + */ + public function setNotesAttribute($value) + { + if($value == '') { + $value = null; + } + $this->attributes['notes'] = $value; + } + + /** + * @param $value + */ + public function setCompletionDateAttribute($value) + { + if($value == '' || $value == "0000-00-00") { + $value = null; + } + $this->attributes['completion_date'] = $value; + } + /** * asset * Get asset for this improvement diff --git a/app/Models/AssetModel.php b/app/Models/AssetModel.php index 7fd0a476ef..6bde3a6b9f 100755 --- a/app/Models/AssetModel.php +++ b/app/Models/AssetModel.php @@ -41,6 +41,13 @@ class AssetModel extends SnipeModel protected $injectUniqueIdentifier = true; use ValidatingTrait; + public function setEolAttribute($value) { + if ($value == '') { + $value = 0; + } + + $this->attributes['eol'] = $value; + } /** * The attributes that are mass assignable. * diff --git a/app/Models/License.php b/app/Models/License.php index 6bb93a586f..56422b6bda 100755 --- a/app/Models/License.php +++ b/app/Models/License.php @@ -33,6 +33,22 @@ class License extends Depreciable 'company_id' => 'integer|nullable', ); + public function setExpirationDateAttribute($value) + { + if($value == '' || $value == '0000-00-00') { + $value = null; + } + $this->attributes['expiration_date'] = $value; + } + + public function setTerminationDateAttribute($value) + { + if($value == '' || $value == '0000-00-00') { + $value = null; + } + $this->attributes['termination_date'] = $value; + } + public function company() { return $this->belongsTo('\App\Models\Company', 'company_id'); diff --git a/app/Models/SnipeModel.php b/app/Models/SnipeModel.php index 4b049804c7..e83c0fc317 100644 --- a/app/Models/SnipeModel.php +++ b/app/Models/SnipeModel.php @@ -2,10 +2,72 @@ namespace App\Models; +use App\Helpers\Helper; use Illuminate\Database\Eloquent\Model; class SnipeModel extends Model { + // Setters that are appropriate across multiple models. + public function setPurchaseDateAttribute($value) + { + if($value == '') { + $value = null; + } + $this->attributes['purchase_date'] = $value; + } + + /** + * @param $value + */ + public function setPurchaseCostAttribute($value) + { + $value = Helper::ParseFloat($value); + if($value == '0.0') { + $value = null; + } + $this->attributes['purchase_cost'] = $value; + } + + public function setLocationIdAttribute($value) + { + if($value == '') { + $value = null; + } + $this->attributes['location_id'] = $value; + } + + public function setCategoryIdAttribute($value) + { + if($value == '') { + $value = null; + } + $this->attributes['category_id'] = $value; + } + + public function setSupplierIdAttribute($value) + { + if($value == '') { + $value = null; + } + $this->attributes['supplier_id'] = $value; + } + + public function setDepreciationIdAttribute($value) + { + if($value == '') { + $value = null; + } + $this->attributes['depreciation_id'] = $value; + } + + public function setManufacturerIdAttribute($value) + { + if($value == '') { + $value = null; + } + $this->attributes['manufacturer_id'] = $value; + } + // public function getDisplayNameAttribute() { diff --git a/resources/lang/en/admin/asset_maintenances/message.php b/resources/lang/en/admin/asset_maintenances/message.php index ca4256efbe..d121115825 100644 --- a/resources/lang/en/admin/asset_maintenances/message.php +++ b/resources/lang/en/admin/asset_maintenances/message.php @@ -11,6 +11,10 @@ 'error' => 'Asset Maintenance was not created, please try again.', 'success' => 'Asset Maintenance created successfully.' ], + 'edit' => [ + 'error' => 'Asset Maintenance was not edited, please try again.', + 'success' => 'Asset Maintenance edited successfully.' + ], 'asset_maintenance_incomplete' => 'Not Completed Yet', 'warranty' => 'Warranty', 'not_warranty' => 'Not Warranty', diff --git a/tests/unit.suite.yml b/tests/unit.suite.yml index 1f8cb0a998..38d0e4f3f0 100644 --- a/tests/unit.suite.yml +++ b/tests/unit.suite.yml @@ -5,5 +5,6 @@ class_name: UnitTester modules: enabled: - \Helper\Unit + - Asserts - Laravel5: environment_file: .env.tests diff --git a/tests/unit/AssetMaintenanceTest.php b/tests/unit/AssetMaintenanceTest.php new file mode 100644 index 0000000000..be2d5e1282 --- /dev/null +++ b/tests/unit/AssetMaintenanceTest.php @@ -0,0 +1,65 @@ +is_warranty = ''; + $this->assertTrue($c->is_warranty === 0); + $c->is_warranty = '4'; + $this->assertTrue($c->is_warranty==4); + } + + /** + * @test + */ + public function it_sets_costs_appropriately() + { + $c = new AssetMaintenance(); + $c->cost = '0.00'; + $this->assertTrue($c->cost === null); + $c->cost = '9.54'; + $this->assertTrue($c->cost===9.54); + $c->cost = '9.50'; + $this->assertTrue($c->cost===9.5); + } + + /** + * @test + */ + public function it_nulls_out_notes_if_blank() + { + $c = new AssetMaintenance; + $c->notes = ''; + $this->assertTrue($c->notes === null); + $c->notes = 'This is a long note'; + $this->assertTrue($c->notes==='This is a long note'); + } + + /** + * @test + */ + public function it_nulls_out_completion_date_if_blank_or_invalid() + { + $c = new AssetMaintenance; + $c->completion_date = ''; + $this->assertTrue($c->completion_date === null); + $c->completion_date = '0000-00-00'; + $this->assertTrue($c->completion_date===null); + $c->completion_date = '2017-05-12'; + $this->assertTrue($c->completion_date==='2017-05-12'); + + } +} \ No newline at end of file diff --git a/tests/unit/AssetModelTest.php b/tests/unit/AssetModelTest.php index 4d2f2d0a82..483c9b9dbf 100644 --- a/tests/unit/AssetModelTest.php +++ b/tests/unit/AssetModelTest.php @@ -27,4 +27,15 @@ class AssetModelTest extends \Codeception\TestCase\Test $this->tester->seeRecord('models', $values); } + /** + * @test + */ + public function it_zeros_blank_eol_but_not_others() + { + $am = new AssetModel; + $am->eol = ''; + $this->assertTrue($am->eol === 0); + $am->eol = '4'; + $this->assertTrue($am->eol==4); + } } diff --git a/tests/unit/SnipeModelTest.php b/tests/unit/SnipeModelTest.php new file mode 100644 index 0000000000..1cd0f51589 --- /dev/null +++ b/tests/unit/SnipeModelTest.php @@ -0,0 +1,98 @@ +purchase_date = ''; + $this->assertTrue($c->purchase_date === null); + $c->purchase_date = '2016-03-25 12:35:50'; + $this->assertTrue($c->purchase_date==='2016-03-25 12:35:50'); + } + + /** + * @test + */ + public function it_sets_purchase_costs_appropriately() + { + $c = new SnipeModel; + $c->purchase_cost = '0.00'; + $this->assertTrue($c->purchase_cost === null); + $c->purchase_cost = '9.54'; + $this->assertTrue($c->purchase_cost===9.54); + $c->purchase_cost = '9.50'; + $this->assertTrue($c->purchase_cost===9.5); + } + + /** + * @test + */ + public function it_nulls_blank_location_ids_but_not_others() + { + $c = new SnipeModel; + $c->location_id = ''; + $this->assertTrue($c->location_id === null); + $c->location_id = '5'; + $this->assertTrue($c->location_id==5); + } + /** + * @test + */ + public function it_nulls_blank_categories_but_not_others() + { + $c = new SnipeModel; + $c->category_id = ''; + $this->assertTrue($c->category_id === null); + $c->category_id = '1'; + $this->assertTrue($c->category_id==1); + } + + /** + * @test + */ + public function it_nulls_blank_suppliers_but_not_others() + { + $c = new SnipeModel; + $c->supplier_id = ''; + $this->assertTrue($c->supplier_id === null); + $c->supplier_id = '4'; + $this->assertTrue($c->supplier_id==4); + } + + /** + * @test + */ + public function it_nulls_blank_depreciations_but_not_others() + { + $c = new SnipeModel; + $c->depreciation_id = ''; + $this->assertTrue($c->depreciation_id === null); + $c->depreciation_id = '4'; + $this->assertTrue($c->depreciation_id==4); + } + + /** + * @test + */ + public function it_nulls_blank_manufacturers_but_not_others() + { + $c = new SnipeModel; + $c->manufacturer_id = ''; + $this->assertTrue($c->manufacturer_id === null); + $c->manufacturer_id = '4'; + $this->assertTrue($c->manufacturer_id==4); + } +} \ No newline at end of file