-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[5.7] Make any column searchable with like
in PostgreSQL (and fix Global Search in Laravel Nova)
#25698
Conversation
This new feature definitely needs tests. |
Yes, it requires tests. Just added assertions on grammar with |
The pull request targets Laravel 5.7, but since Nova works with Laravel 5.6 too, would it make sense to fix it in 5.6 too? |
return $this->whereBasicLike($query, $where); | ||
} | ||
|
||
$value = $this->parameter($where['value']); |
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.
You can use return parent::whereBasic($query, $where);
.
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.
If this is the case there is no point in overwriting the method at all.
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.
How do you mean? I meant that the lines 40 and 42 do exactly what the parent method does.
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.
Sorry, you pointed the wrong lines, forget what I said you are right.
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.
My comment points to line 40, but GitHub also shows the previous lines.
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.
Changed to parent call.
{ | ||
// Special treatment for like clauses. In order to make | ||
// any column searchable, we need to convert it to text. | ||
if (Str::contains($where['operator'], 'like')) { |
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.
strtolower($where['operator'])
to handle uppercase LIKE
.
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.
implemented and added an extra assertion for this case
/** | ||
* {@inheritdoc} | ||
* | ||
* @param Builder $query |
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.
The PHPDoc of both methods is incorrect, take a look at whereDate()
.
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.
Updated the docs, are they ok this way?
That’s right, after the if condition it does the same as the parent method.
I’ll add the parent call when I’m back at my laptop...
…--- edit:
rm unnecessary content
|
A request has been made to fix this in Laravel 5.6 too (see here). Will this get merged to 5.6 or should I close this PR and open a new one with a fix for 5.6? |
like
in PostgreSQL (and fix Global Search in Laravel Nova)like
in PostgreSQL (and fix Global Search in Laravel Nova)
Thanks for merging! \o/ |
This small change actually became necessary to make Global Search in Laravel Nova on PostgreSQL possible.
See related issue here: laravel/nova-issues#46
Postgres doesn't like to compare non text fields using like without strict type conversion, so we need to add it.