sonata-project/SonataCoreBundle

Duplicate translation in flash messages

Closed this issue ยท 16 comments

Question Answer
Bundle version 3.1.1
Symfony version 2.8.4
php version 5.5

Subject

The flash messages are translated multiple times.

Steps to reproduce

Add a new (translated) message to the FlashBag.

Expected results

No translation is done.

Actual results

bildschirmfoto 2016-09-18 um 08 28 14

Solution

We should remove the translation in the `FlashManager, because that's no good way to handle missing translation of the developer.

We should remove the translation in the `FlashManager, because that's no good way to handle missing translation of the developer.

What's a better way in your opinion?

  1. Don't translate ALL messages with a default translation domain, aka remove the translation
  2. Find all untranslated translation keys and translated them before adding to the flashbag

IMO we can't do this with any BC.

IMO we can't do this with any BC.

I think it might be possible :

+$untranslatedMessage = $message
 $message = $this->getTranslator()->trans($message, array(), $domain);
+if ($untranslatedMessage != $message) {
+    trigger_error('Not translating a message before feeding it to the flash manager is deprecated')
+}

To avoid double translation, we could add a container parameter to disable translation in the flash manager. People enabling this flag should translate their message before feeding it to the flash manager. This parameter would be ignored in 4.0, and removed in 5.0 .

Makes sense?

Another problem with this approach is translation parameters. It's not possible to use translations with parameters at this point (in fact, it is necessary to translate them yourself but you will get the missing translation warning on the already translated message). One option could be to also carry a bag of parameters with each message but frankly I think translating shouldn't be the responsibility of the FlashManager class anyway.

As an intermediate measure we could indeed control this behavior with a config setting somehow.

Also found this article about (not) passing translation keys to the flash bag: https://coderwall.com/p/ybup5a/symfony2-translating-flash-messages

@verheyenkoen would you be willing to contribute that deprecation?

I guess I can find some time and give it a shot.

Would that be on the 3.x branch? I guess we can make it BC by having the config setting's default value for translating on by default?

Also, I have another PR open on the 3.x branch. Can I make a new branch in my fork and work on the PR from there? Otherwise I guess both PR's would interfere.

Would that be on the 3.x branch? I guess we can make it BC by having the config setting's default value for translating on by default?

Yes, completely.

Also, I have another PR open on the 3.x branch. Can I make a new branch in my fork and work on the PR from there? Otherwise I guess both PR's would interfere.

Sure , if you think the other PR is going to be merged first. You will just have to rebase it on 3.x once it is merged, otherwise we will see the commits of both PRs in the second PR.

If it is #411, I don't see how it interferes though.

Not really interfering code-wise. I just don't get how Github assigns new commits to pull requests. Is it smart enough to see which commits are related to which PR's?

It just compares 2 branches, and I think you should restart your second PR, which has nothing do to with the other one, on 3.x, not on your previous merge. Don't make both PRs dependent on each other.

Is there any article on how to develop on a bundle and trying your changes at the same time (without commit/push to github)? My previous PR changes were small enough to not test them, but now it's gonna need more than that. I tried to find some composer way to require a local package, but can't get it to work. How do you guys do this?

I use inline aliases, or develop directly in the vendors of my project. Or just test with unit tests :P

Same issue here with duplicate translations. Any news ? Should be good to have a fix, 2 years after this issue was opened :).

@flolivaud would you like to contribute it?

Hi
sonata core: 3.16.1

the translation strategy parameter could be put in sonata_core.yaml

sonata_core:

    flashmessage:
        success:
            types:
                - { type: admin_success, domain: admin, translation_strategy: managed }

        warning:
            types:
                - { type: admin_warning, domain: admin, translation_strategy: managed }

        error:
            types:
                - { type: admin_error, domain: admin, translation_strategy: managed }

translation_strategy could take 3 values

  • legacy : actual mode with double call to trans()
  • managed : only one call to trans()
  • passtrough : no internal call to trans()