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

PM-12736: Dismiss badge for autofill once enabled #1020

Merged
merged 1 commit into from
Oct 9, 2024
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
74 changes: 46 additions & 28 deletions BitwardenShared/UI/Platform/Application/AppProcessor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,17 @@ public class AppProcessor {
/// - Parameters:
/// - appExtensionDelegate: A delegate used to communicate with the app extension.
/// - appModule: The root module to use to create sub-coordinators.
/// - debugDidEnterBackground: A closure that is called in debug builds for testing after the
/// processor finishes its work when the app enters the background.
/// - debugWillEnterForeground: A closure that is called in debug builds for testing after the
/// processor finishes its work when the app enters the foreground.
/// - services: The services used by the app.
///
public init(
appExtensionDelegate: AppExtensionDelegate? = nil,
appModule: AppModule,
debugDidEnterBackground: (() -> Void)? = nil,
debugWillEnterForeground: (() -> Void)? = nil,
services: ServiceContainer
) {
self.appExtensionDelegate = appExtensionDelegate
Expand All @@ -77,6 +83,10 @@ public class AppProcessor {
for await _ in services.notificationCenterService.willEnterForegroundPublisher() {
startEventTimer()
await checkAccountsForTimeout()
await completeAutofillAccountSetupIfEnabled()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

โ„น๏ธ This was the primary change here, adding this check when the app is foregrounded.

#if DEBUG
debugWillEnterForeground?()
#endif
}
}

Expand All @@ -91,6 +101,9 @@ public class AppProcessor {
} catch {
services.errorReporter.log(error: error)
}
#if DEBUG
debugDidEnterBackground?()
#endif
}
}
}
Expand Down Expand Up @@ -166,10 +179,10 @@ public class AppProcessor {
/// - Parameter incomingURL: The URL handled from AppLinks.
///
public func handleAppLinks(incomingURL: URL) {
guard let sanatizedUrl = URL(
guard let sanitizedUrl = URL(
string: incomingURL.absoluteString.replacingOccurrences(of: "/redirect-connector.html#", with: "/")
),
let components = URLComponents(url: sanatizedUrl, resolvingAgainstBaseURL: true) else {
let components = URLComponents(url: sanitizedUrl, resolvingAgainstBaseURL: true) else {
return
}

Expand Down Expand Up @@ -202,27 +215,6 @@ public class AppProcessor {

// MARK: Autofill Methods

/// If the native create account feature flag and the autofill extension are enabled, this marks
/// any user's autofill account setup completed. This should be called on app startup.
///
func completeAutofillAccountSetupIfEnabled() async {
guard await services.configService.getFeatureFlag(.nativeCreateAccountFlow),
await services.autofillCredentialService.isAutofillCredentialsEnabled()
else { return }
do {
let accounts = try await services.stateService.getAccounts()
for account in accounts {
let userId = account.profile.userId
guard let progress = await services.stateService.getAccountSetupAutofill(userId: userId),
progress != .complete
else { continue }
try await services.stateService.setAccountSetupAutofill(.complete, userId: userId)
}
} catch {
services.errorReporter.log(error: error)
}
}

/// Returns a `ASPasswordCredential` that matches the user-requested credential which can be
/// used for autofill.
///
Expand Down Expand Up @@ -284,6 +276,11 @@ public class AppProcessor {
coordinator?.showAlert(alert)
}

/// Show the debug menu.
public func showDebugMenu() {
coordinator?.navigate(to: .debugMenu)
}

// MARK: Notification Methods

/// Called when the app has registered for push notifications.
Expand Down Expand Up @@ -322,7 +319,9 @@ public class AppProcessor {
notificationTapped: notificationTapped
)
}
}

extension AppProcessor {
// MARK: Private Methods

/// Checks if any accounts have timed out.
Expand Down Expand Up @@ -357,6 +356,30 @@ public class AppProcessor {
}
}

/// If the native create account feature flag and the autofill extension are enabled, this marks
/// any user's autofill account setup completed. This should be called on app startup.
///
private func completeAutofillAccountSetupIfEnabled() async {
// Don't mark the user's progress as complete in the extension, otherwise the app may not
// see that the user's progress needs to be updated to publish new values to subscribers.
guard appExtensionDelegate?.isInAppExtension != true,
await services.configService.getFeatureFlag(.nativeCreateAccountFlow),
await services.autofillCredentialService.isAutofillCredentialsEnabled()
else { return }
do {
let accounts = try await services.stateService.getAccounts()
for account in accounts {
let userId = account.profile.userId
guard let progress = await services.stateService.getAccountSetupAutofill(userId: userId),
progress != .complete
else { continue }
try await services.stateService.setAccountSetupAutofill(.complete, userId: userId)
}
} catch {
services.errorReporter.log(error: error)
}
}

/// Starts timer to send organization events regularly
private func startEventTimer() {
sendEventTimer = Timer.scheduledTimer(withTimeInterval: 5 * 60, repeats: true) { _ in
Expand Down Expand Up @@ -398,11 +421,6 @@ public class AppProcessor {
backgroundTaskId = nil
}
}

/// Show the debug menu.
public func showDebugMenu() {
coordinator?.navigate(to: .debugMenu)
}
}

// MARK: - NotificationServiceDelegate
Expand Down
67 changes: 65 additions & 2 deletions BitwardenShared/UI/Platform/Application/AppProcessorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
var vaultRepository: MockVaultRepository!
var vaultTimeoutService: MockVaultTimeoutService!

var didEnterBackgroundCalled = 0
var willEnterForegroundCalled = 0

// MARK: Setup & Teardown

override func setUp() {
Expand Down Expand Up @@ -57,6 +60,8 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body

subject = AppProcessor(
appModule: appModule,
debugDidEnterBackground: { [weak self] in self?.didEnterBackgroundCalled += 1 },
debugWillEnterForeground: { [weak self] in self?.willEnterForegroundCalled += 1 },
services: ServiceContainer.withMocks(
authRepository: authRepository,
autofillCredentialService: autofillCredentialService,
Expand Down Expand Up @@ -218,6 +223,26 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertFalse(authRepository.logoutUserInitiated)
}

/// `init()` subscribes to will enter foreground events ands completes the user's autofill setup
/// process if autofill is enabled and they previously choose to set it up later.
@MainActor
func test_init_appForeground_completeAutofillAccountSetup() async throws {
// The processor checks for autofill completion when entering the foreground. Wait for the
// initial check to finish when the test starts before continuing.
try await waitForAsync { self.willEnterForegroundCalled == 1 }

autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
stateService.accounts = [.fixture()]
stateService.accountSetupAutofill["1"] = .setUpLater

notificationCenterService.willEnterForegroundSubject.send()
try await waitForAsync { self.willEnterForegroundCalled == 2 }

XCTAssertEqual(stateService.accountSetupAutofill, ["1": .complete])
}

/// `init()` sets the `AppProcessor` as the delegate of any necessary services.
func test_init_setDelegates() {
XCTAssertIdentical(notificationService.delegate, subject)
Expand Down Expand Up @@ -641,6 +666,36 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(migrationService.didPerformMigrations, true)
}

/// `start(navigator:)` doesn't complete the accounts autofill setup when running in an app extension.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_appExtension() async throws {
let delegate = MockAppExtensionDelegate()
delegate.isInAppExtension = true
var willEnterForegroundCalled = 0
let subject = AppProcessor(
appExtensionDelegate: delegate,
appModule: appModule,
debugWillEnterForeground: { willEnterForegroundCalled += 1 },
services: ServiceContainer.withMocks(
autofillCredentialService: autofillCredentialService,
configService: configService,
stateService: stateService
)
)
try await waitForAsync { willEnterForegroundCalled == 1 }

autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
stateService.accounts = [.fixture()]
stateService.accountSetupAutofill["1"] = .setUpLater

let rootNavigator = MockRootNavigator()
await subject.start(appContext: .mainApp, navigator: rootNavigator, window: nil)

XCTAssertEqual(stateService.accountSetupAutofill, ["1": .setUpLater])
}

/// `start(navigator:)` doesn't complete the accounts autofill setup if autofill is disabled.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_autofillDisabled() async {
Expand Down Expand Up @@ -674,7 +729,11 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body

/// `start(navigator:)` logs an error if one occurs while updating the account's autofill setup.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_error() async {
func test_start_completeAutofillAccountSetupIfEnabled_error() async throws {
// The processor checks for autofill completion when entering the foreground. Wait for the
// initial check to finish when the test starts before continuing.
try await waitForAsync { self.willEnterForegroundCalled == 1 }

autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.accounts = [.fixture()]
Expand Down Expand Up @@ -706,7 +765,11 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
/// `start(navigator:)` completes the user's autofill setup progress if autofill is enabled and
/// they previously choose to set it up later.
@MainActor
func test_start_completeAutofillAccountSetupIfEnabled_success() async {
func test_start_completeAutofillAccountSetupIfEnabled_success() async throws {
// The processor checks for autofill completion when entering the foreground. Wait for the
// initial check to finish when the test starts before continuing.
try await waitForAsync { self.willEnterForegroundCalled == 1 }

autofillCredentialService.isAutofillCredentialsEnabled = true
configService.featureFlagsBool[.nativeCreateAccountFlow] = true
stateService.activeAccount = .fixture()
Expand Down
Loading