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

[BITAU-124] Refresh Item List When Foregrounding the App #178

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2e5aa95
[BITAU-132] Move to Bitwarden Long Press on Local Items
brant-livefront Oct 24, 2024
5207553
[BITAU-130] Add Option to Manual Key Entry for Adding to Bitwarden
brant-livefront Oct 24, 2024
85c59a8
Updated per PR review: removed BW shortening and renamed hash function
brant-livefront Oct 25, 2024
f664ce3
Merge in latest from brant/BITAU-132-move-to-bitwarden-long-press-on-โ€ฆ
brant-livefront Oct 25, 2024
dfe9ef0
Fixed one missed merge conflict
brant-livefront Oct 25, 2024
016ad2f
Merged in latest from main, fixed conflicts
brant-livefront Oct 25, 2024
7837fe7
[BITAU-131] Add Popup to QR Code Scan to Add Code to Bitwarden
brant-livefront Oct 28, 2024
d060aca
Increase sleep time to allow test to succeed on CI
brant-livefront Oct 28, 2024
46a383f
Respond to PR feedback
brant-livefront Oct 29, 2024
c2acc5c
Removed dead code that Codecov revealed
brant-livefront Oct 29, 2024
bf971bb
Copy changes from the UX team
brant-livefront Oct 29, 2024
73b9869
[BITAU-124] Refersh Item List When Foregrounding the App
brant-livefront Oct 30, 2024
67bae02
Merge branch 'main' into brant/BITAU-130-add-option-to-manual-key-entโ€ฆ
brant-livefront Oct 31, 2024
d50a68b
Merge in latest from main; fix conflicts
brant-livefront Oct 31, 2024
3f01f91
Add tests and handle case where QR code scan is done with the sync noโ€ฆ
brant-livefront Oct 31, 2024
4798034
Merge branch 'brant/BITAU-131-add-popup-to-qr-code-scan-to-add-code-tโ€ฆ
brant-livefront Oct 31, 2024
22c6f77
Added NotificationCenter property and tests to cover NotificationCentโ€ฆ
brant-livefront Oct 31, 2024
2983145
Updated strings to put quotes around default save options
brant-livefront Oct 31, 2024
836e087
Merge branch 'main' into brant/BITAU-130-add-option-to-manual-key-entโ€ฆ
brant-livefront Nov 1, 2024
bcc76d3
Merge branch 'brant/BITAU-130-add-option-to-manual-key-entry-for-addiโ€ฆ
brant-livefront Nov 1, 2024
1be6c0e
Merge branch 'brant/BITAU-131-add-popup-to-qr-code-scan-to-add-code-tโ€ฆ
brant-livefront Nov 1, 2024
d74bbe0
Update with PR suggestions
brant-livefront Nov 6, 2024
d01ffd1
Merged in latest from main
brant-livefront Nov 7, 2024
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
@@ -0,0 +1,53 @@
import Combine
import UIKit

// MARK: - NotificationCenterService

/// A protocol for a `NotificationCenterService` which accesses the app's notification center.
///
protocol NotificationCenterService: AnyObject {
/// A publisher for when the app enters the background.
///
func didEnterBackgroundPublisher() -> AnyPublisher<Void, Never>
victor-livefront marked this conversation as resolved.
Show resolved Hide resolved

/// A publisher for when the app enters the foreground.
///
func willEnterForegroundPublisher() -> AnyPublisher<Void, Never>
}

// MARK: - DefaultNotificationCenterService

