diff --git a/Modules/Sources/JetpackStats/Services/PostLikesStore.swift b/Modules/Sources/JetpackStats/Services/PostLikesStore.swift new file mode 100644 index 000000000000..56d9bf5fbe4b --- /dev/null +++ b/Modules/Sources/JetpackStats/Services/PostLikesStore.swift @@ -0,0 +1,60 @@ +import Foundation +@preconcurrency import WordPressKit + +/// A single liker in the shape the app's shared likes cache expects. +/// +/// Built directly from the API response so the cache receives full-fidelity +/// data even though the stats UI renders only name and avatar: the Likes list +/// cell shows an `@username` subtitle, and the cache sorts and pages by the +/// parsed like date, so dropping either field here would degrade seeded rows. +/// The date is kept as the server-formatted string because the cache stores +/// that string verbatim as its paging cursor. +public struct PostLikeSeed: Equatable, Sendable { + public let userID: Int + public let displayName: String + public let username: String? + public let avatarURL: String? + public let dateLikedString: String? + + public init( + userID: Int, + displayName: String, + username: String?, + avatarURL: String?, + dateLikedString: String? + ) { + self.userID = userID + self.displayName = displayName + self.username = username + self.avatarURL = avatarURL + self.dateLikedString = dateLikedString + } +} + +extension PostLikeSeed { + init(remoteUser: RemoteLikeUser) { + self.init( + userID: remoteUser.userID?.intValue ?? 0, + displayName: remoteUser.displayName ?? remoteUser.username ?? "", + username: remoteUser.username, + avatarURL: remoteUser.avatarURL, + dateLikedString: remoteUser.dateLiked + ) + } +} + +/// App-injected sink that persists likers fetched by Post Stats into the +/// app's shared likes cache, so the Likes list screen can seed itself from +/// cache instead of starting empty. The package deliberately knows nothing +/// about the cache's implementation; `nil` (previews, mocks) disables seeding. +public protocol PostLikesStore: Sendable { + /// Persists the likers fetched by Post Stats for a post. + /// + /// `totalCount` is the post's authoritative like count from the same fetch. + /// When it is `0` the store must clear any cached likers for the post: Post + /// Stats only ever seeds the first page, so a plain upsert of the empty + /// `likes` would leave stale rows that the Likes list would show under a + /// "0 likes" title (including offline, where its own refresh cannot run). + /// A positive `totalCount` upserts the partial seeds without purging. + func storeLikes(_ likes: [PostLikeSeed], totalCount: Int, forPost postID: Int) async +} diff --git a/Modules/Sources/JetpackStats/Services/StatsService.swift b/Modules/Sources/JetpackStats/Services/StatsService.swift index a64b1ea62391..4ce18aeadd72 100644 --- a/Modules/Sources/JetpackStats/Services/StatsService.swift +++ b/Modules/Sources/JetpackStats/Services/StatsService.swift @@ -14,6 +14,7 @@ actor StatsService: StatsServiceProtocol { private let siteTimeZone: TimeZone // Temporary private var mocks: MockStatsService + private let postLikesStore: (any PostLikesStore)? // Cache private var siteStatsCache: [SiteStatsCacheKey: CachedEntity] = [:] @@ -46,7 +47,7 @@ actor StatsService: StatsServiceProtocol { } } - init(siteID: Int, api: WordPressComRestApi, timeZone: TimeZone) { + init(siteID: Int, api: WordPressComRestApi, timeZone: TimeZone, postLikesStore: (any PostLikesStore)? = nil) { self.siteID = siteID self.api = api self.service = StatsServiceRemoteV2( @@ -56,6 +57,7 @@ actor StatsService: StatsServiceProtocol { ) self.siteTimeZone = timeZone self.mocks = MockStatsService(timeZone: timeZone) + self.postLikesStore = postLikesStore } // MARK: - StatsServiceProtocol @@ -80,14 +82,25 @@ actor StatsService: StatsServiceProtocol { return data } - private func fetchSiteStats(interval: DateInterval, granularity: DateRangeGranularity) async throws -> SiteMetricsResponse { + private func fetchSiteStats( + interval: DateInterval, + granularity: DateRangeGranularity + ) async throws -> SiteMetricsResponse { let interval = convertDateIntervalSiteToLocal(interval) if granularity == .hour { // Hourly data is available only for "Views", so the service has to // make a separate request to fetch the total metrics. - async let hourlyResponseTask: WordPressKit.StatsSiteMetricsResponse = service.getData(interval: interval, unit: .init(granularity), limit: 0) - async let dailyResponseTask: WordPressKit.StatsSiteMetricsResponse = service.getData(interval: interval, unit: .init(.day), limit: 0) + async let hourlyResponseTask: WordPressKit.StatsSiteMetricsResponse = service.getData( + interval: interval, + unit: .init(granularity), + limit: 0 + ) + async let dailyResponseTask: WordPressKit.StatsSiteMetricsResponse = service.getData( + interval: interval, + unit: .init(.day), + limit: 0 + ) let (hourlyResponse, dailyResponse) = try await (hourlyResponseTask, dailyResponseTask) @@ -95,7 +108,11 @@ actor StatsService: StatsServiceProtocol { data.total = mapSiteMetricsResponse(dailyResponse).total return data } else { - let response: WordPressKit.StatsSiteMetricsResponse = try await service.getData(interval: interval, unit: .init(granularity), limit: 0) + let response: WordPressKit.StatsSiteMetricsResponse = try await service.getData( + interval: interval, + unit: .init(granularity), + limit: 0 + ) return mapSiteMetricsResponse(response) } } @@ -124,7 +141,10 @@ actor StatsService: StatsServiceProtocol { try await service.getWordAdsEarnings() } - private func fetchWordAdsStats(date: Date, granularity: DateRangeGranularity) async throws -> WordAdsMetricsResponse { + private func fetchWordAdsStats( + date: Date, + granularity: DateRangeGranularity + ) async throws -> WordAdsMetricsResponse { let localDate = convertDateSiteToLocal(date) let response: WordPressKit.StatsWordAdsResponse = try await service.getData( @@ -145,7 +165,10 @@ actor StatsService: StatsServiceProtocol { let now = Date.now - func makeDataPoint(from data: WordPressKit.StatsWordAdsResponse.PeriodData, metric: WordPressKit.StatsWordAdsResponse.Metric) -> DataPoint? { + func makeDataPoint( + from data: WordPressKit.StatsWordAdsResponse.PeriodData, + metric: WordPressKit.StatsWordAdsResponse.Metric + ) -> DataPoint? { guard let value = data[metric] else { return nil } @@ -180,16 +203,37 @@ actor StatsService: StatsServiceProtocol { return WordAdsMetricsResponse(total: total, metrics: metrics) } - func getTopListData(_ item: TopListItemType, metric: SiteMetric, interval: DateInterval, granularity: DateRangeGranularity, limit: Int?, options: TopListItemOptions) async throws -> TopListResponse { + func getTopListData( + _ item: TopListItemType, + metric: SiteMetric, + interval: DateInterval, + granularity: DateRangeGranularity, + limit: Int?, + options: TopListItemOptions + ) async throws -> TopListResponse { // Check cache first - let cacheKey = TopListCacheKey(item: item, metric: metric, options: options, interval: interval, granularity: granularity, limit: limit) + let cacheKey = TopListCacheKey( + item: item, + metric: metric, + options: options, + interval: interval, + granularity: granularity, + limit: limit + ) if let cached = topListCache[cacheKey], !cached.isExpired { return cached.data } // Fetch fresh data do { - let data = try await _getTopListData(item, metric: metric, interval: interval, granularity: granularity, limit: limit, options: options) + let data = try await _getTopListData( + item, + metric: metric, + interval: interval, + granularity: granularity, + limit: limit, + options: options + ) // Cache the result // Historical data never expires (ttl = nil), current period data expires after 30 seconds @@ -205,14 +249,22 @@ actor StatsService: StatsServiceProtocol { // when there are no recoreded periods (happens when the entire requested // period is _before_ the site creation). if let error = error as? StatsServiceRemoteV2.ResponseError, - error == .emptySummary { + error == .emptySummary + { return TopListResponse(items: []) } throw error } } - private func _getTopListData(_ item: TopListItemType, metric: SiteMetric, interval: DateInterval, granularity: DateRangeGranularity, limit: Int?, options: TopListItemOptions) async throws -> TopListResponse { + private func _getTopListData( + _ item: TopListItemType, + metric: SiteMetric, + interval: DateInterval, + granularity: DateRangeGranularity, + limit: Int?, + options: TopListItemOptions + ) async throws -> TopListResponse { func getData( _ type: T.Type, @@ -220,7 +272,13 @@ actor StatsService: StatsServiceProtocol { ) async throws -> T where T: Sendable { /// The `summarize: true` feature works correctly only with the `.day` granularity. let interval = convertDateIntervalSiteToLocal(interval) - return try await service.getData(interval: interval, unit: .day, summarize: true, limit: limit ?? 0, parameters: parameters) + return try await service.getData( + interval: interval, + unit: .day, + summarize: true, + limit: limit ?? 0, + parameters: parameters + ) } // Helper function to sort items by metric value (descending), then by displayName, and then by itemID for stable ordering @@ -277,7 +335,11 @@ actor StatsService: StatsServiceProtocol { } let convertedInterval = convertDateIntervalSiteToLocal(interval) - let data = try await service.getDeviceStats(breakdown: breakdown, startDate: convertedInterval.start, endDate: convertedInterval.end) + let data = try await service.getDeviceStats( + breakdown: breakdown, + startDate: convertedInterval.start, + endDate: convertedInterval.end + ) // TEMPORARY WORKAROUND (CMM-1168): // The screensize breakdown returns percentages (e.g., 73.8 for 73.8%), but SiteMetricsSet @@ -402,7 +464,8 @@ actor StatsService: StatsServiceProtocol { ) // Fetch likes using the REST API - let result = try await withCheckedThrowingContinuation { continuation in + let (result, seeds): (PostLikesData, [PostLikeSeed]) = try await withCheckedThrowingContinuation { + continuation in postService.getLikesForPostID( NSNumber(value: postID), count: NSNumber(value: count), @@ -418,8 +481,9 @@ actor StatsService: StatsServiceProtocol { avatarURL: remoteLike.avatarURL.flatMap(URL.init) ) } + let seeds = users.map(PostLikeSeed.init(remoteUser:)) let postLikes = PostLikesData(users: likeUsers, totalCount: found.intValue) - continuation.resume(returning: postLikes) + continuation.resume(returning: (postLikes, seeds)) }, failure: { error in continuation.resume(throwing: error ?? StatsServiceError.unknown) @@ -427,9 +491,46 @@ actor StatsService: StatsServiceProtocol { ) } + await storeSeeds(seeds, totalCount: result.totalCount, postID: postID) + return result } + /// Seeds the shared likes cache with the just-fetched likers. + /// + /// A positive `totalCount` seeds fire-and-forget: seeding real likers must + /// never delay or fail the UI, and a slightly late partial page is benign. + /// A confirmed zero result is awaited, because it purges the post's cache + /// and this method returns before Post Stats exposes the "0 likes" total: a + /// fast tap-through to the Likes list would otherwise snapshot stale seeded + /// rows that, offline, it never refetches or observes being deleted. + func storeSeeds(_ seeds: [PostLikeSeed], totalCount: Int, postID: Int) async { + let task = storeSeedsIfNeeded(seeds, totalCount: totalCount, postID: postID) + if totalCount == 0 { + await task?.value + } + } + + /// Hands freshly fetched likers to the app's shared likes cache. + /// + /// Write-only by design: Post Stats refetches on every screen entry (this + /// service instance is created per screen), and the shared cache carries + /// no total like count, so reading it back here would render a wrong + /// total in the likes strip. The write exists solely to seed the Likes + /// list screen. `totalCount` is forwarded so the store can clear the post's + /// cache on a confirmed zero-like result instead of leaving stale rows. + /// Returns the seeding task so callers can await it (see `storeSeeds`). + @discardableResult + func storeSeedsIfNeeded(_ seeds: [PostLikeSeed], totalCount: Int, postID: Int) -> Task? { + guard let postLikesStore else { + return nil + } + + return Task { + await postLikesStore.storeLikes(seeds, totalCount: totalCount, forPost: postID) + } + } + func getEmailOpens(for postID: Int) async throws -> StatsEmailOpensData { try await service.getEmailOpens(for: postID) } @@ -518,7 +619,10 @@ actor StatsService: StatsServiceProtocol { let now = Date.now - func makeDataPoint(from data: WordPressKit.StatsSiteMetricsResponse.PeriodData, metric: WordPressKit.StatsSiteMetricsResponse.Metric) -> DataPoint? { + func makeDataPoint( + from data: WordPressKit.StatsSiteMetricsResponse.PeriodData, + metric: WordPressKit.StatsSiteMetricsResponse.Metric + ) -> DataPoint? { guard let value = data[metric] else { return nil } diff --git a/Modules/Sources/JetpackStats/StatsContext.swift b/Modules/Sources/JetpackStats/StatsContext.swift index a169b703a4f2..17fbb17ea603 100644 --- a/Modules/Sources/JetpackStats/StatsContext.swift +++ b/Modules/Sources/JetpackStats/StatsContext.swift @@ -16,8 +16,17 @@ public struct StatsContext: Sendable { /// URL to upgrade the site's plan public var upgradeURL: URL? - public init(timeZone: TimeZone, siteID: Int, api: WordPressComRestApi) { - self.init(timeZone: timeZone, siteID: siteID, service: StatsService(siteID: siteID, api: api, timeZone: timeZone)) + public init( + timeZone: TimeZone, + siteID: Int, + api: WordPressComRestApi, + postLikesStore: (any PostLikesStore)? = nil + ) { + self.init( + timeZone: timeZone, + siteID: siteID, + service: StatsService(siteID: siteID, api: api, timeZone: timeZone, postLikesStore: postLikesStore) + ) } init(timeZone: TimeZone, siteID: Int, service: (any StatsServiceProtocol)) { @@ -37,10 +46,10 @@ public struct StatsContext: Sendable { public static let demo: StatsContext = { var context = StatsContext(timeZone: .current, siteID: 1, service: MockStatsService()) -#if DEBUG + #if DEBUG context.tracker = MockStatsTracker.shared context.upgradeURL = URL(string: "https://wordpress.com/pricing/") -#endif + #endif return context }() diff --git a/Modules/Tests/JetpackStatsTests/PostLikeSeedTests.swift b/Modules/Tests/JetpackStatsTests/PostLikeSeedTests.swift new file mode 100644 index 000000000000..55b3321f4cd6 --- /dev/null +++ b/Modules/Tests/JetpackStatsTests/PostLikeSeedTests.swift @@ -0,0 +1,41 @@ +import Testing +import Foundation +import WordPressKit +@testable import JetpackStats + +@Suite +struct PostLikeSeedTests { + @Test func mapsAllFieldsFromRemoteLikeUser() { + let dictionary: [String: Any] = [ + "ID": 101, + "login": "testlogin", + "name": "Test Name", + "site_ID": 20, + "avatar_URL": "https://example.com/avatar.jpg", + "date_liked": "2026-01-24T04:02:42+0000" + ] + let remoteUser = RemoteLikeUser(dictionary: dictionary, postID: 55, siteID: 20) + + let seed = PostLikeSeed(remoteUser: remoteUser) + + #expect(seed.userID == 101) + #expect(seed.displayName == "Test Name") + #expect(seed.username == "testlogin") + #expect(seed.avatarURL == "https://example.com/avatar.jpg") + #expect(seed.dateLikedString == "2026-01-24T04:02:42+0000") + } + + @Test func fallsBackToUsernameWhenDisplayNameMissing() { + let dictionary: [String: Any] = [ + "ID": 101, + "login": "testlogin" + ] + let remoteUser = RemoteLikeUser(dictionary: dictionary, postID: 55, siteID: 20) + + let seed = PostLikeSeed(remoteUser: remoteUser) + + #expect(seed.displayName == "testlogin") + #expect(seed.avatarURL == nil) + #expect(seed.dateLikedString == nil) + } +} diff --git a/Modules/Tests/JetpackStatsTests/StatsServicePostLikesStoreTests.swift b/Modules/Tests/JetpackStatsTests/StatsServicePostLikesStoreTests.swift new file mode 100644 index 000000000000..c9fe0403cedf --- /dev/null +++ b/Modules/Tests/JetpackStatsTests/StatsServicePostLikesStoreTests.swift @@ -0,0 +1,137 @@ +import Testing +import Foundation +import WordPressKit +@testable import JetpackStats + +private actor MockPostLikesStore: PostLikesStore { + private(set) var storedCalls: [(postID: Int, totalCount: Int, likes: [PostLikeSeed])] = [] + + func storeLikes(_ likes: [PostLikeSeed], totalCount: Int, forPost postID: Int) async { + storedCalls.append((postID, totalCount, likes)) + } +} + +private actor DoneFlag { + private(set) var isDone = false + func markDone() { isDone = true } +} + +/// A store whose `storeLikes` blocks on an explicit gate, so tests can observe +/// whether the caller awaited it. +private actor GatedMockPostLikesStore: PostLikesStore { + private(set) var completed = false + private var startedFlag = false + private var gate: CheckedContinuation? + private var startedWaiter: CheckedContinuation? + + func storeLikes(_ likes: [PostLikeSeed], totalCount: Int, forPost postID: Int) async { + startedFlag = true + startedWaiter?.resume() + startedWaiter = nil + await withCheckedContinuation { gate = $0 } + completed = true + } + + func waitUntilStarted() async { + if startedFlag { return } + await withCheckedContinuation { startedWaiter = $0 } + } + + func open() { + gate?.resume() + gate = nil + } +} + +@Suite +struct StatsServicePostLikesStoreTests { + private let seeds = [ + PostLikeSeed( + userID: 1, + displayName: "Test Name", + username: "testlogin", + avatarURL: nil, + dateLikedString: "2026-01-24T04:02:42+0000" + ) + ] + + private func makeService(store: (any PostLikesStore)?) -> StatsService { + StatsService( + siteID: 20, + api: WordPressComRestApi(oAuthToken: "fake-token", userAgent: "test"), + timeZone: .current, + postLikesStore: store + ) + } + + @Test func forwardsSeedsAndTotalCountToInjectedStore() async { + let store = MockPostLikesStore() + let service = makeService(store: store) + + let task = await service.storeSeedsIfNeeded(seeds, totalCount: 42, postID: 55) + await task?.value + + let calls = await store.storedCalls + #expect(calls.count == 1) + #expect(calls.first?.postID == 55) + #expect(calls.first?.totalCount == 42) + #expect(calls.first?.likes == seeds) + } + + @Test func forwardsConfirmedZeroTotalToInjectedStore() async { + let store = MockPostLikesStore() + let service = makeService(store: store) + + let task = await service.storeSeedsIfNeeded([], totalCount: 0, postID: 55) + await task?.value + + let calls = await store.storedCalls + #expect(calls.count == 1) + #expect(calls.first?.totalCount == 0) + #expect(calls.first?.likes.isEmpty == true) + } + + @Test func skipsSilentlyWithoutStore() async { + let service = makeService(store: nil) + + let task = await service.storeSeedsIfNeeded(seeds, totalCount: 1, postID: 55) + + #expect(task == nil) + } + + @Test func awaitsPurgeForConfirmedZero() async { + let store = GatedMockPostLikesStore() + let service = makeService(store: store) + let flag = DoneFlag() + + async let call: Void = { + await service.storeSeeds([], totalCount: 0, postID: 55) + await flag.markDone() + }() + + // storeLikes has entered but is blocked on the gate. + await store.waitUntilStarted() + // Because `storeSeeds` awaits a confirmed-zero purge, the call has not + // returned while the store is still blocked. + #expect(await flag.isDone == false) + + await store.open() + await call + + #expect(await flag.isDone == true) + #expect(await store.completed == true) + } + + @Test func doesNotAwaitSeedingForPositiveTotal() async { + let store = GatedMockPostLikesStore() + let service = makeService(store: store) + + // A positive total seeds fire-and-forget: this returns without waiting + // for the gated store. If it awaited, the test would deadlock. + await service.storeSeeds(seeds, totalCount: 5, postID: 55) + + // Release the background seeding task so it does not leak. + await store.waitUntilStarted() + await store.open() + } +} diff --git a/Tests/KeystoneTests/Tests/Services/LikeUserSeedUpsertTests.swift b/Tests/KeystoneTests/Tests/Services/LikeUserSeedUpsertTests.swift new file mode 100644 index 000000000000..ccd3d8305273 --- /dev/null +++ b/Tests/KeystoneTests/Tests/Services/LikeUserSeedUpsertTests.swift @@ -0,0 +1,128 @@ +@testable import WordPress +import WordPressData +import WordPressKit +import XCTest + +class LikeUserSeedUpsertTests: CoreDataTestCase { + + private let siteID: Int64 = 20 + private let postID: Int64 = 55 + + private func makeSeed(userID: Int64, displayName: String = "Seed Name") -> LikeUserSeed { + LikeUserSeed( + userID: userID, + displayName: displayName, + username: "seedlogin", + avatarUrl: "https://example.com/avatar.jpg", + dateLikedString: "2026-01-24T04:02:42+0000" + ) + } + + // Creates a full-fidelity cached row (bio + preferred blog) via the existing write path. + private func insertExistingLikeUser(userID: Int64) -> LikeUser { + let dictionary: [String: Any] = [ + "ID": Int(userID), + "login": "existinglogin", + "name": "Existing Name", + "site_ID": Int(siteID), + "avatar_URL": "https://example.com/old-avatar.jpg", + "bio": "existing bio", + "date_liked": "2025-11-24T04:02:42+0000", + "preferred_blog": [ + "id": 1, + "url": "https://example.com", + "name": "Existing Blog", + "icon": ["img": "someimage.jpg"] + ] as [String: Any] + ] + let remoteUser = RemoteLikeUser( + dictionary: dictionary, + postID: NSNumber(value: postID), + siteID: NSNumber(value: siteID) + ) + return LikeUserHelper.createOrUpdateFrom(remoteUser: remoteUser, context: mainContext) + } + + private func fetchAllLikeUsers() -> [LikeUser] { + let request = LikeUser.fetchRequest() as NSFetchRequest + return (try? mainContext.fetch(request)) ?? [] + } + + func testUpsertInsertsNewRows() throws { + LikeUserHelper.upsert( + seeds: [makeSeed(userID: 1), makeSeed(userID: 2)], + siteID: siteID, + postID: postID, + in: mainContext + ) + try mainContext.save() + + let users = fetchAllLikeUsers() + XCTAssertEqual(users.count, 2) + let user = try XCTUnwrap(users.first { $0.userID == 1 }) + XCTAssertEqual(user.displayName, "Seed Name") + XCTAssertEqual(user.username, "seedlogin") + XCTAssertEqual(user.avatarUrl, "https://example.com/avatar.jpg") + XCTAssertEqual(user.dateLikedString, "2026-01-24T04:02:42+0000") + XCTAssertNotNil(user.dateLiked) + XCTAssertNotNil(user.dateFetched) + XCTAssertEqual(user.likedSiteID, siteID) + XCTAssertEqual(user.likedPostID, postID) + XCTAssertEqual(user.likedCommentID, 0) + } + + func testUpsertUpdatesExistingRowPreservingRicherFields() throws { + let existing = insertExistingLikeUser(userID: 1) + try mainContext.save() + + LikeUserHelper.upsert( + seeds: [makeSeed(userID: 1, displayName: "Updated Name")], + siteID: siteID, + postID: postID, + in: mainContext + ) + try mainContext.save() + + XCTAssertEqual(fetchAllLikeUsers().count, 1) + XCTAssertEqual(existing.displayName, "Updated Name") + XCTAssertEqual(existing.username, "seedlogin") + // Fields the seed does not carry keep their richer cached values. + XCTAssertEqual(existing.bio, "existing bio") + XCTAssertNotNil(existing.preferredBlog) + } + + func testUpsertDoesNotTouchOtherRows() throws { + let otherPostUser = insertExistingLikeUser(userID: 9) + otherPostUser.likedPostID = postID + 1 + try mainContext.save() + + LikeUserHelper.upsert( + seeds: [makeSeed(userID: 1)], + siteID: siteID, + postID: postID, + in: mainContext + ) + try mainContext.save() + + // The row for the other post is neither deleted nor modified. + XCTAssertEqual(fetchAllLikeUsers().count, 2) + XCTAssertEqual(otherPostUser.displayName, "Existing Name") + } + + func testDeleteLikesRemovesOnlyThePostsRows() throws { + _ = insertExistingLikeUser(userID: 1) + let otherPost = insertExistingLikeUser(userID: 2) + otherPost.likedPostID = postID + 1 + try mainContext.save() + XCTAssertEqual(fetchAllLikeUsers().count, 2) + + LikeUserHelper.deleteLikes(forPost: postID, siteID: siteID, in: mainContext) + try mainContext.save() + + // Only the target post's rows are removed; the other post survives. + let remaining = fetchAllLikeUsers() + XCTAssertEqual(remaining.count, 1) + XCTAssertEqual(remaining.first?.userID, 2) + XCTAssertEqual(remaining.first?.likedPostID, postID + 1) + } +} diff --git a/Tests/KeystoneTests/Tests/Services/PostServiceLikesTests.swift b/Tests/KeystoneTests/Tests/Services/PostServiceLikesTests.swift new file mode 100644 index 000000000000..a2738766c3a7 --- /dev/null +++ b/Tests/KeystoneTests/Tests/Services/PostServiceLikesTests.swift @@ -0,0 +1,112 @@ +@testable import WordPress +@testable import WordPressData +@testable import WordPressKit +import XCTest + +class PostServiceLikesTests: CoreDataTestCase { + + private let siteID: NSNumber = 20 + private let postID: NSNumber = 55 + + override func setUp() { + super.setUp() + // createNewUsers persists via ContextManager.shared; route it to the test stack. + contextManager.useAsSharedInstance(untilTestFinished: self) + } + + private func makeService(returning remote: PostServiceRemoteREST) -> PostService { + let factory = PostServiceRemoteFactoryMock() + factory.remoteToReturn = remote + return PostService(managedObjectContext: mainContext, postServiceRemoteFactory: factory) + } + + private func insertCachedLikeUser() { + let dictionary: [String: Any] = [ + "ID": 1, + "login": "testlogin", + "name": "Test Name", + "site_ID": siteID.intValue, + "date_liked": "2025-11-24T04:02:42+0000" + ] + let remoteUser = RemoteLikeUser(dictionary: dictionary, postID: postID, siteID: siteID) + _ = LikeUserHelper.createOrUpdateFrom(remoteUser: remoteUser, context: mainContext) + // Save synchronously so the row is committed to the store before the + // purge's background context fetches it. `contextManager.save(_:)` is + // asynchronous, which would let the empty-page purge run against a store + // that does not yet contain the seeded row. + contextManager.saveContextAndWait(mainContext) + } + + private func fetchCachedLikeUsers() -> [LikeUser] { + let request = LikeUser.fetchRequest() as NSFetchRequest + return (try? mainContext.fetch(request)) ?? [] + } + + func testEmptyFirstPagePurgesCachedLikes() { + insertCachedLikeUser() + XCTAssertEqual(fetchCachedLikeUsers().count, 1) + + let service = makeService(returning: EmptyLikesRemoteMock()) + let completion = expectation(description: "getLikesFor completes") + service.getLikesFor( + postID: postID, + siteID: siteID, + success: { users, totalLikes, _ in + XCTAssertEqual(users.count, 0) + XCTAssertEqual(totalLikes, 0) + completion.fulfill() + }, + failure: { _ in + XCTFail("The request should succeed") + } + ) + waitForExpectations(timeout: 5) + + // A successful empty first page means the post has no likes; the cache must be cleared. + XCTAssertEqual(fetchCachedLikeUsers().count, 0) + } + + func testEmptyLaterPageDoesNotPurge() { + insertCachedLikeUser() + + let service = makeService(returning: EmptyLikesRemoteMock()) + let completion = expectation(description: "getLikesFor completes") + service.getLikesFor( + postID: postID, + siteID: siteID, + purgeExisting: false, + success: { _, _, _ in + completion.fulfill() + }, + failure: { _ in + XCTFail("The request should succeed") + } + ) + waitForExpectations(timeout: 5) + + // An empty non-first page just means pagination ended; the cache stays. + XCTAssertEqual(fetchCachedLikeUsers().count, 1) + } +} + +private class PostServiceRemoteFactoryMock: PostServiceRemoteFactory { + var remoteToReturn: PostServiceRemoteREST? + + override func restRemoteFor(siteID: NSNumber, context: NSManagedObjectContext) -> PostServiceRemoteREST? { + remoteToReturn + } +} + +// Simulates a successful likes response with zero users ("found": 0). +private class EmptyLikesRemoteMock: PostServiceRemoteREST { + override func getLikesForPostID( + _ postID: NSNumber, + count: NSNumber, + before: String?, + excludeUserIDs: [NSNumber]?, + success: (([RemoteLikeUser], NSNumber) -> Void)?, + failure: ((Error?) -> Void)? + ) { + success?([], 0) + } +} diff --git a/WordPress/Classes/Services/LikeUserHelpers.swift b/WordPress/Classes/Services/LikeUserHelpers.swift index e5cc051f6a9b..7b599697c477 100644 --- a/WordPress/Classes/Services/LikeUserHelpers.swift +++ b/WordPress/Classes/Services/LikeUserHelpers.swift @@ -36,8 +36,13 @@ import WordPressKit let commentID = remoteUser.likedCommentID ?? 0 let request = LikeUser.fetchRequest() as NSFetchRequest - request.predicate = NSPredicate(format: "userID = %@ AND likedSiteID = %@ AND likedPostID = %@ AND likedCommentID = %@", - userID, siteID, postID, commentID) + request.predicate = NSPredicate( + format: "userID = %@ AND likedSiteID = %@ AND likedPostID = %@ AND likedCommentID = %@", + userID, + siteID, + postID, + commentID + ) return try? context.fetch(request).first } @@ -48,13 +53,23 @@ import WordPressKit @param siteID The ID of the site that contains the post. @param after Filter results to likes after this Date. Optional. */ - class func likeUsersFor(commentID: NSNumber, siteID: NSNumber, after: Date? = nil, in context: NSManagedObjectContext) -> [LikeUser] { + class func likeUsersFor( + commentID: NSNumber, + siteID: NSNumber, + after: Date? = nil, + in context: NSManagedObjectContext + ) -> [LikeUser] { let request = LikeUser.fetchRequest() as NSFetchRequest request.predicate = { if let after { // The date comparison is 'less than' because Likes are in descending order. - return NSPredicate(format: "likedSiteID = %@ AND likedCommentID = %@ AND dateLiked < %@", siteID, commentID, after as CVarArg) + return NSPredicate( + format: "likedSiteID = %@ AND likedCommentID = %@ AND dateLiked < %@", + siteID, + commentID, + after as CVarArg + ) } return NSPredicate(format: "likedSiteID = %@ AND likedCommentID = %@", siteID, commentID) @@ -69,7 +84,11 @@ import WordPressKit return [LikeUser]() } - private class func updatePreferredBlog(for user: LikeUser, with remoteUser: RemoteLikeUser, context: NSManagedObjectContext) { + private class func updatePreferredBlog( + for user: LikeUser, + with remoteUser: RemoteLikeUser, + context: NSManagedObjectContext + ) { guard let remotePreferredBlog = remoteUser.preferredBlog else { if let existingPreferredBlog = user.preferredBlog { context.deleteObject(existingPreferredBlog) @@ -106,3 +125,76 @@ import WordPressKit } } } + +/// A liker fetched by the new Stats screens, carrying only the fields that +/// fetch provides. See `LikeUserHelper.upsert(seeds:siteID:postID:in:)`. +struct LikeUserSeed: Sendable { + let userID: Int64 + let displayName: String + let username: String + let avatarUrl: String + let dateLikedString: String +} + +extension LikeUserHelper { + /// Merges likers fetched by Post Stats into the shared likes cache so the + /// Likes list screen can seed itself from cache instead of starting empty. + /// + /// Unlike the fetch path built on `createOrUpdateFrom(remoteUser:context:)`, + /// this never deletes other rows for the post: the caller holds only the + /// first page of likers, and purging here could wipe a fuller previously + /// cached list. Fields the seed does not carry (bio, preferred blog, + /// primary blog) are left untouched on existing rows so a richer earlier + /// fetch is not degraded. + class func upsert(seeds: [LikeUserSeed], siteID: Int64, postID: Int64, in context: NSManagedObjectContext) { + for seed in seeds { + let request = LikeUser.fetchRequest() as NSFetchRequest + request.predicate = NSPredicate( + format: "userID = %@ AND likedSiteID = %@ AND likedPostID = %@ AND likedCommentID = 0", + NSNumber(value: seed.userID), + NSNumber(value: siteID), + NSNumber(value: postID) + ) + let existing = try? context.fetch(request).first + + let liker = existing ?? LikeUser(context: context) + if existing == nil { + // New rows need the seed-less attributes initialized. + liker.bio = "" + liker.primaryBlogID = 0 + } + liker.userID = seed.userID + liker.displayName = seed.displayName + liker.username = seed.username + liker.avatarUrl = seed.avatarUrl + liker.dateLikedString = seed.dateLikedString + liker.dateLiked = Date.dateFromServerDate(seed.dateLikedString) ?? .now + liker.likedSiteID = siteID + liker.likedPostID = postID + liker.likedCommentID = 0 + liker.dateFetched = Date() + } + } + + /// Removes every cached post liker for the given post. + /// + /// Used to seed a confirmed zero-like result: when Post Stats reports a post + /// has no likes, clearing the cache keeps the Likes list from showing stale + /// likers under a "0 likes" title. Comment likes (`likedCommentID != 0`) are + /// untouched. + class func deleteLikes(forPost postID: Int64, siteID: Int64, in context: NSManagedObjectContext) { + let request = LikeUser.fetchRequest() as NSFetchRequest + request.predicate = NSPredicate( + format: "likedSiteID = %@ AND likedPostID = %@ AND likedCommentID = 0", + NSNumber(value: siteID), + NSNumber(value: postID) + ) + + do { + let users = try context.fetch(request) + users.forEach { context.delete($0) } + } catch { + DDLogError("Error fetching post Like Users to delete: \(error)") + } + } +} diff --git a/WordPress/Classes/Services/PostService+Likes.swift b/WordPress/Classes/Services/PostService+Likes.swift index c87635a86ffd..7222ba01f238 100644 --- a/WordPress/Classes/Services/PostService+Likes.swift +++ b/WordPress/Classes/Services/PostService+Likes.swift @@ -5,7 +5,10 @@ final class PostService { let managedObjectContext: NSManagedObjectContext let postServiceRemoteFactory: PostServiceRemoteFactory - init(managedObjectContext: NSManagedObjectContext, postServiceRemoteFactory: PostServiceRemoteFactory = PostServiceRemoteFactory()) { + init( + managedObjectContext: NSManagedObjectContext, + postServiceRemoteFactory: PostServiceRemoteFactory = PostServiceRemoteFactory() + ) { self.managedObjectContext = managedObjectContext self.postServiceRemoteFactory = postServiceRemoteFactory } @@ -29,14 +32,16 @@ extension PostService { - Number of likes per fetch @param failure A failure block */ - func getLikesFor(postID: NSNumber, - siteID: NSNumber, - count: Int = 90, - before: String? = nil, - excludingIDs: [NSNumber]? = nil, - purgeExisting: Bool = true, - success: @escaping (([LikeUser], Int, Int) -> Void), - failure: @escaping ((Error?) -> Void)) { + func getLikesFor( + postID: NSNumber, + siteID: NSNumber, + count: Int = 90, + before: String? = nil, + excludingIDs: [NSNumber]? = nil, + purgeExisting: Bool = true, + success: @escaping (([LikeUser], Int, Int) -> Void), + failure: @escaping ((Error?) -> Void) + ) { guard let remote = postServiceRemoteFactory.restRemoteFor(siteID: siteID, context: managedObjectContext) else { DDLogError("Unable to create a REST remote for posts.") @@ -44,22 +49,27 @@ extension PostService { return } - remote.getLikesForPostID(postID, - count: NSNumber(value: count), - before: before, - excludeUserIDs: excludingIDs, - success: { remoteLikeUsers, totalLikes in - self.createNewUsers(from: remoteLikeUsers, - postID: postID, - siteID: siteID, - purgeExisting: purgeExisting) { - let users = self.likeUsersFor(postID: postID, siteID: siteID) - success(users, totalLikes.intValue, count) - } - }, failure: { error in - DDLogError("\(String(describing: error))") - failure(error) - }) + remote.getLikesForPostID( + postID, + count: NSNumber(value: count), + before: before, + excludeUserIDs: excludingIDs, + success: { remoteLikeUsers, totalLikes in + self.createNewUsers( + from: remoteLikeUsers, + postID: postID, + siteID: siteID, + purgeExisting: purgeExisting + ) { + let users = self.likeUsersFor(postID: postID, siteID: siteID) + success(users, totalLikes.intValue, count) + } + }, + failure: { error in + DDLogError("\(String(describing: error))") + failure(error) + } + ) } /** @@ -75,7 +85,12 @@ extension PostService { request.predicate = { if let after { // The date comparison is 'less than' because Likes are in descending order. - return NSPredicate(format: "likedSiteID = %@ AND likedPostID = %@ AND dateLiked < %@", siteID, postID, after as CVarArg) + return NSPredicate( + format: "likedSiteID = %@ AND likedPostID = %@ AND dateLiked < %@", + siteID, + postID, + after as CVarArg + ) } return NSPredicate(format: "likedSiteID = %@ AND likedPostID = %@", siteID, postID) @@ -93,36 +108,79 @@ extension PostService { private extension PostService { - func createNewUsers(from remoteLikeUsers: [RemoteLikeUser]?, - postID: NSNumber, - siteID: NSNumber, - purgeExisting: Bool, - onComplete: @escaping (() -> Void)) { + func createNewUsers( + from remoteLikeUsers: [RemoteLikeUser]?, + postID: NSNumber, + siteID: NSNumber, + purgeExisting: Bool, + onComplete: @escaping (() -> Void) + ) { guard let remoteLikeUsers, - !remoteLikeUsers.isEmpty else { - DispatchQueue.main.async { - onComplete() + !remoteLikeUsers.isEmpty + else { + guard purgeExisting else { + DispatchQueue.main.async { + onComplete() + } + return } + + // A successful, empty first page means the post currently has no + // likes. Clear cached rows (seeded by Post Stats or left over from + // an earlier fetch) so the list cannot show stale likers under a + // zero total. Empty later pages skip this via the guard above. + ContextManager.shared.performAndSave( + { derivedContext in + self.deleteExistingUsersFor(postID: postID, siteID: siteID, from: derivedContext, likesToKeep: []) + }, + completion: onComplete, + on: .main + ) return } - ContextManager.shared.performAndSave({ derivedContext in - let likers = remoteLikeUsers.map { remoteUser in - LikeUserHelper.createOrUpdateFrom(remoteUser: remoteUser, context: derivedContext) - } - - if purgeExisting { - self.deleteExistingUsersFor(postID: postID, siteID: siteID, from: derivedContext, likesToKeep: likers) - } - - LikeUserHelper.purgeStaleLikes(fromContext: derivedContext) - }, completion: onComplete, on: .main) + ContextManager.shared.performAndSave( + { derivedContext in + let likers = remoteLikeUsers.map { remoteUser in + LikeUserHelper.createOrUpdateFrom(remoteUser: remoteUser, context: derivedContext) + } + + if purgeExisting { + self.deleteExistingUsersFor( + postID: postID, + siteID: siteID, + from: derivedContext, + likesToKeep: likers + ) + } + + LikeUserHelper.purgeStaleLikes(fromContext: derivedContext) + }, + completion: onComplete, + on: .main + ) } - func deleteExistingUsersFor(postID: NSNumber, siteID: NSNumber, from context: NSManagedObjectContext, likesToKeep: [LikeUser]) { + func deleteExistingUsersFor( + postID: NSNumber, + siteID: NSNumber, + from context: NSManagedObjectContext, + likesToKeep: [LikeUser] + ) { let request = LikeUser.fetchRequest() as NSFetchRequest - request.predicate = NSPredicate(format: "likedSiteID = %@ AND likedPostID = %@ AND NOT (self IN %@)", siteID, postID, likesToKeep) + // Core Data's SQLite store does not reliably evaluate `NOT (self IN %@)` + // against an empty array (it can match no rows), so when nothing is kept + // delete every cached row for the post with a plain predicate instead. + request.predicate = + likesToKeep.isEmpty + ? NSPredicate(format: "likedSiteID = %@ AND likedPostID = %@", siteID, postID) + : NSPredicate( + format: "likedSiteID = %@ AND likedPostID = %@ AND NOT (self IN %@)", + siteID, + postID, + likesToKeep + ) do { let users = try context.fetch(request) diff --git a/WordPress/Classes/ViewRelated/Stats/StatsHostingViewController.swift b/WordPress/Classes/ViewRelated/Stats/StatsHostingViewController.swift index 74aebd4f00ad..057ffe7faef8 100644 --- a/WordPress/Classes/ViewRelated/Stats/StatsHostingViewController.swift +++ b/WordPress/Classes/ViewRelated/Stats/StatsHostingViewController.swift @@ -9,7 +9,11 @@ import BuildSettingsKit /// A UIViewController wrapper for the new SwiftUI StatsMainView class StatsHostingViewController: UIViewController { - static func makeNewTrafficViewController(blog: Blog? = nil, parentViewController: UIViewController, isDemo: Bool = false) -> UIViewController? { + static func makeNewTrafficViewController( + blog: Blog? = nil, + parentViewController: UIViewController, + isDemo: Bool = false + ) -> UIViewController? { let context: StatsContext if isDemo { context = StatsContext.demo @@ -54,14 +58,16 @@ class StatsHostingViewController: UIViewController { extension StatsContext { init?(blog: Blog) { guard let siteID = blog.dotComID?.intValue, - let api = blog.account?.wordPressComRestApi else { + let api = blog.account?.wordPressComRestApi + else { wpAssertionFailure("required context missing") return nil } self.init( timeZone: blog.timeZone ?? .current, siteID: siteID, - api: api + api: api, + postLikesStore: StatsPostLikesStore(siteID: Int64(siteID)) ) // Configure avatar preprocessing using Gravatar diff --git a/WordPress/Classes/ViewRelated/Stats/StatsPostLikesStore.swift b/WordPress/Classes/ViewRelated/Stats/StatsPostLikesStore.swift new file mode 100644 index 000000000000..2299ec248ca8 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Stats/StatsPostLikesStore.swift @@ -0,0 +1,43 @@ +import Foundation +import JetpackStats +import WordPressData + +/// Bridges likers fetched by the JetpackStats package into the app's shared +/// `LikeUser` cache, so the Likes list screen (`LikesListController`) can +/// seed itself from cache instead of starting empty. +struct StatsPostLikesStore: PostLikesStore { + let siteID: Int64 + + func storeLikes(_ likes: [PostLikeSeed], totalCount: Int, forPost postID: Int) async { + let seeds = likes.map { like in + LikeUserSeed( + userID: Int64(like.userID), + displayName: like.displayName, + username: like.username ?? "", + avatarUrl: like.avatarURL ?? "", + dateLikedString: like.dateLikedString ?? "" + ) + } + let siteID = self.siteID + let postID = Int64(postID) + await withCheckedContinuation { continuation in + ContextManager.shared.performAndSave( + { context in + // A confirmed zero-like result clears the post's cache so the + // Likes list cannot seed stale likers under a "0 likes" title. + // A positive total upserts the partial first page without + // purging a possibly fuller previously cached list. + if totalCount == 0 { + LikeUserHelper.deleteLikes(forPost: postID, siteID: siteID, in: context) + } else { + LikeUserHelper.upsert(seeds: seeds, siteID: siteID, postID: postID, in: context) + } + }, + completion: { + continuation.resume() + }, + on: .main + ) + } + } +}