Merge pull request silverstripe#881 from creative-commoners/pulls/5.4…

…/resilient-recipients

Add presence validation for EmailRecipient recipient, add error handling

code/Control/UserDefinedFormController.php

@@ -2,13 +2,16 @@
 
 namespace SilverStripe\UserForms\Control;
 
+use Exception;
 use PageController;
+use Psr\Log\LoggerInterface;
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Upload;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Email\Email;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Core\Manifest\ModuleLoader;
 use SilverStripe\Forms\Form;
 use SilverStripe\i18n\i18n;
@@ -23,6 +26,7 @@
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\Requirements;
 use SilverStripe\View\SSViewer;
+use Swift_RfcComplianceException;
 
 /**
  * Controller for the {@link UserDefinedForm} page type.
@@ -350,16 +354,30 @@ public function process($data, $form)
 
  // check to see if they are a dynamic reciever eg based on a dropdown field a user selected
  $emailTo = $recipient->SendEmailToField();
- if ($emailTo && $emailTo->exists()) {
- $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailToField()->Name);
 
- if ($submittedFormField && is_string($submittedFormField->Value)) {
- $email->setTo(explode(',', $submittedFormField->Value));
+ try {
+ if ($emailTo && $emailTo->exists()) {
+ $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailToField()->Name);
+
+ if ($submittedFormField && is_string($submittedFormField->Value)) {
+ $email->setTo(explode(',', $submittedFormField->Value));
+ } else {
+ $email->setTo(explode(',', $recipient->EmailAddress));
+ }
  } else {
  $email->setTo(explode(',', $recipient->EmailAddress));
  }
- } else {
- $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

code/Model/Recipient/EmailRecipient.php

@@ -550,6 +550,12 @@ public function validate()
  if (!$this->EmailFrom && empty(Email::getSendAllEmailsFrom()) && empty(Email::config()->get('admin_email'))) {
  $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;
  }
 

tests/Model/Recipient/EmailRecipientTest.php

@@ -27,4 +27,15 @@ public function testShortcodesAreRenderedInEmailPreviewContent()
  $result = $recipient->getEmailBodyContent();
  $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 = '[email protected]';
+ $recipient->write();
+ }
 }

tests/Model/UserDefinedFormTest.php

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