Allow defaultSort to accept Livewire

[archilex]

Description

This PR allows defaultSort() to accept Livewire as a closure.

return $table
     ->defaultSort(function (Component $livewire) {
          return $livewire->activeTab === 'inStock' ? 'qty' : 'created_at';
     })

Of course passing in Builder or a string continues to work as well.

Note: my first attempt was to keep the original condition and then evaluate the closure inside getDefaultSortQuery(), however applyDefaultSortingToTableQuery() calls getDefaultSortColumn() before getDefaultSortQuery() so we have to evaluate there. This turned out to be cleaner anyway.

• Code style has been fixed by running the composer cs command.
• Changes have been tested to not break existing functionality.
• Documentation is up-to-date.

[danharrin]

Is there a way to refactor defaultSortColumn and defaultSortQuery into a single defaultSort property? Because I don't think they should both be set at the same time, right? And it feels really weird to set one property from the getter of the other.

[archilex]

Hi Dan, I refactored this to a single defaultSort property as requested. I tested with each possible way this could be configured and everything worked as expected (string, closure, builder). And the methods I removed aren't used anywhere else so we should be good to go.

I guess if there were any app/plugins that tapping into one of these methods (cough cough AdvancedTables), this would be a breaking change...not sure how you want to handle that.

Finally, I did notice that inside

if ($defaultSort instanceof Closure) {
    app()->call($defaultSort, [
        'direction' => $sortDirection,
        'query' => $query,
    ]);

    return $query;
}

the direction parameter has no affect at all. This doesn't appear to be because of this PR as I couldn't get this to work in the current release either. In other words:

->defaultSort(fn (Builder $query) => $query->orderBy('status'), 'desc')

doesn't sort the table by desc. For that to work you apply it directly to the query:

->defaultSort(fn (Builder $query) => $query->orderBy('status', 'desc'))

Just wanted you to know.

[danharrin]

doesn't sort the table by desc. For that to work you apply it directly to the query:

It is only useful when using a string column, not a query function. If you pass a column name, it does apply. If you pass a query function, you need to accept $direction as a param of the function and pass it to your orderBy()

[archilex]

It is only useful when using a string column, not a query function. If you pass a column name, it does apply.

hmmm...but when passing a column name, the column and sort direction was applied to the query before it reached this method. And when it was a query and reached this method then it wouldn't be applied...at least in my testing.

Want to make sure I'm testing against all the different scenarios to make sure this isn't breaking anything.

[danharrin]

Just tested and the second param does still seem to work when passing a column name like ->defaultSort('name', 'desc') :)

[danharrin]

Thanks for your help here!

[archilex]

Just tested and the second param does still seem to work when passing a column name like ->defaultSort('name', 'desc') :)

Yes, that all works fine. I'm referring to something different:

public function getDefaultSort(Builder $query, string $direction): string | Builder | null
{
    return $this->evaluate($this->defaultSort, [
        'direction' => $direction, // this line doesn't appear to do anything
        'query' => $query,
    ]);
}

Evaluating $direction doesn't have an affect when you pass in a query. If you pass in a query and want the query to be sorted in a different direction, you have to add it to the query itself. Again, this is how it was previously too.

So my question is, since it appears that $direction doesnt do anything here in ->evaluate(), can we just remove it from being evaluated (and also passed into getDefaultSort()? (We wouldn't remove it from ->defaultSort() of course since that's needed for string columns).

[danharrin]

It gives the user access to it:

->defaultSort(fn ($query, $direction) => $query->orderBy('name', $direction))

If they are sharing the function between multiple defaultSort() calls with different directions :)