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

Fixed threshold explanation and min qty math #14601

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

Conversation

Godmartinz
Copy link
Collaborator

@Godmartinz Godmartinz commented Apr 15, 2024

Description

This changes the min qty on Asset models, Components, Accessories, Consumables and Licenses to 'Increase Alert Threshold' giving a better understanding of how this value will interact with the Settings threshold. The Helper text has been updated as well. Also the way the min_amt is calculated in the notification center has been updated to total the asset model and settting threshold values.

A Helper method was made to better handle the sum for transformers.

image Fixes #14604 [sc-25227]

Type of change

Please delete options that are not relevant.

  • [ x] Bug fix (non-breaking change which fixes an issue)

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

  • Test A
  • Test B

Test Configuration:

  • PHP version:
  • MySQL version
  • Webserver version
  • OS version

Checklist:

Copy link

what-the-diff bot commented Apr 15, 2024

PR Summary

  • Adding Low Inventory Alert Threshold Feature
    An enhancement has been added to the Helper.php file. Now, our system will check for low inventory by comparing minimum amounts with a newly introduced alert threshold.
  • Dependency Addition In Controller
    To facilitate this, AssetModelsController.php is now linked to the Setting model. This helps us access all application settings directly from the controller.
  • Enhancing Edit Method in Controller
    In the same file (AssetModelsController.php), additional 'setting' data has been attached to the 'view'. This allows more data to be sent to the webpage when someone is editing an asset model.
  • Updating User Interface
    In the edit.blade.php file, new user interface elements such as a label for the 'Increase Alert Threshold' and a corresponding input field have been introduced. Standard threshold values are now also displayed for better context.
  • Added Tooltip
    For ease of understanding, a tooltip and information icon have been added next to the input field inside edit.blade.php. This tooltip provides helpful hints about what the input field is for.
  • Translation Updates
    The explanatory text associated with the minimum amount field (min_amt_help) in the general.php language file has been updated for better clarity and understanding.

@Godmartinz Godmartinz changed the title fixed threshold explanation and min qty math Fixed threshold explanation and min qty math Apr 15, 2024
@snipe
Copy link
Owner

snipe commented Apr 18, 2024

Wouldn't we want to apply this to all of the spots where we use minimum quantities?

@Godmartinz
Copy link
Collaborator Author

@snipe changes applied to the partial blade. controllers and transformers updated as well 🙂

@snipe snipe requested a review from uberbrady April 27, 2024 02:46
Copy link
Collaborator

@marcusmoore marcusmoore left a comment

Choose a reason for hiding this comment

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

Code looks good 👍🏾

(didn't run in browser since we have other reviewers checking it out as well)

Comment on lines +1461 to +1464
public static function sumThreshold($min_qty = null){

return $min_qty + Setting::getSettings()->alert_threshold;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a hunch this method name can be improved but I don't have a good suggestion...

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.

None yet

3 participants