sethjohnson1 / conflist5

Conflist cakePHP5
1 stars 0 forks source link

comma separated emails save fails #25

Closed sethjohnson1 closed 3 weeks ago

sethjohnson1 commented 1 month ago

Noticed if I put two emails:

email1@example.com, email2@example.com

I think we should only ask for one. If not, we need to check for commas, make an array, validate each, and also make sure and count (i.e. if more than x entries, ignore)

nilesjohnson commented 1 month ago

Strange; is the error happening in validation? I get a similar error (not the validation message) if I have a single mal-formatted email.

I guess if we need to set a limit on the number of to addresses, it might as well be one. Looking at the live site, there are some cases (like 2 or 3 in the past 20 announcements) where people list multiple addresses. Probably they are organizing a conference together, and so the person making the post included the other organizers for reference. If they had to enter only one address, they could just forward the confirmation later.

On the other hand, preg_split does the checking and splitting pretty easily:

https://github.com/sethjohnson1/conflist5/blob/a09c8a09f095d12c433b8529d89040520f6541e9/src/Controller/ConferencesController.php#L519

If there's no match, it just returns an array with the original string as its only entry.

nilesjohnson commented 1 month ago

oh, I just realized that in the previous version I use a custom validation method multiEmail with preg_split and then looping over the entries validating each one. So, we could just stop having that if you don't want to deal with it.

sethjohnson1 commented 1 month ago

Ah, yeah that old function was a good point in the right direction. I added a max of 5 (set via app.php config) so spammers don't use it as a personal delivery system.

And yeah, validation error handling in general was wonky. Should be better now, but help with testing would be good.

I think now you might need to check for array/loop on your email send. Will add a note to #24

nilesjohnson commented 3 weeks ago

this one is done now too!!