Skip to content

Remove FXIOS-11952 ⁃ Remove jump back in feature flag #26225

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion BrowserKit/Sources/Shared/Prefs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ public struct PrefsKeys {
public static let DebugSuffixKey = "DebugKey"
public static let FirefoxSuggest = "FirefoxSuggest"
public static let InactiveTabs = "InactiveTabsUserPrefsKey"
public static let JumpBackInSection = "JumpBackInSectionUserPrefsKey"
public static let SearchBarPosition = "SearchBarPositionUsersPrefsKey"
public static let SentFromFirefox = "SentFromFirefoxUserPrefsKey"
}
Expand Down Expand Up @@ -109,6 +108,7 @@ public struct PrefsKeys {
public struct UserFeatureFlagPrefs {
public static let ASPocketStories = "ASPocketStoriesUserPrefsKey"
public static let BookmarksSection = "BookmarksSectionUserPrefsKey"
public static let JumpBackInSection = "JumpBackInSectionUserPrefsKey"
public static let SponsoredShortcuts = "SponsoredShortcutsUserPrefsKey"
public static let StartAtHome = "StartAtHomeUserPrefsKey"
public static let TopSiteSection = "TopSitesUserPrefsKey"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ enum NimbusFeatureFlagID: String, CaseIterable {
case firefoxSuggestFeature
case homepageRebuild
case inactiveTabs
case jumpBackIn
case loginAutofill
case menuRefactor
case menuRefactorHint
Expand Down Expand Up @@ -109,8 +108,6 @@ struct NimbusFlaggableFeature: HasNimbusSearchBar {
return FlagKeys.FirefoxSuggest
case .inactiveTabs:
return FlagKeys.InactiveTabs
case .jumpBackIn:
return FlagKeys.JumpBackInSection
case .sentFromFirefox:
return FlagKeys.SentFromFirefox
// Cases where users do not have the option to manipulate a setting.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct JumpBackInSectionState: StateType, Equatable, Hashable {
windowUUID: WindowUUID
) {
// TODO: FXIOS-11412 / 11226 - Move profile dependency and show section also based on feature flags
let shouldShowSection = profile.prefs.boolForKey(PrefsKeys.FeatureFlags.JumpBackInSection) ?? true
let shouldShowSection = profile.prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.JumpBackInSection) ?? true
self.init(
windowUUID: windowUUID,
jumpBackInTabs: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,8 @@ extension JumpBackInViewModel: HomepageViewModelProtocol {
}

var isEnabled: Bool {
guard featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildAndUser) else { return false }

return !isPrivate
let isEnabled = profile.prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.JumpBackInSection) ?? true
return !isPrivate && isEnabled
}

func numberOfItemsInSection() -> Int {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ class HomePageSettingViewController: SettingsTableViewController, FeatureFlaggab
var hasHomePage = false
var wallpaperManager: WallpaperManagerInterface

var isJumpBackInSectionEnabled: Bool {
return featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildOnly)
}

var isWallpaperSectionEnabled: Bool {
return wallpaperManager.canSettingsBeShown
}
Expand Down Expand Up @@ -128,27 +124,27 @@ class HomePageSettingViewController: SettingsTableViewController, FeatureFlaggab
format: .Settings.Homepage.CustomizeFirefoxHome.ThoughtProvokingStoriesSubtitle,
PocketAppName.shortName.rawValue)

let jumpBackInSetting = BoolSetting(
with: .jumpBackIn,
titleText: NSAttributedString(string: .Settings.Homepage.CustomizeFirefoxHome.JumpBackIn)
) { value in
store.dispatch(
JumpBackInAction(
isEnabled: value,
windowUUID: self.windowUUID,
actionType: JumpBackInActionType.toggleShowSectionSetting
)
)
}

// Section ordering
sectionItems.append(TopSitesSettings(settings: self))

if isJumpBackInSectionEnabled {
if let profile {
let jumpBackInSetting = BoolSetting(
prefs: profile.prefs,
theme: themeManager.getCurrentTheme(for: windowUUID),
prefKey: PrefsKeys.UserFeatureFlagPrefs.JumpBackInSection,
defaultValue: true,
titleText: .Settings.Homepage.CustomizeFirefoxHome.JumpBackIn
) { value in
store.dispatch(
JumpBackInAction(
isEnabled: value,
windowUUID: self.windowUUID,
actionType: JumpBackInActionType.toggleShowSectionSetting
)
)
}
sectionItems.append(jumpBackInSetting)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have another if let profile { on the line below, we could combine both jumpBackInSetting and bookmarksSetting in the same if so we only unwrap the profile once?

if let profile {
let bookmarksSetting = BoolSetting(
prefs: profile.prefs,
theme: themeManager.getCurrentTheme(for: windowUUID),
Expand Down
19 changes: 0 additions & 19 deletions firefox-ios/Client/Nimbus/NimbusFeatureFlagLayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ final class NimbusFeatureFlagLayer {
case .creditCardAutofillStatus:
return checkNimbusForCreditCardAutofill(for: featureID, from: nimbus)

case .jumpBackIn:
return checkHomescreenSectionsFeature(for: featureID, from: nimbus)

case .firefoxSuggestFeature:
return checkFirefoxSuggestFeature(from: nimbus)

Expand Down Expand Up @@ -183,22 +180,6 @@ final class NimbusFeatureFlagLayer {
}
}

private func checkHomescreenSectionsFeature(for featureID: NimbusFeatureFlagID,
from nimbus: FxNimbus
) -> Bool {
let config = nimbus.features.homescreenFeature.value()
var nimbusID: HomeScreenSection

switch featureID {
case .jumpBackIn: nimbusID = HomeScreenSection.jumpBackIn
default: return false
}

guard let status = config.sectionsEnabled[nimbusID] else { return false }

return status
}

private func checkHomepageFeature(from nimbus: FxNimbus) -> Bool {
let config = nimbus.features.homepageRebuildFeature.value()
return config.enabled
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Telemetry/TelemetryWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
}

// Homepage section preferences
let isJumpBackInEnabled = featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildAndUser)
let isJumpBackInEnabled = profile.prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.JumpBackInSection) ?? true
GleanMetrics.Preferences.jumpBackIn.set(isJumpBackInEnabled)

let isBookmarksEnabled = prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.BookmarksSection) ?? true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ class FeatureFlagManagerTests: XCTestCase, FeatureFlaggable {
XCTAssertTrue(featureFlags.isFeatureEnabled(.bottomSearchBar, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.inactiveTabs, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.inactiveTabs, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.reportSiteIssue, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.reportSiteIssue, checking: .userOnly))
}
Expand All @@ -46,22 +44,6 @@ class FeatureFlagManagerTests: XCTestCase, FeatureFlaggable {
XCTAssertEqual(featureFlags.getCustomState(for: .searchBarPosition), SearchBarPosition.top)
}

// Changing the prefs manually, to make sure settings are respected through
// the FFMs interface
func testManagerRespectsProfileChangesForBoolSettings() {
let mockProfile = MockProfile(databasePrefix: "FeatureFlagsManagerTests_")
mockProfile.prefs.clearAll()
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: mockProfile)

XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .userOnly))
// Changing the prefs manually, to make sure settings are respected through
// the FFMs interface
mockProfile.prefs.setBool(false, forKey: PrefsKeys.FeatureFlags.JumpBackInSection)
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildOnly))
XCTAssertFalse(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .userOnly))
}

