Tucker-Eric / EloquentFilter

An Eloquent Way To Filter Laravel Models And Their Relationships
http://tucker-eric.github.io/EloquentFilter
MIT License
1.72k stars 120 forks source link

small issue #114

Closed almokhtarbr closed 4 years ago

almokhtarbr commented 4 years ago

Hello Thanks for this great package first, and secondly i have a small problem. the problem is i have a bulk of payments(table name = payment , fields: payment_date , payment_type : monthly, yearly ...) and as user can add one payment or more (payment 1, payment 2... +) it can be more than a payment. when i search with filter it getting the results but merging the payment 1 payment_date field with payment 2 payment_type field it like combining between 2 payments in the same time cause the payments belongs to a claim Note : a claim has many payment .

Tucker-Eric commented 4 years ago

It sounds like an issue with the join and/or subquery grouping. Do you have an example of your filter? Or the query being ran?

almokhtarbr commented 4 years ago

@Tucker-Eric

     * Filter by payment date
     *
     * @param string $date
     * @return mixed
     */
    public function paymentDate($date)
    {
        return $this->whereHasMorph('claimable', 'App\WgClaim', function ($query) use ($date) {
            $query->whereHas('plan', function ($query) use ($date) {
                $query->whereHas('payments', function ($query) use ($date) {
                    if (request()->get('payment_paid')) {
                        return $query->whereDate('payment_date', $date)
                            ->where('paid', request()->get('payment_paid'));
                    }

                    return $query->whereDate('payment_date', $date);
                });
            });
        })->get();
    }

    /**
     * Filter by payment paid
     *
     * @param string $paid
     * @return mixed
     */
    public function paymentPaid($paid)
    {
        return $this->whereHasMorph('claimable', 'App\WgClaim', function ($query) use ($paid) {
            $query->whereHas('plan', function ($query) use ($paid) {
                $query->whereHas('payments', function ($query) use ($paid) {
                    if (request()->get('payment_date')) {
                        return $query->whereDate('payment_date', request()->get('payment_date'))
                            ->where('paid', $paid);
                    }

                    return $query->where('paid', $paid);
                });
            });
        })->get();
    }

    /**
     * Filter by payment type
     *
     * @param $paymentType
     * @return mixed
     */
    public function paymentType($paymentType){
        return $this->whereHasMorph('claimable', 'App\WgClaim', function ($query) use ($paymentType) {
            $query->whereHas('plan', function ($query) use ($paymentType) {
                $query->whereHas('payments', function ($query) use ($paymentType) {
                    return $query->where('payment_type', $paymentType);
                });
            });
        })->get();
    }
Tucker-Eric commented 4 years ago

calling ->get() at the end of each filter method may be doing it. That is executing each query inside each method instead of aggregating the queries. I'd check to see if that issue persists if you remove the ->get() from each method in that filter.

almokhtarbr commented 4 years ago

I had removed all ->get() still dosen't work, i had sent you the queries via email . thank you

Tucker-Eric commented 4 years ago

That is a hefty query. I don't see anything immediately alarming with it. There is a lot of repetition and looking at your filter and queries, I would suggest moving those 3 methods to a filter for WgClaim so you'd be able to take advantage of the related method in the filters. If you used a hasManyThrough relation from wg_claims to payments then that filter would do all aggregation for you to make this a little easier to troubleshoot. You should also try running that query directly against the database and see if you're getting the results you expect.

From looking just at the query you sent me, it doesn't seem this is an issue with EloquentFilter because there are no joins that would cause any of your data to get mutated at the query level and the issue could be outside of the query after you get your result out of the database.

almokhtarbr commented 4 years ago

Hello, Thanks for the response. It all works up until when I try to query using different conditions on the payments table, as you have seen from the query I sent you if I try to filter paid & payment_date the join is wrong. I have tried to move those filters for WgClaim to a separate filter, however, I couldn’t set it up due to the polymorphic relationship I have. That way I can refactor that code to remove the duplicate methods an use the related method in the filters Can you please walk me through how to setup polymorphic filters?

Tucker-Eric commented 4 years ago

If you move that filter to WgClaim it would be the easiest to throw a setUp in your ClaimFilter to kick off the logic. It would look like:

Add a setup method In your current ClaimFilter:

public function setup()
{
    if(!empty($paymentFilters = \Illuminate\Support\Arr::only($this->input(), ['payment_date', 'payment_paid', 'payment_type']))) {
        $this->whereHasMorph('claimable', 'App\WgClaim', function ($query) use ($paymentFilters) {
            $query->filter($paymentFilters);
        });
    }
}

Your WgClaimFilter


class WgClaimFilter extends ModelFilter
{
    /**
     * Filter by payment date
     *
     * @param string $date
     * @return mixed
     */
    public function paymentDate($date)
    {
        $query->related('plan.payments', function ($query) use ($date) {
            return $query->whereDate('payment_date', $date);
        });
    }

    /**
     * Filter by payment paid
     *
     * @param string $paid
     * @return mixed
     */
    public function paymentPaid($paid)
    {        
        $query->related('plan.payments', function ($query) use ($paid) {
            return $query->where('paid', $paid);
        });
    }

    /**
     * Filter by payment type
     *
     * @param $paymentType
     * @return mixed
     */
    public function paymentType($paymentType)
    {   
        $query->whereHas('plan.payments', function ($query) use ($paymentType) {            
            return $query->where('payment_type', $paymentType);
        });        
    }
}

Using related in the WgClaimFilter will ensure that all the calls to plan.payments are in the same whereHas block when executed.

almokhtarbr commented 4 years ago
Tucker-Eric commented 4 years ago

Make sure you're on at least version 2.2.0. That's where dot notation for related was supported.

almokhtarbr commented 4 years ago

Thank you, It works