-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature - Improve assets bulk checkin/checkout #15479
base: develop
Are you sure you want to change the base?
Feature - Improve assets bulk checkin/checkout #15479
Conversation
One thing that has prevented us from handling this in the past is the JS selectivity based on the status of the object, For example, if you select all but have a mix of checked in and checkout assets, we should probably deselect the ones that are not eligible for the action selected. The UX on that gets a little squirrely IMHO. What if you accidentally selected the wrong option, now everything you previously selected gets un-selected. We can handle this on the backend, of course - the same way we do with other bulk things, where we let you select it, but once you get to the confirmation interstitial, we tell you why we can't do the thing you're trying to do. We already send the "checkoutability" etc through the API which is handled through the formatters, and determines whether you see an enabled/disabled checkin/checkout button, so this isn't exactly a hard problem to solve, but accidental clicks could definitely be a friction point. |
PR Summary
|
@@ -524,6 +524,10 @@ public function selectlist(Request $request) : array | |||
$assets = $assets->RTD(); | |||
} | |||
|
|||
if ($request->filled('assetStatusType') && $request->input('assetStatusType') === 'Deployed') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already handle this case here I think.
status
has a switch case for Deployed
on line 239
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure to get the point, as index() and selectlist() functions are not calling each other. Or perhaps that's your point? Calling index() from selectlist()?
Hi @snipe, thanks for your quick feedback. There is a lot of code change highlighted, despite some parts are just reindented. I hope it won't be too painful to read :) You are right about such mix can occur when selecting non checkable (in or out) assets. Dynamically unselecting them will definitely need to notify the user. Perhaps such notifications can immediately appear in the Status table for non eligible items, explaining why they don't appear in the assets tag input? I will have a deeper look at the "checkoutability" calls and other bulk actions to find out simple ways to clarify this. |
570db09
to
036d17e
Compare
f390d0f
to
bcaf15d
Compare
Replicate the assets selection method of Bulk Checkout to the Quick Scan Checkin view: - API: filter dynamic assets list to show only Deployed assets Add Checkin and Checkout as Bulk actions for selected items: - API: add ability to handle assets tag or id as parameters to prefill the asset tags list - Scripts: share commun javascript functions by moving them to snipe-it.js - Do an initial checkability verification on selected assets Add additional fields to be set on Bulk checkin: Status, Checkin Date Ability to scan assets tags is preserved.
bcaf15d
to
86220a9
Compare
I resumed development of this PR several days ago after a long stalling period. I also noticed that in the meantime some work on it has been done separately in #15680. So far, there are some substantial differences and additions:
Below are some screenshots of the implemented modifications: |
Description
Needs review/improvements: So far, you can expect some things to be broken.
Main changes
Replicate the assets selection method (dynamic select2 field) of Bulk Checkout to the Quick Scan Checkin view:
Add Checkin and Checkout as Bulk actions for selected items in assets index, asset / user views:
Add additional fields to be set on Bulk checkin: Status, Checkin Date
Ability to scan assets tags is preserved.
Current development status
Screenshots
New Bulk Checkout action
Bulk Checkin result
Bulk Checkout result
Bulk Checkin/Checkout in assets, users and kit views
Linked feature requests: #14466
Type of change
How Has This Been Tested?
Usual use of the bulk checkin/checkout process.
Checklist: