laravel-search-string icon indicating copy to clipboard operation
laravel-search-string copied to clipboard

Database exception raised when sort field doesn't exist

Open kurucu opened this issue 4 years ago • 2 comments

When I set up laravel-search-string, and then use a search string e.g. "sort:invalid-field" then a database exception is raised (Undefined column: 7 ERROR: column table.invalid-field).

I don't think I've done anything wrong (but it's obviously possible).

As search string is exposed to the user, I don't think invalid input should result in an exception, but should fail gracefully. If the native exception can't be caught and handled, then perhaps a the "sortable" columns should be set in the model config?

Note that my fail strategy is set to "all-results".

kurucu avatar May 05 '21 06:05 kurucu

So I can catch the exception and do something like the following. However, I still feel that the default behaviour should be to catch and silently fail in this scenario (or at least according to the fail strategy defined in the config).

try {
    $posts = $client->posts()->usingSearchString($this->search)->paginate(20);
} catch (\Exception $e) {
    $posts = $client->posts()->paginate(20);
    $this->addError('search', 'The search failed, please check the search string');
}

An enhancement might be to observe the config (i.e. catch the exception when asked) but raise a flag on the collection in some way so that it can be presented (i.e. allow simplification of the code above).

kurucu avatar May 05 '21 06:05 kurucu

Hi there 👋

That's a good point and I think we could improve that by adding a check prior to building the query but it would also mean that users have to whitelist the columns that can be sorted through. Which, to be honest, is probably not a bad thing since we might not want users to sort through columns that are not indexed for example.

lorisleiva avatar Jun 17 '21 19:06 lorisleiva