Logging Audit on Checkin/out #16679

akemidx · 2025-04-09T23:34:09Z

Adding in an option to log an audit when you check in or out an asset.

There is less information on the audit itself, but all changes to an asset will be logged on the checkin or out.

NOTE: This action does not allow a user to manually set a next audit date OR upload a picture as these are not options on checkin or checkout

what-the-diff · 2025-04-09T23:34:24Z

PR Summary

Improvement in Asset Tracking
The system now records all instances where assets are either checked in or out. This enhanced tracking feature includes new mechanisms for verifying user permissions and keeping track of these actions in AssetCheckinController and AssetCheckoutController.
Enhanced User Interface Language
New entries have been introduced to the language files to support the newly introduced audit feature. This includes new text for checkboxes and additional help text to guide users through the process.
Check-In View Modification
The check-in form now features a checkbox that allows enabling or disabling the audit log. Its display is subject to the respective user's permissions.
Check-Out View Modification
Much like the check-in view, the check-out form also features a checkbox that governs the audit log. It too is displayed depending on the permissions granted to the user.

resources/views/hardware/checkin.blade.php

snipe · 2025-04-10T14:37:04Z

app/Http/Controllers/Assets/AssetCheckoutController.php

@@ -71,6 +71,10 @@ public function store(AssetCheckoutRequest $request, $assetId) : RedirectRespons

            $admin = auth()->user();

