Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: builder() not working as expected #2054

Open
shawe opened this issue Nov 12, 2024 · 10 comments
Open

[Bug]: builder() not working as expected #2054

shawe opened this issue Nov 12, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@shawe
Copy link

shawe commented Nov 12, 2024

What happened?

Based on your documentation:

https://rappasoft.com/docs/laravel-livewire-tables/v3/usage/the-query#content-using-the-builder-method

Seems that I can do search based on property $model or if defined builder function with a custom query.

I have both defined and I detected this not expected behaviour:

Navigate to https://mybusiness.test/admin/lms/students and after use search box to filter by "lms." string to get one student by user email.

image

No register found.

Because I to a search, the URL was updated to https://mybusiness.test/admin/lms/students?lms_students-search=lms.

I press F5 and now:

image

To me is expected that in this 2 cases the final query and final results would be the same.

The second screenshot is showing the correct query customized on builder function, but not the first.

Third, if I clean the search box, it's still using the previous search value before clear the field:

image

My builder() function is this:

    /**
     * Build the custom query to use.
     * Original query + custom filters.
     *
     * @return Builder
     */
    public function builder(): Builder
    {
        $userTable = (new User())->getTable();
        $studentTable = $this->tableName;
        $builder = parent::builder()
            ->join($userTable, $studentTable . '.user_id', '=', $userTable . '.id');
        if (!empty($this->search)) {
            $builder->where($userTable . '.email', 'like', '%' . $this->search . '%')
                ->orWhere($userTable . '.name', 'like', '%' . $this->search . '%');
        }

        return $builder;
    }

If I add this dd before the return, I can verify that is not using the search value expected to receive according to search box.

        dd([
            'Updating search: ' . $this->search,
            'Query: ' . $builder->toSql()
        ]);

image

How to reproduce the bug

The way to reproduce it was explanined before.

Package Version

3.5.2

PHP Version

8.3.x

Laravel Version

11.30.0

Alpine Version

Default version included with livewire 3.5.2

Theme

Bootstrap 5.x

Notes

No response

Error Message

No response

@shawe shawe added the bug Something isn't working label Nov 12, 2024
@lrljoe
Copy link
Collaborator

lrljoe commented Nov 13, 2024

Not a bug.

Searching is built in, and you simply mark the relevant columns as searchable() with a callback should your search be more complex.

@shawe
Copy link
Author

shawe commented Nov 13, 2024

Is not a bug that the result displayed where from previous search and not the last one?

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 13, 2024

Hmmm, now that might well be a bug, will take a look at the weekend

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 16, 2024

Will need to have a proper look at this one, as it's probably to do with the order that the underlying feature traits are loaded in.

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 22, 2024

So Search should get fixed this weekend hopefully. It's as I suspected, the order that traits are loaded in

@shawe
Copy link
Author

shawe commented Nov 22, 2024

So Search should get fixed this weekend hopefully. It's as I suspected, the order that traits are loaded in

Thank you!

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 22, 2024

@shawe
Although the more I look at what you're doing there, the more I question why you're searching in the builder()

Can you share your full datatable component please.

Looks like you've got a lot of queries running for your table, which isn't ideal long term!

@shawe
Copy link
Author

shawe commented Nov 22, 2024

@shawe Although the more I look at what you're doing there, the more I question why you're searching in the builder()

Can you share your full datatable component please.

Looks like you've got a lot of queries running for your table, which isn't ideal long term!

Tomorrow I can share it, now I’m out.

If it isn’t the correct way, this is what I understand to do.

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 22, 2024

No worries.

I do have a working fix for it, but I need to look at it in more detail, to avoid duplicate calls to the builder() method.

Look forward to seeing your data table example, as it'll help me identify potential better approaches for you.

@shawe
Copy link
Author

shawe commented Dec 4, 2024

@shawe Although the more I look at what you're doing there, the more I question why you're searching in the builder()

Can you share your full datatable component please.

Looks like you've got a lot of queries running for your table, which isn't ideal long term!

Sorry for the delay, I haven't been able to find time to test until now.

I've been reviewing the excess of queries you mentioned, and I've solved it with eager loading. I was loading related data in the worst possible way (N+1 common issue) because I hadn't read your documentation enough and wanted to show progress with my work.

Now are reduced to minimum in all tables. Thanks for pointing this out to me, it was still detected in time and was quick and easy to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants