-
Notifications
You must be signed in to change notification settings - Fork 120
Support recurrence for TimeWindowFilter #266
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
Conversation
b51b184
to
0e88077
Compare
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceRange.cs
Outdated
Show resolved
Hide resolved
|
||
const int WeeklyUnitIntervalDuration = 7; // in days | ||
const int MonthlyUnitIntervalDuration = 28; // in days | ||
const int YearlyUnitIntervalDuration = 365; // in days |
There was a problem hiding this comment.
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.
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceRange.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs
Outdated
Show resolved
Hide resolved
2cb3e59
to
f8d0c34
Compare
src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrencePattern.cs
Outdated
Show resolved
Hide resolved
76f2879
to
92e8a08
Compare
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrencePattern.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs
Outdated
Show resolved
Hide resolved
66b9ca7
to
ac8f8f4
Compare
…-Dotnet into zhiyuanliang/recurring-time-window
} | ||
|
||
[Fact] | ||
public async void RecurrenceEvaluationThroughCacheTest() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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:
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:
prevOccurrenceStart
time
is within the time window:prevOccurrenceStart
~prevOccurrenceStart
+End
-Start