diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index 3aa895ef45..1e04fa1344 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -8,6 +8,8 @@ use App\Helpers\Helper; use App\Models\Location; use App\Http\Transformers\LocationsTransformer; use App\Http\Transformers\SelectlistTransformer; +use Illuminate\Pagination\LengthAwarePaginator; +use Illuminate\Support\Collection; class LocationsController extends Controller { @@ -26,7 +28,7 @@ class LocationsController extends Controller 'updated_at','manager_id','image', 'assigned_assets_count','users_count','assets_count','currency']; - $locations = Location::with('parent', 'manager', 'childLocations')->select([ + $locations = Location::with('parent', 'manager', 'children')->select([ 'locations.id', 'locations.name', 'locations.address', @@ -109,7 +111,7 @@ class LocationsController extends Controller public function show($id) { $this->authorize('view', Location::class); - $location = Location::with('parent', 'manager', 'childLocations') + $location = Location::with('parent', 'manager', 'children') ->select([ 'locations.id', 'locations.name', @@ -181,6 +183,27 @@ class LocationsController extends Controller /** * Gets a paginated collection for the select2 menus * + * This is handled slightly differently as of ~4.7.8-pre, as + * we have to do some recursive magic to get the hierarchy to display + * properly when looking at the parent/child relationship in the + * rich menus. + * + * This means we can't use the normal pagination that we use elsewhere + * in our selectlists, since we have to get the full set before we can + * determine which location is parent/child/grandchild, etc. + * + * This also means that hierarchy display gets a little funky when people + * use the Select2 search functionality, but there's not much we can do about + * that right now. + * + * As a result, instead of paginating as part of the query, we have to grab + * the entire data set, and then invoke a paginator manually and pass that + * through to the SelectListTransformer. + * + * Many thanks to @uberbrady for the help getting this working better. + * Recursion still sucks, but I guess he doesn't have to get in the + * sea... this time. + * * @author [A. Gianotto] [] * @since [v4.0.16] * @see \App\Http\Transformers\SelectlistTransformer @@ -192,25 +215,39 @@ class LocationsController extends Controller $locations = Location::select([ 'locations.id', 'locations.name', + 'locations.parent_id', 'locations.image', ]); + $page = 1; + if ($request->filled('page')) { + $page = $request->input('page'); + } + if ($request->filled('search')) { - $locations = $locations->where('locations.name', 'LIKE', '%'.$request->get('search').'%'); + \Log::debug('Searching... '); + $locations = $locations->where('locations.name', 'LIKE', '%'.$request->input('search').'%'); } - $locations = $locations->orderBy('name', 'ASC')->paginate(50); + $locations = $locations->orderBy('name', 'ASC')->get(); - // Loop through and set some custom properties for the transformer to use. - // This lets us have more flexibility in special cases like assets, where - // they may not have a ->name value but we want to display something anyway + $locations_with_children = []; foreach ($locations as $location) { - $location->use_text = $location->name; - $location->use_image = ($location->image) ? url('/').'/uploads/locations/'.$location->image : null; + if(!array_key_exists($location->parent_id, $locations_with_children)) { + $locations_with_children[$location->parent_id] = []; + } + $locations_with_children[$location->parent_id][] = $location; } - return (new SelectlistTransformer)->transformSelectlist($locations); + $location_options = Location::indenter($locations_with_children); + + $locations_formatted = new Collection($location_options); + $paginated_results = new LengthAwarePaginator($locations_formatted->forPage($page, 500), $locations_formatted->count(), 500, $page, []); + + //return []; + return (new SelectlistTransformer)->transformSelectlist($paginated_results); } + } diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index 23091de859..b3df9efaff 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -57,14 +57,7 @@ class LocationsController extends Controller public function create() { $this->authorize('create', Location::class); - $locations = Location::orderBy('name', 'ASC')->get(); - - $location_options_array = Location::getLocationHierarchy($locations); - $location_options = Location::flattenLocationsArray($location_options_array); - $location_options = array('' => 'Top Level') + $location_options; - return view('locations/edit') - ->with('location_options', $location_options) ->with('item', new Location); } @@ -130,14 +123,8 @@ class LocationsController extends Controller return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist')); } - // Show the page - $locations = Location::orderBy('name', 'ASC')->get(); - $location_options_array = Location::getLocationHierarchy($locations); - $location_options = Location::flattenLocationsArray($location_options_array); - $location_options = array('' => 'Top Level') + $location_options; - return view('locations/edit', compact('item')) - ->with('location_options', $location_options); + return view('locations/edit', compact('item')); } @@ -227,7 +214,7 @@ class LocationsController extends Controller if ($location->users->count() > 0) { return redirect()->to(route('locations.index'))->with('error', trans('admin/locations/message.assoc_users')); - } elseif ($location->childLocations->count() > 0) { + } elseif ($location->children->count() > 0) { return redirect()->to(route('locations.index'))->with('error', trans('admin/locations/message.assoc_child_loc')); } elseif ($location->assets->count() > 0) { diff --git a/app/Http/Transformers/LocationsTransformer.php b/app/Http/Transformers/LocationsTransformer.php index 5d93db4def..b418ebb5f8 100644 --- a/app/Http/Transformers/LocationsTransformer.php +++ b/app/Http/Transformers/LocationsTransformer.php @@ -23,7 +23,7 @@ class LocationsTransformer if ($location) { $children_arr = []; - foreach($location->childLocations as $child) { + foreach($location->children as $child) { $children_arr[] = [ 'id' => (int) $child->id, 'name' => $child->name diff --git a/app/Models/Location.php b/app/Models/Location.php index d0cbec3fdc..d6139ddebd 100755 --- a/app/Models/Location.php +++ b/app/Models/Location.php @@ -113,7 +113,8 @@ class Location extends SnipeModel public function parent() { - return $this->belongsTo('\App\Models\Location', 'parent_id','id'); + return $this->belongsTo('\App\Models\Location', 'parent_id','id') + ->with('parent'); } public function manager() @@ -121,9 +122,9 @@ class Location extends SnipeModel return $this->belongsTo('\App\Models\User', 'manager_id'); } - public function childLocations() - { - return $this->hasMany('\App\Models\Location', 'parent_id'); + public function children() { + return $this->hasMany('\App\Models\Location','parent_id') + ->with('children'); } // I don't think we need this anymore since we de-normed location_id in assets? @@ -137,59 +138,37 @@ class Location extends SnipeModel return $this->attributes['ldap_ou'] = empty($ldap_ou) ? null : $ldap_ou; } - public static function getLocationHierarchy($locations, $parent_id = null) - { + /** + * Query builder scope to order on parent + * + * @param Illuminate\Database\Query\Builder $query Query builder instance + * @param text $order Order + * + * @return Illuminate\Database\Query\Builder Modified query builder + */ - $op = array(); - - foreach ($locations as $location) { - - if ($location['parent_id'] == $parent_id) { - $op[$location['id']] = - array( - 'name' => $location['name'], - 'parent_id' => $location['parent_id'] - ); - - // Using recursion - $children = Location::getLocationHierarchy($locations, $location['id']); - if ($children) { - $op[$location['id']]['children'] = $children; - } - - } - + public static function indenter($locations_with_children, $parent_id = null, $prefix = '') { + $results = Array(); + + if (!array_key_exists($parent_id, $locations_with_children)) { + return []; } - return $op; + + foreach ($locations_with_children[$parent_id] as $location) { + $location->use_text = $prefix.' '.$location->name; + $location->use_image = ($location->image) ? url('/').'/uploads/locations/'.$location->image : null; + $results[] = $location; + //now append the children. (if we have any) + if (array_key_exists($location->id, $locations_with_children)) { + $results = array_merge($results, Location::indenter($locations_with_children, $location->id,$prefix.'--')); + } + } + return $results; } - public static function flattenLocationsArray($location_options_array = null) - { - $location_options = array(); - foreach ($location_options_array as $id => $value) { - // get the top level key value - $location_options[$id] = $value['name']; - - // If there is a key named children, it has child locations and we have to walk it - if (array_key_exists('children', $value)) { - - foreach ($value['children'] as $child_id => $child_location_array) { - $child_location_options = Location::flattenLocationsArray($value['children']); - - foreach ($child_location_options as $child_id => $child_name) { - $location_options[$child_id] = '--'.$child_name; - } - } - - } - - } - - return $location_options; - } /** * Query builder scope to order on parent diff --git a/resources/views/locations/edit.blade.php b/resources/views/locations/edit.blade.php index 253a069ce0..7edfb3b6d1 100755 --- a/resources/views/locations/edit.blade.php +++ b/resources/views/locations/edit.blade.php @@ -10,16 +10,8 @@ @section('inputFields') @include ('partials.forms.edit.name', ['translated_name' => trans('admin/locations/table.name')]) - -
- -
- {!! Form::select('parent_id', $location_options , Input::old('parent_id', $item->parent_id), array('class'=>'select2 parent', 'style'=>'width:350px')) !!} - {!! $errors->first('parent_id', ' :message') !!} -
-
+ +@include ('partials.forms.edit.location-select', ['translated_name' => trans('admin/locations/table.parent'), 'fieldname' => 'parent_id']) @include ('partials.forms.edit.user-select', ['translated_name' => trans('admin/users/table.manager'), 'fieldname' => 'manager_id'])