Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support recurrence for TimeWindowFilter #266

Merged
merged 63 commits into from
Apr 23, 2024

Conversation

zhiyuanliang-ms
Copy link
Contributor

@zhiyuanliang-ms zhiyuanliang-ms commented Sep 22, 2023

Why this PR?

#194

Visible changes

Add-on "Recurrence" parameter for TimeWindowFilter using Outlook-style schema:
https://learn.microsoft.com/en-us/graph/outlook-schedule-recurring-events#using-patterns-and-ranges-to-create-recurring-events

The modification we made to the original outlook schema:

  1. We remove the "StartDate" property in the RecurrenceRange and we force the "Start" property of the time window to be a valid first occurrence (there are validations for "Start" property, checking whether it follows the recurrence pattern).
  2. We remove the "RecurrenceTimeZone" property in the RecurrenceRange and we will use the UTC offset in the "Start" property.
  3. We will only support "Daily" and "Weekly" recurrence pattern type: Support recurrence for TimeWindowFilter #266 (comment)

Some discussion can be found in #256

Other comments

The main logic of how to check whether current time is within any recurring time window:

  1. Find the previous occurrence of the recurring time window, let's call it prevOccurrenceStart
  2. Check whether the current time: time is within the time window: prevOccurrenceStart ~ prevOccurrenceStart + End - Start


const int WeeklyUnitIntervalDuration = 7; // in days
const int MonthlyUnitIntervalDuration = 28; // in days
const int YearlyUnitIntervalDuration = 365; // in days
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should make all these const strings public. You have the RecurrencePattern public, but not their values.

@zhiyuanliang-ms zhiyuanliang-ms changed the title In Progress: Outlook-style schema for recurrence Outlook-style schema for recurrence Sep 29, 2023
@zhiyuanliang-ms zhiyuanliang-ms force-pushed the zhiyuanliang/recurring-time-window branch 2 times, most recently from 2cb3e59 to f8d0c34 Compare September 29, 2023 11:50
@zhiyuanliang-ms zhiyuanliang-ms force-pushed the zhiyuanliang/recurring-time-window branch from 76f2879 to 92e8a08 Compare November 14, 2023 07:23
@zhiyuanliang-ms zhiyuanliang-ms changed the base branch from preview to main November 14, 2023 07:24
@zhiyuanliang-ms zhiyuanliang-ms force-pushed the zhiyuanliang/recurring-time-window branch 2 times, most recently from 66b9ca7 to ac8f8f4 Compare April 16, 2024 16:50
@zhiyuanliang-ms zhiyuanliang-ms merged commit 44033d2 into main Apr 23, 2024
2 checks passed
}

[Fact]
public async void RecurrenceEvaluationThroughCacheTest()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we check if there's an entry in cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just sent a PR #445 to test it.

@zhiyuanliang-ms zhiyuanliang-ms deleted the zhiyuanliang/recurring-time-window branch June 17, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants