#8728: Upgrade/datetimefield format #8729
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
2 tests failed but when I tried to rerun them locally I got no problem. Also, since the tests that failed don't have nothing to do with the files I worked on I guess there was some problem running the test, probably some connection issues. Here is the code for the two tests that are failing: |
BenedekFarkas
changed the title
|
Indeed, the test failures were caused by the domain being unreachable, I've fixed the test since then. Please merge from 1.10.x. |
BenedekFarkas
requested changes
Nov 18, 2024
| @@ -1,7 +1,9 @@ | |||
| using System; | |||
| using System.Runtime.InteropServices.WindowsRuntime; | |||
There was a problem hiding this comment.
Suggested change
| using System.Runtime.InteropServices.WindowsRuntime; |
| using Orchard.ContentManagement; | ||
| using Orchard.ContentManagement.FieldStorage; | ||
| using Orchard.Fields.Settings; | ||
| using Orchard.UI.Notify; |
There was a problem hiding this comment.
Suggested change
| using Orchard.UI.Notify; |
| @@ -10,15 +12,15 @@ public DateTime DateTime { | |||
| get { | |||
| var settings = this.PartFieldDefinition.Settings.GetModel<DateTimeFieldSettings>(); | |||
There was a problem hiding this comment.
Suggested change
| var settings = this.PartFieldDefinition.Settings.GetModel<DateTimeFieldSettings>(); |
| @@ -10,15 +12,15 @@ public DateTime DateTime { | |||
| get { | |||
| var settings = this.PartFieldDefinition.Settings.GetModel<DateTimeFieldSettings>(); | |||
| var value = Storage.Get<DateTime>(); | |||
| if (settings.Display == DateTimeFieldDisplays.DateOnly) { | |||
| if (Display == DateTimeFieldDisplays.DateOnly) { | |||
| return new DateTime(value.Year, value.Month, value.Day); | |||
| } | |||
| return value; | |||
| } | |||
|
|
|||
| set { | |||
| var settings = this.PartFieldDefinition.Settings.GetModel<DateTimeFieldSettings>(); | |||
There was a problem hiding this comment.
Suggested change
| var settings = this.PartFieldDefinition.Settings.GetModel<DateTimeFieldSettings>(); |
| Editor = new DateTimeEditor() { | ||
| Date = showDate ? DateLocalizationServices.ConvertToLocalizedDateString(value, options) : null, | ||
| Time = showTime ? DateLocalizationServices.ConvertToLocalizedTimeString(value, options) : null, | ||
| ShowDate = showDate, | ||
| ShowTime = showTime, | ||
| } | ||
| } |
| @@ -42,6 +42,7 @@ public override IEnumerable<TemplateViewModel> PartFieldEditorUpdate(ContentPart | |||
| builder.WithSetting("DateTimeFieldSettings.Required", model.Required.ToString(CultureInfo.InvariantCulture)); | |||
| model.DefaultValue = model.Editor == null ? model.DefaultValue : _dateLocalizationServices.ConvertFromLocalizedString(model.Editor.Date, model.Editor.Time); | |||
| builder.WithSetting("DateTimeFieldSettings.DefaultValue", model.DefaultValue.HasValue ? model.DefaultValue.Value.ToString(CultureInfo.InvariantCulture) : String.Empty); | |||
| builder.WithSetting("DateTimeFieldSettings.AllowDisplayOptionsOverride", model.AllowDisplayOptionsOverride.ToString(CultureInfo.InvariantCulture)); | |||
There was a problem hiding this comment.
Suggested change
| builder.WithSetting("DateTimeFieldSettings.AllowDisplayOptionsOverride", model.AllowDisplayOptionsOverride.ToString(CultureInfo.InvariantCulture)); | |
| builder.WithSetting("DateTimeFieldSettings.AllowDisplayOptionOverride", model.AllowDisplayOptionsOverride.ToString(CultureInfo.InvariantCulture)); |
|
|
||
| <fieldset> | ||
| <label for="@Html.FieldIdFor(m => Model.Editor.Date)" @if (Model.IsRequired) { <text> class="required" </text> }>@Model.Name</label> | ||
| @Html.EditorFor(m => m.Editor) | ||
| @if (Model.AllowDisplayOptionsOverride) { |
There was a problem hiding this comment.
Suggested change
| @if (Model.AllowDisplayOptionsOverride) { | |
| @if (Model.AllowDisplayOptionOverride) { |
Comment on lines
+48
to
+53
| get { | ||
| return Storage.Get<string>("DisplayOption"); | ||
| } | ||
| set { | ||
| Storage.Set("DisplayOption", value ?? String.Empty); | ||
| } |
There was a problem hiding this comment.
Suggested change
| get { | |
| return Storage.Get<string>("DisplayOption"); | |
| } | |
| set { | |
| Storage.Set("DisplayOption", value ?? String.Empty); | |
| } | |
| get => Storage.Get<string>("DisplayOption"); | |
| set => Storage.Set("DisplayOption", value ?? string.Empty); |
Comment on lines
+42
to
+44
| set { | ||
| DisplayOption = value.ToString(); | ||
| } |
There was a problem hiding this comment.
Suggested change
| set { | |
| DisplayOption = value.ToString(); | |
| } | |
| set => DisplayOption = value.ToString(); |
Comment on lines
+35
to
40
| if (settings.AllowDisplayOptionsOverride) { | ||
| if (Enum.TryParse(DisplayOption, out DateTimeFieldDisplays displayOption)){ | ||
| return displayOption; | ||
| } | ||
| } | ||
| return settings.Display; |
There was a problem hiding this comment.
Suggested change
| if (settings.AllowDisplayOptionsOverride) { | |
| if (Enum.TryParse(DisplayOption, out DateTimeFieldDisplays displayOption)){ | |
| return displayOption; | |
| } | |
| } | |
| return settings.Display; | |
| return settings.AllowDisplayOptionOverride && Enum.TryParse(DisplayOption, out DateTimeFieldDisplays displayOption) | |
| ? displayOption | |
| : settings.Display; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8728
Added the AllowDisplayOptionsOverride property in DateTimeFieldSetting.
Handled this setting in DateTimeField so that it returns the overriding value (if set) or (if there's no overriding value set) the setting value.
Handles a new case in in DefaultDateLocalizationServices.cs: when converting a DateTime to and from UTC with the IgnoreDate setting, if the DateTime had a valid Date (!= DateTime.min) the code was returning a wrong value since it didn't handle the solar time correctly.
Probably this case wasn't handled correctly since DateTimeFields were only allowing the user to insert the time when their Display property was set to TimeOnly, but we decided to show both the Date and Time editors when the AllowDisplayOptionsOverride is set to true so now it's possibile to have to convert DateTime with a valid date and the IgnoreDate option set to true.