diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 79cb8a1d0ea4..3184ac97672f 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,6 +1,7 @@ 23.5 ----- * [*] Fix a crash when the blog's blogging prompt settings contain invalid JSON [#21677] +* [*] [WordPress-only] Fixes an issue where users would land on the Reader after signup while it should not be accessible. [#21751] * [*] Block Editor: Split formatted text on triple Enter [https://github.com/WordPress/gutenberg/pull/53354] * [*] Block Editor: Quote block: Ensure border is visible with block-based themes in dark [https://github.com/WordPress/gutenberg/pull/54964] * [*] (Internal) Remove .nativePhotoPicker feature flag and the disabled code [#21681](https://github.com/wordpress-mobile/WordPress-iOS/pull/21681) diff --git a/WordPress/Classes/Stores/RemoteFeatureFlagStore.swift b/WordPress/Classes/Stores/RemoteFeatureFlagStore.swift index b283adf20b4e..f1484ca9ba19 100644 --- a/WordPress/Classes/Stores/RemoteFeatureFlagStore.swift +++ b/WordPress/Classes/Stores/RemoteFeatureFlagStore.swift @@ -10,6 +10,12 @@ class RemoteFeatureFlagStore { /// Thread Safety Coordinator private var queue: DispatchQueue private var persistenceStore: UserPersistentRepository + private lazy var operationQueue: OperationQueue = { + var queue = OperationQueue() + queue.name = "Feature Flags Refresh Queue" + queue.maxConcurrentOperationCount = 1 + return queue + }() init(queue: DispatchQueue = .remoteFeatureFlagStoreQueue, persistenceStore: UserPersistentRepository = UserDefaults.standard) { @@ -23,18 +29,19 @@ class RemoteFeatureFlagStore { /// - Parameter callback: An optional callback that can be used to update UI following the fetch. It is not called on the UI thread. public func update(using remote: FeatureFlagRemote = FeatureFlagRemote(wordPressComRestApi: WordPressComRestApi.defaultApi()), then callback: FetchCallback? = nil) { - DDLogInfo("🚩 Updating Remote Feature Flags with Device ID: \(deviceID)") - remote.getRemoteFeatureFlags(forDeviceId: deviceID) { [weak self] result in + let refreshOperation = FetchRemoteFeatureFlagsOperation(remote: remote, + deviceID: deviceID, + completion: { [weak self] result in switch result { - case .success(let flags): - self?.cache = flags.dictionaryValue - DDLogInfo("🚩 Successfully updated local feature flags: \(flags)") - callback?() - case .failure(let error): - DDLogError("🚩 Unable to update Feature Flag Store: \(error.localizedDescription)") - callback?() + case .success(let flags): + self?.cache = flags + callback?() + case .failure: + callback?() } - } + }) + operationQueue.cancelAllOperations() + operationQueue.addOperation(refreshOperation) } /// Checks if the local cache has a value for a given `FeatureFlag` @@ -85,3 +92,43 @@ extension RemoteFeatureFlagStore { } } } + +fileprivate final class FetchRemoteFeatureFlagsOperation: AsyncOperation { + private let remote: FeatureFlagRemote + private let deviceID: String + private let completion: OperationCompletionHandler + + typealias OperationCompletionHandler = (Result<[String: Bool], Error>) -> () + + enum OperationError: Error { + case operationCanceled + } + + init(remote: FeatureFlagRemote, deviceID: String, completion: @escaping OperationCompletionHandler) { + self.remote = remote + self.deviceID = deviceID + self.completion = completion + super.init() + } + + override func main() { + DDLogInfo("🚩 Updating Remote Feature Flags with Device ID: \(deviceID)") + remote.getRemoteFeatureFlags(forDeviceId: deviceID) { [weak self] result in + if self?.isCancelled == true { + self?.state = .isFinished + DDLogInfo("🚩 Feature flags update operation canceled") + self?.completion(.failure(OperationError.operationCanceled)) + return + } + switch result { + case .success(let flags): + DDLogInfo("🚩 Successfully updated local feature flags: \(flags.dictionaryValue)") + self?.completion(.success(flags.dictionaryValue)) + case .failure(let error): + DDLogError("🚩 Unable to update Feature Flag Store: \(error.localizedDescription)") + self?.completion(.failure(error)) + } + self?.state = .isFinished + } + } +} diff --git a/WordPress/Classes/System/RootViewCoordinator.swift b/WordPress/Classes/System/RootViewCoordinator.swift index a6e44da107bd..0851fc282aec 100644 --- a/WordPress/Classes/System/RootViewCoordinator.swift +++ b/WordPress/Classes/System/RootViewCoordinator.swift @@ -87,6 +87,18 @@ class RootViewCoordinator { self.rootViewPresenter = nil } + func showPostSignUpTabForNoSites() { + let appUIType = Self.appUIType(featureFlagStore: featureFlagStore) + switch appUIType { + case .normal: + rootViewPresenter?.showReaderTab() + case .simplified: + fallthrough + case .staticScreens: + rootViewPresenter?.showMySitesTab() + } + } + private func createPresenter(_ appType: AppUIType) -> RootViewPresenter { switch appType { case .normal: diff --git a/WordPress/Classes/ViewRelated/NUX/Post Signup Interstitial/PostSignUpInterstitialViewController.swift b/WordPress/Classes/ViewRelated/NUX/Post Signup Interstitial/PostSignUpInterstitialViewController.swift index 15e9fd05ea52..e1fe0313d98a 100644 --- a/WordPress/Classes/ViewRelated/NUX/Post Signup Interstitial/PostSignUpInterstitialViewController.swift +++ b/WordPress/Classes/ViewRelated/NUX/Post Signup Interstitial/PostSignUpInterstitialViewController.swift @@ -104,7 +104,7 @@ class PostSignUpInterstitialViewController: UIViewController { @IBAction func cancel(_ sender: Any) { dismiss?(.none) - RootViewCoordinator.sharedPresenter.showReaderTab() + RootViewCoordinator.shared.showPostSignUpTabForNoSites() tracker.track(click: .dismiss, ifTrackingNotEnabled: { WPAnalytics.track(.welcomeNoSitesInterstitialDismissed) diff --git a/WordPress/WordPressTest/JetpackFeaturesRemovalCoordinatorTests.swift b/WordPress/WordPressTest/JetpackFeaturesRemovalCoordinatorTests.swift index e69c18b9501a..559b45ec02c9 100644 --- a/WordPress/WordPressTest/JetpackFeaturesRemovalCoordinatorTests.swift +++ b/WordPress/WordPressTest/JetpackFeaturesRemovalCoordinatorTests.swift @@ -23,7 +23,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -38,7 +38,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: true, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -52,7 +52,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: true, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -66,7 +66,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: true, phaseTwo: true, phaseThree: true, phaseFour: true, phaseNewUsers: true, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -81,7 +81,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: true) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -95,7 +95,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: true, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: true) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -109,7 +109,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: true, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -123,7 +123,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: false, phaseTwo: true, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -137,7 +137,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: true, phaseTwo: true, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -151,7 +151,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: true, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -165,7 +165,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: true, phaseTwo: true, phaseThree: true, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -179,7 +179,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: false, phaseFour: true, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -193,7 +193,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: true, phaseTwo: true, phaseThree: true, phaseFour: true, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: store) @@ -209,7 +209,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) let flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) // When let phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) @@ -225,7 +225,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { // When var flags = generateFlags(phaseOne: true, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) var phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) // Then @@ -234,7 +234,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { // When flags = generateFlags(phaseOne: false, phaseTwo: true, phaseThree: false, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) remote.flags = flags - store.update(using: remote) + store.update(using: remote, waitOn: self) phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) // Then @@ -243,7 +243,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { // When flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: true, phaseFour: false, phaseNewUsers: false, phaseSelfHosted: false) remote.flags = flags - store.update(using: remote) + store.update(using: remote, waitOn: self) phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) // Then @@ -257,7 +257,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { // When var flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: false, phaseFour: true, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) var phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) // Then @@ -266,7 +266,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { // When flags = generateFlags(phaseOne: false, phaseTwo: false, phaseThree: false, phaseFour: false, phaseNewUsers: true, phaseSelfHosted: false) remote.flags = flags - store.update(using: remote) + store.update(using: remote, waitOn: self) phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) // Then @@ -280,7 +280,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { // When var flags = generateFlags(phaseOne: true, phaseTwo: true, phaseThree: true, phaseFour: true, phaseNewUsers: false, phaseSelfHosted: false) let remote = MockFeatureFlagRemote(flags: flags) - store.update(using: remote) + store.update(using: remote, waitOn: self) var phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) // Then @@ -289,7 +289,7 @@ final class JetpackFeaturesRemovalCoordinatorTests: CoreDataTestCase { // When flags = generateFlags(phaseOne: true, phaseTwo: true, phaseThree: true, phaseFour: false, phaseNewUsers: true, phaseSelfHosted: false) remote.flags = flags - store.update(using: remote) + store.update(using: remote, waitOn: self) phase = JetpackFeaturesRemovalCoordinator.siteCreationPhase(featureFlagStore: store) // Then @@ -346,3 +346,13 @@ private extension Date { from: self) } } + +internal extension RemoteFeatureFlagStore { + func update(using remote: FeatureFlagRemote, waitOn test: XCTestCase) { + let exp = test.expectation(description: "Store finishes update") + update(using: remote) { + exp.fulfill() + } + test.wait(for: [exp], timeout: 1) + } +} diff --git a/WordPress/WordPressTest/RemoteFeatureFlagTests.swift b/WordPress/WordPressTest/RemoteFeatureFlagTests.swift index 7a590a861f83..97ef27913dd2 100644 --- a/WordPress/WordPressTest/RemoteFeatureFlagTests.swift +++ b/WordPress/WordPressTest/RemoteFeatureFlagTests.swift @@ -22,8 +22,8 @@ class RemoteFeatureFlagTests: XCTestCase { exp.fulfill() } - store.update(using: mock) - store.update(using: mock) + store.update(using: mock, waitOn: self) + store.update(using: mock, waitOn: self) wait(for: [exp], timeout: 1.0) } @@ -42,7 +42,7 @@ class RemoteFeatureFlagTests: XCTestCase { let mock = MockFeatureFlagRemote(mockFlags: MockFeatureFlag.remoteCases) let store = RemoteFeatureFlagStore(persistenceStore: mockUserDefaults) - store.update(using: mock) + store.update(using: mock, waitOn: self) // All of the remotely defined values should be present XCTAssertTrue(store.hasValue(for: MockFeatureFlag.remotelyEnabledLocallyEnabledFeature.remoteKey))