When changing recurrance, identical date combinations should not be deleted & recreated.

The MR doesn't correctly account included and excluded dates; that's likely because include/exclude logic is handled in the recurring_events_event_instances_pre_create hook implementation (recurring_events_recurring_events_event_instances_pre_create_alter). This causes an issue with the removal of instances that are no longer in the range.

The proposed code change calls this hook only for events instances to be created while originally it was called for *all* instances.

Steps to reproduce:

1) Create the weekly event series, starting from e.g. the beginning of the year and ending at the end of the year.
2) Observe 51 instances (or whatever the number of weeks is in range) created.
3) Change the included date range to include only one month from the range, save the series.

Expected: all instances except those in the included range are deleted.
Actual: none of the instances are deleted.

Note: I'm talking about the Excluded dates and Included dates fields, which are below the normal range selection.

I've pushed a fix for the issue outlined in #3 (hope @ras-ben doesn't mind; otherwise, feel free to remove/squash my commit),
Attaching a patch to apply via composer.json.

> The MR doesn't correctly account included and excluded dates.

I tested the updated MR by creating a weekly event over a year and excluding 11 of the 12 months. This resulted in removal of the expected events while the others were left untouched.

Bumping this to major since it can cause unexpected data loss.

Let's add some test coverage for this so we can ascertain that this works as expected.

I had a quick look at the patch but in its current form it is not yet taking advantage of the EventInstanceCreator plugin system that was added in ✨ By updating a series, it deletes all the eventinstances and recreates them, which deletes all information stored on the instance. Active with the specific intent to make this use case possible in a flexible way.

We should definitely make use of this, and add a new plugin like an UnchangedDateRangePreservingEventInstanceCreator that users can opt-in to.

Assigning to do an in-depth review.

I think your comments are excellent @pfrenssen, but I dont know if I'll have the time this month to look into fixing them - so, if anyone has the time and expertise, you are very welcome :)

Should this bug be critical? It's on the edge, but it does cause loss of existing data in custom fields added to the instance entity.

@awolfey, no this is not critical. It is also not a bug, this is just how it was originally designed. There is a very clear warning shown to the user that the data loss will occur and they have to confirm it.

This is a known shortcoming. The groundwork for solving this has been laid in ✨ By updating a series, it deletes all the eventinstances and recreates them, which deletes all information stored on the instance. Active . There is also very interesting prior discussion in that issue.

There is also ✨ Provide an EventInstanceCreator plugin that does nothing Active which has a simple but very effective solution that can be implemented as a hook and is waiting for review.