Currently in the Date module 7.x-2.6 version, when using repeat fuctionality on the field.
Repeats - daily.
There is a section with radio buttons - "Stop repeating".
There are two options: "after" and "on".
When the radio button is selecting "on" and a valid date is entered, the value within after is also validated, causing "Count must be a positive integer." validation error.

Possible solution:
I think based on the selection of the radio button, the validation needs to be conditional for the next fields.
Example: When after is selected, validation for "count" field should be skipped.

I need solution or opinion on how much work is this, because when I looked into the form element generation method, I got lost within this 500+ lines element pre-process super method.

CommentFileSizeAuthor
#17 reroll_patch-date-fix-repeat-rule-count-validation-2157469-17.patch2.86 KBPradnya Pingat
#10 date-fix-repeat-rule-count-validation-2157469-10.patch2.95 KBk.minkov
#8 date-fix-repeat-rule-count-validation-2157469-8.patch2.36 KBk.minkov
#6 date-fix-repeat-rule-count-validation-2157469-6.patch2.15 KBk.minkov
#4 date-fix-repeat-rule-count-validation-2157469-4.patch1.44 KBk.minkov
#1 date-fix-repeat-rule-count-validation-2157469.patch1.28 KBk.minkov
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

k.minkov’s picture

k.minkov CreditAttribution: k.minkov commented
Version: 7.x-2.6 » 7.x-2.x-dev
FileSize
date-fix-repeat-rule-count-validation-2157469.patch1.28 KB

I am providing a patch for the issue. It creates a custom validator for the count field that applies validation to it only when "After" is selected. The patch is appliable against version 2.6 too.

k.minkov’s picture

k.minkov CreditAttribution: k.minkov commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: date-fix-repeat-rule-count-validation-2157469.patch, failed testing.

k.minkov’s picture

k.minkov CreditAttribution: k.minkov commented
Status: Needs work » Needs review
FileSize
date-fix-repeat-rule-count-validation-2157469-4.patch1.44 KB

Submitting new version of the patch.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
CreditAttribution: podarok commented
Status: Needs review » Needs work
Issue tags: +Needs tests

#4
should be repeat, not reapeat in function name
and we needs testing here,

k.minkov’s picture

k.minkov CreditAttribution: k.minkov commented
Status: Needs work » Needs review
FileSize
date-fix-repeat-rule-count-validation-2157469-6.patch2.15 KB

Ohh yes that is a stupid mistake. About tests I added a line which I think is enough to validate this behavior , although if you suggest to create a separate test case for this I may think of it.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
CreditAttribution: podarok commented

re #6
code coverage is good, but better to see few check lines for it, cause it is very design specific and someone can never grok it in the feature...

k.minkov’s picture

k.minkov CreditAttribution: k.minkov commented
FileSize
date-fix-repeat-rule-count-validation-2157469-8.patch2.36 KB

This way is a little more self explanatory. Hope it is ok now.

Status: Needs review » Needs work

The last submitted patch, 8: date-fix-repeat-rule-count-validation-2157469-8.patch, failed testing.

k.minkov’s picture

k.minkov CreditAttribution: k.minkov commented
Status: Needs work » Needs review
FileSize
date-fix-repeat-rule-count-validation-2157469-10.patch2.95 KB

Here is a refined version.

ndobromirov’s picture

ndobromirov CreditAttribution: ndobromirov commented

Assuming the last version in #10 is the best one, I am hiding the previous patches, to make it more visible.

andriyun queued 10: date-fix-repeat-rule-count-validation-2157469-10.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 10: date-fix-repeat-rule-count-validation-2157469-10.patch, failed testing.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Clever Systems, MachbarMacher, DruConsult commented
Related issues: +#2726843: Repeat-after usability flaw

Crosslinking related issue: #2726843: Repeat-after usability flaw which is about disabling fields per states api.

ndobromirov’s picture

ndobromirov CreditAttribution: ndobromirov commented
Issue tags: +Needs reroll

Fails tasting, as the patch can not be applied, so needs a re-roll.

Pradnya Pingat’s picture

Pradnya Pingat CreditAttribution: Pradnya Pingat at Blisstering Solutions commented
Assigned: Unassigned » Pradnya Pingat

I am working on rerolling patch.

Pradnya Pingat’s picture

Pradnya Pingat CreditAttribution: Pradnya Pingat at Blisstering Solutions commented
Assigned: Pradnya Pingat » Unassigned
Status: Needs work » Needs review
FileSize
reroll_patch-date-fix-repeat-rule-count-validation-2157469-17.patch2.86 KB

reroll patch added.

Chris Matthews’s picture

Chris Matthews CreditAttribution: Chris Matthews as a volunteer commented
Status: Needs review » Needs work
Issue tags: +Needs rework

The 3 year patch in #17 to date_repeat does not apply to the latest 7.x-2.x-dev and may be too old to easily reroll, but I went ahead and tagged the issue accordingly.

Checking patch date_repeat/date_repeat_form.inc...
Hunk #1 succeeded at 486 (offset -2 lines).
Hunk #2 succeeded at 727 (offset -2 lines).
Checking patch date_repeat/tests/date_repeat_form.test...
error: while searching for:
        $edit += $this->formatDateForRRULEInputs('field_test[und][0][rrule][until_child]', $options, $date);
      }

      // Test date exceptions and/or additions
      if ($exclude_include !== NULL) {
        $exclude_include_edit = array();

error: patch failed: date_repeat/tests/date_repeat_form.test:387
error: date_repeat/tests/date_repeat_form.test: patch does not apply