Skip to content

Commit

Permalink
Remove FXIOS-6735 [v120] remove pocket feature flag (#16825)
Browse files Browse the repository at this point in the history
* Remove .pocket feature flag

* Show pocket settings even when the feature is turned off

* Return directly

* Adjust tests
  • Loading branch information
lmarceau authored Oct 13, 2023
1 parent b9d6833 commit 022abcc
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 43 deletions.
12 changes: 1 addition & 11 deletions Client/FeatureFlags/NimbusFlaggableFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ enum NimbusFeatureFlagID: String, CaseIterable {
case isToolbarCFREnabled
case jumpBackIn
case libraryCoordinatorRefactor
case pocket
case qrCodeCoordinatorRefactor
case recentlySaved
case reduxIntegration
Expand Down Expand Up @@ -68,8 +67,6 @@ struct NimbusFlaggableFeature: HasNimbusSearchBar {
return FlagKeys.InactiveTabs
case .jumpBackIn:
return FlagKeys.JumpBackInSection
case .pocket:
return FlagKeys.ASPocketStories
case .recentlySaved:
return FlagKeys.RecentlySavedSection
case .topSites:
Expand Down Expand Up @@ -113,14 +110,7 @@ struct NimbusFlaggableFeature: HasNimbusSearchBar {
return true
}

let nimbusValue = nimbusLayer.checkNimbusConfigFor(featureID)

switch featureID {
case .pocket:
return nimbusValue && PocketProvider.islocaleSupported(Locale.current.identifier)
default:
return nimbusValue
}
return nimbusLayer.checkNimbusConfigFor(featureID)
}

/// Returns whether or not the feature's state was changed by the user. If no
Expand Down
3 changes: 2 additions & 1 deletion Client/Frontend/Home/HomepageViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,10 @@ class HomepageViewModel: FeatureFlaggable {
historyHighlightsDataAdaptor: historyDataAdaptor,
wallpaperManager: wallpaperManager)

let pocketDataAdaptor = PocketDataAdaptorImplementation(pocketAPI: PocketProvider())
let pocketDataAdaptor = PocketDataAdaptorImplementation(pocketAPI: PocketProvider(prefs: profile.prefs))
self.pocketViewModel = PocketViewModel(pocketDataAdaptor: pocketDataAdaptor,
theme: theme,
prefs: profile.prefs,
wallpaperManager: wallpaperManager)
pocketDataAdaptor.delegate = pocketViewModel

Expand Down
11 changes: 6 additions & 5 deletions Client/Frontend/Home/Pocket/PocketViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ class PocketViewModel {
private var dataAdaptor: PocketDataAdaptor
private var pocketStoriesViewModels = [PocketStandardCellViewModel]()
private var wallpaperManager: WallpaperManager
private var prefs: Prefs

init(pocketDataAdaptor: PocketDataAdaptor,
isZeroSearch: Bool = false,
theme: Theme,
prefs: Prefs,
wallpaperManager: WallpaperManager) {
self.dataAdaptor = pocketDataAdaptor
self.isZeroSearch = isZeroSearch
self.theme = theme
self.prefs = prefs
self.wallpaperManager = wallpaperManager
}

Expand Down Expand Up @@ -119,7 +122,7 @@ class PocketViewModel {
}

// MARK: HomeViewModelProtocol
extension PocketViewModel: HomepageViewModelProtocol, FeatureFlaggable {
extension PocketViewModel: HomepageViewModelProtocol {
var sectionType: HomepageSectionType {
return .pocket
}
Expand Down Expand Up @@ -184,10 +187,8 @@ extension PocketViewModel: HomepageViewModelProtocol, FeatureFlaggable {
}

var isEnabled: Bool {
// For Pocket, the user preference check returns a user preference if it exists in
// UserDefaults, and, if it does not, it will return a default preference based on
// a (nimbus pocket section enabled && Pocket.isLocaleSupported) check
return featureFlags.isFeatureEnabled(.pocket, checking: .buildAndUser)
let isFeatureEnabled = prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.ASPocketStories) ?? true
return isFeatureEnabled && PocketProvider.islocaleSupported(Locale.current.identifier)
}

var hasData: Bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class HomePageSettingViewController: SettingsTableViewController, FeatureFlaggab
}

var isPocketSectionEnabled: Bool {
return featureFlags.isFeatureEnabled(.pocket, checking: .buildOnly)
return PocketProvider.islocaleSupported(Locale.current.identifier)
}

var isHistoryHighlightsSectionEnabled: Bool {
Expand Down Expand Up @@ -133,11 +133,13 @@ class HomePageSettingViewController: SettingsTableViewController, FeatureFlaggab
PocketAppName.shortName.rawValue)

let pocketSetting = BoolSetting(
with: .pocket,
titleText: NSAttributedString(string: .Settings.Homepage.CustomizeFirefoxHome.ThoughtProvokingStories),
statusText: NSAttributedString(string: pocketStatusText)) { [weak self] _ in
self?.tableView.reloadData()
}
prefs: profile.prefs,
theme: themeManager.currentTheme,
prefKey: PrefsKeys.UserFeatureFlagPrefs.ASPocketStories,
defaultValue: true,
titleText: .Settings.Homepage.CustomizeFirefoxHome.ThoughtProvokingStories,
statusText: pocketStatusText
)

let jumpBackInSetting = BoolSetting(with: .jumpBackIn,
titleText: NSAttributedString(string: .Settings.Homepage.CustomizeFirefoxHome.JumpBackIn))
Expand Down
6 changes: 0 additions & 6 deletions Client/Nimbus/NimbusFeatureFlagLayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ final class NimbusFeatureFlagLayer {
return checkCredentialAutofillCoordinatorRefactorFeature(from: nimbus)

case .jumpBackIn,
.pocket,
.recentlySaved,
.historyHighlights,
.topSites:
Expand Down Expand Up @@ -117,7 +116,6 @@ final class NimbusFeatureFlagLayer {
case .jumpBackIn: nimbusID = HomeScreenSection.jumpBackIn
case .recentlySaved: nimbusID = HomeScreenSection.recentlySaved
case .historyHighlights: nimbusID = HomeScreenSection.recentExplorations
case .pocket: nimbusID = HomeScreenSection.pocket
default: return false
}

Expand Down Expand Up @@ -173,10 +171,6 @@ final class NimbusFeatureFlagLayer {
return config.configuration.version.rawValue
}

private func checkNimbusForPocketSponsoredStoriesFeature(using nimbus: FxNimbus) -> Bool {
return nimbus.features.homescreenFeature.value().pocketSponsoredStories
}

private func checkReduxIntegrationFeature(from nimbus: FxNimbus) -> Bool {
let config = nimbus.features.reduxIntegrationFeature.value()
return config.enabled
Expand Down
5 changes: 3 additions & 2 deletions Client/Telemetry/TelemetryWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
func recordFinishedLaunchingPreferenceMetrics(notification: NSNotification) {
guard let profile = self.profile else { return }
// Pocket stories visible
if let pocketStoriesVisible = profile.prefs.boolForKey(PrefsKeys.FeatureFlags.ASPocketStories) {
if let pocketStoriesVisible = profile.prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.ASPocketStories) {
GleanMetrics.FirefoxHomePage.pocketStoriesVisible.set(pocketStoriesVisible)
} else {
GleanMetrics.FirefoxHomePage.pocketStoriesVisible.set(true)
Expand Down Expand Up @@ -324,7 +324,8 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
let isRecentlySavedEnabled = featureFlags.isFeatureEnabled(.recentlySaved, checking: .buildAndUser)
GleanMetrics.Preferences.recentlySaved.set(isRecentlySavedEnabled)

let isPocketEnabled = featureFlags.isFeatureEnabled(.pocket, checking: .buildAndUser)
let isFeatureEnabled = prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.ASPocketStories) ?? true
let isPocketEnabled = isFeatureEnabled && PocketProvider.islocaleSupported(Locale.current.identifier)
GleanMetrics.Preferences.pocket.set(isPocketEnabled)

let startAtHomeOption = prefs.stringForKey(PrefsKeys.UserFeatureFlagPrefs.StartAtHome) ?? StartAtHomeSetting.afterFourHours.rawValue
Expand Down
7 changes: 5 additions & 2 deletions Providers/Pocket/PocketProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class PocketProvider: PocketStoriesProviding, FeatureFlaggable, URLCaching {
private static let SupportedLocales = ["en_CA", "en_US", "en_GB", "en_ZA", "de_DE", "de_AT", "de_CH"]

private let pocketGlobalFeed: String
private var prefs: Prefs

static let GlobalFeed = "https://getpocket.cdn.mozilla.net/v3/firefox/global-recs"
static let MoreStoriesURL = {
Expand All @@ -45,8 +46,10 @@ class PocketProvider: PocketStoriesProviding, FeatureFlaggable, URLCaching {
}()

// Allow endPoint to be overriden for testing
init(endPoint: String = PocketProvider.GlobalFeed) {
init(endPoint: String = PocketProvider.GlobalFeed,
prefs: Prefs) {
self.pocketGlobalFeed = endPoint
self.prefs = prefs
}

var urlCache: URLCache {
Expand All @@ -65,7 +68,7 @@ class PocketProvider: PocketStoriesProviding, FeatureFlaggable, URLCaching {

// Fetch items from the global pocket feed
func fetchStories(items: Int, completion: @escaping (StoryResult) -> Void) {
let isFeatureEnabled = featureFlags.isFeatureEnabled(.pocket, checking: .buildOnly)
let isFeatureEnabled = prefs.boolForKey(PrefsKeys.UserFeatureFlagPrefs.ASPocketStories) ?? true
let isCurrentLocaleSupported = PocketProvider.islocaleSupported(Locale.current.identifier)

// Check if we should use mock data
Expand Down
2 changes: 1 addition & 1 deletion Shared/Prefs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ public struct PrefsKeys {

// For ease of use, please list keys alphabetically.
public struct FeatureFlags {
public static let ASPocketStories = "ASPocketStoriesUserPrefsKey"
public static let CustomWallpaper = "CustomWallpaperUserPrefsKey"
public static let FirefoxSuggest = "FirefoxSuggest"
public static let HistoryHighlightsSection = "HistoryHighlightsSectionUserPrefsKey"
Expand All @@ -81,6 +80,7 @@ public struct PrefsKeys {
}

public struct UserFeatureFlagPrefs {
public static let ASPocketStories = "ASPocketStoriesUserPrefsKey"
public static let SponsoredShortcuts = "SponsoredShortcutsUserPrefsKey"
public static let StartAtHome = "StartAtHomeUserPrefsKey"
}
Expand Down
2 changes: 0 additions & 2 deletions Tests/ClientTests/FeatureFlagManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ class FeatureFlagManagerTests: XCTestCase, FeatureFlaggable {
XCTAssertTrue(featureFlags.isFeatureEnabled(.inactiveTabs, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.jumpBackIn, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.pocket, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.pocket, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.recentlySaved, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.recentlySaved, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.reportSiteIssue, checking: .buildOnly))
Expand Down
3 changes: 2 additions & 1 deletion Tests/ClientTests/Frontend/Home/Pocket/PocketFeedTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import UIKit
import GCDWebServers
import Shared
import XCTest

@testable import Client
Expand Down Expand Up @@ -41,7 +42,7 @@ class PocketStoriesTests: XCTestCase {

func testPocketStoriesCaching() {
let expect = expectation(description: "Pocket")
let pocketFeed = PocketProvider(endPoint: pocketAPI)
let pocketFeed = PocketProvider(endPoint: pocketAPI, prefs: MockProfilePrefs())
let feedNumber = 11

pocketFeed.fetchStories(items: feedNumber) { result in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ final class PocketViewModelTests: XCTestCase, FeatureFlaggable {
}

func testFeatureFlagDisablesSection() {
featureFlags.set(feature: .pocket, to: false)
profile.prefs.setBool(false, forKey: PrefsKeys.UserFeatureFlagPrefs.ASPocketStories)
let subject = createSubject()
XCTAssertFalse(subject.isEnabled)
}
Expand Down Expand Up @@ -218,6 +218,7 @@ extension PocketViewModelTests {
let subject = PocketViewModel(pocketDataAdaptor: adaptor,
isZeroSearch: isZeroSearch,
theme: LightTheme(),
prefs: profile.prefs,
wallpaperManager: WallpaperManager())
trackForMemoryLeaks(subject, file: file, line: line)
return subject
Expand Down
5 changes: 0 additions & 5 deletions nimbus-features/homescreenFeature.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ features:
"jump-back-in": true,
"recently-saved": true,
"recent-explorations": true,
"pocket": true
}
pocket-sponsored-stories:
description: >
Expand All @@ -29,7 +28,6 @@ features:
"jump-back-in": true,
"recently-saved": true,
"recent-explorations": true,
"pocket": true
},
"pocket-sponsored-stories": true
}
Expand All @@ -40,7 +38,6 @@ features:
"jump-back-in": true,
"recently-saved": true,
"recent-explorations": true,
"pocket": true
},
"pocket-sponsored-stories": false
}
Expand All @@ -57,5 +54,3 @@ enums:
description: The tabs the user was looking immediately before being interrupted.
recent-explorations:
description: The tab groups
pocket:
description: The pocket section. This should only be available in the US.

0 comments on commit 022abcc

Please sign in to comment.