Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetpack Focus: Introduce new Static Screens phase #20331

Merged
6 changes: 3 additions & 3 deletions WordPress/Classes/System/RootViewCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -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 {
hassaanelgarem marked this conversation as resolved.
Show resolved Hide resolved
let phase = JetpackFeaturesRemovalCoordinator.generalPhase(featureFlagStore: featureFlagStore)
switch phase {
case .four, .newUsers, .selfHosted:
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions WordPress/Classes/Utility/BuildInformation/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ enum FeatureFlag: Int, CaseIterable, OverrideableFlag {
case jetpackFeaturesRemovalPhaseFour
case jetpackFeaturesRemovalPhaseNewUsers
case jetpackFeaturesRemovalPhaseSelfHosted
case jetpackFeaturesRemovalStaticPosters
case wordPressSupportForum
case jetpackIndividualPluginSupport
case blaze
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 Screens Phase"
case .wordPressSupportForum:
return "Provide support through a forum"
case .jetpackIndividualPluginSupport:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class JetpackBrandingCoordinator {
case .two:
fallthrough
case .three:
fallthrough
case .staticScreens:
return true
default:
return false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand All @@ -139,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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ final class JetpackBrandingMenuCardPresenterTests: CoreDataTestCase {
remoteFeatureFlagsStore.removalPhaseFour = false
remoteFeatureFlagsStore.removalPhaseNewUsers = false
remoteFeatureFlagsStore.removalPhaseSelfHosted = false
remoteFeatureFlagsStore.removalPhaseStaticScreens = false

// Set phase
switch phase {
Expand All @@ -249,6 +250,8 @@ final class JetpackBrandingMenuCardPresenterTests: CoreDataTestCase {
remoteFeatureFlagsStore.removalPhaseNewUsers = true
case .selfHosted:
remoteFeatureFlagsStore.removalPhaseSelfHosted = true
case .staticScreens:
remoteFeatureFlagsStore.removalPhaseStaticScreens = true
}
}
}
3 changes: 3 additions & 0 deletions WordPress/WordPressTest/RemoteFeatureFlagStoreMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -27,6 +28,8 @@ class RemoteFeatureFlagStoreMock: RemoteFeatureFlagStore {
return removalPhaseNewUsers
case .jetpackFeaturesRemovalPhaseSelfHosted:
return removalPhaseSelfHosted
case .jetpackFeaturesRemovalStaticPosters:
return removalPhaseStaticScreens
default:
return super.value(for: flag)
}
Expand Down