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

Fix [FD-43836] PATCH of purchase_cost for assets for comma as decimal separator #15412

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

uberbrady
Copy link
Collaborator

A customer who was using the "1.234,56" number format was having problems doing a PATCH of the purchase_cost for assets.

When he tried to pass "12,34" he got an error that a number was required. When he used an actual float, as in {"purchase_cost": 12.34} - the period got stripped out and the value was changed to 1234.00.

We already do some magic on creating new assets to allow for this. But we were not doing the same magic when doing a PATCH for an existing asset.

I also decided - and maybe this is me getting a little ahead of myself - that if you actually pass in an actual Float via JSON, then that Float is probably what you wanted. That change is a little bit scarier (to me), since it changes the setPurchaseCost mutator in SnipeModel.

I added in some tests for the PATCH method for US and EU formats, for string-numbers and real floats, making sure they still work as expected.

Copy link

what-the-diff bot commented Aug 29, 2024

PR Summary

  • Incorporated relevant application model in UpdateAssetRequest.php
    The update asset request file now makes use of the Setting model in our application. This means that it can now directly interact with application settings related aspects more efficiently.

  • Added conditional parameters in the rules() method
    This change has been applied to the UpdateAssetRequest.php file. This supports more precise evaluation of conditions when an asset update request is created.

  • Enhancement to the setPurchaseCostAttribute() method
    Modifications were made to the SnipeModel.php file, specifically enhancing the setPurchaseCostAttribute() method. This allows for a more rigorous assignment of purchase costs to assets.

  • New test-cases for updating Purchase Cost
    Several test-cases have been added to the UpdateAssetTest.php file. These were specifically designed to test the new and improved asset update request handling, particularly focusing on the purchase cost updating functionality. This will aid us in ensuring our new features work as expected.

@snipe snipe merged commit ba291ed into snipe:develop Aug 29, 2024
5 of 9 checks passed
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