yajra / laravel-datatables

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

addIndexColumn not using database primary keys #2661

Closed schonhoff closed 2 years ago

schonhoff commented 3 years ago

Summary of problem or feature request

Hello,

I have a problem with the addIndexColumn. In my case I want that the index column reference the id of the database object like it is described in the editor case (https://editor.datatables.net/examples/advanced/jsonId.html). DataTables Editor is using DT_RowId as a standard for setting the id attribute. If you use softdeletes or other deletes the row number isn't matching the id anymore. So in case of using datatables editor it will edit a completely other entry instead the right one.

Code snippet of problem

DataTables::of(Geodaten::with('postal_code'))
            ->addIndexColumn()
            ->toJson();

Example data: Geodaten table in database: id name 1 Munich 2 Berlin 3 Bremen 5 Hamburg

This would be result in this json code with addIndexColumn:

{
  data: [
    {DT_RowId: 1, id: 1, name: Munich},
    {DT_RowId: 2, id: 2, name: Berlin},
    {DT_RowId: 3, id: 3, name: Bremen},
    {DT_RowId: 4, id: 5, name: Hamburg},
  ]
}

There is now a mismatch for DT_RowId and id. That can cause plugins like editor to use a false model and update the wrong model. (This was in my case very painful to undo!)

System details

Possible solution:

It would be nice, if the addIndexColumn function is not exclusively used like this:

            if ($this->includeIndex) {
                $value[$indexColumn] = ++$this->start;
            }

in src/Processors/DataProcessor.php

Maybe there is a way to set the IndexColumn to the ID of a model. Or maybe another function?

Current workaround:

https://editor.datatables.net/examples/advanced/jsonId.html Like said in this example of datatables editor I can use another identifier to match the right model.

yajra commented 3 years ago

I think you should use ->setRowId('id') instead.

schonhoff commented 3 years ago

Hello @yajra ,

thanks for the very fast answer. The function ->setRowId('id') is the thing I needed. Thanks. Maybe you can reference it in your documentation here: https://yajrabox.com/docs/laravel-datatables/master/index-column Because it is mention before the setRowId. That would be awesome. Issue can be closed if you don't want to do the documentation change.

Thanks again and have a nice day!

yajra commented 3 years ago

Tagged for documentation. PR is welcome incase you can submit https://github.com/yajra/laravel-datatables-docs.

Thanks!

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] commented 2 years ago

This issue was closed because it has been inactive for 7 days since being marked as stale.