mehdi-fathi / eloquent-filter

Eloquent Filter is a package for filter data of models by the query strings. Easy to use and fully dynamic.
https://mehdi-fathi.github.io/eloquent-filter/
MIT License
436 stars 42 forks source link

ignoreRequest sets "deleted_at is null" #165

Closed ClProsser closed 3 years ago

ClProsser commented 3 years ago

Describe the bug I try to filter on a model including trashed entries. This works as designed, however, adding ignoreRequest() adds where deleted_at is null to the query, which is definitely not intended. I don't see any reason why especially ignoreRequest should add that.

To Reproduce Steps to reproduce the behavior:

  1. Add a filter to a query including trashed entries (::withTrashed()). The result includes trashed entries (if exist).
  2. Add ignoreRequest() to the builder. The result does not include trashed entries anymore.

Here is some boilerblade code:

dump($request->input());
dump(Vehicle::withTrashed()->filter()->toSql());
dump(Vehicle::withTrashed()->ignoreRequest(["id"])->toSql()); //ignoring id for demonstration
dd(Vehicle::withTrashed()->ignoreRequest(["id"])->filter()->toSql());

The result:

array:1 [
  "id" => "1"
]
"select * from `vehicles` where `id` = ?"
"select * from `vehicles` where `vehicles`.`deleted_at` is null"
"select * from `vehicles` where `vehicles`.`deleted_at` is null"

Expected behavior I expect that calling ignoreRequest() does not modify the query in that way (i.e. only fields listed as argument should be ignored).

Workaround Just remove the desired field (which should be ignored) from the request and add it again afterwards.

$id = $request->input("id");
$request->request->remove("id");
$vehicles = Vehicle::withTrashed()->filter()->get(); //Cannot call ignoreRequest(['rent']) as else filtered by deleted_at

//Add it back to make default laravel functions work properly
$request->request->add([
    'id' => $id
]);
dd($vehicles);

Versions:

Additional context I tried this in a fresh laravel project as well, just to confirm my observation.

mehdi-fathi commented 3 years ago

@ClProsser I should check it and make a unit test for this situation as soon. I'll announce you. Thanks for report

ClProsser commented 3 years ago

@mehdi-fathi Sounds great, thanks for the quick reply and the amazing package

mehdi-fathi commented 3 years ago

@ClProsser I fixed it. please upgrade eloquent-filter to the 2.5.2 version.

ClProsser commented 3 years ago

@mehdi-fathi looks like it works now (my tests are passing). Thanks for the quick fix, keep going :)