Merge pull request #717 from creative-commoners/pulls/5.0/better_email_recipient_logic

adds logic for presence of email recipient fields

code/Control/UserDefinedFormController.php

@@ -163,7 +163,6 @@ class UserDefinedFormController extends PageController
      */
     public function generateConditionalJavascript()
     {
-        $default = '';
         $rules = '';
         $form = $this->data();
         $formFields = $form->Fields();
@@ -325,14 +324,6 @@ JS
                     $email->addData($key, $value);
                 }
 
-                $email->setFrom(explode(',', $recipient->EmailFrom));
-                $email->setTo(explode(',', $recipient->EmailAddress));
-                $email->setSubject($recipient->EmailSubject);
-
-                if ($recipient->EmailReplyTo) {
-                    $email->setReplyTo(explode(',', $recipient->EmailReplyTo));
-                }
-
                 // check to see if they are a dynamic reply to. eg based on a email field a user selected
                 if ($recipient->SendEmailFromField()) {
                     $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailFromField()->Name);
@@ -340,14 +331,26 @@ JS
                     if ($submittedFormField && is_string($submittedFormField->Value)) {
                         $email->setReplyTo(explode(',', $submittedFormField->Value));
                     }
+                } elseif ($recipient->EmailReplyTo) {
+                    $email->setReplyTo(explode(',', $recipient->EmailReplyTo));
                 }
+
+                // check for a specified from; otherwise fall back to server defaults
+                if ($recipient->EmailFrom) {
+                    $email->setFrom(explode(',', $recipient->EmailFrom));
+                }
+
                 // check to see if they are a dynamic reciever eg based on a dropdown field a user selected
                 if ($recipient->SendEmailToField()) {
                     $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));
                 }
 
                 // check to see if there is a dynamic subject
@@ -356,7 +359,11 @@ JS
 
                     if ($submittedFormField && trim($submittedFormField->Value)) {
                         $email->setSubject($submittedFormField->Value);
+                    } else {
+                        $email->setSubject($recipient->EmailSubject);
                     }
+                } else {
+                    $email->setSubject($recipient->EmailSubject);
                 }
 
                 $this->extend('updateEmail', $email, $recipient, $emailData);

code/Form/UserForm.php

@@ -174,6 +174,7 @@ class UserForm extends Form
             ->Fields()
             ->filter('Required', true)
             ->column('Name');
+        $requiredNames = array_merge($requiredNames, $this->getEmailRecipientRequiredFields());
         $required = new RequiredFields($requiredNames);
         $this->extend('updateRequiredFields', $required);
         $required->setForm($this);
@@ -203,4 +204,30 @@ class UserForm extends Form
     {
         return $this->config()->get('button_text');
     }
+
+    /**
+     * Push fields into the RequiredFields array if they are used by any Email recipients.
+     * Ignore if there is a backup i.e. the plain string field is set
+     *
+     * @return array required fields names
+     */
+    protected function getEmailRecipientRequiredFields()
+    {
+        $requiredFields = [];
+        $recipientFieldsMap = [
+            'EmailAddress' => 'SendEmailToField',
+            'EmailSubject' => 'SendEmailSubjectField',
+            'EmailReplyTo' => 'SendEmailFromField'
+        ];
+
+        foreach ($this->getController()->data()->EmailRecipients() as $recipient) {
+            foreach ($recipientFieldsMap as $textField => $dynamicFormField) {
+                if (empty($recipient->$textField) && $recipient->getComponent($dynamicFormField)->exists()) {
+                    $requiredFields[] = $recipient->getComponent($dynamicFormField)->Name;
+                }
+            }
+        }
+
+        return $requiredFields;
+    }
 }

code/Model/Recipient/EmailRecipient.php

@@ -612,6 +612,11 @@ class EmailRecipient extends DataObject
                 }
             }
         }
+
+        // if there is no from address and no fallback, you'll have errors if this isn't defined
+        if (!$this->EmailFrom && empty(Email::getSendAllEmailsFrom()) && empty(Email::config()->get('admin_email'))) {
+            $result->addError(_t(__CLASS__.".EMAILFROMREQUIRED", '"Email From" address is required'));
+        }
         return $result;
     }
 }

lang/en.yml

@@ -191,6 +191,7 @@ en:
     EMAILCONTENTTAB: 'Email Content'
     EMAILDETAILSTAB: 'Email Details'
     EMAILFROMINVALID: '"Email From" is not valid'
+    EMAILFROMREQUIRED: '"Email From" address is required'
     EMAILREPLYTOINVALID: '"Email Reply To" is not valid'
     PLURALNAME: 'User Defined Form Email Recipients'
     SINGULARNAME: 'User Defined Form Email Recipient'

tests/Model/UserDefinedFormTest.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\UserForms\Tests\Model;
 
 use SilverStripe\Control\Controller;
+use SilverStripe\Control\Email\Email;
 use SilverStripe\Core\Convert;
 use SilverStripe\Dev\FunctionalTest;
 use SilverStripe\Forms\DropdownField;
@@ -34,6 +35,12 @@ class UserDefinedFormTest extends FunctionalTest
         UserDefinedForm::class => [UserFormFieldEditorExtension::class],
     ];
 
+    protected function setUp()
+    {
+        parent::setUp();
+        Email::config()->update('admin_email', 'no-reply@example.com');
+    }
+
     public function testRollbackToVersion()
     {
         $this->markTestSkipped(
@@ -70,10 +77,10 @@ class UserDefinedFormTest extends FunctionalTest
 
         $fields = $form->getCMSFields();
 
-        $this->assertTrue($fields->dataFieldByName('Fields') !== null);
-        $this->assertTrue($fields->dataFieldByName('EmailRecipients') != null);
-        $this->assertTrue($fields->dataFieldByName('Submissions') != null);
-        $this->assertTrue($fields->dataFieldByName('OnCompleteMessage') != null);
+        $this->assertNotNull($fields->dataFieldByName('Fields'));
+        $this->assertNotNull($fields->dataFieldByName('EmailRecipients'));
+        $this->assertNotNull($fields->dataFieldByName('Submissions'));
+        $this->assertNotNull($fields->dataFieldByName('OnCompleteMessage'));
     }
 
 
@@ -107,12 +114,12 @@ class UserDefinedFormTest extends FunctionalTest
 
         $fields = $popup->getCMSFields();
 
-        $this->assertTrue($fields->dataFieldByName('EmailSubject') !== null);
-        $this->assertTrue($fields->dataFieldByName('EmailFrom') !== null);
-        $this->assertTrue($fields->dataFieldByName('EmailAddress') !== null);
-        $this->assertTrue($fields->dataFieldByName('HideFormData') !== null);
-        $this->assertTrue($fields->dataFieldByName('SendPlain') !== null);
-        $this->assertTrue($fields->dataFieldByName('EmailBody') !== null);
+        $this->assertNotNull($fields->dataFieldByName('EmailSubject'));
+        $this->assertNotNull($fields->dataFieldByName('EmailFrom'));
+        $this->assertNotNull($fields->dataFieldByName('EmailAddress'));
+        $this->assertNotNull($fields->dataFieldByName('HideFormData'));
+        $this->assertNotNull($fields->dataFieldByName('SendPlain'));
+        $this->assertNotNull($fields->dataFieldByName('EmailBody'));
 
         // add an email field, it should now add a or from X address picker
         $email = $this->objFromFixture(EditableEmailField::class, 'email-field');