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-16153: Draw new login action card #1238

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 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
1 change: 1 addition & 0 deletions BitwardenShared/Core/Auth/Services/AuthService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,7 @@ class DefaultAuthService: AuthService { // swiftlint:disable:this type_body_leng
try await stateService.setAccountSetupImportLogins(.incomplete)
}
try await stateService.setAccountSetupVaultUnlock(.incomplete)
await stateService.setLearnNewLoginActionCardStatus(.incomplete)
ezimet-livefront marked this conversation as resolved.
Show resolved Hide resolved
} catch {
errorReporter.log(error: error)
}
Expand Down
20 changes: 20 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,12 @@ protocol StateService: AnyObject {
///
func getShouldTrustDevice(userId: String) async -> Bool?

/// Gets the status of Learn New Login Action Card.
///
/// - Returns: The status of Learn New Login Action Card.
///
func getLearnNewLoginActionCardStatus() async -> AccountSetupProgress?

/// Get whether to show the website icons.
///
/// - Returns: Whether to show the website icons.
Expand Down Expand Up @@ -513,6 +519,12 @@ protocol StateService: AnyObject {
///
func setIntroCarouselShown(_ shown: Bool) async

/// Sets the status of Learn New Login Action Card.
///
/// - Parameter status: The status of Learn New Login Action Card.
///
func setLearnNewLoginActionCardStatus(_ status: AccountSetupProgress) async

/// Sets the last active time within the app.
///
/// - Parameters:
Expand Down Expand Up @@ -1560,6 +1572,10 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
appSettingsStore.shouldTrustDevice(userId: userId)
}

func getLearnNewLoginActionCardStatus() async -> AccountSetupProgress? {
appSettingsStore.learnNewLoginActionCardStatus
}

func getShowWebIcons() async -> Bool {
!appSettingsStore.disableWebIcons
}
Expand Down Expand Up @@ -1769,6 +1785,10 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
appSettingsStore.introCarouselShown = shown
}

func setLearnNewLoginActionCardStatus(_ status: AccountSetupProgress) async {
appSettingsStore.learnNewLoginActionCardStatus = status
}

func setLastActiveTime(_ date: Date?, userId: String?) async throws {
let userId = try userId ?? getActiveAccountUserId()
appSettingsStore.setLastActiveTime(date, userId: userId)
Expand Down
19 changes: 19 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,16 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertTrue(hasShownCarousel)
}

/// `getLearnNewLoginActionCardStatus()` returns the status of the learn new login action card.
func test_getLearnNewLoginActionCardStatus() async {
var learnNewLoginActionCardStatus = await subject.getLearnNewLoginActionCardStatus()
XCTAssertEqual(learnNewLoginActionCardStatus, .incomplete)

appSettingsStore.learnNewLoginActionCardStatus = .complete
learnNewLoginActionCardStatus = await subject.getLearnNewLoginActionCardStatus()
XCTAssertEqual(learnNewLoginActionCardStatus, .complete)
}

/// `getLastActiveTime(userId:)` gets the user's last active time.
func test_getLastActiveTime() async throws {
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))
Expand Down Expand Up @@ -1762,6 +1772,15 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
)
}

/// `setLearnNewLoginActionCardStatus(_:)` sets the learn new login action card status.
func test_setLearnNewLoginActionCardStatus() async {
await subject.setLearnNewLoginActionCardStatus(.incomplete)
XCTAssertEqual(appSettingsStore.learnNewLoginActionCardStatus, .incomplete)

await subject.setLearnNewLoginActionCardStatus(.complete)
XCTAssertEqual(appSettingsStore.learnNewLoginActionCardStatus, .complete)
}

