laravel / nova-issues

554 stars 34 forks source link

sumBy* methods of Trend metric does not accept Illuminate\Contracts\Database\Query\Expression #6270

Closed webard closed 7 months ago

webard commented 7 months ago

Description:

I have Trend metric like this:

 public function calculate(NovaRequest $request)
    {
        return $this
            ->sumByDays(
                $request,
                PhoneCall::where('caller_user_id', $request->resourceId),
                DB::raw('duration_in_seconds/60'),
                'created_at')
            ->suffix(' minutes')
            ->showSumValue();
    }

which using duration_in_seconds column, but in Trend I want to show minutes so I divided value by 60.

Everything works correctly, but Larastan reports error:

 Parameter #3 $column of method Laravel\Nova\Metrics\Trend::sumByDays() expects  
         Illuminate\Database\Query\Expression|string,                                    
         Illuminate\Contracts\Database\Query\Expression given.  

I haven't checked other metrics or methods like sumByMinutes etc, but this error probably also occurs.

PS. I know about transform() method, but Raw expressions should work too.

Detailed steps to reproduce the issue on a fresh Nova installation:

crynobone commented 7 months ago

In newer Laravel, Illuminate\Database\Query\Expression does extends Illuminate\Contracts\Database\Query\Expression and that shouldn't cause Larastan to throw an error. However, we also support version where Illuminate\Database\Query\Expression doesn't extend Illuminate\Contracts\Database\Query\Expression and only implements __toString().

This feel more like a Larastan issue.