// Changing the prefs manually, to make sure settings are respected through
// the FFMs interface
func testManagerRespectsProfileChangesForCustomSettings() {
Expand All @@ -76,14 +58,6 @@ class FeatureFlagManagerTests: XCTestCase, FeatureFlaggable {
XCTAssertEqual(featureFlags.getCustomState(for: .searchBarPosition), SearchBarPosition.bottom)
}

func testManagerInterfaceForUpdatingBoolFlags() {
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .userOnly))
featureFlags.set(feature: .jumpBackIn, to: false)
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildOnly))
XCTAssertFalse(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .userOnly))
}

func testManagerInterfaceForUpdatingCustomFlags() {
// Search Bar
XCTAssertEqual(featureFlags.getCustomState(for: .searchBarPosition), SearchBarPosition.top)
Expand Down
5 changes: 0 additions & 5 deletions firefox-ios/nimbus-features/homescreenFeature.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ features:
type: Map<HomeScreenSection, Boolean>
default:
{
"jump-back-in": true,
"recent-explorations": false,
}
prefer-switch-to-open-tab:
Expand All @@ -22,15 +21,13 @@ features:
- channel: developer
value: {
"sections-enabled": {
"jump-back-in": true,
"recent-explorations": false,
},
"prefer-switch-to-open-tab": true
}
- channel: beta
value: {
"sections-enabled": {
"jump-back-in": true,
"recent-explorations": false,
},
"prefer-switch-to-open-tab": false
Expand All @@ -40,7 +37,5 @@ enums:
HomeScreenSection:
description: The identifiers for the sections of the homescreen.
variants:
jump-back-in:
description: The tabs the user was looking immediately before being interrupted.
recent-explorations:
description: The tab groups