/// `setLoginRequest()` sets the pending login requests.
func test_setLoginRequest() async {
let loginRequest = LoginRequestNotification(id: "1", userId: "10")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ protocol AppSettingsStore: AnyObject {
/// sending the status to the watch if the user is logged out.
var lastUserShouldConnectToWatch: Bool { get set }

/// The status of the learn new login action card.
var learnNewLoginActionCardStatus: AccountSetupProgress { get set }

/// The login request information received from a push notification.
var loginRequest: LoginRequestNotification? { get set }

Expand Down Expand Up @@ -709,6 +712,7 @@ extension DefaultAppSettingsStore: AppSettingsStore {
case encryptedUserKey(userId: String)
case events(userId: String)
case introCarouselShown
case learnNewLoginActionCardStatus
case lastActiveTime(userId: String)
case lastSync(userId: String)
case lastUserShouldConnectToWatch
Expand Down Expand Up @@ -783,6 +787,8 @@ extension DefaultAppSettingsStore: AppSettingsStore {
key = "events_\(userId)"
case .introCarouselShown:
key = "introCarouselShown"
case .learnNewLoginActionCardStatus:
key = "learnNewLoginActionCardStatus"
case let .lastActiveTime(userId):
key = "lastActiveTime_\(userId)"
case let .lastSync(userId):
Expand Down Expand Up @@ -876,6 +882,11 @@ extension DefaultAppSettingsStore: AppSettingsStore {
set { store(newValue, for: .introCarouselShown) }
}

var learnNewLoginActionCardStatus: AccountSetupProgress {
get { fetch(for: .learnNewLoginActionCardStatus) ?? .incomplete }
set { store(newValue, for: .learnNewLoginActionCardStatus) }
}

var lastUserShouldConnectToWatch: Bool {
get { fetch(for: .lastUserShouldConnectToWatch) }
set { store(newValue, for: .lastUserShouldConnectToWatch) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,29 @@ class AppSettingsStoreTests: BitwardenTestCase { // swiftlint:disable:this type_
XCTAssertFalse(subject.isBiometricAuthenticationEnabled(userId: "1"))
}

/// `learnNewLoginActionCardStatus` returns `.incomplete` if there isn't a previously stored value.
func test_learnNewLoginActionCardStatus_isInitiallyIncomplete() {
XCTAssertEqual(subject.learnNewLoginActionCardStatus, .incomplete)
}

/// `learnNewLoginActionCardStatus` can be used to get and set the persisted value in user defaults.
func test_learnNewLoginActionCardStatus_withValues() {
subject.learnNewLoginActionCardStatus = .complete
XCTAssertEqual(subject.learnNewLoginActionCardStatus, .complete)

try XCTAssertEqual(
JSONDecoder().decode(
AccountSetupProgress.self,
from: XCTUnwrap(
userDefaults
.string(forKey: "bwPreferencesStorage:learnNewLoginActionCardStatus")?
.data(using: .utf8)
)
),
AccountSetupProgress.complete
)
}

/// `lastUserShouldConnectToWatch` returns `false` if there isn't a previously stored value.
func test_lastUserShouldConnectToWatch_isInitiallyFalse() {
XCTAssertFalse(subject.lastUserShouldConnectToWatch)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class MockAppSettingsStore: AppSettingsStore { // swiftlint:disable:this type_bo
var disableWebIcons = false
var introCarouselShown = false
var lastUserShouldConnectToWatch = false
var learnNewLoginActionCardStatus: AccountSetupProgress = .incomplete
var loginRequest: LoginRequestNotification?
var migrationVersion = 0
var overrideDebugFeatureFlagCalled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
var isAuthenticated = [String: Bool]()
var isAuthenticatedError: Error?
var lastActiveTime = [String: Date]()
var learnNewLoginActionCardStatus: AccountSetupProgress?
var loginRequest: LoginRequestNotification?
var logoutAccountUserInitiated = false
var getAccountEncryptionKeysError: Error?
Expand Down Expand Up @@ -250,6 +251,10 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
introCarouselShown
}

func getLearnNewLoginActionCardStatus() async -> AccountSetupProgress? {
learnNewLoginActionCardStatus
}

func getLastActiveTime(userId: String?) async throws -> Date? {
let userId = try unwrapUserId(userId)
return lastActiveTime[userId]
Expand Down Expand Up @@ -505,6 +510,10 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
isAuthenticated[account.profile.userId] = true
}

func setLearnNewLoginActionCardStatus(_ status: AccountSetupProgress) async {
learnNewLoginActionCardStatus = status
}

func setLastActiveTime(_ date: Date?, userId: String?) async throws {
let userId = try unwrapUserId(userId)
lastActiveTime[userId] = timeProvider.presentTime
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@
"PossibleMatchingItems" = "Possible matching items";
"Search" = "Search";
"BitwardenAutofillServiceSearch" = "You are searching for an autofill item for \"%1$@\".";
"LearnAboutNewLogins" = "Learn about new logins";
"WeLlWalkYouThroughTheKeyFeaturesToAddANewLogin" = "Weโ€™ll walk you through the key features to add a new login.";
ezimet-livefront marked this conversation as resolved.
Show resolved Hide resolved
"LearnOrg" = "Learn about organizations";
"CannotOpenApp" = "Cannot open the app \"%1$@\".";
"AuthenticatorAppTitle" = "Authenticator app";
Expand Down
5 changes: 4 additions & 1 deletion BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,15 @@ final class VaultCoordinator: Coordinator, HasStackNavigator {
case let .addItem(allowTypeSelection, group, newCipherOptions):
Task {
let hasPremium = try? await services.vaultRepository.doesActiveAccountHavePremium()
let showLearnNewLoginActionCard = await services.stateService
.getLearnNewLoginActionCardStatus() == .incomplete
ezimet-livefront marked this conversation as resolved.
Show resolved Hide resolved
showVaultItem(
route: .addItem(
allowTypeSelection: allowTypeSelection,
group: group,
hasPremium: hasPremium ?? false,
newCipherOptions: newCipherOptions
newCipherOptions: newCipherOptions,
shouldShowLearnNewLoginActionCard: showLearnNewLoginActionCard
),
delegate: context as? CipherItemOperationDelegate
)
Expand Down
40 changes: 38 additions & 2 deletions BitwardenShared/UI/Vault/Vault/VaultCoordinatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class VaultCoordinatorTests: BitwardenTestCase {
var errorReporter: MockErrorReporter!
var module: MockAppModule!
var stackNavigator: MockStackNavigator!
var stateService: MockStateService!
ezimet-livefront marked this conversation as resolved.
Show resolved Hide resolved
var subject: VaultCoordinator!
var vaultRepository: MockVaultRepository!

Expand All @@ -23,13 +24,18 @@ class VaultCoordinatorTests: BitwardenTestCase {
errorReporter = MockErrorReporter()
delegate = MockVaultCoordinatorDelegate()
module = MockAppModule()
stateService = MockStateService()
stackNavigator = MockStackNavigator()
vaultRepository = MockVaultRepository()
subject = VaultCoordinator(
appExtensionDelegate: MockAppExtensionDelegate(),
delegate: delegate,
module: module,
services: ServiceContainer.withMocks(errorReporter: errorReporter, vaultRepository: vaultRepository),
services: ServiceContainer.withMocks(
errorReporter: errorReporter,
stateService: stateService,
vaultRepository: vaultRepository
),
stackNavigator: stackNavigator
)
}
Expand Down Expand Up @@ -103,7 +109,37 @@ class VaultCoordinatorTests: BitwardenTestCase {
let action = try XCTUnwrap(stackNavigator.actions.last)
XCTAssertEqual(action.type, .presented)
XCTAssertTrue(module.vaultItemCoordinator.isStarted)
XCTAssertEqual(module.vaultItemCoordinator.routes.last, .addItem(hasPremium: true))
XCTAssertEqual(
module.vaultItemCoordinator.routes.last,
.addItem(
hasPremium: true,
shouldShowLearnNewLoginActionCard: false
)
)
}

/// `navigate(to:)` with `.addItem` presents the add item view onto the stack navigator.
@MainActor
func test_navigateTo_addItem_showLearnNewLoginActionCard() throws {
stateService.accounts = [Account.fixtureAccountLogin()]
stateService.learnNewLoginActionCardStatus = .incomplete
vaultRepository.isVaultEmptyResult = .success(true)
let coordinator = MockCoordinator<VaultItemRoute, VaultItemEvent>()
module.vaultItemCoordinator = coordinator
subject.navigate(to: .addItem())

waitFor(!stackNavigator.actions.isEmpty)

let action = try XCTUnwrap(stackNavigator.actions.last)
XCTAssertEqual(action.type, .presented)
XCTAssertTrue(module.vaultItemCoordinator.isStarted)
XCTAssertEqual(
module.vaultItemCoordinator.routes.last,
.addItem(
hasPremium: true,
shouldShowLearnNewLoginActionCard: true
)
)
}

/// `.navigate(to:)` with `.editItem` presents the edit item screen.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ enum AddEditItemEffect {
/// The delete option was pressed.
case deletePressed

/// The user tapped the dismiss button on the new login action card.
case dismissNewLoginActionCard

/// Any options that need to be loaded for a cipher (e.g. organizations and folders) should be fetched.
case fetchCipherOptions

Expand All @@ -24,4 +27,7 @@ enum AddEditItemEffect {

/// The setup totp button was pressed.
case setupTotpPressed

/// Show the learn new login guided tour.
case showLearnNewLoginGuidedTour
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ final class AddEditItemProcessor: StateProcessor<// swiftlint:disable:this type_
self.coordinator = coordinator
self.delegate = delegate
self.services = services

super.init(state: state)

if !state.configuration.isAdding {
Expand All @@ -134,6 +133,9 @@ final class AddEditItemProcessor: StateProcessor<// swiftlint:disable:this type_
guard let key = state.loginState.authenticatorKey else { return }
services.pasteboardService.copy(key)
state.toast = Toast(title: Localizations.valueHasBeenCopied(Localizations.authenticatorKeyScanner))
case .dismissNewLoginActionCard:
state.showLearnNewLoginActionCard = false
await services.stateService.setLearnNewLoginActionCardStatus(.complete)
case .fetchCipherOptions:
await fetchCipherOptions()
case .savePressed:
Expand All @@ -142,6 +144,10 @@ final class AddEditItemProcessor: StateProcessor<// swiftlint:disable:this type_
await setupTotp()
case .deletePressed:
await showSoftDeleteConfirmation()
case .showLearnNewLoginGuidedTour:
// TODO: PM-16154
state.showLearnNewLoginActionCard = false
await services.stateService.setLearnNewLoginActionCardStatus(.complete)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,16 @@ class AddEditItemProcessorTests: BitwardenTestCase {
XCTAssertTrue(delegate.itemSoftDeletedCalled)
}

/// `perform(_:)` with `.dismissNewLoginActionCard` will set `.showLearnNewLoginActionCard` to false
/// and updates `.learnNewLoginActionCardShown` via stateService.
@MainActor
func test_perform_dismissNewLoginActionCard() async {
subject.state.showLearnNewLoginActionCard = true
await subject.perform(.dismissNewLoginActionCard)
XCTAssertFalse(subject.state.showLearnNewLoginActionCard)
XCTAssertEqual(stateService.learnNewLoginActionCardStatus, .complete)
}

/// `perform(_:)` with `.fetchCipherOptions` fetches the ownership options for a cipher from the repository.
@MainActor
func test_perform_fetchCipherOptions() async {
Expand Down Expand Up @@ -1403,6 +1413,15 @@ class AddEditItemProcessorTests: BitwardenTestCase {
XCTAssertEqual(coordinator.routes.last, .setupTotpManual)
}

/// `perform(_:)` with `.showLearnNewLoginGuidedTour` sets `showLearnNewLoginActionCard` to `false`.
@MainActor
func test_perform_showLearnNewLoginGuidedTour() async {
subject.state.showLearnNewLoginActionCard = true
await subject.perform(.showLearnNewLoginGuidedTour)
XCTAssertFalse(subject.state.showLearnNewLoginActionCard)
XCTAssertEqual(stateService.learnNewLoginActionCardStatus, .complete)
}

/// `receive(_:)` with `authKeyVisibilityTapped` updates the value in the state.
@MainActor
func test_receive_authKeyVisibilityTapped() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ protocol AddEditItemState: Sendable {
/// If master password reprompt toggle should be shown.
var showMasterPasswordReprompt: Bool { get set }

/// The flag indicating if we should show the learn new login action card.
var shouldShowLearnNewLoginActionCard: Bool { get }

/// If account is eligible for Learn New Login Action Card.
var showLearnNewLoginActionCard: Bool { get set }
ezimet-livefront marked this conversation as resolved.
Show resolved Hide resolved

/// The SSH key item state.
var sshKeyState: SSHKeyItemState { get set }

Expand Down
Loading
Loading