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

Double error translation #162

Closed
2bad2furious opened this issue Aug 4, 2017 · 9 comments
Closed

Double error translation #162

2bad2furious opened this issue Aug 4, 2017 · 9 comments

Comments

@2bad2furious
Copy link
Contributor

@2bad2furious 2bad2furious commented Aug 4, 2017

  • bug report? yes
  • feature request? no
  • version: 2.4.5

Description

I have noticed, that erros from inputs are translated twice. When they arrive to DeafultFormRenderer::renderErrors, they are already translated. However, they are translated again.

@dg

This comment has been minimized.

Copy link
Member

@dg dg commented Aug 5, 2017

Related to #145

ping @vlastavesely

@2bad2furious 2bad2furious changed the title Dvojí překlad errorů Double error translation Aug 5, 2017
@vlastavesely

This comment has been minimized.

Copy link
Contributor

@vlastavesely vlastavesely commented Aug 5, 2017

I prepared test to test it but no translation duplicity found on tested case. @2bad2furious could you prepare failing test?

@2bad2furious

This comment has been minimized.

Copy link
Contributor Author

@2bad2furious 2bad2furious commented Aug 5, 2017

I am not exactly sure how to do that. I triggered error everytime i couldnt find a translation for a message and found the bug when i couldnt get a translation for already translated message. It happened to me with backend form checking using (probably) anonymous function.
$form->addEmail(UserManagement::EMAIL, UserManagement::EMAIL_LABEL) ->setRequired(UserManagement::EMAIL_REQUIRED) ->addRule(function (\Nette\Forms\Controls\TextInput $email) { return $this->getUserManager()->isEmailUnique($email->getValue()); }, UserManagement::EMAIL_EXISTS)

I can try to find exactly the point, where it gets translated for the first time.

@2bad2furious

This comment has been minimized.

Copy link
Contributor Author

@2bad2furious 2bad2furious commented Aug 5, 2017

@vlastavesely Found the first translation. It is called at Forms/Validator::formatMessage() at line 63.

@vlastavesely

This comment has been minimized.

Copy link
Contributor

@vlastavesely vlastavesely commented Aug 5, 2017

I think it is not a problem of the DefaultFormRenderer then. I am not really sure whether it can be even solved. The renderer cannot know whether a message has been translated already by other class or not.

@2bad2furious

This comment has been minimized.

Copy link
Contributor Author

@2bad2furious 2bad2furious commented Aug 5, 2017

I get that. But I still don't think it should be happening. It should be translated in one class or another, not both.

@vlastavesely

This comment has been minimized.

Copy link
Contributor

@vlastavesely vlastavesely commented Aug 17, 2017

I thought about it a little bit.

The only reason for the translation being in the Validators::formatMessage() is the $count parameter that is used in translate() method.

$message = $translator->translate($message, is_int($rule->arg) ? $rule->arg : null);

But this seems to me to be wrong. Assume that you would like to validate whether a number is smaller than 5. Then the $rule->arg will be 5 and this does not have anything in common with count. Maybe I am missing something but I think it could be removed from here. But possibly, it could be a BC break.

@dg what do you think?

@dg

This comment has been minimized.

Copy link
Member

@dg dg commented Aug 17, 2017

It is used for validators like MIN_LENGTH, ie. musí mít alespoň 2 znaky vs 5 znaků

@dg dg closed this in 51a90b2 Aug 17, 2017
dg added a commit that referenced this issue Aug 17, 2017
…loses #162]

This reverts commit 7c4e4ca.
dg added a commit that referenced this issue Aug 17, 2017
…loses #162]

This reverts commit 7c4e4ca.
@dg

This comment has been minimized.

Copy link
Member

@dg dg commented Aug 17, 2017

Fixed this way 1888029

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.