From 8c3451964405e34724eb556ac13af5d2524ee33f Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Tue, 13 Feb 2018 16:24:53 +0000 Subject: [PATCH 1/2] adds logic for presence of email recipient fields --- code/Control/UserDefinedFormController.php | 21 +++++++++-------- code/Form/UserForm.php | 27 ++++++++++++++++++++++ code/Model/Recipient/EmailRecipient.php | 5 ++++ lang/en.yml | 1 + 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/code/Control/UserDefinedFormController.php b/code/Control/UserDefinedFormController.php index 6da7830..320335c 100644 --- a/code/Control/UserDefinedFormController.php +++ b/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,7 +331,15 @@ 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); @@ -348,6 +347,8 @@ JS if ($submittedFormField && is_string($submittedFormField->Value)) { $email->setTo(explode(',', $submittedFormField->Value)); } + } else { + $email->setTo(explode(',', $recipient->EmailAddress)); } // check to see if there is a dynamic subject @@ -357,6 +358,8 @@ JS if ($submittedFormField && trim($submittedFormField->Value)) { $email->setSubject($submittedFormField->Value); } + } else { + $email->setSubject($recipient->EmailSubject); } $this->extend('updateEmail', $email, $recipient, $emailData); diff --git a/code/Form/UserForm.php b/code/Form/UserForm.php index a22ae41..42d4c8e 100644 --- a/code/Form/UserForm.php +++ b/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)) { + $requiredFields[] = $recipient->getComponent($dynamicFormField)->Name; + } + } + } + + return $requiredFields; + } } diff --git a/code/Model/Recipient/EmailRecipient.php b/code/Model/Recipient/EmailRecipient.php index e26fa47..e4c6443 100644 --- a/code/Model/Recipient/EmailRecipient.php +++ b/code/Model/Recipient/EmailRecipient.php @@ -613,6 +613,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; } } diff --git a/lang/en.yml b/lang/en.yml index ebc111a..c4f528e 100644 --- a/lang/en.yml +++ b/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' From 0f6efea12a918e7a6e613e1905ef8bac08d945b3 Mon Sep 17 00:00:00 2001 From: Andrew Aitken-Fincham Date: Tue, 13 Feb 2018 17:21:03 +0000 Subject: [PATCH 2/2] add proper fallbacks to cover tests --- code/Control/UserDefinedFormController.php | 4 ++++ code/Form/UserForm.php | 2 +- code/Model/Recipient/EmailRecipient.php | 2 +- tests/Model/UserDefinedFormTest.php | 27 ++++++++++++++-------- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/code/Control/UserDefinedFormController.php b/code/Control/UserDefinedFormController.php index 320335c..b37bf93 100644 --- a/code/Control/UserDefinedFormController.php +++ b/code/Control/UserDefinedFormController.php @@ -346,6 +346,8 @@ JS if ($submittedFormField && is_string($submittedFormField->Value)) { $email->setTo(explode(',', $submittedFormField->Value)); + } else { + $email->setTo(explode(',', $recipient->EmailAddress)); } } else { $email->setTo(explode(',', $recipient->EmailAddress)); @@ -357,6 +359,8 @@ JS if ($submittedFormField && trim($submittedFormField->Value)) { $email->setSubject($submittedFormField->Value); + } else { + $email->setSubject($recipient->EmailSubject); } } else { $email->setSubject($recipient->EmailSubject); diff --git a/code/Form/UserForm.php b/code/Form/UserForm.php index 42d4c8e..6211dda 100644 --- a/code/Form/UserForm.php +++ b/code/Form/UserForm.php @@ -222,7 +222,7 @@ class UserForm extends Form foreach ($this->getController()->data()->EmailRecipients() as $recipient) { foreach ($recipientFieldsMap as $textField => $dynamicFormField) { - if (empty($recipient->$textField) && $recipient->getComponent($dynamicFormField)) { + if (empty($recipient->$textField) && $recipient->getComponent($dynamicFormField)->exists()) { $requiredFields[] = $recipient->getComponent($dynamicFormField)->Name; } } diff --git a/code/Model/Recipient/EmailRecipient.php b/code/Model/Recipient/EmailRecipient.php index e4c6443..720a5f1 100644 --- a/code/Model/Recipient/EmailRecipient.php +++ b/code/Model/Recipient/EmailRecipient.php @@ -615,7 +615,7 @@ 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'))) { + if (!$this->EmailFrom && empty(Email::getSendAllEmailsFrom()) && empty(Email::config()->get('admin_email'))) { $result->addError(_t(__CLASS__.".EMAILFROMREQUIRED", '"Email From" address is required')); } return $result; diff --git a/tests/Model/UserDefinedFormTest.php b/tests/Model/UserDefinedFormTest.php index a7f38e3..a3cccff 100644 --- a/tests/Model/UserDefinedFormTest.php +++ b/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');