feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
14.97k stars 742 forks source link

fix(knex): Fix Knex adapter date comparison queries #3429

Closed doug-patterson closed 4 months ago

doug-patterson commented 4 months ago

Summary

Other Information

The issue here is that a date is an object, so the code in knex/adapter tries to get the query operators as its keys and of course finds none so it just moves on. The fix here just lets a date be a date in comparisons, too.

Problem I of course want to include some unit tests for this. However, the typing in the tests somehow has been set up with Omit called on the enum for acceptable JSON Schema types - you can't make a json schema with a property that has { type: 'date' } to feed into the the query syntax, so it's impossible to construct a proper test involving dates. I'll 'fess up and admit that I tried to figure this out for several hours on Monday but couldn't.

If someone who knows the codebase better than I do can point me to where { type: 'date' } is ruled out by the typing in there I'll get this finished. Apologies for not finding it myself but that's how it is and I'm hoping someone with more knowledge than me can help me with it.

Thanks! And thanks also to @daffl and all contributors for making the best of all API frameworks in any language!

daffl commented 4 months ago

Thank you for the pull request. This should be fine since the change seems pretty safe and all tests are still passing. Released in v5.0.22

doug-patterson commented 4 months ago

Thanks @daffl - I'll hope to contribute again soon!