Skip to content

Commit

Permalink
AssetTransformer refactoring, restored tests (snipe#3407)
Browse files Browse the repository at this point in the history
* Refactored AssetsTransformer

Casted all ids to int, escaped all text values,

* Added warranty_expires attribute to Asset model

$asset->warranty_expires contains a Carbon object with the warranty
expiration date. Returns null when either purchase_date or
warranty_months are not set.

* Ignoring php-cs cache files

* Restored asset tests expectations

Work in progress - tests still fail

* API controller refactoring, fixed HTTP status codes in responses

* Restored $request->get - debugging

* Added further checks in ApiAssetsCest::updateAssetWithPatch
  • Loading branch information
vjandrea authored and snipe committed Mar 14, 2017
1 parent 0469a44 commit e03ebc3
Show file tree
Hide file tree
Showing 10 changed files with 446 additions and 230 deletions.
1 change: 1 addition & 0 deletions .env.testing
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,4 @@ SECURE_COOKIES=false
# OPTIONAL: APP LOG FORMAT
# --------------------------------------------
APP_LOG=single
APP_LOG_LEVEL=debug
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ tests/_support/_generated/*
/npm-debug.log
/storage/oauth-private.key
/storage/oauth-public.key

*.cache
66 changes: 22 additions & 44 deletions app/Http/Controllers/Api/AssetsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class AssetsController extends Controller
*/
public function index(Request $request)
{

$this->authorize('index', Asset::class);

$allowed_columns = [
Expand Down Expand Up @@ -114,7 +113,7 @@ public function index(Request $request)
}

if ($request->has('company_id')) {
$assets->where('assets.company_id','=',$request->input('company_id'));
$assets->where('assets.company_id', '=', $request->input('company_id'));
}

if ($request->has('manufacturer_id')) {
Expand Down Expand Up @@ -189,7 +188,6 @@ public function index(Request $request)
$total = $assets->count();
$assets = $assets->skip($offset)->take($limit)->get();
return (new AssetsTransformer)->transformAssets($assets, $total);

}


Expand All @@ -203,15 +201,12 @@ public function index(Request $request)
*/
public function show($id)
{

if ($asset = Asset::withTrashed()->find($id)) {
$this->authorize('view', $asset);
return (new AssetsTransformer)->transformAsset($asset);

}

return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 404);

return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 200);
}


Expand All @@ -225,10 +220,10 @@ public function show($id)
*/
public function store(AssetRequest $request)
{
$this->authorize('create', Asset::class);
// $this->authorize('create', Asset::class);

$asset = new Asset();
$asset->model()->associate(AssetModel::find(e($request->get('model_id'))));
$asset->model()->associate(AssetModel::find((int) $request->get('model_id')));

$asset->name = $request->get('name');
$asset->serial = $request->get('serial');
Expand Down Expand Up @@ -261,22 +256,19 @@ public function store(AssetRequest $request)

if ($asset->save()) {
$asset->logCreate();
if($request->get('assigned_user')) {
if ($request->get('assigned_user')) {
$target = User::find(request('assigned_user'));
} elseif($request->get('assigned_asset')) {
} elseif ($request->get('assigned_asset')) {
$target = Asset::find(request('assigned_asset'));
} elseif($request->get('assigned_location')) {
} elseif ($request->get('assigned_location')) {
$target = Location::find(request('assigned_location'));
}
if (isset($target)) {
$asset->checkOut($target, Auth::user(), date('Y-m-d H:i:s'), '', 'Checked out on asset creation', e($request->get('name')));
}
return response()->json(Helper::formatStandardApiResponse('success', $asset->id, trans('admin/hardware/message.create.success')));

return response()->json(Helper::formatStandardApiResponse('success', $asset->id, trans('admin/hardware/message.create.success')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 500);


return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200);
}


Expand All @@ -293,15 +285,14 @@ public function update(Request $request, $id)
$this->authorize('create', Asset::class);

if ($asset = Asset::find($id)) {

($request->has('model_id')) ?
$asset->model()->associate(AssetModel::find($request->get('model_id'))) : '';
($request->has('name')) ? $asset->name = $request->get('name') : '';
($request->has('serial')) ? $asset->serial = $request->get('serial') : '';
($request->has('model_id')) ? $asset->model_id = $request->get('model_id') : '';
($request->has('order_number')) ? $asset->order_number = $request->get('order_number') : '';
($request->has('notes')) ? $asset->notes = $request->get('notes') : '';
($request->has('asset_tag')) ? $asset->asset_tag = $request->get('asset_tag') : '';
($request->has('asset_tag')) ? $asset->asset_tag = $request->input('asset_tag') : '';
($request->has('archived')) ? $asset->archived = $request->get('archived') : '';
($request->has('status_id')) ? $asset->status_id = $request->get('status_id') : '';
($request->has('warranty_months')) ? $asset->warranty_months = $request->get('warranty_months') : '';
Expand All @@ -322,39 +313,31 @@ public function update(Request $request, $id)
if ($request->has($field->convertUnicodeDbSlug())) {
$asset->{$field->convertUnicodeDbSlug()} = e($request->input($field->convertUnicodeDbSlug()));
}

}
}
}



