Skip to content

Commit

Permalink
remove unused gating logic for history roll out (#3075)
Browse files Browse the repository at this point in the history
  • Loading branch information
brindy authored Jul 12, 2024
1 parent c09558c commit 3610c70
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 109 deletions.
5 changes: 0 additions & 5 deletions Core/DefaultVariantManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ extension FeatureName {
// Define your feature e.g.:
// public static let experimentalFeature = FeatureName(rawValue: "experimentalFeature")

public static let history = FeatureName(rawValue: "history")
}

public struct VariantIOS: Variant {
Expand Down Expand Up @@ -62,10 +61,6 @@ public struct VariantIOS: Variant {
VariantIOS(name: "sd", weight: doNotAllocate, isIncluded: When.always, features: []),
VariantIOS(name: "se", weight: doNotAllocate, isIncluded: When.always, features: []),

// This needs to stay until we finish rolling out history to all users...
// This ensures that users who previously had do not lose it.
VariantIOS(name: "md", weight: doNotAllocate, isIncluded: When.inEnglish, features: [.history]),

returningUser
]

Expand Down
3 changes: 0 additions & 3 deletions Core/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public enum FeatureFlag: String {
case incontextSignup
case autoconsentOnByDefault
case history
case historyRollout
case newTabPageSections
case duckPlayer
}
Expand Down Expand Up @@ -62,8 +61,6 @@ extension FeatureFlag: FeatureFlagSourceProviding {
return .remoteReleasable(.subfeature(AutoconsentSubfeature.onByDefault))
case .history:
return .remoteReleasable(.feature(.history))
case .historyRollout:
return .remoteReleasable(.subfeature(HistorySubFeature.onByDefault))
case .newTabPageSections:
return .internalOnly
case .duckPlayer:
Expand Down
36 changes: 1 addition & 35 deletions Core/HistoryManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,9 @@ public protocol HistoryManaging {

}

// Used for controlling incremental rollout
public enum HistorySubFeature: String, PrivacySubfeature {
public var parent: PrivacyFeature {
.history
}

case onByDefault
}

public class HistoryManager: HistoryManaging {

let privacyConfigManager: PrivacyConfigurationManaging
let variantManager: VariantManager
let internalUserDecider: InternalUserDecider
let dbCoordinator: HistoryCoordinator

public var historyCoordinator: HistoryCoordinating {
Expand All @@ -66,39 +55,19 @@ public class HistoryManager: HistoryManaging {

/// Use `make()`
init(privacyConfigManager: PrivacyConfigurationManaging,
variantManager: VariantManager,
internalUserDecider: InternalUserDecider,
dbCoordinator: HistoryCoordinator,
isAutocompleteEnabledByUser: @autoclosure @escaping () -> Bool,
isRecentlyVisitedSitesEnabledByUser: @autoclosure @escaping () -> Bool) {

self.privacyConfigManager = privacyConfigManager
self.variantManager = variantManager
self.internalUserDecider = internalUserDecider
self.dbCoordinator = dbCoordinator
self.isAutocompleteEnabledByUser = isAutocompleteEnabledByUser
self.isRecentlyVisitedSitesEnabledByUser = isRecentlyVisitedSitesEnabledByUser
}

/// Determines if the history feature is enabled. This code will need to be cleaned up once the roll out is at 100%
public func isHistoryFeatureEnabled() -> Bool {
guard privacyConfigManager.privacyConfig.isEnabled(featureKey: .history) else {
// Whatever happens if this is disabled then disable the feature
return false
}

if internalUserDecider.isInternalUser {
// Internal users get the feature
return true
}

if variantManager.isSupported(feature: .history) {
// Users in the experiment get the feature
return true
}

// Handles incremental roll out to everyone else
return privacyConfigManager.privacyConfig.isSubfeatureEnabled(HistorySubFeature.onByDefault)
return privacyConfigManager.privacyConfig.isEnabled(featureKey: .history)
}

public func removeAllHistory() async {
Expand Down Expand Up @@ -234,7 +203,6 @@ extension HistoryManager {
/// Should only be called once in the app
public static func make(isAutocompleteEnabledByUser: @autoclosure @escaping () -> Bool,
isRecentlyVisitedSitesEnabledByUser: @autoclosure @escaping () -> Bool,
internalUserDecider: InternalUserDecider,
privacyConfigManager: PrivacyConfigurationManaging) -> Result<HistoryManager, Error> {

let database = HistoryDatabase.make()
Expand All @@ -251,8 +219,6 @@ extension HistoryManager {
let dbCoordinator = HistoryCoordinator(historyStoring: HistoryStore(context: context, eventMapper: HistoryStoreEventMapper()))

let historyManager = HistoryManager(privacyConfigManager: privacyConfigManager,
variantManager: DefaultVariantManager(),
internalUserDecider: internalUserDecider,
dbCoordinator: dbCoordinator,
isAutocompleteEnabledByUser: isAutocompleteEnabledByUser(),
isRecentlyVisitedSitesEnabledByUser: isRecentlyVisitedSitesEnabledByUser())
Expand Down
7 changes: 2 additions & 5 deletions DuckDuckGo/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ import WebKit
}
}

// swiftlint:disable:next function_body_length cyclomatic_complexity
// swiftlint:disable:next function_body_length
func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {

// SKAD4 support
Expand Down Expand Up @@ -237,10 +237,8 @@ import WebKit
variantManager.assignVariantIfNeeded { _ in
// MARK: perform first time launch logic here
DaxDialogs.shared.primeForUse()
historyMessageManager.dismiss()
}

if variantManager.isSupported(feature: .history) {
// New users don't see the message
historyMessageManager.dismiss()
}

Expand Down Expand Up @@ -380,7 +378,6 @@ import WebKit

switch HistoryManager.make(isAutocompleteEnabledByUser: settings.autocomplete,
isRecentlyVisitedSitesEnabledByUser: settings.recentlyVisitedSites,
internalUserDecider: AppDependencyProvider.shared.internalUserDecider,
privacyConfigManager: ContentBlocking.shared.privacyConfigurationManager) {

case .failure(let error):
Expand Down
88 changes: 27 additions & 61 deletions DuckDuckGoTests/HistoryManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,73 +28,43 @@ final class HistoryManagerTests: XCTestCase {

let privacyConfig = MockPrivacyConfiguration()
let privacyConfigManager = MockPrivacyConfigurationManager()
var variantManager = MockVariantManager()
let internalUserStore = MockInternalUserStoring()

func test() {

struct Condition {

let privacyConfig: Bool
let variant: Bool
let inRollOut: Bool
let internalUser: Bool
let expected: Bool

func testWhenEnabledInPrivacyConfig_ThenFeatureIsEnabled() {
privacyConfig.isFeatureKeyEnabled = { feature, _ in
XCTAssertEqual(feature, .history)
return true
}

let conditions = [
// Users in the experiment should get the feature
Condition(privacyConfig: true, variant: true, inRollOut: false, internalUser: false, expected: true),
Condition(privacyConfig: true, variant: true, inRollOut: true, internalUser: false, expected: true),

// If not previously in the experiment then check for the rollout
Condition(privacyConfig: true, variant: false, inRollOut: false, internalUser: false, expected: false),
Condition(privacyConfig: true, variant: false, inRollOut: true, internalUser: false, expected: true),

// Internal users also get the feature
Condition(privacyConfig: true, variant: false, inRollOut: false, internalUser: true, expected: true),
Condition(privacyConfig: true, variant: false, inRollOut: true, internalUser: true, expected: true),

// Privacy config is the ultimate on/off switch though
Condition(privacyConfig: false, variant: true, inRollOut: true, internalUser: true, expected: false),
]

for index in conditions.indices {
let condition = conditions[index]
privacyConfig.isFeatureKeyEnabled = { feature, _ in
XCTAssertEqual(feature, .history)
return condition.privacyConfig
}

privacyConfig.isSubfeatureKeyEnabled = { subFeature, _ in
XCTAssertEqual(subFeature as? HistorySubFeature, HistorySubFeature.onByDefault)
return condition.inRollOut
}

internalUserStore.isInternalUser = condition.internalUser
privacyConfigManager.privacyConfig = privacyConfig
variantManager.isSupportedReturns = condition.variant
let model = CoreDataDatabase.loadModel(from: History.bundle, named: "BrowsingHistory")!
let db = CoreDataDatabase(name: "Test", containerLocation: tempDBDir(), model: model)
db.loadStore()

let model = CoreDataDatabase.loadModel(from: History.bundle, named: "BrowsingHistory")!
let db = CoreDataDatabase(name: "Test", containerLocation: tempDBDir(), model: model)
db.loadStore()
let historyManager = makeHistoryManager(db) {
XCTFail("DB Error \($0)")
}

let historyManager = makeHistoryManager(db) {
XCTFail("DB Error \($0)")
}
XCTAssertTrue(historyManager.isHistoryFeatureEnabled())
XCTAssertTrue(historyManager.historyCoordinator is HistoryCoordinator)
}

let result = historyManager.isHistoryFeatureEnabled()
XCTAssertEqual(condition.expected, result, "\(index): \(condition)")
func testWhenDisabledInPrivacyConfig_ThenFeatureIsDisabled() {
privacyConfig.isFeatureKeyEnabled = { feature, _ in
XCTAssertEqual(feature, .history)
return false
}

privacyConfigManager.privacyConfig = privacyConfig

if condition.expected {
XCTAssertTrue(historyManager.historyCoordinator is HistoryCoordinator)
} else {
XCTAssertTrue(historyManager.historyCoordinator is NullHistoryCoordinator)
}
let model = CoreDataDatabase.loadModel(from: History.bundle, named: "BrowsingHistory")!
let db = CoreDataDatabase(name: "Test", containerLocation: tempDBDir(), model: model)
db.loadStore()

let historyManager = makeHistoryManager(db) {
XCTFail("DB Error \($0)")
}

XCTAssertFalse(historyManager.isHistoryFeatureEnabled())
XCTAssertTrue(historyManager.historyCoordinator is NullHistoryCoordinator)
}

func test_WhenUserHasDisabledAutocompleteSitesSetting_ThenDontStoreOrLoadHistory() {
Expand All @@ -104,7 +74,6 @@ final class HistoryManagerTests: XCTestCase {
return true
}

internalUserStore.isInternalUser = true
privacyConfigManager.privacyConfig = privacyConfig
autocompleteEnabledByUser = false

Expand All @@ -126,7 +95,6 @@ final class HistoryManagerTests: XCTestCase {
return true
}

internalUserStore.isInternalUser = true
privacyConfigManager.privacyConfig = privacyConfig
recentlyVisitedSitesEnabledByUser = false

Expand All @@ -147,8 +115,6 @@ final class HistoryManagerTests: XCTestCase {
let dbCoordinator = HistoryCoordinator(historyStoring: store)

return HistoryManager(privacyConfigManager: privacyConfigManager,
variantManager: variantManager,
internalUserDecider: DefaultInternalUserDecider(mockedStore: internalUserStore),
dbCoordinator: dbCoordinator,
isAutocompleteEnabledByUser: self.autocompleteEnabledByUser,
isRecentlyVisitedSitesEnabledByUser: self.recentlyVisitedSitesEnabledByUser)
Expand Down

0 comments on commit 3610c70

Please sign in to comment.