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

[8.0] Added optional merge of config raw columns to rawColumns method #1960

Merged
merged 3 commits into from
Jan 29, 2019

Conversation

Spodnet
Copy link
Contributor

@Spodnet Spodnet commented Jan 16, 2019

This pull request allows the optional merging of the raw columns defined in the config, and any new ones that you may want to add.

From my point of view, it seems odd that rawColumns was removing the ones defined in config, we use the ones defined on config as default across all tables, e.g. actions, so setting rawColumn(['state']) caused actions to disappear.

The pull request is written for backwards compatiblity, it won't merge unless true is passed.

Example:

In config

    /*
     * Default columns definition of dataTable utility functions.
     */
    'columns' => [
        
        /*
         * List of columns that are allowed to display html content.
         * Note: Adding columns to list will make us available to XSS attacks.
         */
        'raw' => ['action'],

Existing functionality

    return DataTables::eloquent($model)
                ->addColumn('link', '<a href="#">Html Column</a>')
                ->addColumn('action', 'path.to.view')
                ->rawColumns(['state'])
                ->toJson();

Results in raw columns as just state, the action column is no longer a raw column.

New merge functionality

    return DataTables::eloquent($model)
                ->addColumn('link', '<a href="#">Html Column</a>')
                ->addColumn('action', 'path.to.view')
                ->rawColumns(['state'], true)
                ->toJson();

Results in raw columns as state and action.

A variation in this idea could be a new method rawColumnsMerge, so existing method is left alone, and a new that just does merge could be used.

Copy link
Owner

@yajra yajra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with some minor typo adjustments. Thanks!

@@ -249,12 +249,21 @@ public function escapeColumns($columns = '*')
/**
* Set columns that should not be escaped.
*
* Optionally merge the defaults from config
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing period . and please remove the new line before this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No probs, updated.

@yajra yajra merged commit 909adca into yajra:8.0 Jan 29, 2019
@yajra yajra changed the title Added optional merge of config raw columns to rawColumns method [8.0] Added optional merge of config raw columns to rawColumns method Jan 29, 2019
@yajra
Copy link
Owner

yajra commented Jan 29, 2019

Released on v8.13.4, thanks! 🚀

@tegos
Copy link

tegos commented Feb 24, 2021

it is not clear. seems this method with second param true should stack raw columns if have several calls of this method, but it will stacked only with config raw columns, the previouse call will be ignored.

$datatables = datatables($query);
$raw_columns = ['category', 'article', 'name', 'image'];
$datatables->rawColumns($raw_columns, true);
// another manipulation
$raw_columns = ['warehouse'];
$datatables->rawColumns($raw_columns, true);

which result will be for raw columns ?

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

Successfully merging this pull request may close these issues.

3 participants