mirror of
https://github.com/snipe/snipe-it.git
synced 2026-08-18 03:06:23 +00:00
Fixed #19398 - restore simple location layout for dropdowns
This commit is contained in:
@ -506,22 +506,13 @@ class LocationsController extends Controller
|
||||
}
|
||||
|
||||
if ($request->filled('search')) {
|
||||
// Search results are cherry-picked out of the tree so the
|
||||
// pre-search Location::indenter walk cannot be reused as-is.
|
||||
// Instead, walk each match's parent chain and inline the
|
||||
// ancestors with the same `›` breadcrumb separator that
|
||||
// Location::indenter uses on the tree-order branch, so both
|
||||
// views share one visual style: `DC1 › Rack 1` distinguishes
|
||||
// Rack 1 under DC1 from Rack 1 under DC2.
|
||||
$locations->load('parent');
|
||||
// Search results are cherry-picked out of the tree — no useful
|
||||
// indent depth to apply — so just use the plain name. The user
|
||||
// is filtering by typed text so context comes from the search
|
||||
// term rather than dropdown position (see #19398 for why we
|
||||
// stopped inlining the parent chain here too).
|
||||
foreach ($locations as $location) {
|
||||
$chain = [$location->name];
|
||||
$ancestor = $location->parent;
|
||||
while ($ancestor) {
|
||||
array_unshift($chain, $ancestor->name);
|
||||
$ancestor = $ancestor->parent;
|
||||
}
|
||||
$location->use_text = implode(' › ', $chain);
|
||||
$location->use_text = $location->name;
|
||||
}
|
||||
$locations_formatted = $locations;
|
||||
} else {
|
||||
|
||||
@ -374,13 +374,13 @@ class Location extends SnipeModel
|
||||
* level". Using 0 (not null) avoids PHP 8.4's deprecation of null array
|
||||
* offsets when callers build the map from `$location->parent_id`.
|
||||
*
|
||||
* `$prefix` is the accumulated breadcrumb path of the current node's
|
||||
* ancestors. Empty at the top-level call, then each recursion passes down
|
||||
* the full parent chain so every entry's `use_text` shows the whole path
|
||||
* (e.g. `DC1 › Rack 1 › Rack 1a`) rather than just an indent-depth marker.
|
||||
* The `›` separator matches the parent-chain breadcrumb rendered in the
|
||||
* location info-panel at resources/views/blade/info-panel/index.blade.php
|
||||
* so the select2 dropdown and the info-panel display are visually unified.
|
||||
* `$prefix` is the accumulated indent marker for the current recursion
|
||||
* depth (two dashes per level), so `use_text` reads e.g.
|
||||
* `-- Rack 1` or `---- Rack 1a` in the dropdown. Locations can nest
|
||||
* arbitrarily deep and the previous `A › B › C` breadcrumb form produced
|
||||
* unreadable long strings for even modestly nested hierarchies
|
||||
* (see #19398). Company::indenter keeps the breadcrumb form because its
|
||||
* hierarchy is capped at one parent level and reads cleanly there.
|
||||
*/
|
||||
public static function indenter($locations_with_children, int $parent_id = 0, $prefix = '')
|
||||
{
|
||||
@ -391,15 +391,13 @@ class Location extends SnipeModel
|
||||
}
|
||||
|
||||
foreach ($locations_with_children[$parent_id] as $location) {
|
||||
$breadcrumb = $prefix === ''
|
||||
$location->use_text = $prefix === ''
|
||||
? $location->name
|
||||
: $prefix.' › '.$location->name;
|
||||
|
||||
$location->use_text = $breadcrumb;
|
||||
: $prefix.' '.$location->name;
|
||||
$location->use_image = ($location->image) ? Storage::disk('public')->url('locations/'.$location->image) : null;
|
||||
$results[] = $location;
|
||||
if (array_key_exists($location->id, $locations_with_children)) {
|
||||
$results = array_merge($results, self::indenter($locations_with_children, $location->id, $breadcrumb));
|
||||
$results = array_merge($results, self::indenter($locations_with_children, $location->id, $prefix.'--'));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -56,13 +56,13 @@ class LocationsForSelectListTest extends TestCase
|
||||
->assertOk();
|
||||
}
|
||||
|
||||
public function test_search_result_shows_parent_chain_in_breadcrumb(): void
|
||||
public function test_search_result_shows_plain_names_without_parent_chain(): void
|
||||
{
|
||||
// Two data centers each with their own rack. Location::name is
|
||||
// `unique_undeleted` today so two children literally named
|
||||
// "Rack 1" cannot coexist, but the disambiguation the breadcrumb
|
||||
// provides is still valuable whenever the child names share a
|
||||
// prefix or the tree is deep.
|
||||
// Per #19398, the location dropdown reverted from the breadcrumb
|
||||
// form (`DC1 › RackA`) to plain indentation. Search results are
|
||||
// cherry-picked out of the tree so there's no depth to indent by;
|
||||
// they render as plain names and the user's search term supplies
|
||||
// the disambiguation context.
|
||||
$dc1 = Location::factory()->create(['name' => 'DC1']);
|
||||
$dc2 = Location::factory()->create(['name' => 'DC2']);
|
||||
Location::factory()->create(['name' => 'RackA', 'parent_id' => $dc1->id]);
|
||||
@ -73,14 +73,14 @@ class LocationsForSelectListTest extends TestCase
|
||||
->assertOk();
|
||||
|
||||
$texts = collect($response->json('results'))->pluck('text');
|
||||
$this->assertTrue($texts->contains('DC1 › RackA'));
|
||||
$this->assertTrue($texts->contains('DC2 › RackB'));
|
||||
$this->assertTrue($texts->contains('RackA'));
|
||||
$this->assertTrue($texts->contains('RackB'));
|
||||
}
|
||||
|
||||
public function test_search_result_walks_multiple_ancestor_levels(): void
|
||||
public function test_search_result_for_deeply_nested_match_shows_plain_name(): void
|
||||
{
|
||||
// Deeper tree: HQ > DC1 > Rack 1. The chain should show every
|
||||
// ancestor level.
|
||||
// Deeper tree: HQ > DC1 > Rack 1. Only the matched leaf's name
|
||||
// renders in the search result — no ancestor chain.
|
||||
$hq = Location::factory()->create(['name' => 'HQ']);
|
||||
$dc1 = Location::factory()->create(['name' => 'DC1', 'parent_id' => $hq->id]);
|
||||
Location::factory()->create(['name' => 'Rack 1', 'parent_id' => $dc1->id]);
|
||||
@ -90,7 +90,7 @@ class LocationsForSelectListTest extends TestCase
|
||||
->assertOk();
|
||||
|
||||
$texts = collect($response->json('results'))->pluck('text');
|
||||
$this->assertTrue($texts->contains('HQ › DC1 › Rack 1'));
|
||||
$this->assertTrue($texts->contains('Rack 1'));
|
||||
}
|
||||
|
||||
public function test_search_result_for_top_level_location_has_no_prefix(): void
|
||||
@ -106,4 +106,22 @@ class LocationsForSelectListTest extends TestCase
|
||||
$texts = collect($response->json('results'))->pluck('text');
|
||||
$this->assertTrue($texts->contains('Standalone Site'));
|
||||
}
|
||||
|
||||
public function test_unsearched_dropdown_uses_dash_indentation_for_nested_locations(): void
|
||||
{
|
||||
// Pins the reverted-to-old-style dropdown display. Root shows plain
|
||||
// name, children get "-- " prefix, grandchildren "---- ", etc.
|
||||
$hq = Location::factory()->create(['name' => 'HQ']);
|
||||
$dc1 = Location::factory()->create(['name' => 'DC1', 'parent_id' => $hq->id]);
|
||||
Location::factory()->create(['name' => 'Rack 1', 'parent_id' => $dc1->id]);
|
||||
|
||||
$response = $this->actingAsForApi(User::factory()->createUsers()->create())
|
||||
->getJson(route('api.locations.selectlist'))
|
||||
->assertOk();
|
||||
|
||||
$texts = collect($response->json('results'))->pluck('text');
|
||||
$this->assertTrue($texts->contains('HQ'), 'Top-level location renders without indent prefix.');
|
||||
$this->assertTrue($texts->contains('-- DC1'), 'One-level-deep location gets a two-dash indent.');
|
||||
$this->assertTrue($texts->contains('---- Rack 1'), 'Two-levels-deep location gets a four-dash indent.');
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user