-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
WIP Adds bulk transfer ability during bulk checkout of Assets #18060
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
base: develop
Are you sure you want to change the base?
Conversation
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.
A couple notes from me skimming the code and then trying it out.
It feels weird to mix and match bulk checkout and bulk transfer in one action to me. Under the hood they are very similar so maybe I'm just getting hung up on it but it seems like there should be two menu items: one for bulk checkout and one for transfer. If we keep it under one menu item then we should let the user know which items they have selected are currently checked out and will be transferred. Just for their information.
| public function onAssetsTransferredInBulk(AssetsTransferredInBulk $event): void | ||
| { | ||
| Log::debug('event passed to the listener:'); | ||
| $event->transferable->logCheckout($event->note, $event->transferredTo, $event->transferable->last_checkout, $event->originalValues, true); |
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.
Hit an exception here: Property [last_checkout] does not exist on this collection instance.
| * 3. The item should send an email at check-in/check-out | ||
| */ | ||
|
|
||
| if (Context::get('action') === 'transfer') { |
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.
This should probably be bulk_transfer right?
| * 3. The item should send an email at check-in/check-out | ||
| */ | ||
|
|
||
| if (Context::get('action') === 'transfer') { |
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.
This should probably be bulk_transfer right?
This is still a work in progress, but take a look as I think the bones of this PR are good.