mirror/snipe-it
3
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2026-08-18 11:15:42 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #19403 from grokability/19401-fixed-checkin-location-choices

Browse Source 
Fixed #19401 - simplify checkin screen location choices
This commit is contained in:
snipe
2026-07-29 21:22:13 +01:00
committed by GitHub
parent 3e2a441ca5 dbed7122b8
commit 339617cdd3
3 changed files with 120 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
app/Http/Controllers/Assets/AssetCheckinController.php
View File

@ -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();

40
resources/views/hardware/checkin.blade.php
View File

@ -51,8 +51,8 @@
<x-form.row :label="trans('admin/hardware/form.model')" name="model_display" input_div_class="col-md-8">
<x-slot:input>
<p class="form-control-static">
@if (($asset->model) && ($asset->model->name))
{{ $asset->model->name }}
@if ($asset->model)
{!! $asset->model->present()->formattedNameLink !!}
@else
<span class="text-danger text-bold">
<x-icon type="warning" />
@ -67,6 +67,15 @@
</x-slot:input>
</x-form.row>
@if ($asset->defaultLoc)
{{-- Default Location (read-only) --}}
<x-form.row :label="trans('admin/hardware/form.default_location')" name="default_location" input_div_class="col-md-6">
<x-slot:input>
<p class="form-control-static"> {!! $asset->defaultLoc->present()->formattedNameLink() !!}</p>
</x-slot:input>
</x-form.row>
@endif
{{-- Asset name --}}
<x-form.row
:label="trans('general.name')"
@ -127,22 +136,29 @@
</div>
</div>
{{-- 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. --}}
<x-input.location-select
:label="trans('general.location')"
name="location_id"
:help_text="($asset->defaultLoc) ? trans('general.checkin_to_diff_location', ['default_location' => $asset->defaultLoc->name]) : null"
:selected="old('location_id')"
:selected="old('location_id', $asset->rtd_location_id)"
:company_id="$asset->company_id"
/>
{{-- Update actual location --}}
<x-form.radio-row
name="update_default_location"
selected="1"
:options="[
'1' => trans('admin/hardware/form.asset_location'),
'0' => trans('admin/hardware/form.asset_location_update_default_current'),
]"
<x-input.location-select
:label="trans('admin/hardware/form.default_location')"
name="rtd_location_id"
:selected="old('rtd_location_id', $asset->rtd_location_id)"
:company_id="$asset->company_id"
/>
{{-- Checkout/Checkin date. The nested input-group carries

62
tests/Feature/Checkins/Ui/AssetCheckinTest.php
View File

@ -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));