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

[11.x] Fixing a missing translation error #50534

Closed
andrey-helldar opened this issue Mar 13, 2024 · 7 comments · Fixed by #50546
Closed

[11.x] Fixing a missing translation error #50534

andrey-helldar opened this issue Mar 13, 2024 · 7 comments · Fixed by #50546
Labels

Comments

@andrey-helldar
Copy link
Contributor

andrey-helldar commented Mar 13, 2024

Laravel Version

11.0.5

PHP Version

8.3.3

Database Driver & Version

PostgreSQL 14

Description

After upgrading from Laravel 10 to Laravel 11, validation error translation started working incorrectly.

Laravel 10 could translate the phrases (and :count more error) and (and :count more errors), but Laravel 11 cannot. Because of this, English text appears in the response, which looks like an error.

Laravel 10:

[{
    "message": "Le champ foo est obligatoire. (et 3 erreurs en plus)",
    // ...
}]

Laravel 11:

[{
    "message": "Le champ foo est obligatoire. (and 3 more errors)",
    // ...
}].

I have not been able to add a test case to my PR because despite requiring a dependency to be installed for translation, I get an error:

config()->set('app.locale', 'fr');

// Illuminate\Contracts\Container\BindingResolutionException: Target class [config] does not exist.

Also the phrase The given data was invalid. is no longer translated.

The source of the problem is in this location:

protected static function summarize($validator)
{
$messages = $validator->errors()->all();
if (! count($messages) || ! is_string($messages[0])) {
return 'The given data was invalid.';
}
$message = array_shift($messages);
if ($additional = count($messages)) {
$pluralized = $additional === 1 ? 'error' : 'errors';
$message .= " (and {$additional} more {$pluralized})";
}
return $message;
}

Steps To Reproduce

  1. Add the code to the routes/console.php file:
    use Illuminate\Support\Facades\Artisan;
    use Illuminate\Support\Facades\Validator;
    use Illuminate\Validation\ValidationException;
    
    Artisan::command('foo', function () {
        try {
            config()->set('app.locale', 'fr');
    
            app('translator')->setLoaded([
                '*' => [
                    '*' => [
                        'fr' => [
                            ' (and :count more error)' => ' (et :count erreur en plus)',
                            ' (and :count more errors)' => ' (et :count erreurs en plus)',
                            'validation.required' => 'Le champ :attribute est obligatoire.',
                        ]
                    ]
                ]
            ]);
    
            Validator::make([], [
                'foo' => ['required'],
                'bar' => ['required'],
                'baz' => ['required'],
            ])->validate();
        } catch (ValidationException $e) {
            dd($e->getMessage());
        }
    });
  2. Execute the console command php artisan foo
  3. See the error
@crynobone
Copy link
Member

Related to #46378

@andrey-helldar
Copy link
Contributor Author

Responding to @GrahamCampbell comments: okay, the exception doesn't need to know anything about the translation. I agree with that, but how do you translate the phrase (and :count more errors), given that ValidationException itself takes the already translated message about the error itself and appends that text to it via string concatenation?

Now it looks like either we end up with a crookedly translated message, or we need to add array processing to add different variants of messages with a concatenated string with a specific number inside (but this is nonsense), or catch the validator error in the kernel and rebuild it ourselves. That is, do the same thing that the box does, but with a translation call.

On the one hand, the simplest way is to put the phrase (and :count more errors) into a neighboring variable and add it to the main text in the application. But this is very inconvenient and crooked from the end developer's point of view. That is, he will be shifted the responsibility for what can be done "in the box".

And what to do in this situation?

My idea is that if the validator package allows installation of a translation package (here's the proof), then it knows about it and it can be done that way. After all, if the framework has a mechanism for translation, why does it selectively allow it?

What other options are there?

@driesvints
Copy link
Member

We're reverting the original PR. Thanks @andrey-helldar

#50546

@taylorotwell
Copy link
Member

I feel really strongly this message should not be translated. Exception messages aren't meant to be shown to users? Should every exception in the framework by translated?

@andrey-helldar
Copy link
Contributor Author

Several of our applications use the content of the "message" key to display a pop-up notification, and the decryption of each field is used to draw messages under a specific field.

@driesvints
Copy link
Member

@andrey-helldar our stand now is the same as it has been in the past: these messages should never reach end users.

@andrey-helldar
Copy link
Contributor Author

I got it. It's a shame. I've recently managed to convince frontenders that these reports can be trusted, but now I see that they can't. Okay.

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

Successfully merging a pull request may close this issue.

4 participants