Skip to content

Commit

Permalink
Decouple NTP settings persistence from AppUserDefaults (#3354)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/72649045549333/1208273840160821/f
Tech Design URL:
CC:

**Description**:

Extract NTP data persistence into smaller protocols and use
`UserDefaultsWrapper` to implement the actual storing. The change allows
for better separation of concerns and potentially storing data in a
different way on a case-by-case basis.

**Steps to test this PR**:
1. Verify NTP sections and shortcuts settings (order, enabled state) are
preserved across app instances.
2. Verify intro message is dismissed after 3 views (it can be set up
from Debug menu).

**Definition of Done (Internal Only)**:

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

---
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
  • Loading branch information
dus7 authored Sep 17, 2024
1 parent 84f3ac5 commit 7669a95
Show file tree
Hide file tree
Showing 18 changed files with 161 additions and 90 deletions.
8 changes: 8 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,8 @@
6FD8E5202C5BA23200345670 /* NewTabPageModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FD8E51F2C5BA23200345670 /* NewTabPageModel.swift */; };
6FD8E5222C5BA5C400345670 /* NewTabPageIntroMessageSetup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FD8E5212C5BA5C400345670 /* NewTabPageIntroMessageSetup.swift */; };
6FDA1FB32B59584400AC962A /* AddressDisplayHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FDA1FB22B59584400AC962A /* AddressDisplayHelper.swift */; };
6FDC64012C92F4A300DB71B3 /* NewTabPageIntroDataStoring.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FDC64002C92F4A300DB71B3 /* NewTabPageIntroDataStoring.swift */; };
6FDC64032C92F4D600DB71B3 /* NewTabPageSettingsPersistentStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FDC64022C92F4D600DB71B3 /* NewTabPageSettingsPersistentStore.swift */; };
6FE018402C25CB3F001F680D /* FavoritesSectionHeader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FE0183F2C25CB3F001F680D /* FavoritesSectionHeader.swift */; };
6FE095D82BD90AFB00490FF8 /* UniversalOmniBarState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FE095D72BD90AFB00490FF8 /* UniversalOmniBarState.swift */; };
6FE127382C20492500EB5724 /* NewTabPage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FE127372C20492500EB5724 /* NewTabPage.swift */; };
Expand Down Expand Up @@ -1607,6 +1609,8 @@
6FD8E51F2C5BA23200345670 /* NewTabPageModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageModel.swift; sourceTree = "<group>"; };
6FD8E5212C5BA5C400345670 /* NewTabPageIntroMessageSetup.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageIntroMessageSetup.swift; sourceTree = "<group>"; };
6FDA1FB22B59584400AC962A /* AddressDisplayHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddressDisplayHelper.swift; sourceTree = "<group>"; };
6FDC64002C92F4A300DB71B3 /* NewTabPageIntroDataStoring.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageIntroDataStoring.swift; sourceTree = "<group>"; };
6FDC64022C92F4D600DB71B3 /* NewTabPageSettingsPersistentStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageSettingsPersistentStore.swift; sourceTree = "<group>"; };
6FE0183F2C25CB3F001F680D /* FavoritesSectionHeader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FavoritesSectionHeader.swift; sourceTree = "<group>"; };
6FE095D72BD90AFB00490FF8 /* UniversalOmniBarState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UniversalOmniBarState.swift; sourceTree = "<group>"; };
6FE127372C20492500EB5724 /* NewTabPage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPage.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3844,6 +3848,7 @@
6F5345AE2C53F2DE00424A43 /* NewTabPageSettingsPersistentStorage.swift */,
6F9FFE252C579BCD00A238BE /* NewTabPageShortcutsSettingsStorage.swift */,
6F9FFE272C579DEA00A238BE /* NewTabPageSectionsSettingsStorage.swift */,
6FDC64022C92F4D600DB71B3 /* NewTabPageSettingsPersistentStore.swift */,
);
name = Storage;
sourceTree = "<group>";
Expand Down Expand Up @@ -3912,6 +3917,7 @@
children = (
6FD8E51D2C5B84DE00345670 /* NewTabPageIntroMessageView.swift */,
6FD8E5212C5BA5C400345670 /* NewTabPageIntroMessageSetup.swift */,
6FDC64002C92F4A300DB71B3 /* NewTabPageIntroDataStoring.swift */,
);
name = IntroMessage;
sourceTree = "<group>";
Expand Down Expand Up @@ -7299,6 +7305,7 @@
F4E1936625AF722F001D2666 /* HighlightCutOutView.swift in Sources */,
6FB2A67C2C2D9DF0004D20C8 /* FavoritesEmptyStateView.swift in Sources */,
1E162605296840D80004127F /* Triangle.swift in Sources */,
6FDC64012C92F4A300DB71B3 /* NewTabPageIntroDataStoring.swift in Sources */,
B609D5522862EAFF0088CAC2 /* InlineWKDownloadDelegate.swift in Sources */,
BDFF03222BA3D8E200F324C9 /* NetworkProtectionFeatureVisibility.swift in Sources */,
B652DEFD287BE67400C12A9C /* UserScripts.swift in Sources */,
Expand Down Expand Up @@ -7379,6 +7386,7 @@
6FB2A67A2C2C5BAE004D20C8 /* FavoriteEmptyStateItem.swift in Sources */,
6FBF0F8B2BD7C0A900136CF0 /* AllProtectedCell.swift in Sources */,
9F4CC5242C4A4F0D006A96EB /* SwiftUITestUtilities.swift in Sources */,
6FDC64032C92F4D600DB71B3 /* NewTabPageSettingsPersistentStore.swift in Sources */,
1E4F4A5A297193DE00625985 /* MainViewController+CookiesManaged.swift in Sources */,
C12324C32C4697C900FBB26B /* AutofillBreakageReportTableViewCell.swift in Sources */,
8586A10D24CBA7070049720E /* FindInPageActivity.swift in Sources */,
Expand Down
6 changes: 0 additions & 6 deletions DuckDuckGo/AppSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,8 @@ protocol AppSettings: AnyObject, AppDebugSettings {

var duckPlayerMode: DuckPlayerMode { get set }
var duckPlayerAskModeOverlayHidden: Bool { get set }

var newTabPageShortcutsSettings: Data? { get set }
var newTabPageSectionsSettings: Data? { get set }
var newTabPageIntroMessageEnabled: Bool? { get set }
var newTabPageIntroMessageSeenCount: Int { get set }
}

