From e185e49e8c2c6cee6b3c7c2c223562cf97146eda Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 15:06:46 +0200 Subject: [PATCH 1/9] Add: new `jp_removal_static_posters` remote flag --- .../Classes/Utility/BuildInformation/FeatureFlag.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift b/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift index 31a98cefb402..aca9f9d58be0 100644 --- a/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift +++ b/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift @@ -43,6 +43,7 @@ enum FeatureFlag: Int, CaseIterable, OverrideableFlag { case jetpackFeaturesRemovalPhaseFour case jetpackFeaturesRemovalPhaseNewUsers case jetpackFeaturesRemovalPhaseSelfHosted + case jetpackFeaturesRemovalStaticPosters case wordPressSupportForum case jetpackIndividualPluginSupport case blaze @@ -142,6 +143,8 @@ enum FeatureFlag: Int, CaseIterable, OverrideableFlag { return false case .jetpackFeaturesRemovalPhaseSelfHosted: return false + case .jetpackFeaturesRemovalStaticPosters: + return false case .wordPressSupportForum: return true case .jetpackIndividualPluginSupport: @@ -174,6 +177,8 @@ enum FeatureFlag: Int, CaseIterable, OverrideableFlag { return "jp_removal_new_users" case .jetpackFeaturesRemovalPhaseSelfHosted: return "jp_removal_self_hosted" + case .jetpackFeaturesRemovalStaticPosters: + return "jp_removal_static_posters" case .jetpackMigrationPreventDuplicateNotifications: return "prevent_duplicate_notifs_remote_field" case .wordPressSupportForum: @@ -282,6 +287,8 @@ extension FeatureFlag { return "Jetpack Features Removal Phase For New Users" case .jetpackFeaturesRemovalPhaseSelfHosted: return "Jetpack Features Removal Phase For Self-Hosted Sites" + case .jetpackFeaturesRemovalStaticPosters: + return "Jetpack Features Removal Static Posters Phase" case .wordPressSupportForum: return "Provide support through a forum" case .jetpackIndividualPluginSupport: From e30cc2cfac2f755ac6a558571a5ec61b3a007c08 Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 15:59:01 +0200 Subject: [PATCH 2/9] Add: new "static_screens" case to `GeneralPhase` --- .../Coordinator/JetpackFeaturesRemovalCoordinator.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift b/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift index 2a2862646c74..a77eec751d72 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift @@ -12,6 +12,7 @@ class JetpackFeaturesRemovalCoordinator: NSObject { case four case newUsers = "new_users" case selfHosted = "self_hosted" + case staticScreens = "static_screens" var frequencyConfig: OverlayFrequencyTracker.FrequencyConfig { switch self { @@ -99,6 +100,9 @@ class JetpackFeaturesRemovalCoordinator: NSObject { if featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalPhaseFour) { return .four } + if featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalStaticPosters) { + return .staticScreens + } if featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalPhaseThree) { return .three } @@ -118,7 +122,8 @@ class JetpackFeaturesRemovalCoordinator: NSObject { } if featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalPhaseNewUsers) - || featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalPhaseFour) { + || featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalPhaseFour) + || featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalStaticPosters) { return .two } if featureFlagStore.value(for: FeatureFlag.jetpackFeaturesRemovalPhaseThree) From 01c7922098ae9b3e06869b3593f18036a909479f Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 15:59:52 +0200 Subject: [PATCH 3/9] Update: change definition of `jetpackFeaturesEnabled` and add `shouldShowJetpackFeatures` --- .../Classes/System/RootViewCoordinator.swift | 6 +-- .../JetpackFeaturesRemovalCoordinator.swift | 39 +++++++++++++++---- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/WordPress/Classes/System/RootViewCoordinator.swift b/WordPress/Classes/System/RootViewCoordinator.swift index 2696cc2fd563..aea2c75dde60 100644 --- a/WordPress/Classes/System/RootViewCoordinator.swift +++ b/WordPress/Classes/System/RootViewCoordinator.swift @@ -48,7 +48,7 @@ class RootViewCoordinator { windowManager: WindowManager?) { self.featureFlagStore = featureFlagStore self.windowManager = windowManager - if Self.shouldEnableJetpackFeatures(featureFlagStore: featureFlagStore) { + if Self.shouldShowJetpackFeaturesBasedOnCurrentPhase(featureFlagStore: featureFlagStore) { self.currentAppUIType = .normal self.rootViewPresenter = WPTabBarController() } @@ -64,7 +64,7 @@ class RootViewCoordinator { // MARK: JP Features State /// Used to determine if the Jetpack features are enabled based on the removal phase. - private static func shouldEnableJetpackFeatures(featureFlagStore: RemoteFeatureFlagStore) -> Bool { + private static func shouldShowJetpackFeaturesBasedOnCurrentPhase(featureFlagStore: RemoteFeatureFlagStore) -> Bool { let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: featureFlagStore) switch phase { case .four, .newUsers, .selfHosted: @@ -84,7 +84,7 @@ class RootViewCoordinator { /// - Returns: Boolean value describing whether the UI was reloaded or not. @discardableResult func reloadUIIfNeeded(blog: Blog?) -> Bool { - let newUIType: AppUIType = Self.shouldEnableJetpackFeatures(featureFlagStore: featureFlagStore) ? .normal : .simplified + let newUIType: AppUIType = Self.shouldShowJetpackFeaturesBasedOnCurrentPhase(featureFlagStore: featureFlagStore) ? .normal : .simplified let oldUIType = currentAppUIType guard newUIType != oldUIType, let windowManager else { return false diff --git a/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift b/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift index a77eec751d72..567cfc6e7f48 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackFeaturesRemovalCoordinator.swift @@ -144,31 +144,54 @@ class JetpackFeaturesRemovalCoordinator: NSObject { return formatter.date(from: dateString) } - /// Used to determine if the Jetpack features are enabled based on the current app UI type. + /// Used to determine if the Jetpack features are enabled based on the current removal phase. + /// It is possible for JP features to be disabled, but still be displayed (`shouldShowJetpackFeatures`) + /// This will happen in the "Static Screens" phase. + @objc + static func jetpackFeaturesEnabled() -> Bool { + return jetpackFeaturesEnabled(featureFlagStore: RemoteFeatureFlagStore()) + } + + /// Used to determine if the Jetpack features are enabled based on the current removal phase. + /// It is possible for JP features to be disabled, but still be displayed (`shouldShowJetpackFeatures`) + /// This will happen in the "Static Screens" phase. + /// Using two separate methods (rather than one method with a default argument) because Obj-C. + /// - Returns: `true` if UI type is normal, and `false` if UI type is simplified. + static func jetpackFeaturesEnabled(featureFlagStore: RemoteFeatureFlagStore) -> Bool { + let phase = generalPhase(featureFlagStore: featureFlagStore) + switch phase { + case .four, .newUsers, .selfHosted, .staticScreens: + return false + default: + return true + } + } + + /// Used to determine if the Jetpack features are to be displayed based on the current app UI type. /// This way we ensure features are not removed before reloading the UI. /// But if the current app UI type is not set, we determine if the Jetpack Features /// are enabled based on the removal phase regardless of the app UI state. @objc - static func jetpackFeaturesEnabled() -> Bool { - return jetpackFeaturesEnabled(featureFlagStore: RemoteFeatureFlagStore()) + static func shouldShowJetpackFeatures() -> Bool { + return shouldShowJetpackFeatures(featureFlagStore: RemoteFeatureFlagStore()) } - /// Used to determine if the Jetpack features are enabled based on the current app UI type. + /// Used to determine if the Jetpack features are to be displayed based on the current app UI type. /// This way we ensure features are not removed before reloading the UI. /// But if the current app UI type is not set, we determine if the Jetpack Features /// are enabled based on the removal phase regardless of the app UI state. /// Using two separate methods (rather than one method with a default argument) because Obj-C. /// - Returns: `true` if UI type is normal, and `false` if UI type is simplified. - static func jetpackFeaturesEnabled(featureFlagStore: RemoteFeatureFlagStore) -> Bool { + static func shouldShowJetpackFeatures(featureFlagStore: RemoteFeatureFlagStore) -> Bool { guard let currentAppUIType else { - return shouldEnableJetpackFeatures(featureFlagStore: featureFlagStore) + return shouldShowJetpackFeaturesBasedOnCurrentPhase(featureFlagStore: featureFlagStore) } return currentAppUIType == .normal } - /// Used to determine if the Jetpack features are enabled based on the removal phase regardless of the app UI state. - private static func shouldEnableJetpackFeatures(featureFlagStore: RemoteFeatureFlagStore) -> Bool { + /// Used to determine if the Jetpack features are to be displayed or not based on the removal phase regardless of the app UI state. + private static func shouldShowJetpackFeaturesBasedOnCurrentPhase(featureFlagStore: RemoteFeatureFlagStore) -> Bool { let phase = generalPhase(featureFlagStore: featureFlagStore) switch phase { case .four, .newUsers, .selfHosted: From ac0489bb9278e50141596ea3363857c40c6b5934 Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 19:12:12 +0200 Subject: [PATCH 4/9] Update: show JP features during "Static screens" phase --- .../BlogDetailsViewController+SectionHelpers.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift index a2cf3cd70e37..60c068268d99 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift +++ b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift @@ -56,39 +56,39 @@ extension BlogDetailsViewController { } @objc func shouldShowStats() -> Bool { - return JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled() + return JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures() } @objc func shouldAddJetpackSection() -> Bool { - guard JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled() else { + guard JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures() else { return false } return blog.shouldShowJetpackSection } @objc func shouldAddGeneralSection() -> Bool { - guard JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled() else { + guard JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures() else { return false } return blog.shouldShowJetpackSection == false } @objc func shouldAddPersonalizeSection() -> Bool { - guard JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled() else { + guard JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures() else { return false } return blog.supports(.themeBrowsing) || blog.supports(.menus) } @objc func shouldAddSharingRow() -> Bool { - guard JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled() else { + guard JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures() else { return false } return blog.supports(.sharing) } @objc func shouldAddPeopleRow() -> Bool { - guard JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled() else { + guard JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures() else { return false } return blog.supports(.people) From 91b87ee6f866fb0271e9df56d0a31aaa793a32c3 Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 19:12:33 +0200 Subject: [PATCH 5/9] Update: Hide menu cards during "Static screens" phase --- .../Branding/Menu Card/JetpackBrandingMenuCardPresenter.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Jetpack/Branding/Menu Card/JetpackBrandingMenuCardPresenter.swift b/WordPress/Classes/ViewRelated/Jetpack/Branding/Menu Card/JetpackBrandingMenuCardPresenter.swift index 498d18e630ff..85b7fdd7c123 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Branding/Menu Card/JetpackBrandingMenuCardPresenter.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Branding/Menu Card/JetpackBrandingMenuCardPresenter.swift @@ -62,7 +62,7 @@ class JetpackBrandingMenuCardPresenter { guard isCardEnabled() else { return false } - let jetpackFeaturesEnabled = JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled(featureFlagStore: featureFlagStore) + let jetpackFeaturesEnabled = JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures(featureFlagStore: featureFlagStore) switch (phase, jetpackFeaturesEnabled) { case (.three, true): return true @@ -77,7 +77,7 @@ class JetpackBrandingMenuCardPresenter { guard isCardEnabled() else { return false } - let jetpackFeaturesEnabled = JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled(featureFlagStore: featureFlagStore) + let jetpackFeaturesEnabled = JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures(featureFlagStore: featureFlagStore) switch (phase, jetpackFeaturesEnabled) { case (.four, false): fallthrough From d0598d6e14aed46526a8cff3f2cd211a6b671520 Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 19:15:11 +0200 Subject: [PATCH 6/9] Update: show banners and badges during "Static Screens" phase --- .../Branding/Coordinator/JetpackBrandingCoordinator.swift | 2 ++ .../Jetpack/Branding/JetpackBrandingVisibility.swift | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackBrandingCoordinator.swift b/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackBrandingCoordinator.swift index e9bbd10768a4..092df34f75e1 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackBrandingCoordinator.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Branding/Coordinator/JetpackBrandingCoordinator.swift @@ -27,6 +27,8 @@ class JetpackBrandingCoordinator { case .two: fallthrough case .three: + fallthrough + case .staticScreens: return true default: return false diff --git a/WordPress/Classes/ViewRelated/Jetpack/Branding/JetpackBrandingVisibility.swift b/WordPress/Classes/ViewRelated/Jetpack/Branding/JetpackBrandingVisibility.swift index 6582d4462547..73599cbe2678 100644 --- a/WordPress/Classes/ViewRelated/Jetpack/Branding/JetpackBrandingVisibility.swift +++ b/WordPress/Classes/ViewRelated/Jetpack/Branding/JetpackBrandingVisibility.swift @@ -16,7 +16,7 @@ enum JetpackBrandingVisibility { return AppConfiguration.isWordPress && AccountHelper.isDotcomAvailable() && FeatureFlag.jetpackPowered.enabled && - JetpackFeaturesRemovalCoordinator.jetpackFeaturesEnabled() + JetpackFeaturesRemovalCoordinator.shouldShowJetpackFeatures() case .wordPressApp: return AppConfiguration.isWordPress case .dotcomAccounts: From a8f5e9594ac6e11f1471defede3f514d2d3f1834 Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 19:16:02 +0200 Subject: [PATCH 7/9] Update: rename visible name for static screens flag --- WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift b/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift index aca9f9d58be0..fdfb7e7c6cb5 100644 --- a/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift +++ b/WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift @@ -288,7 +288,7 @@ extension FeatureFlag { case .jetpackFeaturesRemovalPhaseSelfHosted: return "Jetpack Features Removal Phase For Self-Hosted Sites" case .jetpackFeaturesRemovalStaticPosters: - return "Jetpack Features Removal Static Posters Phase" + return "Jetpack Features Removal Static Screens Phase" case .wordPressSupportForum: return "Provide support through a forum" case .jetpackIndividualPluginSupport: From 815619f4ea8ab64c471329955ca5dbfca38d2418 Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Wed, 15 Mar 2023 19:57:16 +0200 Subject: [PATCH 8/9] Tests: fix tests build errors --- .../WordPressTest/JetpackBrandingMenuCardPresenterTests.swift | 3 +++ WordPress/WordPressTest/RemoteFeatureFlagStoreMock.swift | 3 +++ 2 files changed, 6 insertions(+) diff --git a/WordPress/WordPressTest/JetpackBrandingMenuCardPresenterTests.swift b/WordPress/WordPressTest/JetpackBrandingMenuCardPresenterTests.swift index 0ff6f3267bbc..7703ff0e6c78 100644 --- a/WordPress/WordPressTest/JetpackBrandingMenuCardPresenterTests.swift +++ b/WordPress/WordPressTest/JetpackBrandingMenuCardPresenterTests.swift @@ -232,6 +232,7 @@ final class JetpackBrandingMenuCardPresenterTests: CoreDataTestCase { remoteFeatureFlagsStore.removalPhaseFour = false remoteFeatureFlagsStore.removalPhaseNewUsers = false remoteFeatureFlagsStore.removalPhaseSelfHosted = false + remoteFeatureFlagsStore.removalPhaseStaticScreens = false // Set phase switch phase { @@ -249,6 +250,8 @@ final class JetpackBrandingMenuCardPresenterTests: CoreDataTestCase { remoteFeatureFlagsStore.removalPhaseNewUsers = true case .selfHosted: remoteFeatureFlagsStore.removalPhaseSelfHosted = true + case .staticScreens: + remoteFeatureFlagsStore.removalPhaseStaticScreens = true } } } diff --git a/WordPress/WordPressTest/RemoteFeatureFlagStoreMock.swift b/WordPress/WordPressTest/RemoteFeatureFlagStoreMock.swift index 405f2ae813e2..baf67dcba7e5 100644 --- a/WordPress/WordPressTest/RemoteFeatureFlagStoreMock.swift +++ b/WordPress/WordPressTest/RemoteFeatureFlagStoreMock.swift @@ -9,6 +9,7 @@ class RemoteFeatureFlagStoreMock: RemoteFeatureFlagStore { var removalPhaseFour = false var removalPhaseNewUsers = false var removalPhaseSelfHosted = false + var removalPhaseStaticScreens = false override func value(for flag: OverrideableFlag) -> Bool { guard let flag = flag as? WordPress.FeatureFlag else { @@ -27,6 +28,8 @@ class RemoteFeatureFlagStoreMock: RemoteFeatureFlagStore { return removalPhaseNewUsers case .jetpackFeaturesRemovalPhaseSelfHosted: return removalPhaseSelfHosted + case .jetpackFeaturesRemovalStaticPosters: + return removalPhaseStaticScreens default: return super.value(for: flag) } From bff22377dbbea9435f77fdf2a4bfa9b83ab78624 Mon Sep 17 00:00:00 2001 From: Hassaan El-Garem Date: Thu, 16 Mar 2023 13:30:18 +0200 Subject: [PATCH 9/9] Update: fix inaccuracy in documentation --- WordPress/Classes/System/RootViewCoordinator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/System/RootViewCoordinator.swift b/WordPress/Classes/System/RootViewCoordinator.swift index aea2c75dde60..2bb8d37fbf42 100644 --- a/WordPress/Classes/System/RootViewCoordinator.swift +++ b/WordPress/Classes/System/RootViewCoordinator.swift @@ -63,7 +63,7 @@ class RootViewCoordinator { // MARK: JP Features State - /// Used to determine if the Jetpack features are enabled based on the removal phase. + /// Used to determine if the Jetpack features are to be displayed or not based on the removal phase. private static func shouldShowJetpackFeaturesBasedOnCurrentPhase(featureFlagStore: RemoteFeatureFlagStore) -> Bool { let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: featureFlagStore) switch phase {