diff --git a/app/Http/Controllers/Assets/AssetCheckinController.php b/app/Http/Controllers/Assets/AssetCheckinController.php index 1929a99f62..7e58b812d6 100644 --- a/app/Http/Controllers/Assets/AssetCheckinController.php +++ b/app/Http/Controllers/Assets/AssetCheckinController.php @@ -15,7 +15,6 @@ use App\Models\Statuslabel; use Illuminate\Contracts\View\View; use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\RedirectResponse; -use Illuminate\Support\Facades\Log; class AssetCheckinController extends Controller { @@ -138,28 +137,40 @@ class AssetCheckinController extends Controller $this->migrateLegacyLocations($asset); - $asset->location_id = $asset->rtd_location_id; - - if ($request->has('location_id')) { - if ($request->filled('location_id')) { - // Resolve via the scoped Location query so non-existent IDs and IDs the actor - // cannot see under FMCS are rejected before we write them to the asset. - $submittedLocation = Location::find($request->input('location_id')); - - if (! $submittedLocation) { - return redirect()->back()->withInput() - ->with('error', trans('admin/hardware/message.create.target_not_found.location')); + // The checkin form ships two location pickers (location_id and + // rtd_location_id), both pre-populated with the asset's existing + // rtd_location_id. The common case (submit without touching either + // field) reproduces the codebase-wide checkin convention: current + // location resets to rtd, default stays put. Fixes #19401. + // + // Both are resolved through the scoped Location query so + // non-existent IDs and IDs the actor cannot see under FMCS are + // rejected before we write them. + foreach (['location_id', 'rtd_location_id'] as $field) { + if (! $request->has($field)) { + // Field wasn't in the submitted form at all (test / API + // caller). Fall back to the pre-fix "reset location to rtd" + // convention for location_id; leave rtd untouched. + if ($field === 'location_id') { + $asset->location_id = $asset->rtd_location_id; } - Log::debug('NEW Location ID: '.$submittedLocation->id); - $asset->location_id = $submittedLocation->id; - if ($request->input('update_default_location') == 0) { - $asset->rtd_location_id = $submittedLocation->id; - } - } else { - // Explicitly submitted as empty — clear the location - $asset->location_id = null; + continue; } + + if (! $request->filled($field)) { + // User cleared the picker via select2's X button. + $asset->{$field} = null; + + continue; + } + + $submittedLocation = Location::find($request->input($field)); + if (! $submittedLocation) { + return redirect()->back()->withInput() + ->with('error', trans('admin/hardware/message.create.target_not_found.location')); + } + $asset->{$field} = $submittedLocation->id; } $originalValues = $asset->getRawOriginal(); diff --git a/resources/views/hardware/checkin.blade.php b/resources/views/hardware/checkin.blade.php index d3887b7f76..85a90ec2da 100755 --- a/resources/views/hardware/checkin.blade.php +++ b/resources/views/hardware/checkin.blade.php @@ -51,8 +51,8 @@

- @if (($asset->model) && ($asset->model->name)) - {{ $asset->model->name }} + @if ($asset->model) + {!! $asset->model->present()->formattedNameLink !!} @else @@ -67,6 +67,15 @@ + @if ($asset->defaultLoc) + {{-- Default Location (read-only) --}} + + +

{!! $asset->defaultLoc->present()->formattedNameLink() !!}

+
+
+ @endif + {{-- Asset name --}} + {{-- Location and default-location pickers. Both are + pre-populated with the asset's rtd_location_id so + the common case (submit without touching either + field) resets `location` to rtd and leaves the + default unchanged, matching the codebase-wide + checkin convention. Fixes #19401, where a blank + submission used to wipe `location` to null. + + Users can override either field by picking a + different location or clearing via the select2 X + button. --}} - {{-- Update actual location --}} - {{-- Checkout/Checkin date. The nested input-group carries diff --git a/tests/Feature/Checkins/Ui/AssetCheckinTest.php b/tests/Feature/Checkins/Ui/AssetCheckinTest.php index 88f81af7a3..4c171bf174 100644 --- a/tests/Feature/Checkins/Ui/AssetCheckinTest.php +++ b/tests/Feature/Checkins/Ui/AssetCheckinTest.php @@ -180,6 +180,62 @@ class AssetCheckinTest extends TestCase $this->assertHasTheseActionLogs($asset, ['create', 'checkin from']); } + /** + * Regression coverage for #19401. Pre-fix, submitting the checkin form + * with the location dropdown blank (which is how a browser submits an + * empty select) wiped the asset's current location to null instead of + * resetting it to rtd_location_id like the field's own helper text + * implied. The new form ships both pickers pre-populated with + * rtd_location_id, so a no-touch submit sends both values back and + * the asset lands at rtd. This test posts the same shape as the browser + * would after a no-touch submit. + */ + public function test_checkin_with_prepopulated_location_lands_at_rtd_not_null() + { + $rtdLocation = Location::factory()->create(); + $userLocation = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create([ + 'location_id' => $userLocation->id, + 'rtd_location_id' => $rtdLocation->id, + ]); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', [$asset]), [ + 'location_id' => $rtdLocation->id, + 'rtd_location_id' => $rtdLocation->id, + ]); + + $fresh = $asset->refresh(); + $this->assertNotNull($fresh->location_id, 'Current location must not be wiped to null on checkin (#19401).'); + $this->assertTrue($fresh->location()->is($rtdLocation), 'Current location should land at rtd_location on default checkin.'); + $this->assertTrue($fresh->defaultLoc()->is($rtdLocation), 'Default location should be unchanged when picker submitted with same rtd value.'); + } + + /** + * The user can explicitly clear either location picker (via select2's + * X button), which posts an empty string for that field. Empty string + * for a submitted field is distinct from "field not present" — it means + * "clear this location." + */ + public function test_checkin_clears_location_when_picker_submitted_empty() + { + $rtdLocation = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create([ + 'location_id' => Location::factory()->create()->id, + 'rtd_location_id' => $rtdLocation->id, + ]); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', [$asset]), [ + 'location_id' => '', + 'rtd_location_id' => $rtdLocation->id, + ]); + + $fresh = $asset->refresh(); + $this->assertNull($fresh->location_id, 'Explicit empty submission of the picker should clear the location.'); + $this->assertTrue($fresh->defaultLoc()->is($rtdLocation), 'Default location should be untouched.'); + } + public function test_checkin_rejects_nonexistent_location_id() { $rtdLocation = Location::factory()->create(); @@ -224,10 +280,14 @@ class AssetCheckinTest extends TestCase $location = Location::factory()->create(); $asset = Asset::factory()->assignedToUser()->create(); + // Post-#19401 the checkin form ships both `location_id` and + // `rtd_location_id` as independent pickers, so updating the default + // location just means submitting the desired value on that field. + // Replaces the older `update_default_location=0` flag semantic. $this->actingAs(User::factory()->checkinAssets()->create()) ->post(route('hardware.checkin.store', [$asset]), [ 'location_id' => $location->id, - 'update_default_location' => 0, + 'rtd_location_id' => $location->id, ]); $this->assertTrue($asset->refresh()->defaultLoc()->is($location));