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-13433: Import login flow from settings #1066

Merged
merged 4 commits into from
Oct 23, 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
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,14 @@ extension AppCoordinator: SendItemDelegate {
// MARK: - SettingsCoordinatorDelegate

extension AppCoordinator: SettingsCoordinatorDelegate {
func didCompleteLoginsImport() {
navigate(to: .tab(.vault(.list)))
showToast(
Localizations.loginsImported,
subtitle: Localizations.rememberToDeleteYourImportedPasswordFileFromYourComputer
)
}

func didDeleteAccount() {
Task {
await handleAuthEvent(.didDeleteAccount)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ class AppCoordinatorTests: BitwardenTestCase { // swiftlint:disable:this type_bo
XCTAssertNil(subject.authCompletionRoute)
}

/// `didCompleteLoginsImport()` navigates to the vault list.
@MainActor
func test_didCompleteLoginsImport() {
subject.didCompleteLoginsImport()
XCTAssertTrue(module.tabCoordinator.isStarted)
XCTAssertEqual(module.tabCoordinator.routes, [.vault(.list)])
}

/// `didDeleteAccount(otherAccounts:)` navigates to the `didDeleteAccount` route.
@MainActor
func test_didDeleteAccount() throws {
Expand Down
13 changes: 13 additions & 0 deletions BitwardenShared/UI/Platform/Application/AppModuleTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,19 @@ class AppModuleTests: BitwardenTestCase {
XCTAssertTrue(navigationController.viewControllers[0] is UIHostingController<ExtensionActivationView>)
}

/// `makeImportLoginsCoordinator` builds the import logins coordinator.
@MainActor
func test_makeImportLoginsCoordinator() {
let navigationController = UINavigationController()
let coordinator = subject.makeImportLoginsCoordinator(
delegate: MockImportLoginsCoordinatorDelegate(),
stackNavigator: navigationController
)
coordinator.navigate(to: .importLogins(.vault))
XCTAssertEqual(navigationController.viewControllers.count, 1)
XCTAssertTrue(navigationController.viewControllers[0] is UIHostingController<ImportLoginsView>)
}

/// `makeSendCoordinator()` builds the send coordinator.
@MainActor
func test_makeSendCoordinator() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1049,3 +1049,5 @@
"NoAccountFoundPleaseLogInAgainIfYouContinueToSeeThisError" = "No account found. Please log in again if you continue to see this error.";
"ImportError" = "Import error";
"NoLoginsWereImported" = "No logins were imported";
"LoginsImported" = "Logins imported";
"RememberToDeleteYourImportedPasswordFileFromYourComputer" = "Remember to delete your imported password file from your computer.";
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ extension Navigator {
///
func showLoadingOverlay(_ state: LoadingOverlayState) {
guard let rootViewController else { return }
LoadingOverlayDisplayHelper.show(in: rootViewController.topmostViewController(), state: state)
LoadingOverlayDisplayHelper.show(in: rootViewController, state: state)
}

/// Hides the loading overlay view.
///
func hideLoadingOverlay() {
guard let rootViewController else { return }
LoadingOverlayDisplayHelper.hide(from: rootViewController.topmostViewController())
LoadingOverlayDisplayHelper.hide(from: rootViewController)
}

/// Shows the toast.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@

// Position the toast view on the window with appropriate bottom padding above the tab bar.
window.addSubview(viewController.view)
let bottomPadding = window.safeAreaInsets.bottom + getSafeArea(from: parentViewController).bottom + 14
let bottomPadding = getSafeArea(from: parentViewController).bottom + 16
viewController.view.translatesAutoresizingMaskIntoConstraints = false
viewController.view.bottomAnchor.constraint(equalTo: window.bottomAnchor, constant: -bottomPadding)
.isActive = true
viewController.view.centerXAnchor.constraint(equalTo: window.centerXAnchor).isActive = true
viewController.view.leadingAnchor.constraint(equalTo: window.leadingAnchor).isActive = true
viewController.view.trailingAnchor.constraint(equalTo: window.trailingAnchor).isActive = true

// Animate the toast in.
UIView.animate(withDuration: UI.duration(transitionDuration)) {
Expand All @@ -46,7 +47,7 @@

// Dismiss the toast after 3 seconds.
Timer.scheduledTimer(withTimeInterval: duration, repeats: false) { _ in
hide(from: parentViewController)
hide(viewController.view)
}
}

Expand All @@ -65,19 +66,16 @@
let selected = tabBarController?.selectedViewController,
let topViewController = (selected as? UINavigationController)?.topViewController,
!topViewController.hidesBottomBarWhenPushed {
let height = tabBar.bounds.height - tabBar.safeAreaInsets.bottom
return UIEdgeInsets(top: 0, left: 0, bottom: height, right: 0)
return UIEdgeInsets(top: 0, left: 0, bottom: tabBar.bounds.height, right: 0)

Check warning on line 69 in BitwardenShared/UI/Platform/Application/Views/ToastDisplayHelper.swift

View check run for this annotation

Codecov / codecov/patch

BitwardenShared/UI/Platform/Application/Views/ToastDisplayHelper.swift#L69

Added line #L69 was not covered by tests
}
return .zero
return parentViewController.view.safeAreaInsets
}

/// Hides the toast from showing over the specified view controller
///
/// - Parameter parentViewController: The parent view controller that the toast is shown in.
/// - Parameter view: The toast view to hide.
///
private static func hide(from parentViewController: UIViewController) {
guard let view = parentViewController.view.window?.viewWithTag(toastTag) else { return }

private static func hide(_ view: UIView) {
UIView.animate(withDuration: UI.duration(transitionDuration)) {
view.layer.opacity = 0
} completion: { _ in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ final class VaultSettingsProcessor: StateProcessor<VaultSettingsState, VaultSett
self.state.url = self.services.environmentService.importItemsURL
})
case .showImportLogins:
// TODO: PM-13467 Navigate to import logins
// coordinator.navigate(to: .importLogins)
break
coordinator.navigate(to: .importLogins)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,12 @@ class VaultSettingsProcessorTests: BitwardenTestCase {
try await alert.tapAction(title: Localizations.continue)
XCTAssertEqual(subject.state.url, environmentService.importItemsURL)
}

/// `receive(_:)` with `.showImportLogins` navigates to the import logins screen.
@MainActor
func test_receive_showImportLogins() {
subject.receive(.showImportLogins)

XCTAssertEqual(coordinator.routes.last, .importLogins)
}
}
34 changes: 33 additions & 1 deletion BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import SwiftUI
///
@MainActor
public protocol SettingsCoordinatorDelegate: AnyObject {
/// Called when the user completes the import navigation flow and should be navigated to the vault tab.
///
func didCompleteLoginsImport()

/// Called when the active user's account has been deleted.
///
func didDeleteAccount()
Expand Down Expand Up @@ -45,6 +49,7 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d

/// The module types required by this coordinator for creating child coordinators.
typealias Module = AuthModule
& ImportLoginsModule
& LoginRequestModule

typealias Services = HasAccountAPIService
Expand Down Expand Up @@ -149,6 +154,8 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d
showExportVault()
case .folders:
showFolders()
case .importLogins:
showImportLogins()
case let .loginRequest(loginRequest):
showLoginRequest(loginRequest, delegate: context as? LoginRequestDelegate)
case .other:
Expand Down Expand Up @@ -350,6 +357,21 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d
stackNavigator?.push(viewController, navigationTitle: Localizations.folders)
}

/// Shows the import login items screen.
///
private func showImportLogins() {
let navigationController = UINavigationController()
navigationController.modalPresentationStyle = .overFullScreen
let coordinator = module.makeImportLoginsCoordinator(
delegate: self,
stackNavigator: navigationController
)
coordinator.start()
coordinator.navigate(to: .importLogins(.settings))

stackNavigator?.present(navigationController)
}

/// Shows the login request.
///
/// - Parameters:
Expand Down Expand Up @@ -446,7 +468,17 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d
}
}

// MARK: SettingsProcessorDelegate
// MARK: - ImportLoginsCoordinatorDelegate

extension SettingsCoordinator: ImportLoginsCoordinatorDelegate {
func didCompleteLoginsImport() {
stackNavigator?.dismiss {
self.delegate?.didCompleteLoginsImport()
}
}
}

// MARK: - SettingsProcessorDelegate

extension SettingsCoordinator: SettingsProcessorDelegate {
func updateSettingsTabBadge(_ badgeValue: String?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ class SettingsCoordinatorTests: BitwardenTestCase {

// MARK: Tests

/// `didCompleteLoginsImport()` notifies the delegate that the user completed importing their
/// logins and dismisses the import logins flow.
@MainActor
func test_didCompleteLoginsImport() throws {
subject.didCompleteLoginsImport()

XCTAssertTrue(delegate.didCompleteLoginsImportCalled)
let action = try XCTUnwrap(stackNavigator.actions.last)
XCTAssertEqual(action.type, .dismissedWithCompletionHandler)
}

/// `navigate(to:)` with `.about` pushes the about view onto the stack navigator.
@MainActor
func test_navigateTo_about() throws {
Expand Down Expand Up @@ -165,6 +176,18 @@ class SettingsCoordinatorTests: BitwardenTestCase {
XCTAssertTrue(navigationController.viewControllers.first is UIHostingController<ExportVaultView>)
}

/// `navigate(to:)` with `.importLogins` presents the import logins flow.
@MainActor
func test_navigateTo_importLogins() throws {
subject.navigate(to: .importLogins)

let action = try XCTUnwrap(stackNavigator.actions.last)
XCTAssertEqual(action.type, .presented)
XCTAssertTrue(action.view is UINavigationController)
XCTAssertTrue(module.importLoginsCoordinator.isStarted)
XCTAssertEqual(module.importLoginsCoordinator.routes.last, .importLogins(.settings))
}

/// `navigate(to:)` with `.lockVault` navigates the user to the login view.
@MainActor
func test_navigateTo_lockVault() async throws {
Expand Down Expand Up @@ -331,6 +354,7 @@ class SettingsCoordinatorTests: BitwardenTestCase {
}

class MockSettingsCoordinatorDelegate: SettingsCoordinatorDelegate {
var didCompleteLoginsImportCalled = false
var didDeleteAccountCalled = false
var didLockVaultCalled = false
var didLogoutCalled = false
Expand All @@ -341,6 +365,10 @@ class MockSettingsCoordinatorDelegate: SettingsCoordinatorDelegate {
var wasLogoutUserInitiated: Bool?
var wasSwitchAutomatic: Bool?

func didCompleteLoginsImport() {
didCompleteLoginsImportCalled = true
}

func didDeleteAccount() {
didDeleteAccountCalled = true
}
Expand Down
3 changes: 3 additions & 0 deletions BitwardenShared/UI/Platform/Settings/SettingsRoute.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ public enum SettingsRoute: Equatable, Hashable {
/// A route to view the folders in the vault.
case folders

/// A route to the import logins screen.
case importLogins

/// A route to view a login request.
///
/// - Parameter loginRequest: The login request to display.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class ImportLoginsProcessor: StateProcessor<ImportLoginsState, ImportLoginsActio
// MARK: Private Properties

/// The coordinator that handles navigation.
private let coordinator: AnyCoordinator<VaultRoute, AuthAction>
private let coordinator: AnyCoordinator<ImportLoginsRoute, ImportLoginsEvent>

/// The services used by this processor.
private let services: Services
Expand All @@ -28,7 +28,7 @@ class ImportLoginsProcessor: StateProcessor<ImportLoginsState, ImportLoginsActio
/// - state: The initial state of the processor.
///
init(
coordinator: AnyCoordinator<VaultRoute, AuthAction>,
coordinator: AnyCoordinator<ImportLoginsRoute, ImportLoginsEvent>,
services: Services,
state: ImportLoginsState
) {
Expand Down Expand Up @@ -135,7 +135,6 @@ class ImportLoginsProcessor: StateProcessor<ImportLoginsState, ImportLoginsActio

do {
try await services.settingsRepository.fetchSync()
coordinator.hideLoadingOverlay()

guard try await !services.vaultRepository.isVaultEmpty() else {
showImportLoginsEmptyAlert()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import XCTest
class ImportLoginsProcessorTests: BitwardenTestCase {
// MARK: Properties

var coordinator: MockCoordinator<VaultRoute, AuthAction>!
var coordinator: MockCoordinator<ImportLoginsRoute, ImportLoginsEvent>!
var errorReporter: MockErrorReporter!
var settingsRepository: MockSettingsRepository!
var stateService: MockStateService!
Expand All @@ -31,7 +31,7 @@ class ImportLoginsProcessorTests: BitwardenTestCase {
stateService: stateService,
vaultRepository: vaultRepository
),
state: ImportLoginsState()
state: ImportLoginsState(mode: .vault)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,19 @@

/// An object that defines the current state of a `ImportLoginsView`.
///
struct ImportLoginsState: Equatable, Sendable {
public struct ImportLoginsState: Equatable, Sendable {
// MARK: Types

/// The modes that define where the import logins flow started from.
///
public enum Mode: Equatable, Sendable {
/// Import logins from the app's settings.
case settings

/// Import logins from the vault list.
case vault
}

/// An enumeration of the instruction pages that the user can navigate between.
///
enum Page: Int {
Expand All @@ -26,9 +36,19 @@ struct ImportLoginsState: Equatable, Sendable {

// MARK: Properties

/// The mode of the view based on where the import logins flow was started from.
var mode: Mode

/// The current page.
var page = Page.intro

/// The hostname of the web vault URL.
var webVaultHost = Constants.defaultWebVaultHost

// MARK: Computed Properties

/// Whether the import logins later button should be shown.
var shouldShowImportLoginsLater: Bool {
mode == .vault
}
}
Loading
Loading