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

add pagination for queries using havings #540

Closed
wants to merge 5 commits into from
Closed

add pagination for queries using havings #540

wants to merge 5 commits into from

Conversation

mjauvin
Copy link
Contributor

@mjauvin mjauvin commented Nov 13, 2020

Apply Laravel 7.x PR to fix this:

ref. laravel/framework#32688

Thanks to @alvaro-canepa and @acasar for this!

@alvaro-canepa
Copy link
Contributor

https://github.com/justbetter/laravel-pagination-with-havings
This is the original repo.

@alvaro-canepa
Copy link
Contributor

I tested with withCount + having + pagination and works fine!

    public function scopeClosed($obQuery)
    {
        return $obQuery->withCount(['orders', 'items'])
            ->having('orders_count', '=', \DB::raw('items_count'));
    }

@LukeTowers
Copy link
Contributor

Looks like it was added directly to Laravel in 7.x. Would be better to use that version as a base instead: laravel/framework#32624.

Also, looks like @acasar had the solution all along, great job man! :)

@mjauvin
Copy link
Contributor Author

mjauvin commented Nov 13, 2020

This version was merged, the other one had errors:
laravel/framework#32688

@mjauvin
Copy link
Contributor Author

mjauvin commented Nov 13, 2020

Now I need to add the change to $this->groups

@mjauvin
Copy link
Contributor Author

mjauvin commented Nov 13, 2020

@alvaro-canepa can you test this new implementation to make sure it still work as expected?

@mjauvin
Copy link
Contributor Author

mjauvin commented Nov 23, 2020

@alvaro-canepa did you get a chance to test this?

@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@mjauvin
Copy link
Contributor Author

mjauvin commented Jan 23, 2021

please keep active.

@alvaro-canepa
Copy link
Contributor

alvaro-canepa commented Jan 24, 2021

@alvaro-canepa can you test this new implementation to make sure it still work as expected?

@mjauvin Sorry, I haven't had a chance to try it yet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants