I'm testing the 8.x-2.2 release. I'm now kinda wishing #2855645: Re-allow multiple recipients didn't go into the 8.x-2.x branch. :/
It seems fairly strange that forward_max_recipients_error is a config setting. Some limitations I see from this:
- It really wants to use
TranslationInterface::formatPlural(), which is impossible as a config setting. - It's only translatable if you go through all the trouble to have separate translations of your forward.settings config entity, which is non-trivial.
- Needlessly bloats the settings UI.
I don't see any discussion at #2855645 about why this is the way it is. I vote for ripping this setting out and having a hard-coded (but easily translatable) formatPlural() that doesn't need "recipient(s)" and the like.
While we're at it, instead of $form_state->setErrorByName('', $message); why not set the validation error on the form element that's invalid? I.e. $form_state->setErrorByName('recipient', $message); (or whatever it should be). ;)
Thoughts?
Thanks!
-Derek
Comments
Comment #2
dwwGuess we should call this another sub issue of #2855645...
Comment #3
john.oltman commented@dww I am fine if you want to submit a patch which removes the config setting and implements it differently. I agree with your comments.
Comment #4
dwwSounds good. Not sure when I'll have a chance to write such a patch, but I'm glad to know you'd support it if it came into being. ;) Maybe someone else wants to write it in the meanwhile?
Cheers,
-Derek
Comment #5
john.oltman commentedCircling back, needing to change those error messages is definitely an edge case and having those as config setting is overkill and causes the plural handling issue. The 2 error messages will be removed from config in 2.3 release and the code changed to use a format_plural. If a system admin really wants to change those, they can simply translate (English to English is now fairly easy to do in Drupal 8).
Comment #7
john.oltman commentedComment #8
dwwCommit looks great. Haven't tested (yet), but I'll try to give this a spin in the next few days (along with the rest of the recent goodness in the 8.x-2.x branch) and let you know if I hit any trouble.
Thanks!
-Derek
Comment #9
john.oltman commentedThanks Derek, please test using the release candidate:
https://www.drupal.org/project/forward/releases/8.x-2.3-rc1