[Stats Revamp] Changing "First day of week" results in incorrect numbers in Views & Visitors and Total Likes details (#19829)

* Use weekIncludingDate to determine end of week for Monday - Sunday week

StatsPeriodHelper().weekIncludingDate is used in the Stats to determine start and end of week, when week is Monday - Sunday.

Use this method in the lastDayOfTheWeek to have a consistent start and end of week calculation and avoid bugs with different iOS firstDayOfWeek settings.

* Remove duplicated code from calculateEndDate

calculateEndDate and lastDayOfTheWeek duplicate the same code and comment. Use lastDayOfTheWeek in calculateEndDate.

* Fix TotalLikes details numbers by passing correct parameter to futureEndOfWeekDate

* Remove a -12 hour offset when calculating last day of the week

This is not necessary with the current calculation approach and it produces wrong results with the current date is around the beginning or end of the week

* Returning the current date instead of nil

* Keeping the changes under the Feature Flag

* Use FeatureFlag.statsNewInsights

Author: staskus

WordPress/Classes/ViewRelated/Stats/Helpers/StatsPeriodHelper.swift

@@ -92,9 +92,10 @@ class StatsPeriodHelper {
         }
     }
 
-    func calculateEndDate(from currentDate: Date, offsetBy count: Int = 1, unit: StatsPeriodUnit) -> Date? {
-        let calendar = Calendar.autoupdatingCurrent
-
+    func calculateEndDate(from currentDate: Date,
+                          offsetBy count: Int = 1,
+                          unit: StatsPeriodUnit,
+                          calendar: Calendar = Calendar.autoupdatingCurrent) -> Date? {
         guard let adjustedDate = calendar.date(byAdding: unit.calendarComponent, value: count, to: currentDate) else {
             DDLogError("[Stats] Couldn't do basic math on Calendars in Stats. Returning original value.")
             return currentDate
@@ -105,21 +106,28 @@ class StatsPeriodHelper {
             return adjustedDate.normalizedDate()
 
         case .week:
-
-            // The hours component here is because the `dateInterval` returned by Calendar is a closed range
-            // — so the "end" of a specific week is also simultenously a 'start' of the next one.
-            // This causes problem when calling this math on dates that _already are_ an end/start of a week.
-            // This doesn't work for our calculations, so we force it to rollover using this hack.
-            // (I *think* that's what's happening here. Doing Calendar math on this method has broken my brain.
-            // I spend like 10h on this ~50 LoC method. Beware.)
-            let components = DateComponents(day: 7 * count, hour: -12)
-
-            guard let weekAdjusted = calendar.date(byAdding: components, to: currentDate.normalizedDate()) else {
-                DDLogError("[Stats] Couldn't add a multiple of 7 days and -12 hours to a date in Stats. Returning original value.")
-                return currentDate
+            if FeatureFlag.statsNewInsights.enabled {
+                guard let endDate = currentDate.lastDayOfTheWeek(in: calendar, with: count) else {
+                    DDLogError("[Stats] Couldn't determine the last day of the week for a given date in Stats. Returning original value.")
+                    return currentDate
+                }
+
+                return endDate.normalizedDate()
+            } else {
+                // The hours component here is because the `dateInterval` returned by Calendar is a closed range
+                // — so the "end" of a specific week is also simultenously a 'start' of the next one.
+                // This causes problem when calling this math on dates that _already are_ an end/start of a week.
+                // This doesn't work for our calculations, so we force it to rollover using this hack.
+                // (I *think* that's what's happening here. Doing Calendar math on this method has broken my brain.
+                // I spend like 10h on this ~50 LoC method. Beware.)
+                let components = DateComponents(day: 7 * count, hour: -12)
+
+                guard let weekAdjusted = calendar.date(byAdding: components, to: currentDate.normalizedDate()) else {
+                    DDLogError("[Stats] Couldn't add a multiple of 7 days and -12 hours to a date in Stats. Returning original value.")
+                    return currentDate
+                }
+                return calendar.dateInterval(of: .weekOfYear, for: weekAdjusted)?.end.normalizedDate()
             }
-            return calendar.dateInterval(of: .weekOfYear, for: weekAdjusted)?.end.normalizedDate()
-
         case .month:
             guard let endDate = adjustedDate.lastDayOfTheMonth(in: calendar) else {
                 DDLogError("[Stats] Couldn't determine number of days in a given month in Stats. Returning original value.")
@@ -169,17 +177,11 @@ private extension Date {
     }
 
     func lastDayOfTheWeek(in calendar: Calendar, with offset: Int) -> Date? {
-        // The hours component here is because the `dateInterval` returnd by Calendar is a closed range
-        // — so the "end" of a specific week is also simultenously a 'start' of the next one.
-        // This causes problem when calling this math on dates that _already are_ an end/start of a week.
-        // This doesn't work for our calculations, so we force it to rollover using this hack.
-        // (I *think* that's what's happening here. Doing Calendar math on this method has broken my brain.
-        // I spend like 10h on this ~50 LoC method. Beware.)
-        let components = DateComponents(day: 7 * offset, hour: -12)
+        let components = DateComponents(day: 7 * offset)
 
         guard let weekAdjusted = calendar.date(byAdding: components, to: normalizedDate()),
-        let endOfAdjustedWeek = calendar.dateInterval(of: .weekOfYear, for: weekAdjusted)?.end else {
-            DDLogError("[Stats] Couldn't add a multiple of 7 days and -12 hours to a date in Stats. Returning original value.")
+              let endOfAdjustedWeek = StatsPeriodHelper().weekIncludingDate(weekAdjusted)?.weekEnd else {
+            DDLogError("[Stats] Couldn't add a multiple of 7 days to a date in Stats. Returning original value.")
             return nil
         }
         return endOfAdjustedWeek

WordPress/Classes/ViewRelated/Stats/Shared Views/Stats Detail/SiteStatsInsightsDetailsViewModel.swift

@@ -502,8 +502,8 @@ class SiteStatsInsightsDetailsViewModel: Observable {
         return StatsTotalInsightsData.followersCount(insightsStore: insightsStore)
     }
 
-        func createLikesTotalInsightsRow(periodSummary: StatsSummaryTimeIntervalData?) -> StatsTotalInsightsData {
-        let weekEnd = futureEndOfWeekDate(for: periodStore.getSummary())
+    func createLikesTotalInsightsRow(periodSummary: StatsSummaryTimeIntervalData?) -> StatsTotalInsightsData {
+        let weekEnd = futureEndOfWeekDate(for: periodSummary)
         var data = StatsTotalInsightsData.createTotalInsightsData(periodSummary: periodSummary,
                                                                   insightsStore: insightsStore,
                                                                   statsSummaryType: .likes,

WordPress/WordPress.xcodeproj/project.pbxproj

@@ -150,6 +150,7 @@
 		01CE5013290A890E00A9C2E0 /* TracksConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01CE5010290A890300A9C2E0 /* TracksConfiguration.swift */; };
 		01CE5014290A890E00A9C2E0 /* TracksConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01CE5010290A890300A9C2E0 /* TracksConfiguration.swift */; };
 		01CE5015290A890F00A9C2E0 /* TracksConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01CE5010290A890300A9C2E0 /* TracksConfiguration.swift */; };
+		01E78D1D296EA54F00FB6863 /* StatsPeriodHelperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01E78D1C296EA54F00FB6863 /* StatsPeriodHelperTests.swift */; };
 		02761EC02270072F009BAF0F /* BlogDetailsViewController+SectionHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02761EBF2270072F009BAF0F /* BlogDetailsViewController+SectionHelpers.swift */; };
 		02761EC222700A9C009BAF0F /* BlogDetailsSubsectionToSectionCategoryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02761EC122700A9C009BAF0F /* BlogDetailsSubsectionToSectionCategoryTests.swift */; };
 		02761EC4227010BC009BAF0F /* BlogDetailsSectionIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02761EC3227010BC009BAF0F /* BlogDetailsSectionIndexTests.swift */; };
@@ -5530,6 +5531,7 @@
 		0148CC2A2859C87000CF5D96 /* BlogServiceMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlogServiceMock.swift; sourceTree = "<group>"; };
 		01CE5006290A889F00A9C2E0 /* TracksConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TracksConfiguration.swift; sourceTree = "<group>"; };
 		01CE5010290A890300A9C2E0 /* TracksConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TracksConfiguration.swift; sourceTree = "<group>"; };
+		01E78D1C296EA54F00FB6863 /* StatsPeriodHelperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsPeriodHelperTests.swift; sourceTree = "<group>"; };
 		02761EBF2270072F009BAF0F /* BlogDetailsViewController+SectionHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "BlogDetailsViewController+SectionHelpers.swift"; sourceTree = "<group>"; };
 		02761EC122700A9C009BAF0F /* BlogDetailsSubsectionToSectionCategoryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlogDetailsSubsectionToSectionCategoryTests.swift; sourceTree = "<group>"; };
 		02761EC3227010BC009BAF0F /* BlogDetailsSectionIndexTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlogDetailsSectionIndexTests.swift; sourceTree = "<group>"; };
@@ -11236,6 +11238,7 @@
 				40C403F72215D88100E8C894 /* TopViewedStatsTests.swift */,
 				400A2C942217B68D000A8A59 /* TopViewedVideoStatsRecordValueTests.swift */,
 				400A2C962217B883000A8A59 /* VisitsSummaryStatsRecordValueTests.swift */,
+				01E78D1C296EA54F00FB6863 /* StatsPeriodHelperTests.swift */,
 			);
 			name = Stats;
 			sourceTree = "<group>";
@@ -22560,6 +22563,7 @@
 				010459ED2915519C000C7778 /* JetpackNotificationMigrationServiceTests.swift in Sources */,
 				8B6214E627B1B446001DF7B6 /* BlogDashboardServiceTests.swift in Sources */,
 				C856749A243F4292001A995E /* TenorMockDataHelper.swift in Sources */,
+				01E78D1D296EA54F00FB6863 /* StatsPeriodHelperTests.swift in Sources */,
 				3FFE3C0828FE00D10021BB96 /* StatsSegmentedControlDataTests.swift in Sources */,
 				D81C2F5820F86CEA002AE1F1 /* NetworkStatus.swift in Sources */,
 				E1C545801C6C79BB001CEB0E /* MediaSettingsTests.swift in Sources */,

WordPress/WordPressTest/StatsPeriodHelperTests.swift

@@ -0,0 +1,134 @@
+import XCTest
+@testable import WordPress
+
+final class StatsPeriodHelperTests: XCTestCase {
+    private var sut: StatsPeriodHelper!
+
+    override func setUpWithError() throws {
+        sut = StatsPeriodHelper()
+        try? FeatureFlagOverrideStore().override(FeatureFlag.statsNewInsights, withValue: true)
+    }
+
+    override func tearDownWithError() throws {
+        sut = nil
+        try? FeatureFlagOverrideStore().override(FeatureFlag.statsNewInsights, withValue: false)
+    }
+
+    func testEndOfWeekWhenMondayIsSetAsFirstWeekday() {
+        // A calendar with Monday as first weekday
+        var calendar = Calendar(identifier: .gregorian)
+        calendar.firstWeekday = 2
+
+        // Thursday
+        let dateComponents = DateComponents(year: 2023, month: 02, day: 02, hour: 8, minute: 34)
+
+        let endDate = sut.calculateEndDate(
+            from: calendar.date(from: dateComponents)!,
+            offsetBy: 0,
+            unit: .week,
+            calendar: calendar
+        )
+
+        // 2023-02-05 Sunday should be end of the week
+        XCTAssertEqual(endDate?.dateAndTimeComponents().day, 5)
+    }
+
+    func testEndOfWeekWhenSundayIsSetAsFirstWeekday() {
+        // A calendar with Sunday as first weekday
+        var calendar = Calendar(identifier: .gregorian)
+        calendar.firstWeekday = 1
+
+        // Thursday
+        let dateComponents = DateComponents(year: 2023, month: 02, day: 02, hour: 8, minute: 34)
+
+        let endDate = sut.calculateEndDate(
+            from: calendar.date(from: dateComponents)!,
+            offsetBy: 0,
+            unit: .week,
+            calendar: calendar
+        )
+
+        // 2023-02-05 Sunday should still be end of the week
+        XCTAssertEqual(endDate?.dateAndTimeComponents().day, 5)
+    }
+
+    func testEndOfWeekWhenCurrentDateIsStartOfWeek() {
+        // A calendar with Monday as first weekday
+        var calendar = Calendar(identifier: .gregorian)
+        calendar.firstWeekday = 2
+
+        // Start of Monday
+        let dateComponents = DateComponents(year: 2021, month: 11, day: 15, hour: 00, minute: 00)
+
+        let endDate = sut.calculateEndDate(
+            from: calendar.date(from: dateComponents)!,
+            offsetBy: 0,
+            unit: .week,
+            calendar: calendar
+        )
+
+        // 2021-11-21 Sunday should be end of the week
+        XCTAssertEqual(endDate?.dateAndTimeComponents().day, 21)
+    }
+
+    func testEndOfWeekWhenCurrentDateIsEndOfWeek() {
+        // A calendar with Monday as first weekday
+        var calendar = Calendar(identifier: .gregorian)
+        calendar.firstWeekday = 2
+
+        // End of Sunday
+        let dateComponents = DateComponents(year: 2021, month: 11, day: 21, hour: 23, minute: 59)
+
+        let endDate = sut.calculateEndDate(
+            from: calendar.date(from: dateComponents)!,
+            offsetBy: 0,
+            unit: .week,
+            calendar: calendar
+        )
+
+        // 2021-11-21 Sunday should be end of the week
+        XCTAssertEqual(endDate?.dateAndTimeComponents().day, 21)
+    }
+
+    func testEndOfNextWeek() {
+        // A calendar with Monday as first weekday
+        var calendar = Calendar(identifier: .gregorian)
+        calendar.firstWeekday = 2
+
+        // Thursday
+        let dateComponents = DateComponents(year: 2023, month: 02, day: 02, hour: 8, minute: 34)
+
+        // Get end date of next week
+        let endDate = sut.calculateEndDate(
+            from: calendar.date(from: dateComponents)!,
+            offsetBy: 1,
+            unit: .week,
+            calendar: calendar
+        )
+
+        // 2023-02-12
+        XCTAssertEqual(endDate?.dateAndTimeComponents().month, 2)
+        XCTAssertEqual(endDate?.dateAndTimeComponents().day, 12)
+    }
+
+    func testEndOfLastWeek() {
+        // A calendar with Monday as first weekday
+        var calendar = Calendar(identifier: .gregorian)
+        calendar.firstWeekday = 2
+
+        // Thursday
+        let dateComponents = DateComponents(year: 2023, month: 02, day: 02, hour: 8, minute: 34)
+
+        // Get end date of last week
+        let endDate = sut.calculateEndDate(
+            from: calendar.date(from: dateComponents)!,
+            offsetBy: -1,
+            unit: .week,
+            calendar: calendar
+        )
+
+        // 2023-01-29
+        XCTAssertEqual(endDate?.dateAndTimeComponents().month, 1)
+        XCTAssertEqual(endDate?.dateAndTimeComponents().day, 29)
+    }
+}