fix: guard flexible update notice against double-posting#25681
Open
mvanhorn wants to merge 1 commit into
Open
Conversation
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 wordpress-mobile#25666
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
AppUpdatePresenter.showNotice(using:)now guards against posting a second flexible in-app-update notice while one is already showing, mirroring the existing already-on-top guard inshowBlockingUpdate(using:).Why this matters:
showBlockingUpdatebails when aBlockingUpdateViewControlleris already on top (AppUpdatePresenter.swift:54-59), butshowNoticeposted the flexible notice with no equivalent guard. BecauseAppUpdateCoordinatorcreates a fresh presenter per call and the persisted day-granularity throttles (shouldFetchAppStoreInfo,shouldShowFlexibleUpdate) are frequency caps rather than in-flight dedup, twocheckForAppUpdates()runs racing on cold launch could each reachshowNoticeand queue two identical "update available" notices shown back-to-back (#25666).The fix tags the notice with a stable identifier and bails before posting if a notice with that tag is already the current notice in
NoticeStore. This defends the presentation invariant independent of the fetch-side trigger, so it holds for any future caller that reachesshowNoticetwice. The day-granularity throttles are left untouched.Fixes #25666
Testing instructions
Added presenter-level tests in
AppUpdateCoordinatorTests:showNoticeposts exactly one flexible notice (tagged).showNoticewhile one is already showing is suppressed, and dismissing the current notice leaves no queued duplicate.showNoticecan post again (the guard does not permanently latch).swiftlintpasses on the changed files. The full unit suite runs in CI.