Skip to content

Commit 31bdbbf

Browse files
committed
fix: Address PR nextcloud#3098 review comments on confirmation emails
- Allow form creators to explicitly select which email field to use for confirmations via the `confirmationEmailRecipient` property. - Update `FormsService` to rely on the explicit recipient field when multiple email fields are present. - Display relevant warnings in the settings tab if recipient fields are misconfigured. - Replace basic email validation with Nextcloud's internal `\OCP\Mail\IEmailValidator`. - Expand unit tests to verify explicit recipient selection and multi-email field validation logic.
1 parent d19fe2e commit 31bdbbf

6 files changed

Lines changed: 327 additions & 50 deletions

File tree

lib/Constants.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ class Constants {
147147
];
148148

149149
public const EXTRA_SETTINGS_SHORT = [
150+
'confirmationEmailRecipient' => ['boolean'],
150151
'validationType' => ['string'],
151152
'validationRegex' => ['string'],
152153
];

lib/ResponseDefinitions.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
* dateMax?: int,
2727
* dateMin?: int,
2828
* dateRange?: bool,
29+
* confirmationEmailRecipient?: bool,
2930
* maxAllowedFilesCount?: int,
3031
* maxFileSize?: int,
3132
* optionsHighest?: 2|3|4|5|6|7|8|9|10,

lib/Service/FormsService.php

