Skip to content

Commit

Permalink
Partialize forms (snipe#2884)
Browse files Browse the repository at this point in the history
* Consolidate edit form elements into reusable partials.

This is a large code change that doesn't do much immediately.  It
refactors all of the various edit.blade.php files to reference
standardized partials, so that they all reference the same base html
layout. This has the side effect of moving everything to the new fancy
"required" indicators, and making things look consistent.

In addition, I've gone ahead and renamed a few database fields.  We had
Assetmodel::modelno and Consumable::model_no, I've renamed both to
model_number.  We had items using ::note and ::notes, I've standardized
on ::notes.  Component used total_qty where consumables and accessories
used qty, so I've moved everything to qty (And fixed a few bugs in the
helper file in the process.

TODO includes looking at how/where to place the modal javascripts to
allow for on the fly creation from all places, rather than just the
asset page.

Rename assetmodel::modelno to model_number for clarity and consistency

Rename consumable::model_no to model_number for clarity and consistency

Rename assetmodel::note to notes for clarity and consistency

Port asset and assetmodel to new partials layout.  Adapt all code to the renamed model_number and notes database changes.  Fix some stying.

* Share a settings variable with all views.

* Allow editing the per_page setting.  We showed the value, but we never showed it on the edit page..

* use snipeSettings in all views instead of the long ugly path.

* War on partials. Centralize all bootstrap table javascript

* Use model_number instead of modelno in importer

* Codacy fix.

* More unification/deduplication.  Create an edit form template layout that we use as the base for all edit forms.  This gives the same interface for editing everything and makes the edit.blade.* files much easier to read.

* Use a ViewComposer instead of sharing the variable directly.  Fixes artisan optimize trying to hit the db--which ruins new installs

* Fix DB seeder.

* Base sql dump and csv's to import data from for tests.

* Start some functional tests for creating items.

* Add functional tests for all create methods.  Still need to do tests for edits, deletes, and lots of other things

* Improvements to functional tests.

Use the built in DB seeding mechanism instead of doing it ourselves.
Break the tests into multiple units, rather than testing everything in
each function.

* Some improvements to acceptance tests.

Make sure we're only looking at the "trs" within the bootstrap table.
Creation of assets is now tested at the functional level (and is faster)
so ignore it here.

I'm testing acceptance tests with the
IMPORT_{ASSETS,ACCESSORIES,CONSUMABLES}.csv in the tests/_data folder
imported.

* A few things to make acceptance tests work.  Add a name to the companies table, and make the locations table have the correct name

* Use a .env.tests file for testing functional and unit to allow a separate database.

* Add functional tests for compoents, groups, and licenses.

* Now that the config is in the functional.yml, this just confuses things.

* Start some functional tests for creating items.

* Add functional tests for all create methods.  Still need to do tests for edits, deletes, and lots of other things

* Improvements to functional tests.

Use the built in DB seeding mechanism instead of doing it ourselves.
Break the tests into multiple units, rather than testing everything in
each function.

* Some improvements to acceptance tests.

Make sure we're only looking at the "trs" within the bootstrap table.
Creation of assets is now tested at the functional level (and is faster)
so ignore it here.

I'm testing acceptance tests with the
IMPORT_{ASSETS,ACCESSORIES,CONSUMABLES}.csv in the tests/_data folder
imported.

* update db dump

* Update tests to new reality

* env for the test setup

* only load the database at beginning of tests, not between each Functional test.

* Fix a miss from renaming note to notes.

* Set Termination date when creating an asset.  It was only set on edit.

* Rename serial_number to serial in components for consistency.

* Update validation rules to match limits in database.  Currently we just accepted the values and they were truncated when adding to DB.

* Much more detailed functional testing of creating items.  This checks to make sure all values on form have been successfully persisted to database.
  • Loading branch information
mnemonicly authored and snipe committed Nov 17, 2016
1 parent 84e06f4 commit d722ed3
Show file tree
Hide file tree
Showing 186 changed files with 3,991 additions and 3,670 deletions.
9 changes: 9 additions & 0 deletions .env.tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
APP_ENV=local
APP_DEBUG=true
APP_URL=http://snipe-it.localapp
DB_CONNECTION=mysql
DB_HOST=localhost
DB_DATABASE=snipeittests
DB_USERNAME=snipeit
DB_PASSWORD=snipe
APP_KEY=base64:tu9NRh/a6+dCXBDGvg0Gv/0TcABnFsbT4AKxrr8mwQo=
6 changes: 3 additions & 3 deletions app/Console/Commands/ObjectImportCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public function fire()
$this->locations = Location::All(['name', 'id']);
$this->categories = Category::All(['name', 'category_type', 'id']);
$this->manufacturers = Manufacturer::All(['name', 'id']);
$this->asset_models = AssetModel::All(['name','modelno','category_id','manufacturer_id', 'id']);
$this->asset_models = AssetModel::All(['name','model_number','category_id','manufacturer_id', 'id']);
$this->companies = Company::All(['name', 'id']);
$this->status_labels = Statuslabel::All(['name', 'id']);
$this->suppliers = Supplier::All(['name', 'id']);
Expand Down Expand Up @@ -348,7 +348,7 @@ public function createOrFetchAssetModel(array $row, $category, $manufacturer)
$editingModel = false;
foreach ($this->asset_models as $tempmodel) {
if (strcasecmp($tempmodel->name, $asset_model_name) == 0
&& $tempmodel->modelno == $asset_modelno) {
&& $tempmodel->model_number == $asset_modelno) {
$this->log('A matching model ' . $asset_model_name . ' already exists');
if (!$this->option('update')) {
return $tempmodel;
Expand All @@ -366,7 +366,7 @@ public function createOrFetchAssetModel(array $row, $category, $manufacturer)
$asset_model->name = $asset_model_name;
}
isset($manufacturer) && $manufacturer->exists && $asset_model->manufacturer_id = $manufacturer->id;
isset($asset_modelno) && $asset_model->modelno = $asset_modelno;
isset($asset_modelno) && $asset_model->model_number = $asset_modelno;
if (isset($category) && $category->exists) {
$asset_model->category_id = $category->id;
}
Expand Down
12 changes: 6 additions & 6 deletions app/Helpers/Helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,8 @@ public static function checkLowInventory()
foreach ($consumables as $consumable) {
$avail = $consumable->numRemaining();
if ($avail < ($consumable->min_amt) + \App\Models\Setting::getSettings()->alert_threshold) {
if ($consumable->total_qty > 0) {
$percent = number_format((($consumable->numRemaining() / $consumable->total_qty) * 100), 0);
if ($consumable->qty > 0) {
$percent = number_format((($consumable->numRemaining() / $consumable->qty) * 100), 0);
} else {
$percent = 100;
}
Expand All @@ -456,8 +456,8 @@ public static function checkLowInventory()
$avail = $accessory->numRemaining();
if ($avail < ($accessory->min_amt) + \App\Models\Setting::getSettings()->alert_threshold) {

if ($accessory->total_qty > 0) {
$percent = number_format((($accessory->numRemaining() / $accessory->total_qty) * 100), 0);
if ($accessory->qty > 0) {
$percent = number_format((($accessory->numRemaining() / $accessory->qty) * 100), 0);
} else {
$percent = 100;
}
Expand All @@ -476,8 +476,8 @@ public static function checkLowInventory()
foreach ($components as $component) {
$avail = $component->numRemaining();
if ($avail < ($component->min_amt) + \App\Models\Setting::getSettings()->alert_threshold) {
if ($component->total_qty > 0) {
$percent = number_format((($component->numRemaining() / $component->total_qty) * 100), 0);
if ($component->qty > 0) {
$percent = number_format((($component->numRemaining() / $component->qty) * 100), 0);
} else {
$percent = 100;
}
Expand Down
14 changes: 7 additions & 7 deletions app/Http/Controllers/AccessoriesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
use App\Models\Company;
use App\Models\Setting;
use App\Models\User;
use Auth;
use Carbon\Carbon;
use Config;
use DB;
use Gate;
use Input;
use Lang;
use Mail;
use Redirect;
use Request;
use Slack;
use Str;
use View;
use Auth;
use Request;
use Gate;

/** This controller handles all actions related to Accessories for
* the Snipe-IT Asset Management application.
Expand Down Expand Up @@ -54,7 +54,7 @@ public function getCreate(Request $request)
{
// Show the page
return View::make('accessories/edit')
->with('accessory', new Accessory)
->with('item', new Accessory)
->with('category_list', Helper::categoryList('accessory'))
->with('company_list', Helper::companyList())
->with('location_list', Helper::locationsList())
Expand Down Expand Up @@ -119,14 +119,14 @@ public function postCreate(Request $request)
public function getEdit(Request $request, $accessoryId = null)
{
// Check if the accessory exists
if (is_null($accessory = Accessory::find($accessoryId))) {
if (is_null($item = Accessory::find($accessoryId))) {
// Redirect to the blogs management page
return redirect()->to('admin/accessories')->with('error', trans('admin/accessories/message.does_not_exist'));
} elseif (!Company::isCurrentUserHasAccess($accessory)) {
} elseif (!Company::isCurrentUserHasAccess($item)) {
return redirect()->to('admin/accessories')->with('error', trans('general.insufficient_permissions'));
}

return View::make('accessories/edit', compact('accessory'))
return View::make('accessories/edit', compact('item'))
->with('category_list', Helper::categoryList('accessory'))
->with('company_list', Helper::companyList())
->with('location_list', Helper::locationsList())
Expand Down
22 changes: 11 additions & 11 deletions app/Http/Controllers/AssetModelsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function getCreate()
->with('category_list', $category_list)
->with('depreciation_list', $depreciation_list)
->with('manufacturer_list', $manufacturer_list)
->with('model', new AssetModel);
->with('item', new AssetModel);
}


Expand Down Expand Up @@ -94,10 +94,10 @@ public function postCreate()

// Save the model data
$model->name = e(Input::get('name'));
$model->modelno = e(Input::get('modelno'));
$model->model_number = e(Input::get('model_number'));
$model->manufacturer_id = e(Input::get('manufacturer_id'));
$model->category_id = e(Input::get('category_id'));
$model->note = e(Input::get('note'));
$model->notes = e(Input::get('notes'));
$model->user_id = Auth::user()->id;
$model->requestable = Input::has('requestable');

Expand Down Expand Up @@ -146,7 +146,7 @@ public function store()
$model->name=e(Input::get('name'));
$model->manufacturer_id = e(Input::get('manufacturer_id'));
$model->category_id = e(Input::get('category_id'));
$model->modelno = e(Input::get('modelno'));
$model->model_number = e(Input::get('model_number'));
$model->user_id = Auth::user()->id;
$model->note = e(Input::get('note'));
$model->eol= null;
Expand Down Expand Up @@ -176,15 +176,16 @@ public function store()
public function getEdit($modelId = null)
{
// Check if the model exists
if (is_null($model = AssetModel::find($modelId))) {
if (is_null($item = AssetModel::find($modelId))) {
// Redirect to the model management page
return redirect()->to('assets/models')->with('error', trans('admin/models/message.does_not_exist'));
}

$depreciation_list = Helper::depreciationList();
$manufacturer_list = Helper::manufacturerList();
$category_list = Helper::categoryList('asset');
$view = View::make('models/edit', compact('model'));

$view = View::make('models/edit', compact('item'));
$view->with('category_list', $category_list);
$view->with('depreciation_list', $depreciation_list);
$view->with('manufacturer_list', $manufacturer_list);
Expand Down Expand Up @@ -221,13 +222,12 @@ public function postEdit($modelId = null)
} else {
$model->eol = e(Input::get('eol'));
}

// Update the model data
$model->name = e(Input::get('name'));
$model->modelno = e(Input::get('modelno'));
$model->model_number = e(Input::get('model_number'));
$model->manufacturer_id = e(Input::get('manufacturer_id'));
$model->category_id = e(Input::get('category_id'));
$model->note = e(Input::get('note'));
$model->notes = e(Input::get('notes'));

$model->requestable = Input::has('requestable');

Expand Down Expand Up @@ -442,7 +442,7 @@ public function getDatatable($status = null)
}


$allowed_columns = ['id','name','modelno'];
$allowed_columns = ['id','name','model_number'];
$order = Input::get('order') === 'asc' ? 'asc' : 'desc';
$sort = in_array(Input::get('sort'), $allowed_columns) ? e(Input::get('sort')) : 'created_at';

Expand All @@ -465,7 +465,7 @@ public function getDatatable($status = null)
'manufacturer' => (string)link_to('/admin/settings/manufacturers/'.$model->manufacturer->id.'/view', $model->manufacturer->name),
'name' => (string)link_to('/hardware/models/'.$model->id.'/view', $model->name),
'image' => ($model->image!='') ? '<img src="'.config('app.url').'/uploads/models/'.$model->image.'" height=50 width=50>' : '',
'modelnumber' => $model->modelno,
'modelnumber' => $model->model_number,
'numassets' => $model->assets->count(),
'depreciation' => (($model->depreciation)&&($model->depreciation->id > 0)) ? $model->depreciation->name.' ('.$model->depreciation->months.')' : trans('general.no_depreciation'),
'category' => ($model->category) ? $model->category->name : '',
Expand Down
12 changes: 6 additions & 6 deletions app/Http/Controllers/AssetsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public function getCreate($model_id = null)
$view->with('statuslabel_list', $statuslabel_list);
$view->with('assigned_to', $assigned_to);
$view->with('location_list', $location_list);
$view->with('asset', new Asset);
$view->with('item', new Asset);
$view->with('manufacturer', $manufacturer_list);
$view->with('category', $category_list);
$view->with('statuslabel_types', $statuslabel_types);
Expand Down Expand Up @@ -290,10 +290,10 @@ public function getEdit($assetId = null)
{

// Check if the asset exists
if (!$asset = Asset::find($assetId)) {
if (!$item = Asset::find($assetId)) {
// Redirect to the asset management page
return redirect()->to('hardware')->with('error', trans('admin/hardware/message.does_not_exist'));
} elseif (!Company::isCurrentUserHasAccess($asset)) {
} elseif (!Company::isCurrentUserHasAccess($item)) {
return redirect()->to('hardware')->with('error', trans('general.insufficient_permissions'));
}

Expand All @@ -308,7 +308,7 @@ public function getEdit($assetId = null)
$assigned_to = Helper::usersList();
$statuslabel_types =Helper::statusTypeList();

return View::make('hardware/edit', compact('asset'))
return View::make('hardware/edit', compact('item'))
->with('model_list', $model_list)
->with('supplier_list', $supplier_list)
->with('company_list', $company_list)
Expand Down Expand Up @@ -1038,7 +1038,7 @@ public function getClone($assetId = null)
->with('statuslabel_list', $statuslabel_list)
->with('statuslabel_types', $statuslabel_types)
->with('assigned_to', $assigned_to)
->with('asset', $asset)
->with('item', $asset)
->with('location_list', $location_list)
->with('manufacturer', $manufacturer_list)
->with('category', $category_list)
Expand Down Expand Up @@ -1798,7 +1798,7 @@ public function getDatatable(Request $request, $status = null)
'asset_tag' => '<a title="'.e($asset->asset_tag).'" href="hardware/'.$asset->id.'/view">'.e($asset->asset_tag).'</a>',
'serial' => e($asset->serial),
'model' => ($asset->model) ? (string)link_to('/hardware/models/'.$asset->model->id.'/view', e($asset->model->name)) : 'No model',
'model_number' => ($asset->model && $asset->model->modelno) ? (string)$asset->model->modelno : '',
'model_number' => ($asset->model && $asset->model->model_number) ? (string)$asset->model->model_number : '',
'status_label' => ($asset->assigneduser) ? 'Deployed' : ((e($asset->assetstatus)) ? e($asset->assetstatus->name) : ''),
'assigned_to' => ($asset->assigneduser) ? (string)link_to(config('app.url').'/admin/users/'.$asset->assigned_to.'/view', e($asset->assigneduser->fullName())) : '',
'location' => (($asset->assigneduser) && ($asset->assigneduser->userloc!='')) ? (string)link_to('admin/settings/locations/'.$asset->assigneduser->userloc->id.'/view', e($asset->assigneduser->userloc->name)) : (($asset->defaultLoc!='') ? (string)link_to('admin/settings/locations/'.$asset->defaultLoc->id.'/view', e($asset->defaultLoc->name)) : ''),
Expand Down
6 changes: 3 additions & 3 deletions app/Http/Controllers/CategoriesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function getCreate()
{
// Show the page
$category_types= Helper::categoryTypeList();
return View::make('categories/edit')->with('category', new Category)
return View::make('categories/edit')->with('item', new Category)
->with('category_types', $category_types);
}

Expand Down Expand Up @@ -109,7 +109,7 @@ public function postCreate()
public function getEdit($categoryId = null)
{
// Check if the category exists
if (is_null($category = Category::find($categoryId))) {
if (is_null($item = Category::find($categoryId))) {
// Redirect to the blogs management page
return redirect()->to('admin/settings/categories')->with('error', trans('admin/categories/message.does_not_exist'));
}
Expand All @@ -120,7 +120,7 @@ public function getEdit($categoryId = null)
$category_options = array('' => 'Top Level') + DB::table('categories')->where('id', '!=', $categoryId)->lists('name', 'id');
$category_types= Helper::categoryTypeList();

return View::make('categories/edit', compact('category'))
return View::make('categories/edit', compact('item'))
->with('category_options', $category_options)
->with('category_types', $category_types);
}
Expand Down
6 changes: 3 additions & 3 deletions app/Http/Controllers/CompaniesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function getIndex()
*/
public function getCreate()
{
return View::make('companies/edit')->with('company', new Company);
return View::make('companies/edit')->with('item', new Company);
}

/**
Expand Down Expand Up @@ -74,11 +74,11 @@ public function postCreate()
*/
public function getEdit($companyId)
{
if (is_null($company = Company::find($companyId))) {
if (is_null($item = Company::find($companyId))) {
return redirect()->to('admin/settings/companies')
->with('error', trans('admin/companies/message.does_not_exist'));
} else {
return View::make('companies/edit')->with('company', $company);
return View::make('companies/edit')->with('item', $item);
}
}

Expand Down
Loading

0 comments on commit d722ed3

Please sign in to comment.