From 204140f34f915382d0a010ac8dfd4b87d4b074d8 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 29 Jul 2026 18:33:59 +0100 Subject: [PATCH] Fixed #19398 - restore simple location layout for dropdowns --- .../Controllers/Api/LocationsController.php | 21 +++------- app/Models/Location.php | 22 +++++----- .../Api/LocationsForSelectListTest.php | 42 +++++++++++++------ 3 files changed, 46 insertions(+), 39 deletions(-) diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index 6ceaff8ace..bcdd5cf597 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -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 { diff --git a/app/Models/Location.php b/app/Models/Location.php index 22fa805c60..3989bb59d7 100755 --- a/app/Models/Location.php +++ b/app/Models/Location.php @@ -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.'--')); } } diff --git a/tests/Feature/Locations/Api/LocationsForSelectListTest.php b/tests/Feature/Locations/Api/LocationsForSelectListTest.php index a7a2ad66a5..772c25a727 100644 --- a/tests/Feature/Locations/Api/LocationsForSelectListTest.php +++ b/tests/Feature/Locations/Api/LocationsForSelectListTest.php @@ -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.'); + } }