rappasoft / laravel-livewire-tables

A dynamic table component for Laravel Livewire
https://rappasoft.com/docs/laravel-livewire-tables/v2/introduction
MIT License
1.79k stars 339 forks source link

orQueries should not break searchability while using the builder method #1026

Closed PHPieter closed 1 year ago

PHPieter commented 1 year ago

I needed a more complex than standard query and suddenly the searchability of my table was broken. Wen i do a query dump, i see the search query is only added to the orWhere part of the query. Therefore the first part of the always stays the same and therefore the searchability is not working. The query below broke the searchability.

$query = Employee::query()
            ->whereNull('primary_object_id')
            ->whereNull('expiration_date')
        $query
            ->orWhere(function ($q) use ($searchableFunction) {
                $q
                    ->whereNull('primary_object_id')
                    ->where('expiration_date', '>', Carbon::now())
            });

I could not think of a solution which is in line of the current ->searchable(). The answer below is a potential fix where i think the private methods should go in a trait (MakeSearchableTrait).

public function builder(): Builder
    {
        $searchableFields = ['name', 'surname', 'employee_number', 'email'];
        $searchableFunction = $this->makeSearchable($searchableFields);

        /** @var Builder $query */
        $query = Employee::query()
            ->whereNull('primary_object_id')
            ->whereNull('expiration_date')
            ->where($searchableFunction)
            ->orWhere(function ($q) use ($searchableFunction) {
                $q
                    ->whereNull('primary_object_id')
                    ->where('expiration_date', '>', Carbon::now())
                    ->where($searchableFunction);
            });

        return $query;
    }
private function getSearchKey()
    {
        $searchKey = \Request::get('table')['search'] ?? null;
        if (null === $searchKey) {
            $searchKey = \Request::get('updates')[0]['payload']['value'] ?? null;
        }

        return $searchKey;
    }

private function makeSearchable(array $searchableFields): Closure
    {
        $searchKey = $this->getSearchKey();
        return static function ($q) use ($searchKey, $searchableFields) {
            foreach ($searchableFields as $index => $searchable) {
                if ($index === 0) {
                    $q->where($searchable, 'like', '%' . $searchKey . '%');
                } else {
                    $q->orWhere($searchable, 'like', '%' . $searchKey . '%');
                }
            }
        };
    }
stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lrljoe commented 1 year ago

I'm assuming using the when() function is a bit too painful here?

PHPieter commented 1 year ago

The when function would not provide the expected results because in this case it is not about user input but about a column that can be null or can have an id. Therefore we need the where query. The When method would work if you would know upfront if the result should be a record where expiration_date is null or an id.

At this moment i think the solution above is the most clean one, but not in line with the current flow.

lrljoe commented 1 year ago

When doesn't need a user input, sadly the docs on Laravel don't explain this clearly, the following is a perfectly valid way of using when()

        return User::query()
        ->when('email_verified_at == null', function ($query) {
          Do Something Here
         })

This may help you in simplifying the code above, which may make it a little easier to document/others to adopt!

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.