laravel / nova-issues

554 stars 34 forks source link

Metrics values are sent rounded with zero precision to front-end #2042

Closed zedix closed 5 years ago

zedix commented 5 years ago

Description

Today, metrics values are sent by nova-api/metrics api to front-end rounded with zero precision.

Steps To Reproduce

Create a Value metric card displaying an average value.

<?php

namespace App\Nova\Metrics;

use Illuminate\Http\Request;
use Laravel\Nova\Metrics\Value;
use App\SomeModel;

class AverageNights extends Value
{
    /**
     * Calculate the value of the metric.
     *
     * @param  \Illuminate\Http\Request  $request
     * @return mixed
     */
    public function calculate(Request $request)
    {
        return $this
            ->average($request, SomeModel::class, 'nights')
            ->format([ 'mantissa' => 2 ]);
    }
}

image

Actual result: display "2"

Expected result: display "2.62"

In order to achieve the simple result above, we currently must override the Laravel\Nova\Metrics\Value class like this:

<?php

namespace App\Nova\Metrics;

use Illuminate\Http\Request;
use Illuminate\Database\Eloquent\Builder;
use Laravel\Nova\Metrics\Value as NovaValue;

class Value extends NovaValue
{
   // This is the protected method to add to allow customization
   // of the `round` precision used in the `aggregate` method
    protected function getRoundPrecision()
    {
        return 2;
    }

    /**
     * Return a value result showing the growth of a model over a given time frame.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Illuminate\Database\Eloquent\Builder|string  $model
     * @param  string  $function
     * @param  string|null  $column
     * @param  string|null  $dateColumn
     * @return \Laravel\Nova\Metrics\ValueResult
     */
    protected function aggregate($request, $model, $function, $column = null, $dateColumn = null)
    {
        $query = $model instanceof Builder ? $model : (new $model)->newQuery();

        $column = $column ?? $query->getModel()->getQualifiedKeyName();

        $previousValue = round(with(clone $query)->whereBetween(
            $dateColumn ?? $query->getModel()->getCreatedAtColumn(), $this->previousRange($request->range)
        )->{$function}($column), $this->getRoundPrecision());

        return $this->result(
            round(with(clone $query)->whereBetween(
                $dateColumn ?? $query->getModel()->getCreatedAtColumn(), $this->currentRange($request->range)
            )->{$function}($column), $this->getRoundPrecision())
        )->previous($previousValue);
    }

This would be a great fix/addition.

mrpritchett commented 4 years ago

Was this actually fixed or just closed?

zedix commented 4 years ago

@mrpritchett It was actually fixed in Nova v2.8.0.

You can now change the precision property like this:

use Laravel\Nova\Metrics\Value;

class AverageNights extends Value
{
    /**
     * The value's precision when rounding.
     *
     * @var int
     */
    public $precision = 2;
}
mrpritchett commented 4 years ago

Does this work for Trend metrics?

gpressutto5 commented 4 years ago

@mrpritchett I guess you already figured that out, but for anyone coming into this issue now, it doesn't work for trends.

jonasbrenig commented 4 years ago

@mrpritchett I guess you already figured that out, but for anyone coming into this issue now, it doesn't work for trends.

Well since you fixed it, it does now :-)

https://github.com/laravel/nova/pull/891

For anybody else, just use @zedix code

@mrpritchett It was actually fixed in Nova v2.8.0.

You can now change the precision property like this:

use Laravel\Nova\Metrics\Value;

class AverageNights extends Value
{
    /**
     * The value's precision when rounding.
     *
     * @var int
     */
    public $precision = 2;
}
mrpritchett commented 4 years ago

Thanks, y'all!

rderks88 commented 2 years ago
public $precision = 2;

Since I'm guessing Nova 4.0 this value changed its name to "roundingPrecision"

public $roundingPrecision = 2;
schwindy commented 5 months ago

@mrpritchett I guess you already figured that out, but for anyone coming into this issue now, it doesn't work for trends.

Tested today on latest Nova - it does now work for Trend Metrics :monkey:

This issue is the only place I found to use $roundingPrecision - thanks @rderks88