Lines changed: 59 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OCP\IUser;
3636
use OCP\IUserManager;
3737
use OCP\IUserSession;
38+
use OCP\Mail\IEmailValidator;
3839
use OCP\Mail\IMailer;
3940
use OCP\Search\ISearchQuery;
4041
use OCP\Security\ISecureRandom;
@@ -70,6 +71,7 @@ public function __construct(
7071
private LoggerInterface $logger,
7172
private IEventDispatcher $eventDispatcher,
7273
private IMailer $mailer,
74+
private IEmailValidator $emailValidator,
7375
private AnswerMapper $answerMapper,
7476
) {
7577
$this->currentUser = $userSession->getUser();
@@ -771,12 +773,6 @@ private function sendConfirmationEmail(Form $form, Submission $submission): void
771773
$questions = $this->getQuestions($form->getId());
772774
$answers = $this->answerMapper->findBySubmission($submission->getId());
773775

774-
// Build a map of question IDs to questions and answers
775-
$questionMap = [];
776-
foreach ($questions as $question) {
777-
$questionMap[$question['id']] = $question;
778-
}
779-
780776
$answerMap = [];
781777
foreach ($answers as $answer) {
782778
$questionId = $answer->getQuestionId();
@@ -786,49 +782,25 @@ private function sendConfirmationEmail(Form $form, Submission $submission): void
786782
$answerMap[$questionId][] = $answer->getText();
787783
}
788784

789-
// Find email address from answers
790-
$recipientEmails = [];
791-
foreach ($questions as $question) {
792-
if ($question['type'] !== Constants::ANSWER_TYPE_SHORT) {
793-
continue;
794-
}
795-
796-
$extraSettings = (array)($question['extraSettings'] ?? []);
797-
$validationType = $extraSettings['validationType'] ?? null;
798-
if ($validationType !== 'email') {
799-
continue;
800-
}
801-
802-
$questionId = $question['id'];
803-
if (empty($answerMap[$questionId])) {
804-
continue;
805-
}
806-
807-
$emailValue = $answerMap[$questionId][0];
808-
if ($this->mailer->validateMailAddress($emailValue)) {
809-
$recipientEmails[] = $emailValue;
810-
}
811-
}
812-
813-
// If no email found, cannot send confirmation
814-
if (empty($recipientEmails)) {
815-
$this->logger->debug('No valid email address found in submission for confirmation email', [
785+
$recipientQuestion = $this->getConfirmationEmailRecipientQuestion($questions);
786+
if ($recipientQuestion === null) {
787+
$this->logger->debug('No confirmation email recipient question is available', [
816788
'formId' => $form->getId(),
817789
'submissionId' => $submission->getId(),
818790
]);
819791
return;
820792
}
821793

822-
if (count($recipientEmails) > 1) {
823-
$this->logger->debug('Multiple email fields found in submission for confirmation email, using first match', [
794+
$recipientQuestionId = $recipientQuestion['id'];
795+
$recipientEmail = $answerMap[$recipientQuestionId][0] ?? null;
796+
if ($recipientEmail === null || !$this->emailValidator->isValid($recipientEmail)) {
797+
$this->logger->debug('No valid email address found in submission for confirmation email', [
824798
'formId' => $form->getId(),
825799
'submissionId' => $submission->getId(),
826-
'emailCount' => count($recipientEmails),
827800
]);
801+
return;
828802
}
829803

830-
$recipientEmail = $recipientEmails[0];
831-
832804
// Replace placeholders in subject and body
833805
$replacements = [
834806
'{formTitle}' => $form->getTitle(),
@@ -886,6 +858,55 @@ private function sendConfirmationEmail(Form $form, Submission $submission): void
886858
}
887859
}
888860

861+
/**
862+
* @param list<FormsQuestion> $questions
863+
* @return FormsQuestion|null
864+
*/
865+
private function getConfirmationEmailRecipientQuestion(array $questions): ?array {
866+
$emailQuestions = array_values(array_filter(
867+
$questions,
868+
fn (array $question): bool => $this->isConfirmationEmailQuestion($question),
869+
));
870+
871+
if ($emailQuestions === []) {
872+
return null;
873+
}
874+
875+
$explicitRecipients = array_values(array_filter(
876+
$emailQuestions,
877+
function (array $question): bool {
878+
$extraSettings = (array)($question['extraSettings'] ?? []);
879+
return !empty($extraSettings['confirmationEmailRecipient']);
880+
},
881+
));
882+
883+
if (count($explicitRecipients) === 1) {
884+
return $explicitRecipients[0];
885+
}
886+
887+
if (count($explicitRecipients) > 1) {
888+
return null;
889+
}
890+
891+
if (count($emailQuestions) === 1) {
892+
return $emailQuestions[0];
893+
}
894+
895+
return null;
896+
}
897+
898+
/**
899+
* @param FormsQuestion $question
900+
*/
901+
private function isConfirmationEmailQuestion(array $question): bool {
902+
if (($question['type'] ?? null) !== Constants::ANSWER_TYPE_SHORT) {
903+
return false;
904+
}
905+
906+
$extraSettings = (array)($question['extraSettings'] ?? []);
907+
return ($extraSettings['validationType'] ?? null) === 'email';
908+
}
909+
889910
/**
890911
* Return shares of a form shared with given user
891912
*

src/components/Questions/QuestionShort.vue

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,19 @@
6969
/^[a-z]{3}$/i
7070
<!-- ^ Some example RegExp for the placeholder text -->
7171
</NcActionInput>
72+
<NcActionCheckbox
73+
v-if="validationType === 'email'"
74+
:model-value="isConfirmationEmailRecipient"
75+
@update:model-value="onConfirmationEmailRecipientChange">
76+
{{ t('forms', 'Use for confirmation emails') }}
77+
</NcActionCheckbox>
7278
</NcActions>
7379
</div>
7480
</Question>
7581
</template>
7682

7783
<script>
84+
import NcActionCheckbox from '@nextcloud/vue/components/NcActionCheckbox'
7885
import NcActionInput from '@nextcloud/vue/components/NcActionInput'
7986
import NcActionRadio from '@nextcloud/vue/components/NcActionRadio'
8087
import NcActions from '@nextcloud/vue/components/NcActions'
@@ -89,6 +96,7 @@ export default {
8996
9097
components: {
9198
IconRegex,
99+
NcActionCheckbox,
92100
NcActions,
93101
NcActionInput,
94102
NcActionRadio,
@@ -146,6 +154,10 @@ export default {
146154
validationRegex() {
147155
return this.extraSettings?.validationRegex || ''
148156
},
157+
158+
isConfirmationEmailRecipient() {
159+
return this.extraSettings?.confirmationEmailRecipient ?? false
160+
},
149161
},
150162
151163
methods: {
@@ -185,19 +197,32 @@ export default {
185197
if (validationType === 'regex') {
186198
// Make sure to also submit a regex (even if empty)
187199
this.onExtraSettingsChange({
200+
confirmationEmailRecipient: undefined,
188201
validationType,
189202
validationRegex: this.validationRegex,
190203
})
191204
} else {
192205
// For all other types except regex we close the menu (for regex we keep it open to allow entering a regex)
193206
this.isValidationTypeMenuOpen = false
194207
this.onExtraSettingsChange({
208+
confirmationEmailRecipient:
209+
validationType === 'email'
210+
? this.isConfirmationEmailRecipient || undefined
211+
: undefined,
195212
validationType:
196213
validationType === 'text' ? undefined : validationType,
197214
})
198215
}
199216
},
200217
218+
onConfirmationEmailRecipientChange(confirmationEmailRecipient) {
219+
this.onExtraSettingsChange({
220+
confirmationEmailRecipient: confirmationEmailRecipient
221+
? true
222+
: undefined,
223+
})
224+
},
225+
201226
/**
202227
* Validate and save regex if valid
203228
*

src/components/SidebarTabs/SettingsSidebarTab.vue

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@
182182
{{
183183
t(
184184
'forms',
185-
'Requires an email field in the form. Available placeholders: {formTitle}, {formDescription}, and field names like {name}.',
185+
'Requires an email field in the form. If the form has multiple email fields, mark one question with “Use for confirmation emails”. Available placeholders: {formTitle}, {formDescription}, and field names like {name}.',
186186
)
187187
}}
188188
</p>
@@ -196,12 +196,28 @@
196196
)
197197
" />
198198
<NcNoteCard
199-
v-else-if="form.confirmationEmailEnabled && emailQuestionCount > 1"
200-
type="info"
199+
v-else-if="
200+
form.confirmationEmailEnabled
201+
&& emailQuestionCount > 1
202+
&& confirmationEmailRecipientCount === 0
203+
"
204+
type="error"
201205
:text="
202206
t(
203207
'forms',
204-
'Multiple email fields found. The first email field in the form order will be used.',
208+
'Select one email field and enable “Use for confirmation emails” on that question.',
209+
)
210+
" />
211+
<NcNoteCard
212+
v-else-if="
213+
form.confirmationEmailEnabled
214+
&& confirmationEmailRecipientCount > 1
215+
"
216+
type="error"
217+
:text="
218+
t(
219+
'forms',
220+
'Only one email field can be used for confirmation emails.',
205221
)
206222
" />
207223
<label class="confirmation-email__label">
@@ -221,7 +237,7 @@
221237
<textarea
222238
:value="confirmationEmailBody"
223239
:disabled="locked"
224-
:placeholder="t('forms', 'Thank you for submitting the form.')"
240+
:placeholder="emailBodyPlaceholder"
225241
class="confirmation-email__textarea"
226242
@input="onConfirmationEmailBodyInput"
227243
@blur="onConfirmationEmailBodyChange"></textarea>
@@ -390,11 +406,21 @@ export default {
390406
},
391407
392408
emailQuestionCount() {
409+
return this.confirmationEmailQuestions.length
410+
},
411+
412+
confirmationEmailQuestions() {
393413
const questions = this.form?.questions || []
394414
return questions.filter(
395415
(question) =>
396416
question.type === 'short'
397417
&& question.extraSettings?.validationType === 'email',
418+
)
419+
},
420+
421+
confirmationEmailRecipientCount() {
422+
return this.confirmationEmailQuestions.filter(
423+
(question) => question.extraSettings?.confirmationEmailRecipient,
398424
).length
399425
},
400426
},

0 commit comments

Comments
 (0)