Skip to content

🎥 Bulk Checkout / Checkin: Rejection Warning Side Panel + Related Cleanup - #19322

Merged
snipe merged 4 commits into
developfrom
__nicer-rejection-panel-on-bulk-actions
Jul 20, 2026
Merged

🎥 Bulk Checkout / Checkin: Rejection Warning Side Panel + Related Cleanup#19322
snipe merged 4 commits into
developfrom
__nicer-rejection-panel-on-bulk-actions

Conversation

@snipe

@snipe snipe commented Jul 20, 2026

Copy link
Copy Markdown
Member

This PR moves the "these assets were removed from your selection" warning on bulk checkin and bulk checkout out of the inline form body and into a reusable side-panel component that lives in the right-hand column. Cleans up the surrounding checkout-target-panel width handling so both side panels line up visually. It also migrates the last two inline <script> blocks on the bulk pages into snipeit.js as data-attribute driven handlers, which is part of the broader plan to eventually clean out all of those inline <script> tags to make way for us being able to transition to vite (see PR #19317).

It also fixes a "Do not change" validation regression on bulk checkout status and applies a small N+1 quick fix to the shared asset-select partial that any checkout or checkin page pays for.

All changes are presentation, validation-rule, and query adjustments, nothing on the backend controllers.

New reusable side-panel component

Before

Screenshot 2026-07-20 at 11 54 41 AM

After

Screen.Recording.2026-07-20.at.11.12.31.AM.mov

Note: This seems slow because in recording this, that's when I noticed the N+1 - and also because the recently-modified blade components hadn't been cached yet. I also might move some stuff around, as info-panel and side-panel seem confusingly similar, even though they serve two different purposes here.

resources/views/blade/side-panel/removed-assets.blade.php (new) is a <x-side-panel.removed-assets> component that renders a warning box listing assets that were pulled from the operator's selection because they can't participate in the current bulk action (already assigned on checkout, already unassigned on checkin). We were already doing this via the alert box, but I think this way looks a lot cleaner and is more consistent with the newer side-panel stuff we've been adding.

The table layout mirrors the existing checkout-target-panel so the two side panels read consistently when stacked. The component intentionally sets no col-md-* on its root, so the blade controls the width via the surrounding <x-page-column>.

  • hardware/bulk-checkin.blade.php renders the warning in its own right-hand col-md-5 column.
  • hardware/bulk-checkout.blade.php stacks the warning above the <livewire:checkout-target-panel> in the shared col-md-5 right column. The warning stays visible for the whole session so the operator always knows which assets got rejected. Target panel below reacts to the checkout target selection as before.

Checkout target panel no longer uses its own column wrapper

Previously <livewire:checkout-target-panel> rendered a root <div class="col-md-5"> and blades relied on that to act as the second column of a two-column layout. On bulk-checkout the panel now needs to sit inside a blade-controlled column (alongside the removed-assets warning) which caused nested col-md-* padding accumulation and a visibly indented panel. The <livewire:checkout-target-panel> was only recently added though, so don't worry about it if it's news to you. 😂

Fix:

  • app/Livewire/CheckoutTargetPanel.php no longer accepts or stores a $rootClass.
  • resources/views/livewire/checkout-target-panel.blade.php root element dropped to a plain <div> with no column class.
  • Every non-bulk blade (accessories, components, consumables, hardware, kits, licenses checkout.blade.php) now wraps the panel in <x-page-column class="col-md-5">. Its behavior is visually identical to before for those pages, since the blade-provided wrapper replaces the panel's previous self-emitted wrapper.

Inline script migration on bulk pages

Two inline <script> blocks removed from the bulk pages. Behavior moved into snipeit.js as data-attribute driven handlers (again, in an ongoing effort to get us more able to potentially switch to vite):

  • [data-disable-empty-on-submit] on a form disables empty required inputs on submit so browser HTML5 validation does not block the request before Laravel form-request validation gets a chance to return a nicer error. Non-required empty selects (like "Do not change") pass through with their intentional empty value. This is the reason the "Do not change" bug below existed and was masked.
  • [data-autofocus-select2-search] on a form focuses the first .select2-search__field on load and hides the results dropdown until the operator starts typing, so bulk checkout lands directly in the assets picker. This was previously done in jQuery, and was primarily a benefit for folks using barcode scanners.

Supporting change: resources/views/blade/form/index.blade.php now forwards {{ $attributes }} to the rendered <form> element. Without this, blades could not attach data-* attributes to a form via <x-form>.

"Do not change" for status on bulk checkout

Two contradictions that combined to reject the "Do not change" option even though the controller prepends it as a valid choice:

  1. resources/views/hardware/bulk-checkout.blade.php marked the status select required. That is wrong because the controller (BulkAssetsController::edit) prepends '' => trans('general.do_not_change') as the first option. The required attribute was dropped from the select.
  2. app/Http/Requests/AssetCheckoutRequest.php had 'status_id' => 'exists:status_labels,id,deployable,1' with no nullable. An empty string still triggered the exists check, which fails since no status_label has id="". Added nullable so empty status_id (the "Do not change" intent) skips the rule. The controller's existing if ($request->filled('status_id')) guard already handles the empty case as "do not change the status," so no controller change was needed.

Asset-select partial N+1 quick fix

resources/views/partials/forms/edit/asset-select.blade.php was hitting three queries per asset in its @foreach($asset_ids) loop:

  1. Asset::find($asset_id) for the truthy check.
  2. Asset::find($asset_id) again for the presenter call.
  3. AssetPresenter::fullName() triggered a models table query.

I rewrote both to use @if ($asset = Asset::with('model')->find($asset_id)). One query per iteration instead of three. Still N+1, but the per-asset cost drops to a third and the model relation comes back on the same query. That select is at least paginated, so the change should speed things up and limit the N+1 messiness. (Should still revisit that later, but that seemed too big for this PR because the partial has eight blades that use it across checkout, checkin, edit, and maintenance flows. Messing with the checkout controls was just too spooky for this PR.)

The full fix I think I want to do (whereIn+get, single query for the whole set) can go in a class-backed <x-input.asset-select> component.

Manual Testing

  • Bulk checkout page loads. Assets picker is auto-focused on load. Results dropdown stays hidden until the operator types.
  • Bulk checkout with $removed_assets populated shows the warning panel in the top of the right column. Warning stays visible after picking a checkout target.
  • Bulk checkout picking a target populates the "Assets checked out to this user / asset / location" panel BELOW the warning. Widths match. No visual indent on the target panel.
  • Bulk checkout picking "Do not change" for status and any valid target submits successfully.
  • Bulk checkout with a status picked but no target selected surfaces the correct validation error (not the "status required" error the old behavior produced).
  • Bulk checkin page loads with a two-column layout. Right column is empty unless $removed_assets has entries. Warning shows when it does.
  • Single-asset checkout (hardware/checkout) still renders the checkout-target-panel at the correct width when the operator picks a target.
  • Single-item checkouts on accessories, components, consumables, kits, licenses all render the target panel at the correct width. No visual regression on any of the six pages.
  • DevTools console is clean on bulk-checkin and bulk-checkout on initial load and after picking a target. No Can't find variable: $ or similar errors.
  • DebugBar query count on a bulk-checkout page loaded with 20+ pre-selected assets is meaningfully lower than before (should drop to roughly one third of prior count for the asset-select fetch section). (You'll want to check the /hardware/bulkcheckout entry in the debugbar, NOT the last ajax/livewire that gets loaded.)

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@snipe
snipe merged commit 65e0494 into develop Jul 20, 2026
9 checks passed
@snipe
snipe deleted the __nicer-rejection-panel-on-bulk-actions branch July 20, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant