Event Registration Reminder issues

Thanks your clear write up and for the report @phillamb168. I can reproduce the same, although it sounds like you found workarounds.

Here're my notes to help writing a pull request. I should be able to prepare one shortly.

1) Clicking Add instance on series creates an instance with reminder_date empty
2) When the series reminder_date changes, Instances' reminder_sent is emptied
3) If Series dates changes but reminder options did not, instances are recreated without a reminder_date (note it's expected Instances are recreated when series dates change)

To clarify one point, reminders for past events shouldn't get sent - I see a check for that - but can see how clearing reminder_sent in #2 would mean people could be reminded more than once.

I've attached a patch that address the issues and is quite minimal. But recurring_events is heavily relied on by some sites with large numbers of events and traffic, so it'd be helpful getting another maintainer's review

Here a summary of the changes, which are within recurring_Events_registration:

1. Change hook `eventseries_insert` to `eventinstance_insert`
2. Change criteria in `eventseries_update` to simple yes/ no
3. Do not empty `reminder_sent`

I'll mark this as Needs review.

Hello @ll66382 and @phillamb168.

I understand the two issues raised here, although my comment will focus exclusively on issue #2.

The functionality for sending multiple reminders for an event (by updating the reminder settings in an event series - for example, initially scheduling a reminder for "1 month before" and later adding a new reminder for "1 day before") was introduced in #3283237.

While this feature is valuable, an important detail was overlooked: only future event instances should be marked for sending new reminders. That is exactly what issue #2 here reports, and I agree it makes total sense.

However, I believe the solution implemented for issue #2 in the provided patch is incorrect.

In the original code, recurring_events_reminders_eventseries_update properly checked that reminder settings had actually changed by comparing reminder_amount and reminder_units between the original and the updated entity:

    if ($original->registration_reminders->reminder_amount !== $entity->registration_reminders->reminder_amount
      || $original->registration_reminders->reminder_units !== $entity->registration_reminders->reminder_units
    ) {
            ...

The patch removes this evaluation, which means that any change to the event series - or even simply saving it without modifications - will update the reminder_date and reminder_sent values for all event instances, marking them to send reminders. This creates a new and more severe bug than the one originally reported.

The correct approach should be to retain the check for changes in the reminder settings, and then update reminder_date and reset reminder_sent (NULL) only for instances with future dates.

For that reason, I have created a separate issue specifically describing and resolving #2: Issue 3545215.

Recommendations for this issue:

1. Limit this case so it addresses only problem #1.
2. Remove references to issue #2, since that is now covered properly in #3545215.
3. Update the title to make it more descriptive of the actual problem. The current one is too generic and does not help other developers quickly understand or find the issue.
4. Provide a new patch or merge request that fixes only issue #1. The current patch should be hidden.