Fix GetOccurrences(periodStart) to return still active occurrences

[minichma]

See #825: According to existing code, GetOccurrences(periodStart) seems to be intended to return not only those recurrences that start at or after periodStart, but also those still active at that date-time. There's has been some discussion in #825 about whether to follow the original intention or rather just return those that start at or after periodStart.

This PR introduces a new evaluation option that allows to control the intended behavior. By default only those are returned that start at or after periodStart. By setting the new option EvaluationOptions.IncludeStillActiveOccurrences, also those are returned that started in the past but are still active at the given point in time.

Fixes #825

[minichma]

@ical-org/maintainers Not sure it is the best idea to make the behavior configurable, as it introduces additional complexities, requires additional testing, etc. Please let me know what you think.

If someone would find the time, the PR would also deserve additional tests related to occurrences that should be returned and cross a DST change before periodStart. Not sure this case is handled correctly, probably not (anyhow, its better than today).

[minichma]

@ical-org/maintainers formally the changes are breaking, at least due to classes becoming abstract. However, effectively those are not intended for being constructed/inherited by user code, so I'd consider this acceptable.

[axunonb]

Thanks! Few first comments:

1. Looks like a good solution.
2. Should EvaluationOptions.IncludeStillActiveOccurrences == true be the default behavior (also mentioned in GetOccurrences() with periodStart skips first occurrences if VEVENT has duration #825)? Then we wouldn't even need the new option, because the caller could easily identify ongoing occurrences.
3. Naming: EvaluationOptions.IncludeOngoingOccurrences instead of EvaluationOptions.IncludeStillActiveOccurrences?
4. XmlDoc for IGetOccurrences.GetOccurrences should reflect the behavior regarding ongoing occurrences.
5. Not part of the PR: Update the Wiki

[Edit]

not intended for being constructed/inherited by user code,

Agree, should we then consequently disallow construction inheritance by user code?

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

[minichma]

Then we wouldn't even need the new option, because the caller could easily identify ongoing occurrences.

Agree, so I removed the last commit. Introduced the configuration because I don't like the behavior of filtering by the end time too much because the other way is simpler and easier to understand. Anyhow, the configuration adds even more complexity.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[axunonb]

LGTM - thanks!