-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix Query on whereDate, whereDay, whereMonth, whereYear #2376
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
Conversation
…respective query rather than using basic comparison
(+) fix whereDate operator for month and year
Any update for this one? |
Hello, Please remove styleci changes as this isn't related to this PR. I don't see any check runs for this PR? Thanks! |
tests/QueryTest.php
Outdated
$this->assertCount(5, $month); | ||
} | ||
|
||
public function testWhereYear(): void | ||
{ | ||
$year = Birthday::whereYear('year', '2021')->get(); | ||
$year = Birthday::whereYear('birthday', 2021)->get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't modify current tests to see if it's breaking current applications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as for this, it is breaking change.
Why?
its from comparing between string (current whereYear etc) to comparing datetime (on mongodb) with string/int
So what do i need to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
Just don't touch current tests and add additional to see if it changes in behavior.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Davpyu can you revert old code and additional test cases ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, i've reverted this. Please let me know if there are any commit that need to be reverted
Okay i'll update after working hours |
Hello @Davpyu, Can you please update or I should take care of it? Thanks! |
im sorry for not responding so long because busy with work, yes you can take care of this PR if you want it |
This reverts commit c02bca5.
Hello, i've reverted the styleci for this PR. |
Hello @Davpyu, thanks for your work on this feature. I'm taking over your PR in #2572 to get it merged in the next release.
I found a solution using |
Closing in favour of #2572. |
Fix issues #2334
So basically, current whereDate, whereDay, whereMonth and whereYear is query on different column with specific need (like whereDate you need column that has value YYYY-MM-DD etc) using basic comparison.
As we all know that isnt how whereDate works on sql, this PR will generate query using built in
$dayOfMonth, $month, $year
on whereDay, whereMonth, and whereYear, while whereDate basically generate range date(since mongodb doesnt have date() equivalent like on sql).Example:
Collection on mongodb
Eloquent if this merged
Note: