CRM-21523 add form rule for repetition fields in scheduled reminder form #11377
Conversation
| @@ -350,6 +350,13 @@ public static function formRule($fields, $files, $self) { | |||
| $errors[$recipientKind[$fields['recipient']]['target_id']] = ts('If "Also include" or "Limit to" are selected, you must specify at least one %1', array(1 => $recipientKind[$fields['recipient']]['name'])); | |||
| } | |||
|
|
|||
| //CRM-21523 | |||
| if (!empty($fields['is_repeat']) && | |||
| (empty($fields['repetition_frequency_interval']) || ($fields['end_frequency_interval'] == NULL)) | |||
There was a problem hiding this comment.
@lcdservices because you used == instead of === I don't think those two conditions are actually different.
Php thinks that 0 == NULL but not 0 === NULL.
There was a problem hiding this comment.
I agree that it seems like we should use the strict operator, but it doesn't work when you do. I believe that's because the values we receive are cast as strings, not integers. If we use the strict operator an empty value for the end_frequency_interval field does not trigger the validation.
|
@colemanw this appears like a minor change but stalled in discussion on your review? (It's form validation so a fairly safe area of code) |
|
I'm still slightly confused by the operators but I've tested out the patch and it works as described.
|
Overview
Add form rule to schedule reminder form to require both frequency interval fields if repetition is enabled.
Before
Frequency interval fields were not required. If the value was left blank it caused an SQL error when the scheduled reminder job was triggered.
After
User must enter a value for both fields. No SQL error generated.
Technical Details
The rule is slightly different for the two fields. For repetition_frequency_interval we require a non-null/non-zero value as the frequency of the repetition must be a meaningful value. For end_frequency_interval (until...) we allow a zero value, as that is a valid use case.