Fix issue with Date diff when it includes a Daylight Saving Time (#193)

The application uses local time to calculate the number of days between 2 dates. If a Daylight Saving Time takes place during this period of time (only Clocks Forward 2am->3am), the application will miss 1 day and fail to calculate the number of days/weeks/months between the 2 dates.
image

Description of the changes:
DateCalculationEngine uses local time to modify dates, however, AddDays, AddWeeks,... won't add 24 hours or 7 days if DST happens between the 2 dates, but instead add ~23.9/24.1 hours or ~6.9/7.1 days (depends if it's the DST clock backward or clock forward). When the DST is clock forward, DateCalculationEngine will miss one day.

Solution
use UTC dates to calculate date difference.

Extra Fix:
use calendar->FirstPeriodInThisDay and calendar->FirstHourInThisPeriod in ClipTime (else it will set the time to 12PM (noon) in some regions.
replace OBSERVABLE_PROPERTY_RW by OBSERVABLE_PROPERTY_R when possible.
remove the definition of CheckClipTimeSameDay (implementation missing)

How changes were validated:
Tested manually with different regions (FR, US, ES, JP).

Fixes #178
This commit is contained in:
Rudy Huyn 2019-03-18 11:22:32 -07:00 committed by Howard Wolosky
parent e40791d7ad
commit cd7c266a6b
4 changed files with 76 additions and 32 deletions

View File

@ -12,6 +12,7 @@ using namespace CalculatorApp::Common::DateCalculation;
DateCalculationEngine::DateCalculationEngine(_In_ String^ calendarIdentifier) DateCalculationEngine::DateCalculationEngine(_In_ String^ calendarIdentifier)
{ {
m_calendar = ref new Calendar(); m_calendar = ref new Calendar();
m_calendar->ChangeTimeZone("UTC");
m_calendar->ChangeCalendarSystem(calendarIdentifier); m_calendar->ChangeCalendarSystem(calendarIdentifier);
} }

View File

@ -43,11 +43,7 @@ DateCalculatorViewModel::DateCalculatorViewModel() :
m_StrDateDiffResultAutomationName(L""), m_StrDateDiffResultAutomationName(L""),
m_StrDateDiffResultInDays(L""), m_StrDateDiffResultInDays(L""),
m_StrDateResult(L""), m_StrDateResult(L""),
m_StrDateResultAutomationName(L""), m_StrDateResultAutomationName(L"")
m_fromDate({ 0 }),
m_toDate({ 0 }),
m_startDate({ 0 }),
m_dateResult({ 0 })
{ {
const auto& localizationSettings = LocalizationSettings::GetInstance(); const auto& localizationSettings = LocalizationSettings::GetInstance();
@ -56,19 +52,19 @@ DateCalculatorViewModel::DateCalculatorViewModel() :
// Initialize Date Calc engine // Initialize Date Calc engine
m_dateCalcEngine = make_shared<DateCalculationEngine>(localizationSettings.GetCalendarIdentifier()); m_dateCalcEngine = make_shared<DateCalculationEngine>(localizationSettings.GetCalendarIdentifier());
// Initialize dates of DatePicker controls to today's date // Initialize dates of DatePicker controls to today's date
auto calendar = ref new Calendar(); auto calendar = ref new Calendar();
// We force the timezone to UTC, in order to avoid being affected by Daylight Saving Time
// when we calculate the difference between 2 dates.
calendar->ChangeTimeZone("UTC");
auto today = calendar->GetDateTime(); auto today = calendar->GetDateTime();
// FromDate and ToDate should be clipped (adjusted to a consistent hour in UTC) // FromDate and ToDate should be clipped (adjusted to a consistent hour in UTC)
m_fromDate = today; m_fromDate = ClipTime(today);
m_toDate = today; m_toDate = ClipTime(today);
FromDate = ClipTime(today);
ToDate = ClipTime(today);
// StartDate should not be clipped // StartDate should not be clipped
StartDate = today; m_startDate = today;
m_dateResult = today; m_dateResult = today;
// Initialize the list separator delimiter appended with a space at the end, e.g. ", " // Initialize the list separator delimiter appended with a space at the end, e.g. ", "
@ -86,15 +82,6 @@ DateCalculatorViewModel::DateCalculatorViewModel() :
m_offsetValues->Append(ref new String(numberStr.c_str())); m_offsetValues->Append(ref new String(numberStr.c_str()));
} }
/* In the ClipTime function, we used to change timezone to UTC before clipping the time.
The comment from the previous developers said this was done to eliminate the effects of
Daylight Savings Time. We can't think of a good reason why this change in timezone is
necessary and did find bugs related to the change, therefore, we have removed the
change. Just in case, we will see if the clipped time is ever a different day from the
original day, which would hopefully indicate the change in timezone was actually
necessary. We will collect telemetry if we find this case. If we don't see any
telemetry events after the application has been used for some time, we will feel safe
and can remove this function. */
DayOfWeek trueDayOfWeek = calendar->DayOfWeek; DayOfWeek trueDayOfWeek = calendar->DayOfWeek;
DateTime clippedTime = ClipTime(today); DateTime clippedTime = ClipTime(today);
@ -383,13 +370,14 @@ String^ DateCalculatorViewModel::GetLocalizedNumberString(int value) const
return ref new String(numberStr.c_str()); return ref new String(numberStr.c_str());
} }
// Adjusts the given DateTime to 12AM of the same day // Adjusts the given DateTime to 12AM (UTC) of the same day
DateTime DateCalculatorViewModel::ClipTime(DateTime dateTime) DateTime DateCalculatorViewModel::ClipTime(DateTime dateTime)
{ {
auto calendar = ref new Calendar(); auto calendar = ref new Calendar();
calendar->ChangeTimeZone("UTC");
calendar->SetDateTime(dateTime); calendar->SetDateTime(dateTime);
calendar->Period = 1; calendar->Period = calendar->FirstPeriodInThisDay;
calendar->Hour = 12; calendar->Hour = calendar->FirstHourInThisPeriod;
calendar->Minute = 0; calendar->Minute = 0;
calendar->Second = 0; calendar->Second = 0;
calendar->Nanosecond = 0; calendar->Nanosecond = 0;

View File

@ -23,8 +23,8 @@ namespace CalculatorApp
// Input Properties // Input Properties
OBSERVABLE_PROPERTY_RW(bool, IsDateDiffMode); OBSERVABLE_PROPERTY_RW(bool, IsDateDiffMode);
OBSERVABLE_PROPERTY_RW(bool, IsAddMode); OBSERVABLE_PROPERTY_RW(bool, IsAddMode);
OBSERVABLE_PROPERTY_RW(bool, IsDiffInDays); // If diff is only in days or the dates are the same, OBSERVABLE_PROPERTY_R(bool, IsDiffInDays); // If diff is only in days or the dates are the same,
// then show only one result and avoid redundancy // then show only one result and avoid redundancy
OBSERVABLE_PROPERTY_RW(int, DaysOffset); OBSERVABLE_PROPERTY_RW(int, DaysOffset);
OBSERVABLE_PROPERTY_RW(int, MonthsOffset); OBSERVABLE_PROPERTY_RW(int, MonthsOffset);
@ -82,11 +82,11 @@ namespace CalculatorApp
} }
// Output Properties // Output Properties
OBSERVABLE_PROPERTY_RW(Platform::String^, StrDateDiffResult); OBSERVABLE_PROPERTY_R(Platform::String^, StrDateDiffResult);
OBSERVABLE_PROPERTY_RW(Platform::String^, StrDateDiffResultAutomationName); OBSERVABLE_PROPERTY_R(Platform::String^, StrDateDiffResultAutomationName);
OBSERVABLE_PROPERTY_RW(Platform::String^, StrDateDiffResultInDays); OBSERVABLE_PROPERTY_R(Platform::String^, StrDateDiffResultInDays);
OBSERVABLE_PROPERTY_RW(Platform::String^, StrDateResult); OBSERVABLE_PROPERTY_R(Platform::String^, StrDateResult);
OBSERVABLE_PROPERTY_RW(Platform::String^, StrDateResultAutomationName); OBSERVABLE_PROPERTY_R(Platform::String^, StrDateResultAutomationName);
COMMAND_FOR_METHOD(CopyCommand, DateCalculatorViewModel::OnCopyCommand); COMMAND_FOR_METHOD(CopyCommand, DateCalculatorViewModel::OnCopyCommand);
@ -104,8 +104,6 @@ namespace CalculatorApp
Platform::String^ GetLocalizedNumberString(int value) const; Platform::String^ GetLocalizedNumberString(int value) const;
static Windows::Foundation::DateTime ClipTime(Windows::Foundation::DateTime dateTime); static Windows::Foundation::DateTime ClipTime(Windows::Foundation::DateTime dateTime);
static void CheckClipTimeSameDay(Windows::Globalization::Calendar^ reference);
property bool IsOutOfBound property bool IsOutOfBound
{ {
bool get() { return m_isOutOfBound; } bool get() { return m_isOutOfBound; }

View File

@ -387,6 +387,63 @@ namespace DateCalculationUnitTests
VERIFY_IS_TRUE(StringReference(L"") != viewModel->StrDateResult); VERIFY_IS_TRUE(StringReference(L"") != viewModel->StrDateResult);
} }
TEST_METHOD(DateCalcViewModelDateDiffDaylightSavingTimeTest)
{
auto viewModel = ref new DateCalculatorViewModel();
viewModel->IsDateDiffMode = true;
VERIFY_IS_TRUE(viewModel->IsDateDiffMode);
// 29.02.2008
viewModel->FromDate = DateUtils::SystemTimeToDateTime(datetimeDifftest[5].startDate);
// 31.03.2008
viewModel->ToDate = DateUtils::SystemTimeToDateTime(datetimeDifftest[5].endDate);
//// Assert for the result
VERIFY_IS_FALSE(viewModel->IsDiffInDays);
VERIFY_ARE_EQUAL(StringReference(L"31 days"), viewModel->StrDateDiffResultInDays);
VERIFY_ARE_EQUAL(StringReference(L"1 month, 2 days"), viewModel->StrDateDiffResult);
// Daylight Saving Time - Clock Forward
// 10.03.2019
SYSTEMTIME startDate;
startDate.wYear = 2019;
startDate.wMonth = 03;
startDate.wDay = 10;
startDate.wDayOfWeek = 0;
startDate.wHour = startDate.wMinute = 0;
startDate.wSecond = startDate.wMilliseconds = 0;
viewModel->FromDate = DateUtils::SystemTimeToDateTime(startDate);
// 11.03.2019
SYSTEMTIME endDate;
endDate.wYear = 2019;
endDate.wMonth = 03;
endDate.wDay = 11;
endDate.wDayOfWeek = 0;
endDate.wHour = endDate.wMinute = 0;
endDate.wSecond = endDate.wMilliseconds = 0;
viewModel->ToDate = DateUtils::SystemTimeToDateTime(endDate);
VERIFY_IS_TRUE(viewModel->IsDiffInDays);
VERIFY_ARE_EQUAL(StringReference(L"1 day"), viewModel->StrDateDiffResult);
endDate.wDay += 6;
viewModel->ToDate = DateUtils::SystemTimeToDateTime(endDate);
VERIFY_IS_FALSE(viewModel->IsDiffInDays);
VERIFY_ARE_EQUAL(StringReference(L"1 week"), viewModel->StrDateDiffResult);
// Daylight Saving Time - Clock Backward
// 03.11.2019
startDate.wMonth = 11;
startDate.wDay = 03;
viewModel->FromDate = DateUtils::SystemTimeToDateTime(startDate);
// 04.11.2019
endDate.wMonth = 11;
endDate.wDay = 04;
viewModel->ToDate = DateUtils::SystemTimeToDateTime(endDate);
VERIFY_IS_TRUE(viewModel->IsDiffInDays);
VERIFY_ARE_EQUAL(StringReference(L"1 day"), viewModel->StrDateDiffResult);
}
TEST_METHOD(DateCalcViewModelAddTest) TEST_METHOD(DateCalcViewModelAddTest)
{ {
// TODO - MSFT 10331900, fix this test // TODO - MSFT 10331900, fix this test