Skip to content

Fixed #15600: Adds extra verification step on checkin of device if enabled - #18538

Open
Glukose1 wants to merge 3 commits into
grokability:developfrom
Glukose1:develop
Open

Fixed #15600: Adds extra verification step on checkin of device if enabled#18538
Glukose1 wants to merge 3 commits into
grokability:developfrom
Glukose1:develop

Conversation

@Glukose1

Copy link
Copy Markdown

Fixes #15600

This PR implements a modal that appears when the check-in button is clicked on the asset's check-in page. The user must click a checkbox before they can check in the asset. This behaviour can be enabled or disabled in the general settings tab. The modal has two text fields. One for a general text message in the modal and one next to the checkbox. The text for these fields can also be defined on the general settings page.

Setting disabled:
image

Setting enabled, with text already entered:
image

New verification modal after clicking check-in on the asset page, the asset cannot be checked in:
image

Verification modal after checking the checkbox, the asset can now be checked in:
image

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Different values have been assigned for both settings, and both are saved correctly.
  • The Checkin process has been tested with the setting enabled and disabled.

Test Configuration:

  • PHP version: 8.4.16
  • MySQL version: 11.8.3-MariaDB
  • Webserver version: Apache/2.4.59 (Debian) & Internal Artisan Web Server
  • OS version: Debian GNU/Linux 13.3 (trixie)

Checklist:

@Glukose1
Glukose1 requested a review from snipe as a code owner February 11, 2026 14:49
@snipe

snipe commented Feb 11, 2026

Copy link
Copy Markdown
Member

I'm not sure we'd want this at a settings level though? Also why couldn't you use the checkbox in the required custom fields on checkin/checkout?

@Glukose1

Copy link
Copy Markdown
Author

The required custom field would work; however, I would need to ensure that every asset model where I want the additional verification has the fieldset assigned. For existing models, this is not a problem, but for newly created models, I see it being one. Especially when multiple people, with varying levels of experience, are using the system. Small things like this can easily be forgotten or overlooked in a hurry.

I personally prefer the simple solution of making it a global setting that does not need to be maintained by the user, as the custom fields would.
If people do not want the verification on every check-in, which I totally understand, they can turn off the setting, hiding the additional textboxes to keep the settings page as clean as possible. Your proposed solution using the custom fields remains available for more granular verification on checkin.

@codacy-production

codacy-production Bot commented May 7, 2026

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.

@Glukose1

Glukose1 commented May 8, 2026

Copy link
Copy Markdown
Author

Hey @snipe, I wanted to get back to this PR after trying your proposed solution using custom fields.

Unfortunately, it's not really working for us. Here are the main problems I encountered:

  • When trying to check in an asset where the checkbox is not checked, I am redirected to the Update Asset dialogue instead of the Checkin Asset dialogue. Editing the asset in this dialogue is not an option because Webhook notifications are not triggered when an asset is updated, and I require the Webhook Notification when checking an asset in.
  • Whenever I want to edit the asset, I have to check the checkbox because it is marked as required.
  • As a result of the above, when doing a check-in for an asset, the checkbox is prechecked. So I can just hit check in without being reminded to check that no Apple ID is logged in, which brings me back to my original problem.

If you don't like the exact behaviour of the feature, I am happy to discuss possible changes. However, I would greatly appreciate the introduction of this feature in some form.

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.

2 participants