protocol AppDebugSettings {
var newTabPageSectionsEnabled: Bool { get set }
var onboardingHighlightsEnabled: Bool { get set }
}
15 changes: 0 additions & 15 deletions DuckDuckGo/AppUserDefaults.swift
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,6 @@ public class AppUserDefaults: AppSettings {
userDefaults?.setValue(newValue.rawValue, forKey: Keys.crashCollectionOptInStatus)
}
}

@UserDefaultsWrapper(key: .debugNewTabPageSectionsEnabledKey, defaultValue: false)
var newTabPageSectionsEnabled: Bool

var duckPlayerMode: DuckPlayerMode {
get {
Expand Down Expand Up @@ -418,18 +415,6 @@ public class AppUserDefaults: AppSettings {
}
}

@UserDefaultsWrapper(key: .newTabPageShortcutsSettings, defaultValue: nil)
var newTabPageShortcutsSettings: Data?

@UserDefaultsWrapper(key: .newTabPageSectionsSettings, defaultValue: nil)
var newTabPageSectionsSettings: Data?

@UserDefaultsWrapper(key: .newTabPageIntroMessageEnabled, defaultValue: nil)
var newTabPageIntroMessageEnabled: Bool?

@UserDefaultsWrapper(key: .newTabPageIntroMessageSeenCount, defaultValue: 0)
var newTabPageIntroMessageSeenCount: Int

@UserDefaultsWrapper(key: .debugOnboardingHighlightsEnabledKey, defaultValue: false)
var onboardingHighlightsEnabled: Bool
}
Expand Down
34 changes: 34 additions & 0 deletions DuckDuckGo/NewTabPageIntroDataStoring.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//
// NewTabPageIntroDataStoring.swift
// DuckDuckGo
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import Core

