diff --git a/WordPress/Classes/ViewRelated/Stats/Helpers/StatsPeriodHelper.swift b/WordPress/Classes/ViewRelated/Stats/Helpers/StatsPeriodHelper.swift index e77c7160bdc7..090a4590177b 100644 --- a/WordPress/Classes/ViewRelated/Stats/Helpers/StatsPeriodHelper.swift +++ b/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 diff --git a/WordPress/Classes/ViewRelated/Stats/Shared Views/Stats Detail/SiteStatsInsightsDetailsViewModel.swift b/WordPress/Classes/ViewRelated/Stats/Shared Views/Stats Detail/SiteStatsInsightsDetailsViewModel.swift index d5533fee8fb1..35cdf6a174b6 100644 --- a/WordPress/Classes/ViewRelated/Stats/Shared Views/Stats Detail/SiteStatsInsightsDetailsViewModel.swift +++ b/WordPress/Classes/ViewRelated/Stats/Shared Views/Stats Detail/SiteStatsInsightsDetailsViewModel.swift @@ -498,8 +498,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, diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index 76a87ca0204c..83c2dc396a82 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/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 */; }; @@ -5508,6 +5509,7 @@ 0148CC2A2859C87000CF5D96 /* BlogServiceMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlogServiceMock.swift; sourceTree = ""; }; 01CE5006290A889F00A9C2E0 /* TracksConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TracksConfiguration.swift; sourceTree = ""; }; 01CE5010290A890300A9C2E0 /* TracksConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TracksConfiguration.swift; sourceTree = ""; }; + 01E78D1C296EA54F00FB6863 /* StatsPeriodHelperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsPeriodHelperTests.swift; sourceTree = ""; }; 02761EBF2270072F009BAF0F /* BlogDetailsViewController+SectionHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "BlogDetailsViewController+SectionHelpers.swift"; sourceTree = ""; }; 02761EC122700A9C009BAF0F /* BlogDetailsSubsectionToSectionCategoryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlogDetailsSubsectionToSectionCategoryTests.swift; sourceTree = ""; }; 02761EC3227010BC009BAF0F /* BlogDetailsSectionIndexTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlogDetailsSectionIndexTests.swift; sourceTree = ""; }; @@ -11198,6 +11200,7 @@ 40C403F72215D88100E8C894 /* TopViewedStatsTests.swift */, 400A2C942217B68D000A8A59 /* TopViewedVideoStatsRecordValueTests.swift */, 400A2C962217B883000A8A59 /* VisitsSummaryStatsRecordValueTests.swift */, + 01E78D1C296EA54F00FB6863 /* StatsPeriodHelperTests.swift */, ); name = Stats; sourceTree = ""; @@ -22525,6 +22528,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 */, diff --git a/WordPress/WordPressTest/StatsPeriodHelperTests.swift b/WordPress/WordPressTest/StatsPeriodHelperTests.swift new file mode 100644 index 000000000000..23b24216b17b --- /dev/null +++ b/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) + } +}