From 70079c84056a6ceee8e12cac5e771b3e09f993cc Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Mon, 22 Jun 2026 02:26:38 -0700 Subject: [PATCH] fix: guard flexible update notice against double-posting AppUpdatePresenter.showNotice posted the flexible in-app-update notice with no already-shown guard, so two checkForAppUpdates() runs racing on cold launch could queue two identical notices back-to-back. Tag the notice and bail if a notice with that tag is already the current notice in NoticeStore, mirroring the existing showBlockingUpdate guard. Fixes #25666 --- RELEASE-NOTES.txt | 1 + .../Misc/AppUpdateCoordinatorTests.swift | 62 +++++++++++++++++++ .../AppUpdate/AppUpdatePresenter.swift | 24 ++++++- 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 1d4751b48e0f..2a2bee8b23b2 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,5 +1,6 @@ 27.1 ----- +* [*] [internal] In-app updates: guard the flexible update notice against double-posting when two update checks race [#25666] 27.0 diff --git a/Tests/KeystoneTests/Tests/Features/Misc/AppUpdateCoordinatorTests.swift b/Tests/KeystoneTests/Tests/Features/Misc/AppUpdateCoordinatorTests.swift index 5a37e3d9b3ba..251857fa405d 100644 --- a/Tests/KeystoneTests/Tests/Features/Misc/AppUpdateCoordinatorTests.swift +++ b/Tests/KeystoneTests/Tests/Features/Misc/AppUpdateCoordinatorTests.swift @@ -1,3 +1,4 @@ +import WordPressFlux import XCTest @testable import WordPress @@ -190,6 +191,67 @@ final class AppUpdateCoordinatorTests: XCTestCase { XCTAssertFalse(presenter.didShowNotice) XCTAssertTrue(presenter.didShowBlockingUpdate) } + + // MARK: - AppUpdatePresenter flexible notice guard + + func testShowNoticePostsExactlyOneFlexibleNotice() throws { + // Given + let dispatcher = ActionDispatcher() + let noticeStore = NoticeStore(dispatcher: dispatcher) + let presenter = AppUpdatePresenter(noticeStore: noticeStore, dispatcher: dispatcher) + let appStoreInfo = try makeAppStoreInfo() + + // When + presenter.showNotice(using: appStoreInfo) + + // Then + XCTAssertEqual(noticeStore.currentNotice?.tag, AppUpdatePresenter.flexibleUpdateNoticeTag) + } + + func testShowNoticeSuppressesSecondFlexibleNoticeWhileOneIsShowing() throws { + // Given + let dispatcher = ActionDispatcher() + let noticeStore = NoticeStore(dispatcher: dispatcher) + let presenter = AppUpdatePresenter(noticeStore: noticeStore, dispatcher: dispatcher) + let appStoreInfo = try makeAppStoreInfo() + presenter.showNotice(using: appStoreInfo) + let firstNotice = try XCTUnwrap(noticeStore.currentNotice) + + // When a second presentation races in while the first is still showing + presenter.showNotice(using: appStoreInfo) + + // Then the current notice is still the first one and nothing was queued + XCTAssertEqual(noticeStore.currentNotice, firstNotice) + // Dismissing the current notice leaves no queued duplicate behind + ActionDispatcher.dispatch(NoticeAction.dismiss, dispatcher: dispatcher) + XCTAssertNil(noticeStore.currentNotice) + } + + func testShowNoticeCanPostAgainAfterPreviousNoticeIsCleared() throws { + // Given + let dispatcher = ActionDispatcher() + let noticeStore = NoticeStore(dispatcher: dispatcher) + let presenter = AppUpdatePresenter(noticeStore: noticeStore, dispatcher: dispatcher) + let appStoreInfo = try makeAppStoreInfo() + presenter.showNotice(using: appStoreInfo) + XCTAssertNotNil(noticeStore.currentNotice) + + // When the first notice is cleared and a later legitimate cycle posts again + ActionDispatcher.dispatch(NoticeAction.dismiss, dispatcher: dispatcher) + XCTAssertNil(noticeStore.currentNotice) + presenter.showNotice(using: appStoreInfo) + + // Then the guard does not permanently latch + XCTAssertEqual(noticeStore.currentNotice?.tag, AppUpdatePresenter.flexibleUpdateNoticeTag) + } + + private func makeAppStoreInfo() throws -> AppStoreLookupResponse.AppStoreInfo { + let data = try Bundle.test.json(named: "app-store-lookup-response") + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .iso8601 + let response = try decoder.decode(AppStoreLookupResponse.self, from: data) + return try XCTUnwrap(response.results.first) + } } private final class MockAppStoreSearchService: AppStoreSearchProtocol { diff --git a/WordPress/Classes/Services/AppUpdate/AppUpdatePresenter.swift b/WordPress/Classes/Services/AppUpdate/AppUpdatePresenter.swift index 5b8515a47460..7ef1f00f7398 100644 --- a/WordPress/Classes/Services/AppUpdate/AppUpdatePresenter.swift +++ b/WordPress/Classes/Services/AppUpdate/AppUpdatePresenter.swift @@ -9,7 +9,26 @@ protocol AppUpdatePresenterProtocol { } final class AppUpdatePresenter: AppUpdatePresenterProtocol { + /// Stable tag used to identify the flexible in-app-update notice so a second + /// presentation can be suppressed while one is already showing. + static let flexibleUpdateNoticeTag: Notice.Tag = "in-app-update-flexible" + + private let noticeStore: NoticeStore + private let dispatcher: ActionDispatcher + + init( + noticeStore: NoticeStore = StoreContainer.shared.notice, + dispatcher: ActionDispatcher = .global + ) { + self.noticeStore = noticeStore + self.dispatcher = dispatcher + } + func showNotice(using appStoreInfo: AppStoreLookupResponse.AppStoreInfo) { + guard noticeStore.currentNotice?.tag != Self.flexibleUpdateNoticeTag else { + // Don't post another flexible update notice if one is already showing + return + } let viewModel = AppStoreInfoViewModel(appStoreInfo) let notice = Notice( title: viewModel.title, @@ -17,7 +36,8 @@ final class AppUpdatePresenter: AppUpdatePresenterProtocol { feedbackType: .warning, style: InAppUpdateNoticeStyle(), actionTitle: viewModel.updateButtonTitle, - cancelTitle: viewModel.cancelButtonTitle + cancelTitle: viewModel.cancelButtonTitle, + tag: Self.flexibleUpdateNoticeTag ) { accepted in if accepted { WPAnalytics.track(.inAppUpdateAccepted, properties: ["type": "flexible"]) @@ -26,7 +46,7 @@ final class AppUpdatePresenter: AppUpdatePresenterProtocol { WPAnalytics.track(.inAppUpdateDismissed) } } - ActionDispatcher.dispatch(NoticeAction.post(notice)) + ActionDispatcher.dispatch(NoticeAction.post(notice), dispatcher: dispatcher) WPAnalytics.track(.inAppUpdateShown, properties: ["type": "flexible"]) }