Fix: cast value to a decimal

[wandesnet]

⚡ PowerGrid - Pull Request

Welcome and thank you for your interest in contributing to our project!. You must use this template to submit a Pull Request or it will not be accepted.

---

Motivation

• Bug fix
• Enhancement
• New feature
• Breaking change

Description

This Pull Request adds...

Related Issue(s): #_____.

Documentation

This PR requires Documentation update?

• Yes
• No
• I have already submitted a Documentation pull request.

This Pr fixes a bug, let's understand the problem. Let's say we have an Orders table with the amount field of decimal type in which NULL VALUE is allowed.

In the Order model, define the cast, e.g.:

 protected $casts = [
        'amount' => 'decimal:2',
    ];

Component PowerGrid

 public function datasource(): Builder
    {
        return Order::query();
    }

    public function fields(): PowerGridFields
    {
        return PowerGrid::fields()
            ->add('amount')
            ->add('amount_formatted', fn (Order $model) => formatCurrency($model->amount));

    }

When I access the page, a cast error appears.

Reason for the error, this transforms all values into strings, when the amount field is null in the table, the code below transforms the value of the amount field into an empty string, ""

     e(strval(data_get($model, $fieldName))

this ends up resulting in an error because it is trying to convert an empty string to decimal

Illuminate\Database\Eloquent\Concerns\HasAttributes.php

 protected function asDecimal($value, $decimals)
    {
        
        try {
            return (string) BigDecimal::of($value)->toScale($decimals, RoundingMode::HALF_UP);
        } catch (BrickMathException $e) {
            throw new MathException('Unable to cast value to a decimal.', previous: $e);
        }
    }

There are three ways to solve the problem.

1. accept this PR.
2. send a PR to Laravel, inserting a check, that is, checking whether the $value is a number in the asDecimal() method.
3. add a callable to the field, forcing it to return the original value from the table, e.g.:

 public function fields(): PowerGridFields
    {
        return PowerGrid::fields()
            ->add('amount', fn (Order $model) => $model->amount)
            ->add('amount_formatted', fn (Order $model) => formatCurrency($model->amount));

    }

[luanfreitasdev]

Hello @wandesnet, thank you for submitting this PR!

Can you add any tests?

[dansysanalyst]

In this specific case, shouldn't the user be checking for null in the custom column?

    public function fields(): PowerGridFields
    {
        return PowerGrid::fields()
            ->add('amount')
            ->add('amount_formatted', fn (Order $model) => is_null($model->amount) ? 0 : formatCurrency($model->amount));
    }

In my experience, when dealing with currency amounts, humans don't say "your bill is null dollars". Displaying "$0.00" is often the same as displaying " ".

Regarding the PR, I am not sure if we should remove the e() helper here.

Alternatively, could we just provide an empty string as the default value in data_get?

this->fields[$fieldName] = $closure ?? fn ($model) => e(strval(data_get($model, $fieldName, '')));

[wandesnet]

In this specific case, shouldn't the user be checking for null in the custom column?

    public function fields(): PowerGridFields
    {
        return PowerGrid::fields()
            ->add('amount')
            ->add('amount_formatted', fn (Order $model) => is_null($model->amount) ? 0 : formatCurrency($model->amount));
    }

In this specific case, the error occurs regardless of whether it is a custom column.

In my experience, when dealing with currency amounts, humans don't say "your bill is null dollars". Displaying "$0.00" is often the same as displaying " ".

Regarding the PR, I am not sure if we should remove the e() helper here.

In my opinion, you should not change the values to string, that is, you should always keep the original value/type saved in the table.

Alternatively, could we just provide an empty string as the default value in data_get?

this->fields[$fieldName] = $closure ?? fn ($model) => e(strval(data_get($model, $fieldName, '')));

This way, the error will persist because it continues to return an empty string if the amount field is null, why not keep the original value/type saved in the table? Is there a reason for this?

[dansysanalyst]

I see the point, as it is a cast.

For me, it is more about not removing the e() helper. However, tests must be written here.

So perhaps something like:

if is_callable($closure) {
  $this->fields[$fieldName] = $closure;
   return;
}

$data =  fn ($model) => data_get($model, $fieldName);

if (is_string($data) {
     $data = e($data);
}

$this->fields[$fieldName] = $data

(writing code on the go, from my head and untested)

[wandesnet]

Got it, we can keep e() when it's a string

[dansysanalyst]

👍 L.G.T.M

Thank you very much @wandesnet