Update storage of contact form email recipients

Patch attached for review.

Patch does a good job fixing the issue at admin/build/contact but the list still fails to wrap at admin/build/contact/edit/%n ...we either need to also fix the issue there or change that textarea to a text input (no point in having a textarea when everything's being put on one line).

I'm thinking that we should probably do it the Drupal(tm) way and have them list e-mail address, one per line instead of separated by commas with no spaces. That's just silly.

Revised patch that uses newlines instead of commas. Don't have time to test the upgrade path, but will check tomorrow. Peer review welcomed.

I rerolled it

Does this need to be pushed to 8.x now?

#7: 267492-7.patch queued for re-testing.

added screenshots

no update of config needed, so just testing coverage

Patch makes edit form better

And iteration on list builder, each recipient is valid email so safe markup is unneded

And added to hide column on small screens

Fix tests

Improved list builder to not render list if there's only one recepient and display "No recipients" for empty list

Fix tests

This seems to work now as described with the line breaks.

I wonder if it would be useful to filter for comma's - given that we used to be able to use them ",". They could be just stripped out, right?

More importantly, it would be useful to strip out blank lines (especially the final one). Kinda annoying to be tripped up because of a blank line return.

Thanks for pushing this ahead @andypost - I modified the summary a bit to reflect the new URL.

@mgifford there's no way to have blank lines, do you mean that?

looks that needs test or separate issue because currently you can't enter my@gmail.com, , my@mail.ru

Patch looks good, but we need an update hook to migrate the config from , separated to "\n"

Actually @andypost pointed out they're stored as an array, so no need for the update hook - nice

Yep, no upgrade path needed.

Looks having empty lines needs more tests, to make sure that mail send allows empty recipients (, , ,)

@andypost - I was more worried about having an empty line break at the end of a list of emails. I tested drupal without the patch and my@gmail.com, my@mail.ru, fails too, which is the same problem.

I can set up a different issue though to deal with this as well as my@gmail.com \n my@mail.ru \n & my@gmail.com \n \n my@mail.ru

Needs re-roll after #2557577: Use comma separated item list in contact listing and remove a SafeMarkup::checkPlain()

re-roll + render one item with $row['recipients']['data']['#markup'] = $recipients[0];

maybe better to sort emails before display?

Clean-up, now listing displayed fine so let's keep the comma list

re-roll

Still needs usability review

Please, do not save empty "recipients" lines into storage only to provide way to group them.

for #41

Looks good to me, what needs review here?

Added empty lines removal

Optimized code a bit.

That will fail
Value array ( 0 => 'simpletest&@example.com', 2 => 'simpletest2@example.com', ) is equal to value array ( 0 => 'simpletest&@example.com', 1 => '', 2 => 'simpletest2@example.com', ).
because there's a test for empty lines

Also you have to set the value back to form state to allow other validation to work with cleaned values, also that's require a short comment

+++ b/core/modules/contact/src/MailHandler.php
@@ -102,7 +102,7 @@ public function sendMailMessages(MessageInterface $message, AccountInterface $se
-      $to = implode(', ', $contact_form->getRecipients());
+      $to = implode(', ', array_filter($contact_form->getRecipients()));

array_filter() is not needed if validation clean-ups the empty lines

Fixed test cases, removed extra array_filter

Patch re rolled #52

This patch should fix the inline list-items to a block element.

It's not possible to validate all emails on the same line, separate by spaces or commas.

The instruction “To specify multiple recipients, separate each email address on a new line” works well.

But the result is not perfect :

Each email (on a new line) is between two symbols (a dot and a comma). Maybe only just the dot will be enough ?
Moreover there is no filter to check if an address is duplicated

So the issue needs works

Patch re-rolled #58

Patch re rolled as per comment #57

Comment #60 task is pending

Please update the issue summary.

+++ b/core/modules/contact/src/ContactFormListBuilder.php
@@ -34,8 +37,8 @@ public function buildRow(EntityInterface $entity) {
+      $row['recipients'] = $this->t('Selected user');
+      $row['selected'] = $this->t('No');

@@ -45,7 +48,7 @@ public function buildRow(EntityInterface $entity) {
+      $row['selected'] = ($default_form == $entity->id() ? $this->t('Yes') : $this->t('No'));

These changes are not needed.

Updated the IS to reflect the current state. I tested and the spaces issue does not occur anymore on 9.0.3, at least from what I can tell? Tested entering a long list of emails without spaces and spaces get added on save. It works fine at various screen sizes. (Although a very long email address still breaks mobile, so the fact that the patch hides the column on mobile is good).

Do we still want to update this so that you can use newlines instead of commas? It would address this issue #3111336: comma separated list of recipients is not working.

No spaces:

After save:

Re rolled to 9.2.x

Previously had wrong version, just switched to 9.2 and confirmed
Please check the changes ->

fixed some coding standard issues

Fixed some more bugs

Seems. the config is not updated after updating the form.

Discussed with @Lendude in Slack, there doesn't seem to be any bug here. The form works as described.

If adding the email addresses on new lines is strongly desirable, let's open a new feature request with the details and the reasoning. This issue is so old, it's best to close it to avoid confusion.