Kyslik / laravel-filterable

Laravel 5/6/7 package to handle filtering by query-string
MIT License
116 stars 34 forks source link

Added joins functionality #9

Closed nmc9 closed 6 years ago

nmc9 commented 6 years ago

Added methods to the Filter.php file to allow filters to join from other tables. Also included testCases and stubs for further testing of the join functions

Kyslik commented 6 years ago

Could you explain what is the rationale behind the PR, in the filter itself you have Builder instance available so stock methods ->join*() work.

nmc9 commented 6 years ago

Sorry i should have more clear about what I actually did. Yes you can use the join methods in the builder but it can cause some problems. The main reason for having the join in the filter class is for situations like this: If you have multiple fields on a joined table (like created_at, updated_at) if you try to set them up as separate filters the join duplicates. When it comes time to build the query you end up with multiple joins to the same table. For Example a setup like this:

public function usercreatedat($created_at){
    return $this->getBuilder()->join("user","user.id","role.user_id")
    ->where("user.created_at",$created_at);
}

public function userupdatedat($updated_at){
    return $this->getBuilder()->join("user","user.id","role.user_id")
    ->where("user.updated_at",$updated_at);
}

Would produce this:

select * inner join "user" on "user"."id" = "role"."user_id" inner join "user" on "user"."id" = "role"."user_id" where "user"."created_at" = '2018-08-13 12:33:05' and "user"."updated_at" = '2018-08-13 12:33:05'

So i added a check in the filter table that only adds the join if it's not already in the builder.

Also another option if you are worried about muddying up the Filter class would be to extend it with a JoinableFilter class. Just a thought

Kyslik commented 6 years ago

Makes sense, I will have to dig in to this :) and figure how best it would be, thank you for contribution.

ETA: when Laravel 5.7 comes out.

Kyslik commented 6 years ago

@nmc9 I added trait JoinSupport that adds the methods you've created, but they all accept signatures like Builder::join(...)|leftJoin(...)|rightJoin(..,), even closures etc. Tests are unchanged (the queries themselves) so I hope I did not cripple any functionality.

I will have to work on readme.