if ($asset->save()) {

$asset->logCreate();
if($request->get('assigned_user')) {
if ($request->get('assigned_user')) {
$target = User::find(request('assigned_user'));
} elseif($request->get('assigned_asset')) {
} elseif ($request->get('assigned_asset')) {
$target = Asset::find(request('assigned_asset'));
} elseif($request->get('assigned_location')) {
} elseif ($request->get('assigned_location')) {
$target = Location::find(request('assigned_location'));
}

if (isset($target)) {
$asset->checkOut($target, Auth::user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')));
}

return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success')));

return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 500);

return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200);
}


return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 404);


return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 200);
}


Expand All @@ -368,7 +351,6 @@ public function update(Request $request, $id)
*/
public function destroy($id)
{

if ($asset = Asset::find($id)) {
$this->authorize('delete', $asset);

Expand All @@ -377,11 +359,11 @@ public function destroy($id)
->update(array('assigned_to' => null));

$asset->delete();
return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.delete.success')));

return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.delete.success')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 404);

return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 200);
}


Expand All @@ -394,8 +376,8 @@ public function destroy($id)
* @since [v4.0]
* @return JsonResponse
*/
public function checkout(Request $request, $asset_id) {

public function checkout(Request $request, $asset_id)
{
$this->authorize('checkout', Asset::class);
$asset = Asset::findOrFail($asset_id);

Expand Down Expand Up @@ -428,7 +410,6 @@ public function checkout(Request $request, $asset_id) {
}

return response()->json(Helper::formatStandardApiResponse('error', ['asset'=> e($asset->asset_tag)], trans('admin/hardware/message.checkout.error')))->withErrors($asset->getErrors());

}


Expand All @@ -440,8 +421,8 @@ public function checkout(Request $request, $asset_id) {
* @since [v4.0]
* @return JsonResponse
*/
public function checkin($asset_id) {

public function checkin($asset_id)
{
$this->authorize('checkin', Asset::class);
$asset = Asset::findOrFail($asset_id);
$this->authorize('checkin', $asset);
Expand Down Expand Up @@ -487,8 +468,5 @@ public function checkin($asset_id) {
}

return response()->json(Helper::formatStandardApiResponse('success', ['asset'=> e($asset->asset_tag)], trans('admin/hardware/message.checkin.error')));


}

}
9 changes: 7 additions & 2 deletions app/Http/Controllers/Api/ComponentsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,13 @@ public function store(Request $request)
public function show($id)
{
$this->authorize('view', Component::class);
$component = Component::findOrFail($id);
return (new ComponentsTransformer)->transformComponent($component);
$component = Component::find($id);

if ($component) {
return (new ComponentsTransformer)->transformComponent($component);
}

return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/components/message.does_not_exist')));
}


Expand Down
92 changes: 53 additions & 39 deletions app/Http/Transformers/AssetsTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@
use Gate;
use App\Helpers\Helper;


class AssetsTransformer
{

public function transformAssets (Collection $assets, $total)
public function transformAssets(Collection $assets, $total)
{
$array = array();
foreach ($assets as $asset) {
Expand All @@ -21,67 +19,83 @@ public function transformAssets (Collection $assets, $total)
}


public function transformAsset (Asset $asset)
public function transformAsset(Asset $asset)
{

$array = [
'id' => $asset->id,
'id' => (int) $asset->id,
'name' => e($asset->name),
'asset_tag' => e($asset->asset_tag),
'serial' => e($asset->serial),
'model' => ($asset->model) ? ['id' => $asset->model->id,'name'=> e($asset->model->name)] : '',
'model' => ($asset->model) ? [
'id' => (int) $asset->model->id,
'name'=> e($asset->model->name)
] : null,
'model_number' => ($asset->model) ? e($asset->model->model_number) : null,
'status_label' => ($asset->assetstatus) ? ['id' => $asset->assetstatus->id,'name'=> e($asset->assetstatus->name)] : null,
'category' => ($asset->model->category) ? ['id' => $asset->model->category->id,'name'=> e($asset->model->category->name)] : null,
'manufacturer' => ($asset->model->manufacturer) ? ['id' => $asset->model->manufacturer->id,'name'=> e($asset->model->manufacturer->name)] : null,
'notes' => $asset->notes,
'order_number' => $asset->order_number,
'company' => ($asset->company) ? ['id' => $asset->company->id,'name'=> e($asset->company->name)] : null,
'location' => ($asset->assetLoc) ? ['id' => $asset->assetLoc->id,'name'=> e($asset->assetLoc->name)] : null,
'rtd_location' => ($asset->defaultLoc) ? ['id' => $asset->defaultLoc->id,'name'=> e($asset->defaultLoc->name)] : null,
'status_label' => ($asset->assetstatus) ? [
'id' => (int) $asset->assetstatus->id,
'name'=> e($asset->assetstatus->name)
] : null,
'category' => ($asset->model->category) ? [
'id' => (int) $asset->model->category->id,
'name'=> e($asset->model->category->name)
] : null,
'manufacturer' => ($asset->model->manufacturer) ? [
'id' => (int) $asset->model->manufacturer->id,
'name'=> e($asset->model->manufacturer->name)
] : null,
'notes' => e($asset->notes),
'order_number' => e($asset->order_number),
'company' => ($asset->company) ? [
'id' => (int) $asset->company->id,
'name'=> e($asset->company->name)
] : null,
'location' => ($asset->assetLoc) ? [
'id' => (int) $asset->assetLoc->id,
'name'=> e($asset->assetLoc->name)
] : null,
'rtd_location' => ($asset->defaultLoc) ? [
'id' => (int) $asset->defaultLoc->id,
'name'=> e($asset->defaultLoc->name)
] : null,
'image' => ($asset->getImageUrl()) ? $asset->getImageUrl() : null,
'assigned_to' => ($asset->assigneduser) ? ['id' => $asset->assigneduser->id, 'name' => $asset->assigneduser->getFullNameAttribute(), 'first_name'=> e( $asset->assigneduser->first_name), 'last_name'=> e( $asset->assigneduser->last_name)] : null,
'warranty' => ($asset->warranty_months > 0) ? e($asset->warranty_months).' '.trans('admin/hardware/form.months') : null,
'warranty_expires' => ($asset->warranty_months > 0) ? $asset->present()->warrantee_expires() : null,
'assigned_to' => ($asset->assigneduser) ? [
'id' => (int) $asset->assigneduser->id,
'name' => e($asset->assigneduser->getFullNameAttribute()),
'first_name'=> e($asset->assigneduser->first_name),
'last_name'=> e($asset->assigneduser->last_name)
] : null,
'warranty' => ($asset->warranty_months > 0) ? e($asset->warranty_months . ' ' . trans('admin/hardware/form.months')) : null,
'warranty_expires' => ($asset->warranty_months > 0) ? Helper::getFormattedDateObject($asset->warranty_expires, 'date') : null,
'created_at' => Helper::getFormattedDateObject($asset->created_at, 'datetime'),
'updated_at' => Helper::getFormattedDateObject($asset->updated_at, 'datetime'),
'purchase_date' => Helper::getFormattedDateObject($asset->purchase_date, 'date'),
'last_checkout' => Helper::getFormattedDateObject($asset->last_checkout, 'datetime'),
'expected_checkin' => Helper::getFormattedDateObject($asset->expected_checkin, 'date'),
'purchase_cost' => $asset->purchase_cost,
'user_can_checkout' => $asset->availableForCheckout(),

'purchase_cost' => Helper::formatCurrencyOutput($asset->purchase_cost),
'user_can_checkout' => (bool) $asset->availableForCheckout(),
];


$permissions_array['available_actions'] = [
'checkout' => Gate::allows('checkout', Asset::class) ? true : false,
'checkin' => Gate::allows('checkin', Asset::class) ? true : false,
'update' => Gate::allows('update', Asset::class) ? true : false,
'delete' => Gate::allows('delete', Asset::class) ? true : false,
'checkout' => (bool) Gate::allows('checkout', Asset::class),
'checkin' => (bool) Gate::allows('checkin', Asset::class),
'update' => (bool) Gate::allows('update', Asset::class),
'delete' => (bool) Gate::allows('delete', Asset::class),
];

$array += $permissions_array;



if ($asset->model->fieldset) {

foreach($asset->model->fieldset->fields as $field) {
$fields_array = [$field->name => $asset->{$field->convertUnicodeDbSlug()}];
$array += $fields_array;
}

foreach ($asset->model->fieldset->fields as $field) {
$fields_array = [$field->name => $asset->{$field->convertUnicodeDbSlug()}];
$array += $fields_array;
}
}


return $array;
}

public function transformAssetsDatatable($assets) {
public function transformAssetsDatatable($assets)
{
return (new DatatablesTransformer)->transformDatatables($assets);
}



}
Loading

0 comments on commit e03ebc3

Please sign in to comment.