laravel / nova-issues

556 stars 34 forks source link

id should not be searchable without setup #1339

Closed jarda256sparktech closed 5 years ago

jarda256sparktech commented 5 years ago

Description:

If you have some nova resource and $search property set to ['reference','name']. Id(primary key) should not be searched even if integer is in search field.

ApplySearch method should contain check if key is in searchableColumns().

protected static function applySearch($query, $search)
    {
        return $query->where(function ($query) use ($search) {
            if (is_numeric($search) && in_array($query->getModel()->getKeyType(), ['int', 'integer']) && in_array($query->getModel()->getKeyType(),static::searchableColumns())) {
                $query->orWhere($query->getModel()->getQualifiedKeyName(), $search);
            }

            $model = $query->getModel();

            $connectionType = $query->getModel()->getConnection()->getDriverName();

            $likeOperator = $connectionType == 'pgsql' ? 'ilike' : 'like';

            foreach (static::searchableColumns() as $column) {
                $query->orWhere($model->qualifyColumn($column), $likeOperator, '%'.$search.'%');
            }
        });
    }
davidhemphill commented 5 years ago

Searching by ID is a feature we want included by default regardless of the other specified columns. Also removing this would be a breaking change, which we're not interested in introducing at this time.

shane-smith commented 5 years ago

This is the main thing that has irritated me since I first started using Nova. I updated again today (now 2.0.9) hoping that this was fixed, because it feels so much like a bug. The official documentation implies that the ID field can be easily overridden:

To define which resource fields are searchable, you may assign an array of database columns in the search property of your resource class. This includes id column by default, but you may override it to your needs: https://nova.laravel.com/docs/2.0/search/global-search.html#searchable-columns

This implies that specifying the search property will override the default, which is the ‘id’ field … not append to it. However what it actually does is just append the columns specified and keep the ID field, as mentioned in the comments above. So it’s not consistent with the documentation.

@davidhemphill - if you feel strongly that the behaviour should remain this way, may I please suggest:

davi5e commented 3 years ago

Is there a way to indicate that the ID should not be globally searchable?

Is the breaking change related to the global aspect of the global search bar or just related to the model?

I would think this would go a long to alleviate the issue of typing some meaningful number and getting a bunch of unrelated matches...

crynobone commented 3 years ago

@davi5e https://github.com/laravel/nova-issues/issues/2102#issuecomment-778767901

davi5e commented 3 years ago

The wonders of resuscitating an old thread and finding out the solution was implemented!

Thank you for the speedy reply @crynobone

deffjay commented 1 year ago

Here's my challenge of having the ID forced when searching against a Resource: I am searching for 'Winter 2024' within my resource. It's returning me all of the rows where the table ID has 2024 in it before any of the desired name based searches.

So: 20241 20242 20243 ....

Are matching before anything named Winter 2024. And because there are so many records that match based on ID, I have to go paging to find the content.

Hoping there might be some new thoughts on resurrecting this.

crynobone commented 1 year ago

Hey there,

Unfortunately, we don't support this version of the library anymore. Can you please try to upgrade to the latest version and see if your problem persists? If so, please open up a new issue and we'll help you out.

Thanks!