From eb947a48af42c19e299498125ef4e91049e0cb3e Mon Sep 17 00:00:00 2001 From: "Daniel J. Summers" Date: Mon, 8 Aug 2022 20:39:19 -0400 Subject: [PATCH] Complete NodaTime migration (#41) - Tweak field layout (#38) --- src/PrayerTracker.Data/Entities.fs | 3 +- src/PrayerTracker.Tests/Data/EntitiesTests.fs | 119 +++++++++--------- src/PrayerTracker.Tests/UI/ViewModelsTests.fs | 65 +++++----- src/PrayerTracker.UI/PrayerRequest.fs | 8 +- src/PrayerTracker.UI/ViewModels.fs | 14 ++- src/PrayerTracker/Extensions.fs | 12 +- src/PrayerTracker/PrayerTracker.fsproj | 1 + src/PrayerTracker/wwwroot/css/app.css | 4 +- 8 files changed, 119 insertions(+), 107 deletions(-) diff --git a/src/PrayerTracker.Data/Entities.fs b/src/PrayerTracker.Data/Entities.fs index b38d84f..60f9588 100644 --- a/src/PrayerTracker.Data/Entities.fs +++ b/src/PrayerTracker.Data/Entities.fs @@ -952,7 +952,8 @@ module PrayerRequest = | Automatic, Expecting -> false | Automatic, _ -> // Automatic expiration - asOf.PlusDays -group.Preferences.DaysToExpire > req.UpdatedDate.InZone(SmallGroup.timeZone group).Date + Period.Between(req.UpdatedDate.InZone(SmallGroup.timeZone group).Date, asOf, PeriodUnits.Days).Days + >= group.Preferences.DaysToExpire /// Is an update required for this long-term request? let updateRequired asOf group req = diff --git a/src/PrayerTracker.Tests/Data/EntitiesTests.fs b/src/PrayerTracker.Tests/Data/EntitiesTests.fs index a13bd6d..89fa5e0 100644 --- a/src/PrayerTracker.Tests/Data/EntitiesTests.fs +++ b/src/PrayerTracker.Tests/Data/EntitiesTests.fs @@ -143,6 +143,8 @@ let memberTests = [] let prayerRequestTests = + let instantNow = SystemClock.Instance.GetCurrentInstant + let localDateNow () = (instantNow ()).InUtc().Date testList "PrayerRequest" [ test "empty is as expected" { let mt = PrayerRequest.empty @@ -150,8 +152,8 @@ let prayerRequestTests = Expect.equal mt.RequestType CurrentRequest "The request type should have been Current" Expect.equal mt.UserId.Value Guid.Empty "The user ID should have been an empty GUID" Expect.equal mt.SmallGroupId.Value Guid.Empty "The small group ID should have been an empty GUID" - Expect.equal mt.EnteredDate DateTime.MinValue "The entered date should have been the minimum" - Expect.equal mt.UpdatedDate DateTime.MinValue "The updated date should have been the minimum" + Expect.equal mt.EnteredDate Instant.MinValue "The entered date should have been the minimum" + Expect.equal mt.UpdatedDate Instant.MinValue "The updated date should have been the minimum" Expect.isNone mt.Requestor "The requestor should not exist" Expect.equal mt.Text "" "The request text should have been blank" Expect.isFalse mt.NotifyChaplain "The notify chaplain flag should not have been set" @@ -159,62 +161,60 @@ let prayerRequestTests = Expect.equal mt.User.Id.Value Guid.Empty "The user should have been an empty one" Expect.equal mt.SmallGroup.Id.Value Guid.Empty "The small group should have been an empty one" } - test "IsExpired always returns false for expecting requests" { - let req = { PrayerRequest.empty with RequestType = Expecting } - Expect.isFalse (req.IsExpired DateTime.Now 0) "An expecting request should never be considered expired" + test "isExpired always returns false for expecting requests" { + PrayerRequest.isExpired (localDateNow ()) SmallGroup.empty + { PrayerRequest.empty with RequestType = Expecting } + |> Flip.Expect.isFalse "An expecting request should never be considered expired" } - test "IsExpired always returns false for manually-expired requests" { - let req = { PrayerRequest.empty with UpdatedDate = DateTime.Now.AddMonths -1; Expiration = Manual } - Expect.isFalse (req.IsExpired DateTime.Now 4) "A never-expired request should never be considered expired" + test "isExpired always returns false for manually-expired requests" { + PrayerRequest.isExpired (localDateNow ()) SmallGroup.empty + { PrayerRequest.empty with UpdatedDate = (instantNow ()) - Duration.FromDays 1; Expiration = Manual } + |> Flip.Expect.isFalse "A never-expired request should never be considered expired" } - test "IsExpired always returns false for long term/recurring requests" { - let req = { PrayerRequest.empty with RequestType = LongTermRequest } - Expect.isFalse (req.IsExpired DateTime.Now 0) - "A recurring/long-term request should never be considered expired" + test "isExpired always returns false for long term/recurring requests" { + PrayerRequest.isExpired (localDateNow ()) SmallGroup.empty + { PrayerRequest.empty with RequestType = LongTermRequest } + |> Flip.Expect.isFalse "A recurring/long-term request should never be considered expired" } - test "IsExpired always returns true for force-expired requests" { - let req = { PrayerRequest.empty with UpdatedDate = DateTime.Now; Expiration = Forced } - Expect.isTrue (req.IsExpired DateTime.Now 5) "A force-expired request should always be considered expired" + test "isExpired always returns true for force-expired requests" { + PrayerRequest.isExpired (localDateNow ()) SmallGroup.empty + { PrayerRequest.empty with UpdatedDate = (instantNow ()); Expiration = Forced } + |> Flip.Expect.isTrue "A force-expired request should always be considered expired" } - test "IsExpired returns false for non-expired requests" { - let now = DateTime.Now - let req = { PrayerRequest.empty with UpdatedDate = now.AddDays -5. } - Expect.isFalse (req.IsExpired now 7) "A request updated 5 days ago should not be considered expired" + test "isExpired returns false for non-expired requests" { + let now = instantNow () + PrayerRequest.isExpired (now.InUtc().Date) SmallGroup.empty + { PrayerRequest.empty with UpdatedDate = now - Duration.FromDays 5 } + |> Flip.Expect.isFalse "A request updated 5 days ago should not be considered expired" } - test "IsExpired returns true for expired requests" { - let now = DateTime.Now - let req = { PrayerRequest.empty with UpdatedDate = now.AddDays -8. } - Expect.isTrue (req.IsExpired now 7) "A request updated 8 days ago should be considered expired" + test "isExpired returns true for expired requests" { + let now = instantNow () + PrayerRequest.isExpired (now.InUtc().Date) SmallGroup.empty + { PrayerRequest.empty with UpdatedDate = now - Duration.FromDays 15 } + |> Flip.Expect.isTrue "A request updated 15 days ago should be considered expired" } - test "IsExpired returns true for same-day expired requests" { - let now = DateTime.Now - let req = { PrayerRequest.empty with UpdatedDate = now.Date.AddDays(-7.).AddSeconds -1. } - Expect.isTrue (req.IsExpired now 7) - "A request entered a second before midnight should be considered expired" + test "isExpired returns true for same-day expired requests" { + let now = instantNow () + PrayerRequest.isExpired (now.InUtc().Date) SmallGroup.empty + { PrayerRequest.empty with UpdatedDate = now - (Duration.FromDays 14) - (Duration.FromSeconds 1L) } + |> Flip.Expect.isTrue "A request entered a second before midnight should be considered expired" } - test "UpdateRequired returns false for expired requests" { - let req = { PrayerRequest.empty with Expiration = Forced } - Expect.isFalse (req.UpdateRequired DateTime.Now 7 4) "An expired request should not require an update" + test "updateRequired returns false for expired requests" { + PrayerRequest.updateRequired (localDateNow ()) SmallGroup.empty + { PrayerRequest.empty with Expiration = Forced } + |> Flip.Expect.isFalse "An expired request should not require an update" } - test "UpdateRequired returns false when an update is not required for an active request" { - let now = DateTime.Now - let req = - { PrayerRequest.empty with - RequestType = LongTermRequest - UpdatedDate = now.AddDays -14. - } - Expect.isFalse (req.UpdateRequired now 7 4) - "An active request updated 14 days ago should not require an update until 28 days" + test "updateRequired returns false when an update is not required for an active request" { + let now = instantNow () + PrayerRequest.updateRequired (localDateNow ()) SmallGroup.empty + { PrayerRequest.empty with RequestType = LongTermRequest; UpdatedDate = now - Duration.FromDays 14 } + |> Flip.Expect.isFalse "An active request updated 14 days ago should not require an update until 28 days" } test "UpdateRequired returns true when an update is required for an active request" { - let now = DateTime.Now - let req = - { PrayerRequest.empty with - RequestType = LongTermRequest - UpdatedDate = now.AddDays -34. - } - Expect.isTrue (req.UpdateRequired now 7 4) - "An active request updated 34 days ago should require an update (past 28 days)" + let now = instantNow () + PrayerRequest.updateRequired (localDateNow ()) SmallGroup.empty + { PrayerRequest.empty with RequestType = LongTermRequest; UpdatedDate = now - Duration.FromDays 34 } + |> Flip.Expect.isTrue "An active request updated 34 days ago should require an update (past 28 days)" } ] @@ -288,9 +288,9 @@ let requestSortTests = [] let smallGroupTests = testList "SmallGroup" [ - let now = DateTime (2017, 5, 12, 12, 15, 0, DateTimeKind.Utc) + let now = Instant.FromDateTimeUtc (DateTime (2017, 5, 12, 12, 15, 0, DateTimeKind.Utc)) let withFakeClock f () = - FakeClock (Instant.FromDateTimeUtc now) |> f + FakeClock now |> f yield test "empty is as expected" { let mt = SmallGroup.empty Expect.equal mt.Id.Value Guid.Empty "The small group ID should have been an empty GUID" @@ -311,10 +311,12 @@ let smallGroupTests = { SmallGroup.empty with Preferences = { ListPreferences.empty with TimeZoneId = TimeZoneId "Europe/Berlin" } } - Expect.isGreaterThan (grp.LocalTimeNow clock) now "UTC to Europe/Berlin should have added hours" + let tz = DateTimeZoneProviders.Tzdb["Europe/Berlin"] + Expect.isGreaterThan (SmallGroup.localTimeNow clock grp) (now.InUtc().LocalDateTime) + "UTC to Europe/Berlin should have added hours" "LocalTimeNow adjusts the time behind UTC", fun clock -> - Expect.isLessThan (SmallGroup.empty.LocalTimeNow clock) now + Expect.isLessThan (SmallGroup.localTimeNow clock SmallGroup.empty) (now.InUtc().LocalDateTime) "UTC to America/Denver should have subtracted hours" "LocalTimeNow returns UTC when the time zone is invalid", fun clock -> @@ -322,16 +324,17 @@ let smallGroupTests = { SmallGroup.empty with Preferences = { ListPreferences.empty with TimeZoneId = TimeZoneId "garbage" } } - Expect.equal (grp.LocalTimeNow clock) now "UTC should have been returned for an invalid time zone" + Expect.equal (SmallGroup.localTimeNow clock grp) (now.InUtc().LocalDateTime) + "UTC should have been returned for an invalid time zone" ] - yield test "LocalTimeNow fails when clock is not passed" { - Expect.throws (fun () -> (SmallGroup.empty.LocalTimeNow >> ignore) null) + yield test "localTimeNow fails when clock is not passed" { + Expect.throws (fun () -> (SmallGroup.localTimeNow null SmallGroup.empty |> ignore)) "Should have raised an exception for null clock" } yield test "LocalDateNow returns the date portion" { - let now' = DateTime (2017, 5, 12, 1, 15, 0, DateTimeKind.Utc) - let clock = FakeClock (Instant.FromDateTimeUtc now') - Expect.isLessThan (SmallGroup.empty.LocalDateNow clock) now.Date "The date should have been a day earlier" + let clock = FakeClock (Instant.FromDateTimeUtc (DateTime (2017, 5, 12, 1, 15, 0, DateTimeKind.Utc))) + Expect.isLessThan (SmallGroup.localDateNow clock SmallGroup.empty) (now.InUtc().Date) + "The date should have been a day earlier" } ] diff --git a/src/PrayerTracker.Tests/UI/ViewModelsTests.fs b/src/PrayerTracker.Tests/UI/ViewModelsTests.fs index 629c80f..e52199f 100644 --- a/src/PrayerTracker.Tests/UI/ViewModelsTests.fs +++ b/src/PrayerTracker.Tests/UI/ViewModelsTests.fs @@ -3,6 +3,7 @@ open System open Expecto open Microsoft.AspNetCore.Html +open NodaTime open PrayerTracker.Entities open PrayerTracker.Tests.TestLocalization open PrayerTracker.Utils @@ -121,7 +122,7 @@ let appViewInfoTests = Expect.isNone vi.HelpLink "The help link should have been set to none" Expect.isEmpty vi.Messages "There should have been no messages set" Expect.equal vi.Version "" "The version should have been blank" - Expect.isGreaterThan vi.RequestStart DateTime.MinValue.Ticks "The request start time should have been set" + Expect.equal vi.RequestStart Instant.MinValue "The request start time should have been the minimum value" Expect.isNone vi.User "There should not have been a user" Expect.isNone vi.Group "There should not have been a small group" } @@ -497,30 +498,31 @@ let messageLevelTests = let requestListTests = testList "RequestList" [ let withRequestList f () = - { Requests = [ - { PrayerRequest.empty with - RequestType = CurrentRequest - Requestor = Some "Zeb" - Text = "zyx" - UpdatedDate = DateTime.Today - } - { PrayerRequest.empty with - RequestType = CurrentRequest - Requestor = Some "Aaron" - Text = "abc" - UpdatedDate = DateTime.Today - TimeSpan.FromDays 9. - } - { PrayerRequest.empty with - RequestType = PraiseReport - Text = "nmo" - UpdatedDate = DateTime.Today - } - ] - Date = DateTime.Today - SmallGroup = SmallGroup.empty - ShowHeader = false - Recipients = [] - CanEmail = false + let today = SystemClock.Instance.GetCurrentInstant () + { Requests = [ + { PrayerRequest.empty with + RequestType = CurrentRequest + Requestor = Some "Zeb" + Text = "zyx" + UpdatedDate = today + } + { PrayerRequest.empty with + RequestType = CurrentRequest + Requestor = Some "Aaron" + Text = "abc" + UpdatedDate = today - Duration.FromDays 9 + } + { PrayerRequest.empty with + RequestType = PraiseReport + Text = "nmo" + UpdatedDate = today + } + ] + Date = today.InUtc().Date + SmallGroup = SmallGroup.empty + ShowHeader = false + Recipients = [] + CanEmail = false } |> f yield! testFixture withRequestList [ @@ -574,7 +576,7 @@ let requestListTests = [ """
""" """Prayer Requests
""" """Test HTML Group
""" - htmlList.Date.ToString "MMMM d, yyyy" + htmlList.Date.ToString ("MMMM d, yyyy", null) "

" ] |> String.concat "" @@ -592,7 +594,7 @@ let requestListTests = } let html = htmlList.AsHtml _s let expected = - htmlList.Requests[0].UpdatedDate.ToShortDateString () + htmlList.Requests[0].UpdatedDate.InUtc().Date.ToString ("d", null) |> sprintf """Zeb — zyx  (as of %s)""" // spot check; if one request has it, they all should Expect.stringContains html expected "Expected short as-of date not found" @@ -607,7 +609,7 @@ let requestListTests = } let html = htmlList.AsHtml _s let expected = - htmlList.Requests[0].UpdatedDate.ToLongDateString () + htmlList.Requests[0].UpdatedDate.InUtc().Date.ToString ("D", null) |> sprintf """Zeb — zyx  (as of %s)""" // spot check; if one request has it, they all should Expect.stringContains html expected "Expected long as-of date not found" @@ -617,7 +619,8 @@ let requestListTests = let text = textList.AsText _s Expect.stringContains text $"{textList.SmallGroup.Name}\n" "Small group name not found" Expect.stringContains text "Prayer Requests\n" "List heading not found" - Expect.stringContains text ((textList.Date.ToString "MMMM d, yyyy") + "\n \n") "List date not found" + Expect.stringContains text ((textList.Date.ToString ("MMMM d, yyyy", null)) + "\n \n") + "List date not found" Expect.stringContains text "--------------------\n CURRENT REQUESTS\n--------------------\n" """Heading for category "Current Requests" not found""" Expect.stringContains text " + Zeb - zyx\n" "First request not found" @@ -637,7 +640,7 @@ let requestListTests = } let text = textList.AsText _s let expected = - textList.Requests[0].UpdatedDate.ToShortDateString () + textList.Requests[0].UpdatedDate.InUtc().Date.ToString ("d", null) |> sprintf " + Zeb - zyx (as of %s)" // spot check; if one request has it, they all should Expect.stringContains text expected "Expected short as-of date not found" @@ -652,7 +655,7 @@ let requestListTests = } let text = textList.AsText _s let expected = - textList.Requests[0].UpdatedDate.ToLongDateString () + textList.Requests[0].UpdatedDate.InUtc().Date.ToString ("D", null) |> sprintf " + Zeb - zyx (as of %s)" // spot check; if one request has it, they all should Expect.stringContains text expected "Expected long as-of date not found" diff --git a/src/PrayerTracker.UI/PrayerRequest.fs b/src/PrayerTracker.UI/PrayerRequest.fs index 9c766a5..1823ead 100644 --- a/src/PrayerTracker.UI/PrayerRequest.fs +++ b/src/PrayerTracker.UI/PrayerRequest.fs @@ -38,15 +38,13 @@ let edit (model : EditRequest) today ctx viewInfo = inputField "date" (nameof model.EnteredDate) "" [ _placeholder today ] ] else - // TODO: do these need to be nested like this? div [ _inputField ] [ + br [] div [ _checkboxField ] [ - br [] inputField "checkbox" (nameof model.SkipDateUpdate) "True" [] label [ _for (nameof model.SkipDateUpdate) ] [ locStr s["Check to not update the date"] ] - br [] - small [] [ em [] [ str (s["Typo Corrections"].Value.ToLower ()); rawText ", etc." ] ] ] + small [] [ em [] [ str (s["Typo Corrections"].Value.ToLower ()); rawText ", etc." ] ] ] ] div [ _fieldRow ] [ @@ -57,7 +55,7 @@ let edit (model : EditRequest) today ctx viewInfo = let radioId = String.concat "_" [ nameof model.Expiration; fst exp ] span [ _class "text-nowrap" ] [ radio (nameof model.Expiration) radioId (fst exp) model.Expiration - label [ _for radioId ] [ locStr (snd exp) ] + label [ _for radioId ] [ space; locStr (snd exp) ] rawText "     " ]) |> div [ _class "pt-center-text" ] diff --git a/src/PrayerTracker.UI/ViewModels.fs b/src/PrayerTracker.UI/ViewModels.fs index 58dc8b8..a1118e5 100644 --- a/src/PrayerTracker.UI/ViewModels.fs +++ b/src/PrayerTracker.UI/ViewModels.fs @@ -768,7 +768,7 @@ with /// Is this request new? member this.IsNew (req : PrayerRequest) = let reqDate = req.UpdatedDate.InZone(SmallGroup.timeZone this.SmallGroup).Date - Period.Between(this.Date, reqDate, PeriodUnits.Days).Days <= this.SmallGroup.Preferences.DaysToKeepNew + Period.Between(reqDate, this.Date, PeriodUnits.Days).Days <= this.SmallGroup.Preferences.DaysToKeepNew /// Generate this list as HTML member this.AsHtml (s : IStringLocalizer) = @@ -797,6 +797,7 @@ with ] ] ] + let tz = SmallGroup.timeZone this.SmallGroup reqs |> List.map (fun req -> let bullet = if this.IsNew req then "circle" else "disc" @@ -804,7 +805,7 @@ with match req.Requestor with | Some r when r <> "" -> strong [] [ str r ] - rawText " — " + rawText " – " | Some _ -> () | None -> () rawText req.Text @@ -814,8 +815,8 @@ with | LongDate -> let dt = match p.AsOfDateDisplay with - | ShortDate -> req.UpdatedDate.ToString ("d", null) - | LongDate -> req.UpdatedDate.ToString ("D", null) + | ShortDate -> req.UpdatedDate.InZone(tz).Date.ToString ("d", null) + | LongDate -> req.UpdatedDate.InZone(tz).Date.ToString ("D", null) | _ -> "" i [ _style $"font-size:%.2f{asOfSize}pt" ] [ rawText "  ("; str s["as of"].Value; str " "; str dt; rawText ")" @@ -828,6 +829,7 @@ with /// Generate this list as plain text member this.AsText (s : IStringLocalizer) = + let tz = SmallGroup.timeZone this.SmallGroup seq { this.SmallGroup.Name s["Prayer Requests"].Value @@ -846,8 +848,8 @@ with | _ -> let dt = match this.SmallGroup.Preferences.AsOfDateDisplay with - | ShortDate -> req.UpdatedDate.ToString ("d", null) - | LongDate -> req.UpdatedDate.ToString ("D", null) + | ShortDate -> req.UpdatedDate.InZone(tz).Date.ToString ("d", null) + | LongDate -> req.UpdatedDate.InZone(tz).Date.ToString ("D", null) | _ -> "" $""" ({s["as of"].Value} {dt})""" |> sprintf " %s %s%s%s" bullet requestor (htmlToPlainText req.Text) diff --git a/src/PrayerTracker/Extensions.fs b/src/PrayerTracker/Extensions.fs index cdcfaa1..6c04a4b 100644 --- a/src/PrayerTracker/Extensions.fs +++ b/src/PrayerTracker/Extensions.fs @@ -4,19 +4,26 @@ module PrayerTracker.Extensions open Microsoft.AspNetCore.Http open Microsoft.FSharpLu open Newtonsoft.Json +open NodaTime +open NodaTime.Serialization.JsonNet open PrayerTracker.Entities open PrayerTracker.ViewModels +/// JSON.NET serializer settings for NodaTime +let private jsonSettings = JsonSerializerSettings().ConfigureForNodaTime DateTimeZoneProviders.Tzdb + /// Extensions on the .NET session object type ISession with /// Set an object in the session member this.SetObject key value = - this.SetString (key, JsonConvert.SerializeObject value) + this.SetString (key, JsonConvert.SerializeObject (value, jsonSettings)) /// Get an object from the session member this.GetObject<'T> key = - match this.GetString key with null -> Unchecked.defaultof<'T> | v -> JsonConvert.DeserializeObject<'T> v + match this.GetString key with + | null -> Unchecked.defaultof<'T> + | v -> JsonConvert.DeserializeObject<'T> (v, jsonSettings) /// The currently logged on small group member this.CurrentGroup @@ -63,7 +70,6 @@ type ClaimsPrincipal with open Giraffe -open NodaTime open PrayerTracker /// Extensions on the ASP.NET Core HTTP context diff --git a/src/PrayerTracker/PrayerTracker.fsproj b/src/PrayerTracker/PrayerTracker.fsproj index 902c5c7..0b02431 100644 --- a/src/PrayerTracker/PrayerTracker.fsproj +++ b/src/PrayerTracker/PrayerTracker.fsproj @@ -27,6 +27,7 @@ + diff --git a/src/PrayerTracker/wwwroot/css/app.css b/src/PrayerTracker/wwwroot/css/app.css index e743504..96133cc 100644 --- a/src/PrayerTracker/wwwroot/css/app.css +++ b/src/PrayerTracker/wwwroot/css/app.css @@ -238,6 +238,7 @@ footer a:hover { flex-flow: row wrap; align-items: center; justify-content: center; + gap: 2rem; } .pt-field { display: flex; @@ -255,9 +256,6 @@ footer a:hover { text-transform: uppercase; color: #777; } -.pt-field ~ .pt-field { - margin-left: 3rem; -} .pt-center-text { text-align: center; }