yajra / laravel-datatables

jQuery DataTables API for Laravel
https://yajrabox.com/docs/laravel-datatables
MIT License
4.74k stars 862 forks source link

global search fails when column name with relation has underscore #2324

Open fmcd opened 4 years ago

fmcd commented 4 years ago

Global search failing because EagerLoads collection contains relation names in CamelCase, whereas the founction is passing in the actual table name from the column name. e.g.

column name = child_table.name

Name in EagerLoad collection = childTable

compileQuerySearch in EloquentDataTable.php fails because it needs to convert relation name to camcel Case before calling isNotEagerLoaded:

suggest adding Str::camel call as follows:

protected function compileQuerySearch($query, $columnName, $keyword, $boolean = 'or')
    {
        $parts    = explode('.', $columnName);
        $column   = array_pop($parts);
        $relation = Str::camel( implode('.', $parts) );    // Added convert to camelCase

        if ($this->isNotEagerLoaded( $relation )) {
            return parent::compileQuerySearch($query, $columnName, $keyword, $boolean);
        }

        $query->{$boolean . 'WhereHas'}($relation, function (Builder $query) use ($column, $keyword) {
            parent::compileQuerySearch($query, $column, $keyword, '');
        });
    }

Same issue could be fixed in resolveRelationColumn:

protected function resolveRelationColumn($column)
    {
        $parts      = explode('.', $column);
        $columnName = array_pop($parts);
        $relation   = Str::camel( implode('.', $parts) );   // Added convert to camelCase

        if ($this->isNotEagerLoaded($relation)) {
            return $column;
        }

        return $this->joinEagerLoadedColumn($relation, $columnName);
    }
yajra commented 4 years ago

I think this is an issue that can be fixed by setting the proper name on JS. Something like below.

columns: [
    ...
    {data: 'child_table.name', name: 'childTable.name'}
]

-- Edit --

Found the docs: https://datatables.yajrabox.com/eloquent/relationships

If your relations consists of (2) two or more words, the convention to use is as follows:

{data: 'relation_name.column', name: 'relationName.column'}

yajra commented 4 years ago

On the other hand, I think we can consider adding Str::camel so that it would work either way? Can you please submit a PR.


columns: [
    ...
    {data: 'child_table.name', name: 'childTable.name'}
]

// might work if we add Str::camel
columns: [
    ...
    {data: 'child_table.name'}
]
fmcd commented 4 years ago

After considering your comments, I think perhaps it is better that the HTML builder applies the camel case when it assumed a default 'name' attribute for columns. Please see my PR in the laravel-datatables-html repo: https://github.com/yajra/laravel-datatables-html/pull/123