Fix #28 - NotEmpty validation message cannot be localized

[stefanotorresi]

even after #60, #61 and #63, issue #28 was still reproducible.

Related:

• 'Value is required' message should be localizable #28
• Not set default required error message always #60
• Provide NotEmpty::IS_EMPTY validation message for required input #61
• Ensure custom error messages are used for required missing inputs #63
• Feature/test cleanup #64
• Fix #28 - NotEmpty validation message cannot be localized #67
• Input::prepareRequiredValidationFailureMessage return value #69
• Hotfix/69 #73
• Missing required failure message for CollectionInputFilter #94

[weierophinney]

Translator will always be empty here because you're directly instantiating it. Try pulling it from the validator chain instead, add that will likely use the plugin manager, and thus have access to the translator.

[stefanotorresi]

Hmm I beg to differ. This actually works in a real application.

The test reproduces the issue with the configuration suggested in http://framework.zend.com/manual/current/en/modules/zend.validator.messages.html#using-pre-translated-validation-messages

I also have a functional test in a real world application that reproduces it exactly in the same way.

The translator will not be empty even in new instances, as long as Zend\Validator\AbstractValidator::setDefaultTranslator() is used;
I tried avoding the static method usage, but in my experience the ValidatorPluginManager does not work consistently and I ended up with the translator not being correctly injected.

I can change the new statement with a plugin() invocation from the validator chain, but the static method call will still be necessary.

[Maks3w]

Please take a look to #62

[stefanotorresi]

@Maks3w it comes down to what version are you targeting with that PR. To me it looks like it's a bc break, while this PR is completely BC safe. Your call either way, as far as I'm concerned I have nothing more to add to this one, as I have addressed @weierophinney request to pull the validator from the plugin manager in the last commit.

[Slamdunk]

So @Maks3w , is this PR going to be merged please?

You can tag this in a minor release, and then think about #62 for a future mid-release.
As a non-english-site user, our business apps really need this.

[Maks3w]

/fw @weierophinney

[weierophinney]

@Maks3w Are you wanting me to review this one or #62?

[Maks3w]

@weierophinney #62 still being a PoC for future developments. I don't consider it enough stable. The point I forward this discussion to you is for assign you all the issues and PRs open about the translation of this message.

[weierophinney]

@Maks3w Could you maybe curate a list for me?

[Maks3w]

@weierophinney I've updated the PR comment with a list of related issues/PRs

[johnnymallie]

Hi ! Any news about the release ? Looking forward to it :)

[jaapio]

It would be very helpfull for us to have this merged. It provides a solution to a very common problem. I know this is not the ideal solution but it is a step forward in the right direction

[Slamdunk]

🔔

[stefanotorresi]

@weierophinney I think this is safe to merge for now, and then we can start working with a broader solution encompassing all the other related issues, but ultimately it's your call, of course :)
Let me know if you need me to change anything.