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

Added validation for last_audit_date and next_audit_date #14472

Merged
merged 2 commits into from Mar 25, 2024

Conversation

snipe
Copy link
Owner

@snipe snipe commented Mar 21, 2024

This should fix FD-41210, where the last audit date wasn't being correctly stored on API asset creation. I'm not 100% sure we want to add this though? We don't allow you to manually override that in the asset create/edit screen, since we want you to actually perform an audit, not just make dates up, but I can see a use case for both scenarios.

Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Copy link

what-the-diff bot commented Mar 21, 2024

PR Summary

  • Refinement of Asset Controller Responses
    The responses from the 'store' and 'update' methods in AssetsController.php have been updated. This improvement involves the use of the AssetsTransformer class to process asset data before it's sent back as a response. This means the data returned to the user will be more consistent and easier to interpret.

  • Addition of Audit Date Fields in Asset Model Validation
    In the Asset.php model, two new fields - last_audit_date and last_audit_date - have been included in the validation rules. This enhancement ensures that these critical dates are correctly captured when assets are created or updated, helping maintain the accuracy of our asset management.

@snipe snipe requested review from uberbrady and marcusmoore and removed request for uberbrady March 21, 2024 19:06
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.

I'm not 100% sure we want to add this though

It's documented that this is a valid field but I'm also not sure if that's ideal.

Regarding the test failure (yay we caught a breaking API change!)...it is caused by this line:

'last_audit_date' => '2023-09-03',

Changing it to 'last_audit_date' => '2023-09-03 00:00:00', would make it pass but the api docs say that field should be a date so changing it to a date time would be breaking.

I think a better approach would be to append the time like we do on this line:

$checkin_at = $request->filled('checkin_at') ? $request->input('checkin_at').' '. date('H:i:s') : date('Y-m-d H:i:s');

@ShaunNeighbourSwale
Copy link

Hi. I think it was my support request which flagged this issue. For context, I don't specifically want to create a false audit date, but currently the only way to trigger the Audit Interval is by specifying an initial audit date. If the Audit interval was initially triggered by the asset creation date, that would make more sense.

Currently, I'm using /hardware/audit to set next_audit_date one year from either creation or last update, but this is also populating last_audit_date which is not ideal, but at least makes the "Due for Audit" page display something for my techs.

@snipe snipe merged commit 3f812f6 into develop Mar 25, 2024
4 of 8 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.

None yet

3 participants