Comments

the_glitch created an issue. See original summary.

owenbush’s picture

Thanks for this. I need to tighten up all validation on these recurrence types actually

owenbush’s picture

Component: Code » Recurring Events (Main module)
siddhant.bhosale’s picture

Assigned: Unassigned » siddhant.bhosale
siddhant.bhosale’s picture

Assigned: siddhant.bhosale » Unassigned
Status: Active » Needs review
StatusFileSize
new1.88 KB

Hi, I have fixed the issue and adding the patch for the same.

owenbush’s picture

Thanks for the patch. I'll take a look at this soon.

the_glitch’s picture

StatusFileSize
new121.29 KB

Patch #5 creates bug with the recur type in series and series can't be saved.
screenshot

naresh_bavaskar’s picture

Assigned: Unassigned » naresh_bavaskar
Status: Needs review » Needs work
naresh_bavaskar’s picture

Assigned: naresh_bavaskar » Unassigned
Status: Needs work » Needs review
StatusFileSize
new1.95 KB
new834 bytes

Please review the new patch

owenbush’s picture

StatusFileSize
new23.46 KB

Hey Siddhant and Naresh,

Thanks for the patches towards this issue. However, there is a bigger problem of field widget validation that needs to be addressed. I've attached a patch in which all of the special recurring events field widgets are responsible for their own validation.

The validation is more robust now because it checks individual fields and ensures they are completed as expected. I appreciate the work you put into this, however we need more validation than just checking the entire array is empty.

The patch also addresses some other issues which relate to validation for example checking that the consecutive recurring type is not generating too many instances, and also fixing the broken functionality with modifying date recurrence not triggering a prompt and diff table to approve those changes.

I also took the opportunity to use the StringTranslationTrait to update calls for `t()` to `$this->t()`

owenbush’s picture

StatusFileSize
new23.41 KB
new829 bytes

I found a ksm() I forgot to remove. This patch has that removed.

owenbush’s picture

  • owenbush committed 678ee3b on 8.x-1.x
    Issue #3090658 by owenbush, naresh_bavaskar, siddhant.bhosale,...
owenbush’s picture

Status: Needs review » Fixed

This has been merged and marked as fixed. Thanks all for the patches and issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.