Skip to content
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

Asset Tag validation value #15429

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Robert-Azelis
Copy link
Contributor

Description

This change add validation of Asset Tag value to prevent put incorrect data and keep consistence.

For enabled auto-incremental option:

  • proposed new asset tag is generated on the base system settings with next incremental value

  • if provided asset tag start with prefix, then validate if provided value is compliant
    image

  • if asset tag not start with prefix, allowed is custom value
    image

For not enabled auto-increment option:

  • proposed new asset tag is generated on the base 1st row with keeping format
    image

Additionally:

  • max number of value is allowed up to 100000000000, above this value show error notification.
    image

  • allowed edit only latest tag, previous one is switch to mode read-only to protect value.
    image

Fixes #14829

Type of change

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

Test Configuration:

  • PHP version: 8.2.11
  • MySQL version
  • Webserver version: IIS
  • OS version: Windows server 2019
  • Browser: Google Chrome, MS Edge

Checklist:

Asset Tag validation value
Asset Tag validation error
Copy link

what-the-diff bot commented Aug 30, 2024

PR Summary

  • Enhanced Translation Support
    Two new translation strings were added to the resources/lang/en-US/general.php file to provide more comprehensive localization and improve user understanding.

  • Modifications to Asset Tagging Interface
    There were several changes made to the hardware edit view (in resources/views/hardware/edit.blade.php). The 'asset_tag' input field was updated for clarity, and a new element was added to display error messages more distinctively.

  • Addition & Improvements in Asset Tag Validation
    A new JavaScript function (asset_tag_valid) dedicated to asset tag validation was introduced. This function is called whenever the asset tag input field changes, enhancing the dynamic error checking.

  • Streamlined Asset Tag Handling
    Improvements in the JavaScript code to handle asset tagging include better handling during addition and removal of tags. This ensures continuous and logical increment of tags and further validation when tags are removed.

@snipe snipe requested a review from uberbrady September 9, 2024 03:00
@@ -26,6 +26,8 @@
'asset_report' => 'Asset Report',
'asset_tag' => 'Asset Tag',
'asset_tags' => 'Asset Tags',
'asset_tag_error' => 'Asset Tag number must be less then',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just be able to use the standard validation translations here.

</div>
<div class="col-md-2 col-sm-12">
<button class="add_field_button btn btn-default btn-sm">
<x-icon type="plus" />
<i class="fas fa-plus"></i>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might need to rebase? This would overwrite the blade component stuff I did a few weeks ago

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worked with previous version will check and update.

@@ -290,43 +290,213 @@ function user_add(status_id) {
$(document).ready(function() {

var max_fields = 100; //maximum input boxes allowed
var max_value = 100000000000; //maximum allowed value
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you determining that number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is maximum number where value is properly managed. Over it generated number can be incorrect.

box_html += '<div class="col-md-2 col-sm-12">';
box_html += '<a href="#" class="remove_field btn btn-default btn-sm"><x-icon type="minus" /></a>';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same regression for blade components here

Copy link
Owner

@snipe snipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few regressions here - but also I'm not 100% clear on what this is supposed to do. If you could clarify, I'd appreciate it!

@Robert-Azelis
Copy link
Contributor Author

A few regressions here - but also I'm not 100% clear on what this is supposed to do. If you could clarify, I'd appreciate it!

Hello, are you asking about general purpose of function or some parts of code?

@snipe
Copy link
Owner

snipe commented Sep 9, 2024

IIRC, calculating the max there isn't as simple as just a numeric value - otherwise we'd have already put in that validation. (Per #14607)

@Robert-Azelis
Copy link
Contributor Author

values over 10000000000000000 generate incorrect next tag numbers

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants