spatie / laravel-query-builder

Easily build Eloquent queries from API requests
https://spatie.be/docs/laravel-query-builder
MIT License
4k stars 392 forks source link

Incorrect field being used for join in sort #916

Closed wonder95 closed 3 months ago

wonder95 commented 7 months ago

I have been trying to add a simple sort to a query to sort by a date field in a related table (see here for details), and have the following code:

    public function report(Request $request): Response
    {
        $filter = $request->query("filter") ?? '';
        $between = $filter === "" ? "" : explode(',', $filter['between']) ?? '';

        return Inertia::render('Reports/TreasuryReport', [
            'payments' => TreasuryReportResource::collection(QueryBuilder::for(Dues::class)
                ->allowedIncludes(['payments', 'user'])
                ->allowedFilters([
                    AllowedFilter::callback('between', function (Builder $query, $value) {
                        $query->whereHas('payments', function (Builder $query) use ($value) {
                            $query->whereBetween('payment_date', [$value[0], $value[1]]);
                        });
                    })
                ])
                ->join('payments', 'dues.id', '=', 'payments.dues_id')
                ->defaultSort('-payments.payment_date') // Default sort descending
                ->allowedSorts([
                    AllowedSort::field('paymentDate', 'payments.payment_date')
                ])
                ->paginate(25)
                ->withQueryString()),
            'filters' => $between
        ]);
    }

with this resource

    public function toArray($request)
    {
        return [
            'member_number' => $this->user->member_number,
            'first_name' => $this->user->first_preferred_name,
            'last_name' => $this->user->last_name,
            'amount' => number_format($this->total, 2),
            'payment_date' => $this->payments()->first()->payment_date,
            'mode' => $this->payments()->first()->mode
        ];
    }

this works well enough, but I want it to be in descending date order by payments.payment_date. So, following the docs, I add a - to defaultSort() like so

->defaultSort('-payments.payment_date') 

However, when I do that , my site breaks, because no payments data is returned. Looking at the generated queries, I get this

SELECT * FROM `dues` inner join `payments` on `dues`.`id` = `payments`.`dues_id` WHERE `dues`.`deleted_at` IS NULL ORDER BY `payments`.`payment_date` ASC LIMIT 25 offset 0;

for both cases, with the only difference being I get DESC in the second query (as expected). However, the problem is the next queries that are generated.

First, it gets the user record for first item in the list, which is fine.

SELECT * FROM `users` WHERE `users`.`id` = 269 and `users`.`deleted_at` IS NULL LIMIT 1

and the next query is where it gets the payments record

SELECT * FROM `payments` WHERE `payments`.`dues_id` = 1 and `payments`.`dues_id` IS not NULL and `deleted_at` IS NULL LIMIT 1

The problem is, though, that the query is using the wrong payments table field to get the data for dues_id. In the ascending sort, the id and dues_id values are the same, they work. But you can see it in the descending sort because the two numbers don't match. For instance, in what would be the first record in descending order, id is 1526, and dues_id = 1185, so the correct query would be

SELECT * FROM `payments` WHERE `payments`.`dues_id` = 1185 and `payments`.`dues_id` IS not NULL and `deleted_at` IS NULL LIMIT 1

but instead it is

SELECT * FROM `payments` WHERE `payments`.`dues_id` = 1526 and `payments`.`dues_id` IS not NULL and `deleted_at` IS NULL LIMIT 1

I use these models (Dues and Payment) all over my site without issue, so I know I have them defined correctly.

For grins, here are my relationship definitions, first Payment

    public function dues()
    {
        return $this->belongsTo(Dues::class, 'dues_id');
    }

and Dues:

    public function payments()
    {
        return $this->hasMany(Payment::class);
    }

I stepped through the code via debugger, and I can see that the DESC is applied to the query correctly. The issue is that the wrong column is being used in the join for some reason, even though this is correct.

->join('payments', 'dues.id', '=', 'payments.dues_id')

Is this indeed a QB bug? Or am I doing something wrong?

Ar-Monta commented 4 months ago

You can use sub-query instead of inner-join. In that case you'll be able to set order-by for dues_id.

AlexVanderbist commented 3 months ago

Hey @wonder95, does using a sub query solve your problem? If not, could you please try to PR a failing test (and mention this issue)? That would make reproducing this issue a lot easier on our end. Thanks!