/// A default implementation of the `NotificationCenterService` which accesses the app's notification center.
///
class DefaultNotificationCenterService: NotificationCenterService {
// MARK: Properties

/// The NotificationCenter to use in subscribing to notifications.
let notificationCenter: NotificationCenter

// MARK: Initialization

/// Initialize a `DefaultNotificationCenterService`.
///
/// - Parameter notificationCenter: The NotificationCenter to use in subscribing to notifications.
///
init(notificationCenter: NotificationCenter = NotificationCenter.default) {
self.notificationCenter = notificationCenter
}

// MARK: Methods

func didEnterBackgroundPublisher() -> AnyPublisher<Void, Never> {
notificationCenter
.publisher(for: UIApplication.didEnterBackgroundNotification)
.map { _ in }
.eraseToAnyPublisher()
}

func willEnterForegroundPublisher() -> AnyPublisher<Void, Never> {
notificationCenter
.publisher(for: UIApplication.willEnterForegroundNotification)
.map { _ in }
.eraseToAnyPublisher()
brant-livefront marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import XCTest

@testable import AuthenticatorShared

final class NotificationCenterServiceTests: AuthenticatorTestCase {
// MARK: Properties

var notificationCenter: NotificationCenter!
var subject: DefaultNotificationCenterService!

// MARK: Setup & Teardown

override func setUp() {
notificationCenter = NotificationCenter()
subject = DefaultNotificationCenterService(notificationCenter: notificationCenter)
}

override func tearDown() {
notificationCenter = nil
subject = nil
}

// MARK: Tests

/// `didEnterBackgroundPublisher` publishes a notification when the app enters the background.
func testDidEnterBackgroundPublisher() {
let expectation = XCTestExpectation(description: "Application entered background")
let cancellable = subject.didEnterBackgroundPublisher()
.sink { _ in
expectation.fulfill()
}

notificationCenter.post(
name: UIApplication.didEnterBackgroundNotification,
object: nil
)

wait(for: [expectation], timeout: 1)
cancellable.cancel()
}

/// `willEnterForegroundPublisher` publishes a notification when the app will enter the foreground.
func testWillEnterForegroundPublisher() {
let expectation = XCTestExpectation(description: "Application will enter foreground")
let cancellable = subject.willEnterForegroundPublisher()
.sink { _ in
expectation.fulfill()
}

notificationCenter.post(
name: UIApplication.willEnterForegroundNotification,
object: nil
)

wait(for: [expectation], timeout: 1)
cancellable.cancel()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@
/// The service used to perform app data migrations.
let migrationService: MigrationService

/// The service used to receive foreground and background notifications.
let notificationCenterService: NotificationCenterService

/// The service used by the application for sharing data with other apps.
let pasteboardService: PasteboardService

Expand Down Expand Up @@ -86,6 +89,7 @@
/// - exportItemsService: The service to export items.
/// - importItemsService: The service to import items.
/// - migrationService: The service to do data migrations
/// - notificationCenterService: The service used to receive foreground and background notifications.
/// - pasteboardService: The service used by the application for sharing data with other apps.
/// - stateService: The service for managing account state.
/// - timeProvider: Provides the present time for TOTP Code Calculation.
Expand All @@ -105,6 +109,7 @@
exportItemsService: ExportItemsService,
importItemsService: ImportItemsService,
migrationService: MigrationService,
notificationCenterService: NotificationCenterService,
pasteboardService: PasteboardService,
stateService: StateService,
timeProvider: TimeProvider,
Expand All @@ -123,6 +128,7 @@
self.exportItemsService = exportItemsService
self.importItemsService = importItemsService
self.migrationService = migrationService
self.notificationCenterService = notificationCenterService
self.pasteboardService = pasteboardService
self.timeProvider = timeProvider
self.stateService = stateService
Expand Down Expand Up @@ -189,6 +195,8 @@
keychainRepository: keychainRepository
)

let notificationCenterService = DefaultNotificationCenterService()

Check warning on line 199 in AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift

View check run for this annotation

Codecov / codecov/patch

AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift#L198-L199

Added lines #L198 - L199 were not covered by tests
let totpService = DefaultTOTPService(
clientVault: clientService.clientVault(),
errorReporter: errorReporter,
Expand Down Expand Up @@ -259,6 +267,7 @@
exportItemsService: exportItemsService,
importItemsService: importItemsService,
migrationService: migrationService,
notificationCenterService: notificationCenterService,

Check warning on line 270 in AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift

View check run for this annotation

Codecov / codecov/patch

AuthenticatorShared/Core/Platform/Services/ServiceContainer.swift#L270

Added line #L270 was not covered by tests
pasteboardService: pasteboardService,
stateService: stateService,
timeProvider: timeProvider,
Expand Down
8 changes: 8 additions & 0 deletions AuthenticatorShared/Core/Platform/Services/Services.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ typealias Services = HasAppSettingsStore
& HasErrorReporter
& HasExportItemsService
& HasImportItemsService
& HasNotificationCenterService
& HasPasteboardService
& HasStateService
& HasTOTPService
Expand Down Expand Up @@ -85,6 +86,13 @@ protocol HasImportItemsService {
var importItemsService: ImportItemsService { get }
}

/// Protocol for an object that provides a `NotificationCenterService`.
///
protocol HasNotificationCenterService {
/// The service used to receive foreground and background notifications.
var notificationCenterService: NotificationCenterService { get }
}

/// Protocol for an object that provides a `PasteboardService`.
///
protocol HasPasteboardService {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Combine
import Foundation

@testable import AuthenticatorShared

class MockNotificationCenterService: NotificationCenterService {
var didEnterBackgroundSubject = PassthroughSubject<Void, Never>()
var willEnterForegroundSubject = PassthroughSubject<Void, Never>()

func didEnterBackgroundPublisher() -> AnyPublisher<Void, Never> {
didEnterBackgroundSubject.eraseToAnyPublisher()
}

func willEnterForegroundPublisher() -> AnyPublisher<Void, Never> {
willEnterForegroundSubject.eraseToAnyPublisher()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ extension ServiceContainer {
exportItemsService: ExportItemsService = MockExportItemsService(),
importItemsService: ImportItemsService = MockImportItemsService(),
migrationService: MigrationService = MockMigrationService(),
notificationCenterService: NotificationCenterService = MockNotificationCenterService(),
pasteboardService: PasteboardService = MockPasteboardService(),
stateService: StateService = MockStateService(),
timeProvider: TimeProvider = MockTimeProvider(.currentTime),
Expand All @@ -37,6 +38,7 @@ extension ServiceContainer {
exportItemsService: exportItemsService,
importItemsService: importItemsService,
migrationService: migrationService,
notificationCenterService: notificationCenterService,
pasteboardService: pasteboardService,
stateService: stateService,
timeProvider: timeProvider,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Combine
import Foundation

// swiftlint:disable file_length
Expand All @@ -6,6 +7,8 @@ import Foundation

/// A `Processor` that can process `ItemListAction` and `ItemListEffect` objects.
final class ItemListProcessor: StateProcessor<ItemListState, ItemListAction, ItemListEffect> {
// swiftlint:disable:previous type_body_length

// MARK: Types

typealias Services = HasAppSettingsStore
Expand All @@ -14,12 +17,16 @@ final class ItemListProcessor: StateProcessor<ItemListState, ItemListAction, Ite
& HasCameraService
& HasConfigService
& HasErrorReporter
& HasNotificationCenterService
& HasPasteboardService
& HasTOTPService
& HasTimeProvider

// MARK: Private Properties

/// The set to hold Combine cancellables.
private var cancellables = Set<AnyCancellable>()

/// The `Coordinator` for this processor.
private var coordinator: AnyCoordinator<ItemListRoute, ItemListEvent>

Expand Down Expand Up @@ -56,6 +63,7 @@ final class ItemListProcessor: StateProcessor<ItemListState, ItemListAction, Ite
}
}
)
setupForegroundNotification()
}

deinit {
Expand Down Expand Up @@ -259,6 +267,20 @@ final class ItemListProcessor: StateProcessor<ItemListState, ItemListAction, Ite
return []
}

/// Subscribe to receive foreground notifications so that we can refresh the item list when the app is relaunched.
///
private func setupForegroundNotification() {
services.notificationCenterService
brant-livefront marked this conversation as resolved.
Show resolved Hide resolved
.willEnterForegroundPublisher()
.sink { [weak self] in
guard let self else { return }
Task {
await self.perform(.refresh)
}
}
.store(in: &cancellables)
}

/// Determine if the user has synced with this account previously. If they have not synced previously,
/// this method return `true` indicating that we should show the toast for a newly synced account. It
/// also stores the fact that we've synced with this account in the `AppSettingsStore` for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class ItemListProcessorTests: AuthenticatorTestCase { // swiftlint:disable:this
var configService: MockConfigService!
var coordinator: MockCoordinator<ItemListRoute, ItemListEvent>!
var errorReporter: MockErrorReporter!
var notificationCenterService: MockNotificationCenterService!
var pasteboardService: MockPasteboardService!
var totpService: MockTOTPService!
var subject: ItemListProcessor!
Expand All @@ -32,6 +33,7 @@ class ItemListProcessorTests: AuthenticatorTestCase { // swiftlint:disable:this
configService = MockConfigService()
coordinator = MockCoordinator()
errorReporter = MockErrorReporter()
notificationCenterService = MockNotificationCenterService()
pasteboardService = MockPasteboardService()
totpService = MockTOTPService()

Expand All @@ -42,6 +44,7 @@ class ItemListProcessorTests: AuthenticatorTestCase { // swiftlint:disable:this
cameraService: cameraService,
configService: configService,
errorReporter: errorReporter,
notificationCenterService: notificationCenterService,
pasteboardService: pasteboardService,
totpService: totpService
)
Expand All @@ -56,6 +59,15 @@ class ItemListProcessorTests: AuthenticatorTestCase { // swiftlint:disable:this
override func tearDown() {
super.tearDown()

application = nil
appSettingsStore = nil
authItemRepository = nil
cameraService = nil
configService = nil
errorReporter = nil
notificationCenterService = nil
pasteboardService = nil
totpService = nil
brant-livefront marked this conversation as resolved.
Show resolved Hide resolved
coordinator = nil
subject = nil
}
Expand Down Expand Up @@ -590,6 +602,20 @@ class ItemListProcessorTests: AuthenticatorTestCase { // swiftlint:disable:this
XCTAssertNil(subject.state.url)
}

/// `setupForegroundNotification()` is called as part of `init()` and subscribes to any
/// foreground notification, performing `.refresh` when it receives a notification.
func test_setupForegroundNotification() async throws {
let item = ItemListItem.fixture()
let resultSection = ItemListSection(id: "", items: [item], name: "Items")
authItemRepository.itemListSubject.send([resultSection])
authItemRepository.refreshTotpCodesResult = .success([item])

notificationCenterService.willEnterForegroundSubject.send()

try await waitForAsync { self.subject.state.loadingState != .loading(nil) }
XCTAssertEqual(subject.state.loadingState, .data([resultSection]))
}

// MARK: AuthenticatorKeyCaptureDelegate Tests

/// `didCompleteAutomaticCapture` failure when the user has opted to save locally by default.
Expand Down Expand Up @@ -993,11 +1019,7 @@ class ItemListProcessorTests: AuthenticatorTestCase { // swiftlint:disable:this
dismissAction?.action()
waitFor(!authItemRepository.addAuthItemAuthItems.isEmpty)
waitFor(subject.state.loadingState != .loading(nil))
guard let item = authItemRepository.addAuthItemAuthItems.first
else {
XCTFail("Unable to get authenticator item")
return
}
let item = try XCTUnwrap(authItemRepository.addAuthItemAuthItems.first)
XCTAssertEqual(item.name, "name")
XCTAssertEqual(item.totpKey, String.base32Key)
}
Expand Down
Loading