+            if($request->filled('audit_on_checkinout') == "1") {
+                $this->authorize('audit', Asset::class);


This will probably result in a forbidden error, which is probably not what we want. We probably want to quietly ignore if they don't have permission to audit.

okay yea. i see how that could happen.

unsure how to fix this. originally thought was if the checkbox is filled, then check if they can do it, but don't check if its not a checked box. i am stumped on reversing that logic. may just need to write it out on my whiteboard. reverse causality and such.

We should hide the checkbox in the blade if they're not allowed to audit (using @can) and then check the Gate in the controller.

i think i did this already while i was mulling over how to fix it. if you could double check the more recent changes?

resources/views/hardware/checkout.blade.php

snipe · 2025-04-16T20:38:01Z

app/Http/Controllers/Assets/AssetCheckinController.php

@@ -75,6 +76,12 @@ public function store(AssetCheckinRequest $request, $assetId = null, $backto = n

        $this->authorize('checkin', $asset);

+        if(Gate::allows('audit',$asset)) {
+            if ($request->filled('log_audit') == "1") {
+                $this->authorize('audit', Asset::class);


This duplicates the gate right above, and on failure will cause a forbidden error.

I don't see a gate above it? only the $this->authorize('checking',$asset);
then everything else in the store method is just checking for the asset to exist?

line 79 is what I put there? so you're saying the $this->authorize is what duplicates the gate ya?

snipe · 2025-04-16T20:38:27Z

app/Http/Controllers/Assets/AssetCheckinController.php

@@ -133,7 +140,9 @@ function (Builder $query) use ($asset) {
        $asset->customFieldsForCheckinCheckout('display_checkin');

        if ($asset->save()) {
-
+            if($request->filled('log_audit') == "1") {


This is where the gate belongs.

snipe · 2025-04-16T20:38:40Z

app/Http/Controllers/Assets/AssetCheckoutController.php

@@ -71,6 +72,12 @@ public function store(AssetCheckoutRequest $request, $assetId) : RedirectRespons

            $admin = auth()->user();

+            if(Gate::allows('audit',$asset)) {
+                if ($request->filled('log_audit') == "1") {
+                    $this->authorize('audit', Asset::class);


Same thing here

snipe · 2025-04-16T20:38:59Z

app/Http/Controllers/Assets/AssetCheckoutController.php

@@ -114,6 +121,9 @@ public function store(AssetCheckoutRequest $request, $assetId) : RedirectRespons
            session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]);

            if ($asset->checkOut($target, $admin, $checkout_at, $expected_checkin, $request->get('note'), $request->get('name'))) {
+                if($request->filled('log_audit') == "1") {


snipe · 2025-04-16T20:39:22Z

resources/lang/en-US/admin/settings/general.php

@@ -283,6 +283,8 @@
    'require_accept_signature_help_text'      => 'Enabling this feature will require users to physically sign off on accepting an asset.',
    'require_checkinout_notes'  => 'Require Notes on Checkin/Checkout',
    'require_checkinout_notes_help_text'    => 'Enabling this feature will require the note fields to be populated when checking in or checking out an asset.',
+    'log_audit' => 'Mark as audited',
+    'log_audit_help_text' => 'Check this box to also log an audit in addition to this',


This phrasing seems a little clunky to me.

It does, but words are not my strong suit.
However, this seems clunky since at the END after 'this' it will say checkin or checkout

example:
{{ trans('admin/settings/general.log_audit_help_text') . " " . trans('general.checkin') . "." }}

See my example in the PR review comment. I suggested different phrasing altogether :)

snipe · 2025-04-16T20:39:45Z

resources/views/settings/general.blade.php

@@ -253,7 +253,6 @@
                   </div>
                   <!-- /.form-group -->

-


Try not to commit changed files that only contain whitespace please.

funny story about this, Marcus actually said to ignore it cause it was removing empty white space. but yes, I understand not wanting that one extra file to be changed.

snipe

This is a good start, but I'd like to see a few changes.

There's no real reason why we can't include the audit image upload and next audit date on that form. If the checkbox is checked, we show those fields. If it's unchecked, we hide them.

Additionally, I think I'd prefer the UI to look a little more like:

[no visible label] [x] Also log an audit

Once that checkbox is clicked, the image upload, audit note, and next audit date fields are shown.

akemidx · 2025-04-16T21:27:45Z

okay, overall made the requested changed, but I think we still would get the 403 on the checkbox check->authroize yea?

I'll keep thinking about it.

akemidx · 2025-04-16T21:50:05Z

okay, overall made the requested changed, but I think we still would get the 403 on the checkbox check->authroize yea?

I'll keep thinking about it.

wait, wouldn't that just get removed, and only check for the gate/authorization like on the save. cause THAT is where the actual log action is happening.

snipe · 2025-04-16T21:55:58Z

app/Http/Controllers/Assets/AssetCheckinController.php

@@ -75,6 +76,10 @@ public function store(AssetCheckinRequest $request, $assetId = null, $backto = n

        $this->authorize('checkin', $asset);

+        if ($request->filled('log_audit') == "1") {
+            $this->authorize('audit', Asset::class);


This will cause a forbidden error. We don't need the entire block in this section.

akemidx added 5 commits April 9, 2025 16:52

beginning of audit on checkin/out

be0fdb3

less overthinking

26aa3ee

checkboxes on the checkin/out screen

0b288ed

operational stuffhoohasdss

02357a8

removing unneededdnessnesneed

1943bcd

akemidx requested a review from snipe as a code owner April 9, 2025 23:34

probot-autolabeler bot added backend frontend translations labels Apr 9, 2025

snipe reviewed Apr 10, 2025

View reviewed changes

resources/views/hardware/checkin.blade.php Outdated Show resolved Hide resolved

snipe reviewed Apr 10, 2025

View reviewed changes

resources/views/hardware/checkout.blade.php Outdated Show resolved Hide resolved

akemidx added 3 commits April 10, 2025 17:47

requested variable changes

6617fa9

forgot the other bits

cc686b1

trying to avoid a 403 when checking if the log_audit is chosen

2b54fbb

akemidx requested a review from snipe April 16, 2025 19:20

snipe reviewed Apr 16, 2025

View reviewed changes

snipe requested changes Apr 16, 2025

View reviewed changes

akemidx added 2 commits April 16, 2025 17:09

fixing white space

802f03e

changing Gate location

6f2d314

snipe reviewed Apr 16, 2025

View reviewed changes

js script

4cad217

akemidx added 2 commits April 16, 2025 18:15

js script

e9be4ef

removing unneeded blocks

ea94123

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logging Audit on Checkin/out #16679

Logging Audit on Checkin/out #16679

akemidx commented Apr 9, 2025

what-the-diff bot commented Apr 9, 2025

snipe Apr 10, 2025

akemidx Apr 10, 2025

akemidx Apr 15, 2025

snipe Apr 16, 2025 •

edited

Loading

akemidx Apr 16, 2025

snipe Apr 16, 2025

akemidx Apr 16, 2025

snipe Apr 16, 2025

akemidx Apr 16, 2025

snipe Apr 16, 2025

snipe Apr 16, 2025

snipe Apr 16, 2025

snipe Apr 16, 2025

akemidx Apr 16, 2025 •

edited

Loading

snipe Apr 16, 2025

snipe Apr 16, 2025

akemidx Apr 16, 2025

snipe left a comment

akemidx commented Apr 16, 2025

akemidx commented Apr 16, 2025

snipe Apr 16, 2025

Logging Audit on Checkin/out #16679

Are you sure you want to change the base?

Logging Audit on Checkin/out #16679

Conversation

akemidx commented Apr 9, 2025

what-the-diff bot commented Apr 9, 2025

PR Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snipe Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akemidx Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snipe left a comment

Choose a reason for hiding this comment

akemidx commented Apr 16, 2025

akemidx commented Apr 16, 2025

Choose a reason for hiding this comment

snipe Apr 16, 2025 •

edited

Loading

akemidx Apr 16, 2025 •

edited

Loading