search

gearbox-solutions / eloquent-filemaker

A Model extension and Eloquent driver for Laravel connecting to FileMaker through the Data API
https://gearboxgo.com
MIT License
54 stars 16 forks source link

Bugfix/date casting #41

Closed macbookandrew closed 1 year ago

macbookandrew commented 1 year ago

The 1.0.1 release broke our usage of this with a date (not datetime) cast and date field.

This PR fixes and simplifies the date-casting logic, adds tests, and fixes some other failing tests as well.

Smef commented 1 year ago

What's the error you're getting? We've actually got a whole separate repository of tests, but this 1.0.1 change doesn't seem to have affected any of those and everything appears to be working correctly.

The casting logic check is definitely good, though.

macbookandrew commented 1 year ago

Saving a model to a layout with a date field resulted in this error:

BlueFeather\EloquentFileMaker\Exceptions\FileMakerDataApiException with message 'Date value does not meet validation entry options'

The actual data being sent for the date-cast property was 2/23/22023 12:00:00; with this change, it’s just 2/23/2023 and works correctly.

Smef commented 1 year ago

There were a few other situations in our tests where the fixes in here wouldn't have handled all cases, like custom datetime casts, so I couldn't merge this in, but you did have some good suggestions. I've reworked how it processes a bit and pushed up a new 1.0.2 build which should solve this.

We didn't have the right tests run before after the package name change, so some of these test failures were missed in the 1.0.1 release. Hopefully we'll be good going forward now. Thanks for your input on this.

macbookandrew commented 1 year ago

Thanks!

It would be great if you can incorporate those tests into this package so people can contribute more easily without breaking anything.

Smef commented 1 year ago

Yeah, we're looking at refactoring our private test repo to build that into the main package.