Skip to content
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

Database exception raised when sort field doesn't exist #36

Open
kurucu opened this issue May 5, 2021 · 2 comments
Open

Database exception raised when sort field doesn't exist #36

kurucu opened this issue May 5, 2021 · 2 comments

Comments

@kurucu
Copy link

kurucu commented May 5, 2021

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
Copy link
Author

kurucu commented May 5, 2021

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).

@lorisleiva
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants