Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validator Error message not being created correctly #51849

Closed
timack opened this issue Jun 20, 2024 · 8 comments
Closed

Validator Error message not being created correctly #51849

timack opened this issue Jun 20, 2024 · 8 comments
Labels

Comments

@timack
Copy link

timack commented Jun 20, 2024

Laravel Version

11.11.0

PHP Version

8.2

Database Driver & Version

n/a

Description

The validator is not generating error messages correctly.

Might be related to this MR - #51805 - will investigate further tonight if time.

Steps To Reproduce

In version 11.10.0:

> Validator::make(['total' => 0], ['total' => ['gt:0']])->messages()->all();
= [
    "The total field must be greater than 0.",
  ]

In 11.11.0 the same code produces:

 > Validator::make(['total' => 0], ['total' => ['gt:0']])->messages()->all();
= [
    "The total field must be greater than validation.attributes.",
  ]
@timack timack changed the title Validator Error message not being created Validator Error message not being created correctly Jun 20, 2024
@driesvints
Copy link
Member

Ping @owenandrews

@driesvints
Copy link
Member

Might be best if we work out a test for this so we can actually verify a fix.

@seriquynh
Copy link
Contributor

I figure out why this happens. I'll try my best to explain. Let's take a look at this code.

<?php

namespace Illuminate\Validation\Concerns;

// imports

trait FormatsMessages
{
    protected function getAttributeFromTranslations($name)
    {
        return $this->getAttributeFromLocalArray(
            $name, Arr::dot((array) $this->translator->get('validation.attributes'))
        );
    }
}

When validation messages are formatted, all fields or rule parameters are finally passed through the getAttributeFromTranslations method.

In this example, when formatting the message "The :attribute field must be greater than :value."

  1. "total" is passed through getAttributeFromTranslations, it returns "total" and ":field" must be replaces with "total".

  2. "0" is passed through getAttributeFromTranslations, it has to return "0" and ":value" must be replaces with "0". But it does not work as expected due to "validation.attributes" from the translator.

If lang/en/validation.php does not define attributes or define it as an emty array. It means $this->translator->get('validation.attributes') returns "validation.attributes" as a string instead of an array.

<?php

// $source = [0 => 'validation.attributes'];
$source = Arr::dot((array) $this->translator->get('validation.attributes'));

// $name = '0'; as key accidently exists in the array above and getAttributeFromLocalArray returns "validation.attributes"

// That's why it replaces :value with "validation.attributes" and the final message is
$this->getAttributeFromLocalArray($name, $source); "The total field must be greater than validation.attributes."

I create these examples to show how replacements are done.

Example 1 - attributes are not empty in locale validation.php file.

<?php

namespace Illuminate\Tests\Validation;

class ValidationValidatorTest extends TestCase
{
    public function test_validation_attributes_are_set()
    {
        // $source = ['foo' => 'bar'];
        // $name = '0'; as key does not exists in $source. -> ":value" is replaced with "0".
        $trans = $this->getIlluminateArrayTranslator();
        $trans->addLines(['validation.gt.numeric' => ':attribute is greater than :value.'], 'en');
        $trans->addLines(['validation.attributes' => ['foo' => 'bar']], 'en');
        $v = new Validator($trans, ['total' => 0], ['total' => 'gt:0']);
        $this->assertSame("total is greater than 0.", $v->messages()->first('total'));
    }
}

Example 2 - comparison value is not zero.

<?php

namespace Illuminate\Tests\Validation;

class ValidationValidatorTest extends TestCase
{
    public function test_comparison_value_is_not_zero()
    {
        // $source = [0 => 'validation.attributes'];
        // $name = '1' or '-1'; as key does not exists in $source. -> ":value" is replaced with "1" or "-1".

        $trans = $this->getIlluminateArrayTranslator();
        $trans->addLines(['validation.gt.numeric' => ':attribute is greater than :value.'], 'en');

        $v = new Validator($trans, ['total' => 1], ['total' => 'gt:1']);
        $this->assertSame("total is greater than 1.", $v->messages()->first('total'));

        $v = new Validator($trans, ['total' => -1], ['total' => 'gt:-1']);
        $this->assertSame("total is greater than -1.", $v->messages()->first('total'));
    }
}

Example 3 - validtion attributes contains "0" or 0 as key.

<?php

namespace Illuminate\Tests\Validation;

class ValidationValidatorTest extends TestCase
{
    public function test_validation_attributes_contain_0_as_key()
    {
        // $source = [0 => 'Integer Zero'];
        // $name = '0'; as key exists in $source. -> ":value" is replaced with "Integer Zero"
        $trans = $this->getIlluminateArrayTranslator();
        $trans->addLines(['validation.gt.numeric' => ':attribute is greater than :value.'], 'en');
        $trans->addLines(['validation.attributes' => [0 => 'Integer Zero']], 'en');
        $v = new Validator($trans, ['total' => 0], ['total' => 'gt:0']);
        $this->assertSame("total is greater than Integer Zero.", $v->messages()->first('total'));

// $source = ['0' => 'String Zero'];
        // $name = '0'; as key exists in $source. -> ":value" is replaced with "String Zero"
        $trans = $this->getIlluminateArrayTranslator();
        $trans->addLines(['validation.gt.numeric' => ':attribute is greater than :value.'], 'en');
        $trans->addLines(['validation.attributes' => ['0' => 'String Zero']], 'en');
        $v = new Validator($trans, ['total' => 0], ['total' => 'gt:0']);
        $this->assertSame("total is greater than String Zero.", $v->messages()->first('total'));
    }
}

@owenandrews
Copy link
Contributor

owenandrews commented Jun 21, 2024

Thanks for the tests @seriquynh, I'll get it reproduced locally and get back to you.

@owenandrews
Copy link
Contributor

owenandrews commented Jun 22, 2024

@seriquynh @timack I've created a branch here that I think resolves this issue. I've also added a test to check that messages are returned correctly when the translated attributes array is empty or missing. You can see that failing against the current branch here.

Could you confirm this resolves this issue and if so I'll create a PR.

@seriquynh
Copy link
Contributor

I think your fix's good for this specific situation. But I'm not sure there are any other use cases in which we need to add more tests. It's fine for me. Let's wait for other thoughts, use cases or recommendations.

@timack
Copy link
Author

timack commented Jun 24, 2024

@owenandrews Can confirm issue is resolved for me when using that branch. Nice work 👍

@driesvints
Copy link
Member

Will be in today's release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants