Closed simonworkhouse closed 7 months ago
Can you explain the issue this is solving?
Apologies, I must have neglected to mention that. The issue that this solves is that the current return type declaration of string
is not compatible with Laravel's \Illuminate\Database\Query\Builder::orderBy($column, $direction = 'asc')
method which supports additional types such as those in the following parameter definition:
@param \Closure|\Illuminate\Database\Query\Builder|\Illuminate\Database\Eloquent\Builder|\Illuminate\Contracts\Database\Query\Expression|string $column
This change allows us to be able to specify a more complex sort ordering, for example:
'order_by' => (new \Illuminate\Database\Query\Expression('IFNULL(finalised_at, updated_at)')),
Or even update them dynamically like so:
config()->set('runway.resources.{resource}.order_by', function () {...});
Here's a reference to a previous PR https://github.com/statamic-rad-pack/runway/pull/427
I've opened a PR with an alternative approach: https://github.com/statamic-rad-pack/runway/pull/434
Closing in favour of #434, which will make this possible using the query scope approach, while allowing users to override the filtering.
The order_by
and order_by_direction
config settings are only intended for "simple" sorting - anything more complicated (like using expressions), should be done elsewhere.
This is to maintain compatibility with Laravel's
\Illuminate\Database\Query\Builder::orderBy($column, $direction = 'asc')
type declaration in a non-breaking way.Other solutions, such as using the
runwayListing
scope, are all potentially breaking changes and require a significant amount more effort to implement.