adonisjs
fix: compare DateTime in newUpIfMissing
🔗 Linked issue
#1018
❓ Type of change
- [x] 🐞 Bug fix (a non-breaking change that fixes an issue)
📚 Description
Resolves #1018
Fixes bug in updateOrCreateMany, fetchOrCreateMany and fetchOrNewUpMany (which uses newUpIfMissing) where it does not compare DateTime values correctly and therefore creates duplicate rows.
https://github.com/adonisjs/lucid/blob/develop/src/orm/base_model/index.ts#L212
📝 Checklist
- [x] I have linked an issue or discussion.
Hey! 👋🏻
Thanks for your contribution! It seems that the fail, may you link into it?
@RomainLanz updated. I had to change transformValue method so drivers get Date object instead in queries.
I see there is a bigger problem here because the newUpIfMissing will fail for any sort of any non-literal values.
Yes, we might be able to fix it for DateTime instances, but the same check will fail if some property holds an array of values or an object.
@thetutlage @RomainLanz I managed to get it working for all dialects. The comments are incorporated.
However this is not the cleanest solution when it comes to transforming the DateTime values, it should be ok for now. I think it would be best to transform all values passed to .where using the same prepare logic on column as when inserting the data. I can create a PR for that after this one, if it makes sense to you.
@adamcikado There is a type-checking error in the code. Can you please fix that and then we can release this change.
Sorry it took a while :)
@thetutlage Thank you and fixed!
Regarding what I mentioned about transforming values passed to .where() in the last comment, what's the best place to discuss it (if it makes sense to you)?
Regarding what I mentioned about transforming values passed to .where() in the last comment, what's the best place to discuss it (if it makes sense to you)?
I think the simplest option will be to add whereDate, whereDay, whereTime helpers in the query builder and do not perform automatic transformations.