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

Fix date conversion on the server-side #16841

Merged
merged 1 commit into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/Umbraco.Core/Extensions/ObjectExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.

Check notice on line 1 in src/Umbraco.Core/Extensions/ObjectExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

ℹ Getting worse: Overall Code Complexity

The mean cyclomatic complexity increases from 7.24 to 7.43, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
// See LICENSE for more details.

using System.Collections;
Expand Down Expand Up @@ -219,6 +219,24 @@
}
}

if (target == typeof(DateTime) && input is DateTimeOffset dateTimeOffset)
{
// IMPORTANT: for compatability with various editors, we must discard any Offset information and assume UTC time here
return Attempt.Succeed((object?)new DateTime(
new DateOnly(dateTimeOffset.Year, dateTimeOffset.Month, dateTimeOffset.Day),
new TimeOnly(dateTimeOffset.Hour, dateTimeOffset.Minute, dateTimeOffset.Second, dateTimeOffset.Millisecond, dateTimeOffset.Microsecond),
DateTimeKind.Utc));
}

if (target == typeof(DateTimeOffset) && input is DateTime dateTime)
{
// IMPORTANT: for compatability with various editors, we must discard any DateTimeKind information and assume UTC time here
return Attempt.Succeed((object?)new DateTimeOffset(
new DateOnly(dateTime.Year, dateTime.Month, dateTime.Day),
new TimeOnly(dateTime.Hour, dateTime.Minute, dateTime.Second, dateTime.Millisecond, dateTime.Microsecond),
TimeSpan.Zero));
}

Check warning on line 239 in src/Umbraco.Core/Extensions/ObjectExtensions.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

❌ Getting worse: Complex Method

TryConvertTo increases in cyclomatic complexity from 28 to 32, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
TypeConverter? inputConverter = GetCachedSourceTypeConverter(inputType, target);
if (inputConverter != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,19 @@ public override object ConvertSourceToIntermediate(IPublishedElement owner, IPub

internal static DateTime ParseDateTimeValue(object? source)
{
if (source == null)
if (source is null)
{
return DateTime.MinValue;
}

// in XML a DateTime is: string - format "yyyy-MM-ddTHH:mm:ss"
// Actually, not always sometimes it is formatted in UTC style with 'Z' suffixed on the end but that is due to this bug:
// http://issues.umbraco.org/issue/U4-4145, http://issues.umbraco.org/issue/U4-3894
// We should just be using TryConvertTo instead.
if (source is string sourceString)
if (source is DateTime dateTimeValue)
{
Attempt<DateTime> attempt = sourceString.TryConvertTo<DateTime>();
return attempt.Success == false ? DateTime.MinValue : attempt.Result;
return dateTimeValue;
}

// in the database a DateTime is: DateTime
// default value is: DateTime.MinValue
return source is DateTime dateTimeValue ? dateTimeValue : DateTime.MinValue;
Attempt<DateTime> attempt = source.TryConvertTo<DateTime>();
return attempt.Success
? attempt.Result
: DateTime.MinValue;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.

Check notice on line 1 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/CoreThings/ObjectExtensionsTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

ℹ Getting worse: Code Duplication

introduced similar code in: CanConvertDateTimeOffsetToDateTime,DiscardsOffsetWhenConvertingDateTimeOffsetToDateTime. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.
// See LICENSE for more details.

using System;
Expand All @@ -11,6 +11,7 @@
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Tests.Common.TestHelpers;
using Umbraco.Extensions;
using DateTimeOffset = System.DateTimeOffset;

namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.CoreThings;

Expand Down Expand Up @@ -332,6 +333,50 @@
Assert.AreEqual("This is a string", conv.Result);
}

[Test]
public void CanConvertDateTimeOffsetToDateTime()
{
var dateTimeOffset = new DateTimeOffset(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03), TimeSpan.Zero);
var result = dateTimeOffset.TryConvertTo<DateTime>();
Assert.IsTrue(result.Success);
Assert.Multiple(() =>
{
Assert.AreEqual(new DateTime(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03)), result.Result);
Assert.AreEqual(DateTimeKind.Utc, result.Result.Kind);
});
}

[Test]
public void CanConvertDateTimeToDateTimeOffset()
{
var dateTime = new DateTime(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03), DateTimeKind.Utc);
var result = dateTime.TryConvertTo<DateTimeOffset>();
Assert.IsTrue(result.Success);
Assert.AreEqual(new DateTimeOffset(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03), TimeSpan.Zero), result.Result);
}

[Test]
public void DiscardsOffsetWhenConvertingDateTimeOffsetToDateTime()
{
var dateTimeOffset = new DateTimeOffset(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03), TimeSpan.FromHours(2));
var result = dateTimeOffset.TryConvertTo<DateTime>();
Assert.IsTrue(result.Success);
Assert.Multiple(() =>
{
Assert.AreEqual(new DateTime(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03)), result.Result);
Assert.AreEqual(DateTimeKind.Utc, result.Result.Kind);
});
}

[Test]
public void DiscardsDateTimeKindWhenConvertingDateTimeToDateTimeOffset()
{
var dateTime = new DateTime(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03), DateTimeKind.Local);
var result = dateTime.TryConvertTo<DateTimeOffset>();
Assert.IsTrue(result.Success);
Assert.AreEqual(new DateTimeOffset(new DateOnly(2024, 07, 05), new TimeOnly(12, 30, 01, 02, 03), TimeSpan.Zero), result.Result);
}

[Test]
public void Value_Editor_Can_Convert_Decimal_To_Decimal_Clr_Type()
{
Expand All @@ -342,6 +387,23 @@
Assert.AreEqual(12.34d, result.Result);
}

[Test]
public void Value_Editor_Can_Convert_DateTimeOffset_To_DateTime_Clr_Type()
{
var valueEditor = MockedValueEditors.CreateDataValueEditor(ValueTypes.Date);

var result = valueEditor.TryConvertValueToCrlType(new DateTimeOffset(new DateOnly(2024, 07, 05), new TimeOnly(12, 30), TimeSpan.Zero));
Assert.IsTrue(result.Success);
Assert.IsTrue(result.Result is DateTime);

var dateTime = (DateTime)result.Result;
Assert.Multiple(() =>
{
Assert.AreEqual(new DateTime(new DateOnly(2024, 07, 05), new TimeOnly(12, 30)), dateTime);
Assert.AreEqual(DateTimeKind.Utc, dateTime.Kind);
});
}

private class MyTestObject
{
public override string ToString() => "Hello world";
Expand Down
Loading