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

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

Open
fmcd opened this issue Feb 20, 2020 · 3 comments
Open

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

fmcd opened this issue Feb 20, 2020 · 3 comments

Comments

@fmcd
Copy link

fmcd commented Feb 20, 2020

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

yajra commented Feb 21, 2020

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

yajra commented Feb 21, 2020

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

fmcd commented Feb 21, 2020

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:
yajra/laravel-datatables-html#123

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

No branches or pull requests

2 participants