protocol NewTabPageIntroDataStoring: AnyObject {
var newTabPageIntroMessageEnabled: Bool? { get set }
var newTabPageIntroMessageSeenCount: Int { get set }
}

final class NewTabPageIntroDataUserDefaultsStorage: NewTabPageIntroDataStoring {
@UserDefaultsWrapper(key: .newTabPageIntroMessageEnabled, defaultValue: nil)
var newTabPageIntroMessageEnabled: Bool?

@UserDefaultsWrapper(key: .newTabPageIntroMessageSeenCount, defaultValue: 0)
var newTabPageIntroMessageSeenCount: Int
}
10 changes: 5 additions & 5 deletions DuckDuckGo/NewTabPageIntroMessageSetup.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,24 @@ import BrowserServicesKit
import Core

struct NewTabPageIntroMessageSetup {
let appSettings: AppSettings
let storage: NewTabPageIntroDataStoring
let statistics: StatisticsStore
let newTabPageManager: NewTabPageManaging

init(appSettings: AppSettings = AppDependencyProvider.shared.appSettings,
init(storage: NewTabPageIntroDataStoring = NewTabPageIntroDataUserDefaultsStorage(),
statistics: StatisticsStore = StatisticsUserDefaults(),
newTabPageManager: NewTabPageManaging = NewTabPageManager()) {
self.appSettings = appSettings
self.storage = storage
self.statistics = statistics
self.newTabPageManager = newTabPageManager
}

func perform() {

let isNotSetUp = appSettings.newTabPageIntroMessageEnabled == nil
let isNotSetUp = storage.newTabPageIntroMessageEnabled == nil
guard newTabPageManager.isAvailableInPublicRelease && isNotSetUp else { return }

// For new users we **don't** want intro message
appSettings.newTabPageIntroMessageEnabled = statistics.installDate != nil
storage.newTabPageIntroMessageEnabled = statistics.installDate != nil
}
}
19 changes: 14 additions & 5 deletions DuckDuckGo/NewTabPageManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,26 @@ protocol NewTabPageDebugging: NewTabPageManaging {
var isFeatureFlagEnabled: Bool { get }
}

protocol NewTabPageLocalFlagStoring: AnyObject {
var newTabPageSectionsEnabled: Bool { get set }
}

final class NewTabPageLocalFlagUserDefaultsStorage: NewTabPageLocalFlagStoring {
@UserDefaultsWrapper(key: .debugNewTabPageSectionsEnabledKey, defaultValue: false)
var newTabPageSectionsEnabled: Bool
}

final class NewTabPageManager: NewTabPageManaging, NewTabPageDebugging {

var appDefaults: AppDebugSettings
let localFlagStorage: NewTabPageLocalFlagStoring
let featureFlagger: FeatureFlagger
let internalUserDecider: InternalUserDecider

init(appDefaults: AppDebugSettings = AppDependencyProvider.shared.appSettings,
init(localFlagStorage: NewTabPageLocalFlagStoring = NewTabPageLocalFlagUserDefaultsStorage(),
featureFlager: FeatureFlagger = AppDependencyProvider.shared.featureFlagger,
internalUserDecider: InternalUserDecider = AppDependencyProvider.shared.internalUserDecider) {

self.appDefaults = appDefaults
self.localFlagStorage = localFlagStorage
self.featureFlagger = featureFlager
self.internalUserDecider = internalUserDecider
}
Expand All @@ -67,10 +76,10 @@ final class NewTabPageManager: NewTabPageManaging, NewTabPageDebugging {

var isLocalFlagEnabled: Bool {
get {
appDefaults.newTabPageSectionsEnabled
localFlagStorage.newTabPageSectionsEnabled
}
set {
appDefaults.newTabPageSectionsEnabled = newValue
localFlagStorage.newTabPageSectionsEnabled = newValue
}
}

Expand Down
16 changes: 8 additions & 8 deletions DuckDuckGo/NewTabPageModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,32 +26,32 @@ final class NewTabPageModel: ObservableObject {
@Published private(set) var isOnboarding: Bool
@Published var isShowingSettings: Bool

private let appSettings: AppSettings
private var introDataStorage: NewTabPageIntroDataStoring
private let pixelFiring: PixelFiring.Type

init(appSettings: AppSettings = AppDependencyProvider.shared.appSettings,
init(introDataStorage: NewTabPageIntroDataStoring = NewTabPageIntroDataUserDefaultsStorage(),
pixelFiring: PixelFiring.Type = Pixel.self) {
self.appSettings = appSettings
self.introDataStorage = introDataStorage
self.pixelFiring = pixelFiring

isIntroMessageVisible = appSettings.newTabPageIntroMessageEnabled ?? false
isIntroMessageVisible = introDataStorage.newTabPageIntroMessageEnabled ?? false
isOnboarding = false
isShowingSettings = false
}

func introMessageDisplayed() {
pixelFiring.fire(.newTabPageMessageDisplayed, withAdditionalParameters: [:])

appSettings.newTabPageIntroMessageSeenCount += 1
if appSettings.newTabPageIntroMessageSeenCount >= 3 {
appSettings.newTabPageIntroMessageEnabled = false
introDataStorage.newTabPageIntroMessageSeenCount += 1
if introDataStorage.newTabPageIntroMessageSeenCount >= 3 {
introDataStorage.newTabPageIntroMessageEnabled = false
}
}

func dismissIntroMessage() {
pixelFiring.fire(.newTabPageMessageDismissed, withAdditionalParameters: [:])

appSettings.newTabPageIntroMessageEnabled = false
introDataStorage.newTabPageIntroMessageEnabled = false
isIntroMessageVisible = false
}

Expand Down
26 changes: 13 additions & 13 deletions DuckDuckGo/NewTabPageSectionsDebugView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import SwiftUI
struct NewTabPageSectionsDebugView: View {

private var newTabPageDebugging: NewTabPageDebugging
private var appSettings: AppSettings
private let introDataStorage: NewTabPageIntroDataStoring

@State private var isFeatureEnabled: Bool
@State private var introMessageCount: Int
@State private var isIntroMessageInitialized: Bool
Expand All @@ -34,24 +34,24 @@ struct NewTabPageSectionsDebugView: View {
} set: {
newTabPageDebugging.isLocalFlagEnabled = $0
isFeatureEnabled = newTabPageDebugging.isNewTabPageSectionsEnabled
isIntroMessageInitialized = appSettings.newTabPageIntroMessageEnabled != nil
isIntroMessageInitialized = introDataStorage.newTabPageIntroMessageEnabled != nil
}
}

private var introMessageEnabled: Binding<Bool> {
Binding {
appSettings.newTabPageIntroMessageEnabled ?? false
introDataStorage.newTabPageIntroMessageEnabled ?? false
} set: {
appSettings.newTabPageIntroMessageEnabled = $0
isIntroMessageInitialized = appSettings.newTabPageIntroMessageEnabled != nil
introDataStorage.newTabPageIntroMessageEnabled = $0
isIntroMessageInitialized = introDataStorage.newTabPageIntroMessageEnabled != nil
}
}

private var introMessageCountBinding: Binding<Int> {
Binding {
appSettings.newTabPageIntroMessageSeenCount
introDataStorage.newTabPageIntroMessageSeenCount
} set: {
appSettings.newTabPageIntroMessageSeenCount = $0
introDataStorage.newTabPageIntroMessageSeenCount = $0
introMessageCount = $0
}
}
Expand All @@ -60,10 +60,10 @@ struct NewTabPageSectionsDebugView: View {
let manager = NewTabPageManager()
newTabPageDebugging = manager
isFeatureEnabled = manager.isNewTabPageSectionsEnabled
appSettings = AppDependencyProvider.shared.appSettings
introMessageCount = appSettings.newTabPageIntroMessageSeenCount
isIntroMessageInitialized = appSettings.newTabPageIntroMessageEnabled != nil

introDataStorage = NewTabPageIntroDataUserDefaultsStorage()
introMessageCount = introDataStorage.newTabPageIntroMessageSeenCount
isIntroMessageInitialized = introDataStorage.newTabPageIntroMessageEnabled != nil
}

var body: some View {
Expand Down Expand Up @@ -134,7 +134,7 @@ struct NewTabPageSectionsDebugView: View {
})

Button("Reset intro message", action: {
appSettings.newTabPageIntroMessageEnabled = nil
introDataStorage.newTabPageIntroMessageEnabled = nil
introMessageCountBinding.wrappedValue = 0
isIntroMessageInitialized = false
})
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/NewTabPageSectionsSettingsStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ typealias NewTabPageSectionsSettingsStorage = NewTabPageSettingsPersistentStorag

extension NewTabPageSettingsPersistentStorage<NewTabPageSection> {
convenience init() {
self.init(keyPath: \.newTabPageSectionsSettings,
self.init(persistentStore: NewTabPageSectionsSettingsStore(),
defaultOrder: NewTabPageSection.allCases,
defaultEnabledItems: NewTabPageSection.allCases)
}
Expand Down
17 changes: 9 additions & 8 deletions DuckDuckGo/NewTabPageSettingsPersistentStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,20 @@ private struct NewTabPageItemSettings<Item: NewTabPageSettingsStorageItem>: Coda
let enabledItems: Set<Item>
}

protocol NewTabPageSettingsPersistentStore: AnyObject {
var data: Data? { get set }
}

final class NewTabPageSettingsPersistentStorage<Item: NewTabPageSettingsStorageItem>: NewTabPageSettingsStorage {
private(set) var itemsOrder: [Item]
private var enabledItems: Set<Item>

private var appSettings: AppSettings
private let keyPath: WritableKeyPath<AppSettings, Data?>
private var persistentStore: any NewTabPageSettingsPersistentStore

init(appSettings: AppSettings = AppDependencyProvider.shared.appSettings,
keyPath: WritableKeyPath<AppSettings, Data?>,
init(persistentStore: NewTabPageSettingsPersistentStore,
defaultOrder: [Item],
defaultEnabledItems: [Item]) {
self.appSettings = appSettings
self.keyPath = keyPath
self.persistentStore = persistentStore
self.itemsOrder = defaultOrder
self.enabledItems = Set(defaultEnabledItems)

Expand All @@ -62,12 +63,12 @@ final class NewTabPageSettingsPersistentStorage<Item: NewTabPageSettingsStorageI
func save() {
let newSettings = NewTabPageItemSettings(itemsOrder: itemsOrder, enabledItems: enabledItems)
if let data = try? JSONEncoder().encode(newSettings) {
appSettings[keyPath: keyPath] = data
persistentStore.data = data
}
}

private func load() {
if let settingsData = appSettings[keyPath: keyPath],
if let settingsData = persistentStore.data,
let settings = try? JSONDecoder().decode(NewTabPageItemSettings<Item>.self, from: settingsData) {
itemsOrder = settings.itemsOrder
enabledItems = settings.enabledItems
Expand Down
31 changes: 31 additions & 0 deletions DuckDuckGo/NewTabPageSettingsPersistentStore.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//
// NewTabPageSettingsPersistentStore.swift
// DuckDuckGo
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import Core

final class NewTabPageShorctutsSettingsStore: NewTabPageSettingsPersistentStore {
@UserDefaultsWrapper(key: .newTabPageShortcutsSettings, defaultValue: nil)
var data: Data?
}

final class NewTabPageSectionsSettingsStore: NewTabPageSettingsPersistentStore {
@UserDefaultsWrapper(key: .newTabPageSectionsSettings, defaultValue: nil)
var data: Data?
}
Loading

0 comments on commit 7669a95

Please sign in to comment.