-
-
Notifications
You must be signed in to change notification settings - Fork 197
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
fix: compare DateTime in newUpIfMissing #991
Conversation
Hey! ππ» Thanks for your contribution! |
@RomainLanz updated. I had to change transformValue method so drivers get |
I see there is a bigger problem here because the 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. |
0dcbaa9
to
54033e0
Compare
@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 |
@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)? |
I think the simplest option will be to add |
π Linked issue
#1018
β Type of change
π Description
Resolves #1018
Fixes bug in
updateOrCreateMany
,fetchOrCreateMany
andfetchOrNewUpMany
(which usesnewUpIfMissing
) 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