FeatureToggle icon indicating copy to clipboard operation
FeatureToggle copied to clipboard

Enable Time Zone / UTC Specification in DateTime Based Toggles

Open raleighbuckner opened this issue 9 years ago • 2 comments

Initial thoughts on this:

  • Revise FeatureToggle.Core.ConfigurationDateParser.ExpectedDateFormat to be: @"dd-MMM-yyyy HH:mm:ss K"
  • Revise FeatureToggle.Toggles.EnabledBetweenDatesFeatureToggle, FeatureToggle.Toggles.EnabledOnOrBeforeDateFeatureToggle, and FeatureToggle.Toggles.EnabledOnOrAfterDateFeatureToggle to:
    • have NowProvider to return DateTime.UTCNow
    • have date comparisons use .ToUniversalTime()

Unfortunately, this would be a breaking change since the new ExpectedDateFormat would not be compatible with older configuration settings that don't have the time zone information.

A possible other solution would be to:

  • Revise FeatureToggle.Core.ConfigurationDateParser.ParseDateTimeConfigString to accept an optional "format" parameter that would be used instead of the default ExpectedDateFormat
  • Revise the FeatureToggle.Core.IDateTimeToggleValueProvider and FeatureToggle.Core.ITimePeriodProvider interface methods to accept the same optional "format" parameter.
  • Revise the implementations of FeatureToggle.Core.IDateTimeToggleValueProvider and FeatureToggle.Core.ITimePeriodProvider to pass the parameter along to FeatureToggle.Core.ConfigurationDateParser.ParseDateTimeConfigString
  • Revise FeatureToggle.Toggles.EnabledBetweenDatesFeatureToggle, FeatureToggle.Toggles.EnabledOnOrBeforeDateFeatureToggle, and FeatureToggle.Toggles.EnabledOnOrAfterDateFeatureToggle to have the "format" specified easily in subclasses - either as a property that can be set in a constructor or perhaps as a parameter to the base constructor

Let me know what you think and I should be able to create a pull request for either route. Thanks!

raleighbuckner avatar Sep 09 '16 20:09 raleighbuckner

Oh - reasoning for this... As it stands now, the times are being parsed as Local times, so the timezone is that of the machine where the app is running. This can cause the features to be enabled/disabled at the wrong time if the machine executing the code is in a different time zone than the developer. For example, developer in Eastern TZ in the US, but the server is set to UTC.

raleighbuckner avatar Sep 09 '16 20:09 raleighbuckner

Thanks for the suggestion @raleighbuckner - it makes sense. Let me have a little think about the best way forward. I'm not against breaking changes, the project uses semantic versioning so we'd have to move to v4.x.x to represent this breaking change. We'd still want the ability to be able to configure the datetime-based toggles to use local datetime or utc with time zone - what do you think?

jason-roberts avatar Sep 12 '16 02:09 jason-roberts