Skip to content

Added ability to have custom sort classes.#91

Merged
AlexVanderbist merged 4 commits intospatie:masterfrom
gcw07:custom-sort
Jan 11, 2019
Merged

Added ability to have custom sort classes.#91
AlexVanderbist merged 4 commits intospatie:masterfrom
gcw07:custom-sort

Conversation

@gcw07
Copy link
Contributor

@gcw07 gcw07 commented Aug 16, 2018

This adds the ability to have custom sort classes. So why would you need or want a custom sort class? Doing so would allow users to sort by related models. Normal usage is the same as before. So if you just want to keep the standard sorting, you just add the sort fields like normal:

// GET /users?sort=name,-street
$users = QueryBuilder::for(User::class)
    ->allowedSorts('name', 'street')
    ->get();

So to add a custom sort you would so something like:

$posts = QueryBuilder::for(Post::class)
    ->allowedSorts('name', Sort::custom('user', UserCustomSort::class))
    ->get();

And a basic custom class would look like:

use Illuminate\Database\Eloquent\Builder;
use Spatie\QueryBuilder\Sorts\Sort;

class UserCustomSort implements Sort
{
    public function __invoke(Builder $query, $descending, string $property) : Builder
    {
        return $query->orderBy('name', $descending ? 'desc' : 'asc');
    }
}

Obviously you wouldn't want just a basic custom sort, but something that would be able to sort the related fields. This can be achieved by doing a sub select or you could do a join.

use Illuminate\Database\Eloquent\Builder;
use Spatie\QueryBuilder\Sorts\Sort;

class UserCustomSort implements Sort
{
    public function __invoke(Builder $query, $descending, string $property) : Builder
    {
        return $query
           ->addSubSelect('user', \App\User::select('name')->whereRaw('posts.user_id = users.id'))
           ->orderBy('name', $descending ? 'desc' : 'asc');
    }
}

While it passes all tests, this more than anything is a proof of concept. I think it could be improved upon. Let me know what you think. Thanks for providing such a great package.

@gcw07
Copy link
Contributor Author

gcw07 commented Aug 16, 2018

Just a note, addSubSelect() is a macro that Jonathan Reinink showed off in his great Laracon Online 2018 talk. His talk repo can be seen here. It is also where the idea for the sub select sorting is from.

https://github.com/reinink/laracon2018

use Illuminate\Database\Eloquent\Builder;

Builder::macro('addSubSelect', function ($column, $query) {
    if (is_null($this->getQuery()->columns)) {
        $this->select($this->getQuery()->from.'.*');
    }

    return $this->selectSub($query->limit(1)->getQuery(), $column);
});

@gizburdt
Copy link

gizburdt commented Nov 7, 2018

Would be awesome if this could be merged :)

@freekmurze
Copy link
Member

Could you rebase this against the current master?

@gcw07
Copy link
Contributor Author

gcw07 commented Nov 13, 2018

Sorry, meant to respond earlier today. Yes I can rebase it. Though in my version I had to overwrite (and call the parent class) of several query methods (getQuery, paginate, simplePaginate). I'm not sure if I need to actually overwrite those methods. You only previously overrode the get method. Do you know if the other methods like paginate end up calling get and run the get method within the QueryBuilder class?

@elightbo
Copy link

elightbo commented Dec 1, 2018

Currently working on a project that could use this functionality. Happy to help out if needed.

@matthew-inamdar
Copy link
Contributor

Would love to see this added! 👍

@gcw07
Copy link
Contributor Author

gcw07 commented Dec 7, 2018

OK, I merged all the updates that have happened to master back into this pull request. Not sure if I did it correctly or not, but I think it is good. Still wasn't sure if we really need to do all the overriding of methods or if just overriding the "get" method would work.

@AlexVanderbist
Copy link
Member

Thanks, this looks really good! Will take a look at this and probably merge it later this week :)

@zolotov88
Copy link

What about merge? This feature is really useful and I wait for it.

@matthew-inamdar
Copy link
Contributor

Still waiting on this 😬

@gcw07 I can give a hand with this if you need?

I'm conscious if you have enough time to work on this? And myself and others are really looking forward to using it!

@gcw07
Copy link
Contributor Author

gcw07 commented Jan 9, 2019

@matthew-inamdar I rebased this around a month ago. So it is really up to the maintainers at this point if they want to merge it or implement their own version (either way is fine with me, this was intended more as a proof of concept than anything else).

@AlexVanderbist AlexVanderbist merged commit fb5694a into spatie:master Jan 11, 2019
@AlexVanderbist
Copy link
Member

Sorry for the delay. It's been merged, now going through the broken tests to make sure everything works together with other recently merged PRs. Will tag a new version including these custom sort classes soon

@matthew-inamdar
Copy link
Contributor

Thanks for the work @gcw07 and @AlexVanderbist 👍

@elightbo
Copy link

Thanks for all the work on this. This is going to be extremely helpful for the project I'm working on.

dominikb pushed a commit to dominikb/laravel-query-builder that referenced this pull request Jan 29, 2019
@KevinBeckers
Copy link

Any news on this?

@elightbo
Copy link

elightbo commented Feb 8, 2019

This was added to one of the latest releases. It has been working great!

@robsontenorio
Copy link
Contributor

There is no docs on README?

@farshadff
Copy link

can any one please make an example about how to do this without this macro ?? i mean how to sort by relationship values

@OzanKurt
Copy link

Where can we find the documentation about this?

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.

Comments