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 crashing on date formatting helper when value is not actually a date #11249

Merged
merged 4 commits into from
Jun 3, 2022

Conversation

snipe
Copy link
Owner

@snipe snipe commented Jun 3, 2022

This just wraps the Carbon method in a try/catch to prevent crashing on bad data, so if the $date value isn't actually valid, we don't crash out completely.

While this shouldn't typically happen since we validate dates before entering them into the database (and we use date/datetime fields for native fields in the system), it is a possible scenario that a custom field could be created as an "ANY" field, data gets added, and then the custom field format gets edited to DATE or DATETIME later. If someone put bad data in the database before then - or if they manually edited the field's value - it will crash with:

DateTime::__construct(): Failed to parse time string (TBV) at position 0 (T): The timezone could not be found in the database (View: /Users/snipe/Sites/snipe-it/snipe-it/resources/views/hardware/view.blade.php)

This will now not crash and will instead return an error that the date is invalid.

Screen Shot 2022-06-03 at 3 37 08 PM

While I find it ugly to return a string (instead of false, etc), this method was flat-out crashing as it is now (when there was bad data). A further refractor might be in order down the line, but this at least fixes the immediate problem.

snipe added 4 commits June 3, 2022 15:25
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Copy link
Collaborator

@uberbrady uberbrady left a comment

Choose a reason for hiding this comment

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

This makes a ton of sense - I agree there's a slight wonkitude to returning a string (and a non-internationalized one, at that), but this is still much better than uncaught exceptions all over the place. I think this is going to make our lives easier, and our user's lives easier. Nice work!

@snipe snipe merged commit be0933a into develop Jun 3, 2022
@snipe snipe deleted the fixes/better_handle_bad_date_values branch June 6, 2022 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants