kdion4891 / laravel-livewire-tables

A dynamic, responsive Laravel Livewire table component with searching, sorting, checkboxes, and pagination.
302 stars 41 forks source link

Sortable columns with JSON path #5

Closed mikemand closed 4 years ago

mikemand commented 4 years ago

Hi Kevin,

I'm wondering if you would find it useful to be able to do sorting on a column's JSON path? For example, with Laravel I can sort using the following:

$models->orderBy('total->amount');

Where the total column is JSON with a Money object ({"amount": 23500, "currency": "USD"}) stored.

I have an implementation already working on my fork if you want to see: https://github.com/mikemand/laravel-livewire-tables/commit/8d9ccb3a11a86dee493d644499433654604f23d8

My only problem, however, is how MySQL (I don't know about other RDBMSes) sorts numeric values. It is very literal, so 11 comes before 2 (e.g.: 1, 11, 3, 5, 7, 9). I could solve this by checking if the $sort_attribute contains -> and then using:

$models->orderByRaw("cast({$sort_attribute}->'$.{$jsonPath}' as unsigned) {$this->sort_direction}")

instead of the typical orderBy, but I don't know if you want this kind of complexity in your package? What are your thoughts?

kdion4891 commented 4 years ago

This seems too specific to be included in the package.

Perhaps there could be a new column method that lets you specify a callback for ordering it.

mikemand commented 4 years ago

Interesting idea. How would you go about getting the Column from the $sort_attribute in the models() method? Just a simple loop over $this->columns() comparing attributes?

kdion4891 commented 4 years ago

Prob

mikemand commented 4 years ago

Hi Kevin,

Here's an attempt at sorting with a callback: https://github.com/mikemand/laravel-livewire-tables/commit/9f28eabc385e4f41aa15af6a4a9b859326874043 We could potentially use Laravel's container to resolve the callback, rather than call_user_func, in case someone needs to inject some dependency for their sorting callback. I also thought about adding a method for the sorting callback but couldn't settle on a name (and can't use the same name as the method in this case, since we want to execute it as a function). Maybe sortUsing()? I think Laravel has a few *Using() methods, so we'd be following the lead there.

I've noticed you don't use docblocks or typehints, so if you want me to remove them I can do that. Do you have a style guide you want me to follow? My IDE keeps wanting to reformat the else blocks, lol.

mikemand commented 4 years ago

Hi Kevin,

I've updated my branch to use a new method called sortUsing() instead of hijacking the sortable() method. I've also updated the documentation: https://github.com/mikemand/laravel-livewire-tables/tree/feature/custom-sorting-callback#sortusingcallback

Any thoughts or comments? Otherwise, I'll open up a PR.

kdion4891 commented 4 years ago

Please make a PR.

Also you need to make sure there is no possibility of SQL injections, so use ? in your raw query with the appended parameter array.

mikemand commented 4 years ago

Thank you for the note about SQL injection. I totally overlooked that. Typically when I use the *Raw methods, I don't allow user input (other than already sanitized data from the database). Livewire will keep me on my toes for sure.

kdion4891 commented 4 years ago

Closing. Update PR when you’ve got time.