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:

  1. It really wants to use TranslationInterface::formatPlural(), which is impossible as a config setting.
  2. 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.
  3. 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

dww created an issue. See original summary.

dww’s picture

Guess we should call this another sub issue of #2855645...

john.oltman’s picture

@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.

dww’s picture

Sounds 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

john.oltman’s picture

Assigned: Unassigned » john.oltman

Circling 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).

  • john.oltman committed 8c7c9a8 on 8.x-2.x
    Issue #3034570: Limitations from having 'forward_max_recipients_error'...
john.oltman’s picture

Status: Active » Fixed
dww’s picture

Commit 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

john.oltman’s picture

Thanks Derek, please test using the release candidate:

https://www.drupal.org/project/forward/releases/8.x-2.3-rc1

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.