Use TranslatorInterface for typehint

[MarioBlazek]

Question	Answer
Tickets	N/A
Bug fix?	yes
New feature?	no
BC breaks?	no
Tests pass?	yes
Doc needed?	no

In some cases, autowire fails with following error:
[Symfony\Component\DependencyInjection\Exception\RuntimeException] Cannot autowire service "EzSystems\EzPlatformAdminUi\UI\Config\Mapper\FieldType\RichText\CustomTag": argument "$translator" of method "__construct()" references class "Symfony\Component\Translation\Translator" but no such service exi sts. Try changing the type-hint to "Symfony\Component\Translation\TranslatorInterface" instead.

[adamwojs]

+1, but type hint on $translator property should be adjusted as well 😉

[mikadamczyk]

Please change also type hint in src/lib/Tests/UI/Config/Mapper/FieldType/RichText/CustomTagTest.php

[adamwojs]

Unfortunately, unit tests failure is related to the proposed changes:

Trying to configure method "getCatalogue" which cannot be configured because it does not exist, has not been specified, is final, or is static

According to the discussion under original PR, it seems that the Translator was used intentionally there: #1007 (comment)

[MarioBlazek]

@adamwojs I will undo changes in the test then.

[mikadamczyk]

This is not a problem in the tests. I think it is a problem with autowiring. We could explicitly add Translator as an argument or we should have two separate services injected. Discussion was here #1007 (comment)

[Nattfarinn]

CustomTag class explicitly calls getCatalogue method which is not part of an interface. TypeHint has to remain Symfony\Component\Translation\Translator.

[webhdx]

Since I'm not the biggest fan of autowiring I'd say adding @translator to arguments in the service definition should be enough to resolve the issue.

[adamwojs]

I agree with @Nattfarinn and @webhdx. Reverting b89404a is not a solution.

[MarioBlazek]

@adamwojs @Nattfarinn @webhdx changes are back to Translator class. I have set @translator service to be injected explicitly.

[emodric]

IMO, autowiring should've never been used in eZ admin UI or any vendor code. Autowiring is great when developing apps or websites to enable devs to quickly get features up and running.

Using it anywhere else (basically, anywhere in vendor) is just messy, unreadable, hard to debug and error prone.

[MarioBlazek]

I think we should go with TranslatorInterface typehint as it is already on the master branch.

[MarioBlazek]

@mikadamczyk @adamwojs @Nattfarinn @webhdx what do you suggest to do here? Problem is I have a broken project now.

[MarioBlazek]

Ping @mikadamczyk @adamwojs @Nattfarinn @webhdx

[Nattfarinn]

@MarioBlazek There's a bug on master in that case as well - we'll fix it.

As for current code review: Revert change on src/lib/UI/Config/Mapper/FieldType/RichText/CustomTag.php file as right now this docblock is inaccurate. Typehint remains as Translator, not TranslatorInterface.

[hknezevic]

We have some new findings on this problem, which should help at least reproducing the issue.

One of our bundles has a dependency on Lexik Translation bundle ( https://packagist.org/packages/lexik/translation-bundle ). This bundle replaces the default class in @translator with its own implementation that extends and enriches the default Symfony translator service. When activated on the project using eZ Platform Admin UI, it will cause an error reported in this PR.

Was this issue ever addressed in the kernel?

[mateuszbieniek]

As it seems that PR is abandoned, I allowed myself to open a new one:
#1335

[emodric]

@mateuszbieniek How is this PR abandoned? Couldn't you just ask @MarioBlazek to rebase it?

[mateuszbieniek]

@emodric:
@Nattfarinn requested changes more than half a year ago without any reply from @MarioBlazek and due that this PR seemed abandoned to me.

Is this OK with you @MarioBlazek?

[MarioBlazek]

@mateuszbieniek as the new PR is already created, please proceed with it, and tag fix asap.

[emodric]

That doesn't make it abandoned. @MarioBlazek also asked you for feedback 6 months ago and it was ignored. So not that you actually need this fix, instead of asking @MarioBlazek to finish this PR, you're just going to ignore it, again. Not really nice thing to do.

[mateuszbieniek]

That doesn't make it abandoned. @MarioBlazek also asked you for feedback 6 months ago and it was ignored. So not that you actually need this fix, instead of asking @MarioBlazek to finish this PR, you're just going to ignore it, again. Not really nice thing to do.

I'm sorry that you feel that way. As long as @MarioBlazek is OK, we can continue in either PR.

[emodric]

If @MarioBlazek is okay with it, I am too. I just feel like his efforts were not recognized as they should be.

[mateuszbieniek]

If @MarioBlazek is okay with it, I am too. I just feel like his efforts were not recognized as they should be.

You are right - did not think it trough. I hope that @MarioBlazek does not feel that way. If so, I apologize.

[mateuszbieniek]

Hi @MarioBlazek,
I'm closing this PR as the issue was fixed in #1335

Thank you for your contribution!