[Trans] Auto trans on devs content should not be a thing

[cavasinf]

Edit: You committed something about that in menu.html.twig @kevinpapst
added method to configure translation domain for menu items (#150)

But still the case for the navbar

TablerBundle/templates/includes/navbar_user.html.twig

Line 28 in 28287dc

<a href="#" class="dropdown-item">{{ link.label|trans }}</a>

I'm currently experiencing some translation errors when using the menu and navbar.

What I got

From a subscriber, I implement some items to the menu.
These items have labels that are already translated inside the subscriber.

$changePassword = new MenuItemModel(
    'change_password', 
    $this->translator->trans('action.edit_profile'), // <----- HERE
    'user_edit', 
    ['user_uuid' => $user->getUuid()]
);
$event->addLink($changePassword);

What TablerBundle does

When TablerBundle print the twig result of the menu item, it tries to trans a content that is already translated.
Like {{ 'Modifier mon profil'|trans }}.

What is the problem?

Because of this, Symfony tells me that I have missing/wrong translations.

What is the solution?

1. Add the trans domain to any trans function/filter that the bundle uses
   ❌ I don't think this is a good idea:

• Translations like that are not auto discovered by bin/console translation:extract (huge nono for me)
• I found it easier to expect as a dev, to do a "raw" print by the bundle if my label is a message ID.

2. DO NOT try to translate content that devs define.
   Only word/phrase that we define in the bundle.
   ✔️ Rude, but I'll take it!

[kevinpapst]

I find it disturbing to use the translator in the backend, even though this is a frontend centric piece of the backend.

For myself I don't see an issue when Symfony warns about missing translations.
That happens all the time with form choices and such.

But I don't know, if there are any official best practice for such cases in bundles.
What about adding the same translation domain logic to the user navbar?

[cavasinf]

For myself I don't see an issue when Symfony warns about missing translations.

This is not a missing translation error.
Those are error from a translation of a translated label.

That happens all the time with form choices and such.

How?
Do you not translate your labels and/or choices?

$builder
    ->add('nom', TextType::class, [
        'label' => $this->getTranslator()->trans('label.name'),
    ])
    ->add('statut', EnumType::class, [
        'label'        => $this->getTranslator()->trans('label.status'),
        'class'        => DocumentStatutEnum::class,
        'choice_label' => fn(DocumentStatutEnum $statut) => $statut->trans($this->getTranslator()),
    ])
;

enum DocumentStatutEnum: string implements TranslatableInterface
{
    case Nouveau   = 'nouveau';
    case Brouillon = 'brouillon';
    case EnCours   = 'en_cours';
    case Valide    = 'valide';
    case Refuse    = 'refuse';

    public function trans(TranslatorInterface $translator, string $locale = null): string
    {
        return match ($this) {
            self::Nouveau   => $translator->trans('label.new', locale: $locale),
            self::Brouillon => $translator->trans('label.draft', locale: $locale),
            self::EnCours   => $translator->trans('label.in_progress', locale: $locale),
            self::Valide    => $translator->trans('label.valid', locale: $locale),
            self::Refuse    => $translator->trans('label.refused', locale: $locale),
        };
    }
}

But I don't know, if there are any official best practice for such cases in bundles.

There's no best practice about that.
https://symfony.com/doc/current/best_practices.html#internationalization

But I know it is not a good practice to try to translate dynamically a variable, even in twig than PHP back-end.
And this is what we are doing in the menu template.

❌	✔️
{# this translation uses a Twig variable #} {# so it won't be detected #} {% set message = 'This is wrong' %} {{ message|trans }}	{# Great use #} {% set message = 'This is great'|trans %} {{ message }}

What about adding the same translation domain logic to the user navbar?

As I said before, it's a big nono for me.
Even with you last commit with the domain in parameter.
A menu item like that will not be found and extracted by the symfony translation command:

$changePassword = new MenuItemModel(
    'change_password',
    'action.edit_profile', // <----- That will not be detected
    'user_edit',
    ['user_uuid' => $user->getUuid()]
);

php bin/console translation:extract --force fr will not create an input for action.edit_profile

I'd rather have something not happen because I'm not using it properly.

labels that are not translated because I have not translated them myself.

Than something that is being done by someone/something else and I can't do anything about it.

Having errors from something I did right

[kevinpapst]

There is a lot of data that cannot be translated, e.g. Cities, Numbers ... but that is not the point, right?

I see what you are saying. It works very well for me since years, but that doesn't mean it is the correct way.

Just changing it will cause massive BC breaks (at least in my projects), so please propose a way forward that is BC safe. My project uses plugins from third party developers and I am not able to simply rip that feature out.
It needs time and a sunsetting phase with deprecation notices.

[cavasinf]

There is a lot of data that cannot be translated, e.g. Cities, Numbers ... but that is not the point, right?

TRUE! Some data can be translated and some can't.
So why do we force all data to be translated? That is indeed the point.

Just changing it will cause massive BC breaks

Also true, unfortunately 😞
I don't even know how to handle the "deprecated" state to warn other devs.
Because it is part of the variable content, not a "use way"/import/function/template.
This is also why it is not a correct way, we have no control over what the devs give us NOR how they use it.

[kevinpapst]

You know the AdminLTE Bundle, right?! It has the same logic and it was used and installed (and still is) a couple of times every day: https://packagist.org/packages/kevinpapst/adminlte-bundle/stats
Nobody every complained about the logic. I just mention that, because ...

This is also why it is not a correct way

... I don't like these "there is only one correct way to do it and it's mine" expressions 😁

I am a fan of having all translation calls in the frontend, but then I never used that translation command as well 🤷 so that's probably why I never really thought about it twice.

But I do accept your arguments and that's why we should concentrate on finding a way forward:

What about add a new flag (isTranslated = false) to the MenuItem class and check that in the template when displaying the label?

If isTranslated == true, simply show the label.
But if isTranslated == false then trigger a deprecation and append the |trans filter.

That could work? At some point in the future we need to remove that extra flag isTranslated again, but in that case it does not work without that BC code.

[cavasinf]

... I don't like these "there is only one correct way to do it and it's mine" expressions 😁

I am not here to impose my wishes / my needs.
Otherwise this will be PR not an issue 😉

In my company, we have some tests to make sure there's no missing translation.
php bin/console debug:translation --only-missing $LOCALE --domain=$DOMAIN
If this command returns an error code, we will not be able to deploy.

ATE, we strictly do not use variable with trans function to be able to run that command.
As Symfony doc recommend it: https://symfony.com/doc/current/translation.html#how-to-find-missing-or-unused-translation-messages

The extractors can't find messages translated outside templates (like form labels or controllers) unless using Translations or calling the trans() method on a translator (since Symfony 5.3). Dynamic translations using variables or expressions in templates are not detected either

---

What about add a new flag (isTranslated = false)

Sounds good to me 👍

[kevinpapst]

Sounds like a good approach! I'll play with that as well and see how I like it.

And don't hesitate to change the Menu Interface if necessary (like I recently did). This is also a BC break, but a "minor" one that is identifiable and fixable in 5 minutes. And I do not expect that many users implement their own MenuItemModel.

[akkushopJK]

i'am also working with a UserDetailsSubscriber and for example
$event->addLink(new MenuItemModel('profile', 'Change password', 'app_change_password'));
Translation works if i manuall ad it to messages.de.xlf but if i wan't ti extract it with
php bin/console translation:extract --force --clean de
it isn't recognised and added to the messages.de.xlf file.
Is there any workaround for this?

also spotted that there
https://github.com/kevinpapst/TablerBundle/blob/main/templates/includes/navbar_user.html.twig#L26
https://github.com/kevinpapst/TablerBundle/blob/main/templates/includes/navbar_user.html.twig#L28
the optional translation domain is missing

[cavasinf]

@akkushopJK Pass a translated param to the MenuItemModel label (from the TranslatorInterface).

From this:

$changePassword = new MenuItemModel(
    'change_password',
    'action.edit_profile', // <----- That will not be detected
    'user_edit',
    ['user_uuid' => $user->getUuid()]
);

To this:

$changePassword = new MenuItemModel(
    'change_password', 
    $this->translator->trans('action.edit_profile'), // <----- HERE
    'user_edit', 
    ['user_uuid' => $user->getUuid()]
);

You will get a warning from Symfony that you are translating something that has already been translated.
But better that than data loss 👍

[kevinpapst]

I am fine with adapting the logic, but someone needs to do this change and we should do it in a new branch for the 1.0 bump.