Add presence validation for EmailRecipient recipient, add error handling

This commit is contained in:
Garion Herman 2019-05-03 15:55:59 +12:00
parent b3dc50d253
commit 2f0aea847a
4 changed files with 44 additions and 7 deletions

View File

@ -2,13 +2,16 @@
namespace SilverStripe\UserForms\Control; namespace SilverStripe\UserForms\Control;
use Exception;
use PageController; use PageController;
use Psr\Log\LoggerInterface;
use SilverStripe\Assets\File; use SilverStripe\Assets\File;
use SilverStripe\Assets\Upload; use SilverStripe\Assets\Upload;
use SilverStripe\Control\Controller; use SilverStripe\Control\Controller;
use SilverStripe\Control\Email\Email; use SilverStripe\Control\Email\Email;
use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse;
use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPRequest;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\Core\Manifest\ModuleLoader;
use SilverStripe\Forms\Form; use SilverStripe\Forms\Form;
use SilverStripe\i18n\i18n; use SilverStripe\i18n\i18n;
@ -23,6 +26,7 @@ use SilverStripe\UserForms\Model\Submission\SubmittedForm;
use SilverStripe\View\ArrayData; use SilverStripe\View\ArrayData;
use SilverStripe\View\Requirements; use SilverStripe\View\Requirements;
use SilverStripe\View\SSViewer; use SilverStripe\View\SSViewer;
use Swift_RfcComplianceException;
/** /**
* Controller for the {@link UserDefinedForm} page type. * Controller for the {@link UserDefinedForm} page type.
@ -350,6 +354,8 @@ JS
// check to see if they are a dynamic reciever eg based on a dropdown field a user selected // check to see if they are a dynamic reciever eg based on a dropdown field a user selected
$emailTo = $recipient->SendEmailToField(); $emailTo = $recipient->SendEmailToField();
try {
if ($emailTo && $emailTo->exists()) { if ($emailTo && $emailTo->exists()) {
$submittedFormField = $submittedFields->find('Name', $recipient->SendEmailToField()->Name); $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailToField()->Name);
@ -361,6 +367,18 @@ JS
} else { } else {
$email->setTo(explode(',', $recipient->EmailAddress)); $email->setTo(explode(',', $recipient->EmailAddress));
} }
} catch (Swift_RfcComplianceException $e) {
// The sending address is empty and/or invalid. Log and skip sending.
$error = sprintf(
'Failed to set sender for userform submission %s: %s',
$submittedForm->ID,
$e->getMessage()
);
Injector::inst()->get(LoggerInterface::class)->notice($error);
continue;
}
// check to see if there is a dynamic subject // check to see if there is a dynamic subject
$emailSubject = $recipient->SendEmailSubjectField(); $emailSubject = $recipient->SendEmailSubjectField();

View File

@ -550,6 +550,12 @@ class EmailRecipient extends DataObject
if (!$this->EmailFrom && empty(Email::getSendAllEmailsFrom()) && empty(Email::config()->get('admin_email'))) { if (!$this->EmailFrom && empty(Email::getSendAllEmailsFrom()) && empty(Email::config()->get('admin_email'))) {
$result->addError(_t(__CLASS__.".EMAILFROMREQUIRED", '"Email From" address is required')); $result->addError(_t(__CLASS__.".EMAILFROMREQUIRED", '"Email From" address is required'));
} }
// Sending will also fail if there's no recipient defined
if (!$this->EmailAddress && !$this->SendEmailToFieldID) {
$result->addError(_t(__CLASS__.".EMAILTOREQUIRED", '"Send email to" address or field is required'));
}
return $result; return $result;
} }

View File

@ -27,4 +27,15 @@ class EmailRecipientTest extends SapphireTest
$result = $recipient->getEmailBodyContent(); $result = $recipient->getEmailBodyContent();
$this->assertContains('/about-us/', $result); $this->assertContains('/about-us/', $result);
} }
/**
* @expectedException SilverStripe\ORM\ValidationException
* @expectedExceptionMessage "Send email to" address or field is required
*/
public function testEmptyRecipientFailsValidation()
{
$recipient = new EmailRecipient();
$recipient->EmailFrom = 'test@example.com';
$recipient->write();
}
} }

View File

@ -104,7 +104,6 @@ class UserDefinedFormTest extends FunctionalTest
$this->assertNotContains('SummaryHide', array_keys($summaryFields), 'Summary field showing displayed field'); $this->assertNotContains('SummaryHide', array_keys($summaryFields), 'Summary field showing displayed field');
} }
public function testEmailRecipientPopup() public function testEmailRecipientPopup()
{ {
$this->logInWithPermission('ADMIN'); $this->logInWithPermission('ADMIN');
@ -114,6 +113,7 @@ class UserDefinedFormTest extends FunctionalTest
$popup = new EmailRecipient(); $popup = new EmailRecipient();
$popup->FormID = $form->ID; $popup->FormID = $form->ID;
$popup->FormClass = UserDefinedForm::class; $popup->FormClass = UserDefinedForm::class;
$popup->EmailAddress = 'test@example.com';
$fields = $popup->getCMSFields(); $fields = $popup->getCMSFields();
@ -146,6 +146,7 @@ class UserDefinedFormTest extends FunctionalTest
public function testGetEmailBodyContent() public function testGetEmailBodyContent()
{ {
$recipient = new EmailRecipient(); $recipient = new EmailRecipient();
$recipient->EmailAddress = 'test@example.com';
$emailBody = 'not html'; $emailBody = 'not html';
$emailBodyHtml = '<p>html</p>'; $emailBodyHtml = '<p>html</p>';
@ -185,6 +186,7 @@ class UserDefinedFormTest extends FunctionalTest
$recipient = new EmailRecipient(); $recipient = new EmailRecipient();
$recipient->FormID = $page->ID; $recipient->FormID = $page->ID;
$recipient->FormClass = UserDefinedForm::class; $recipient->FormClass = UserDefinedForm::class;
$recipient->EmailAddress = 'test@example.com';
// Set the default template // Set the default template
$recipient->EmailTemplate = current(array_keys($recipient->getEmailTemplateDropdownValues())); $recipient->EmailTemplate = current(array_keys($recipient->getEmailTemplateDropdownValues()));