Fix a bug where the security banner has the wrong state when out of sync. (#3511)

This commit is contained in:
Doug 2024-11-18 11:38:20 +00:00 committed by GitHub
parent c91a9bd9c9
commit bef1eda533
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 168 additions and 31 deletions

View File

@ -66,10 +66,28 @@ enum HomeScreenRoomListMode: CustomStringConvertible {
}
}
enum HomeScreenBannerMode {
enum HomeScreenSecurityBannerMode: Equatable {
case none
case dismissed
case show
case show(HomeScreenRecoveryKeyConfirmationBanner.State)
var isDismissed: Bool {
switch self {
case .dismissed: true
default: false
}
}
var isShown: Bool {
switch self {
case .show: true
default: false
}
}
}
enum HomeScreenMigrationBannerMode {
case none, show, dismissed
}
struct HomeScreenViewState: BindableState {
@ -77,8 +95,8 @@ struct HomeScreenViewState: BindableState {
var userDisplayName: String?
var userAvatarURL: URL?
var securityBannerMode = HomeScreenBannerMode.none
var slidingSyncMigrationBannerMode = HomeScreenBannerMode.none
var securityBannerMode = HomeScreenSecurityBannerMode.none
var slidingSyncMigrationBannerMode = HomeScreenMigrationBannerMode.none
var requiresExtraAccountSetup = false

View File

@ -55,16 +55,15 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
.sink { [weak self] securityState in
guard let self else { return }
switch (securityState.verificationState, securityState.recoveryState) {
case (.verified, .disabled):
switch securityState.recoveryState {
case .disabled:
state.requiresExtraAccountSetup = true
state.securityBannerMode = .show
case (.verified, .incomplete):
state.requiresExtraAccountSetup = true
if state.securityBannerMode != .dismissed {
state.securityBannerMode = .show
if !state.securityBannerMode.isDismissed {
state.securityBannerMode = .show(.setUpRecovery)
}
case .incomplete:
state.requiresExtraAccountSetup = true
state.securityBannerMode = .show(.recoveryOutOfSync)
default:
state.securityBannerMode = .none
state.requiresExtraAccountSetup = false

View File

@ -120,7 +120,7 @@ struct HomeScreenContent: View {
private var topSection: some View {
// An empty VStack causes glitches within the room list
if context.viewState.shouldShowFilters ||
context.viewState.securityBannerMode == .show ||
context.viewState.securityBannerMode.isShown ||
context.viewState.slidingSyncMigrationBannerMode == .show {
VStack(spacing: 0) {
if context.viewState.shouldShowFilters {
@ -129,8 +129,8 @@ struct HomeScreenContent: View {
if context.viewState.slidingSyncMigrationBannerMode == .show {
HomeScreenSlidingSyncMigrationBanner(context: context)
} else if context.viewState.securityBannerMode == .show {
HomeScreenRecoveryKeyConfirmationBanner(requiresExtraAccountSetup: context.viewState.requiresExtraAccountSetup, context: context)
} else if case let .show(state) = context.viewState.securityBannerMode {
HomeScreenRecoveryKeyConfirmationBanner(state: state, context: context)
}
}
.background(Color.compound.bgCanvasDefault)

View File

@ -10,13 +10,37 @@ import Compound
import SwiftUI
struct HomeScreenRecoveryKeyConfirmationBanner: View {
let requiresExtraAccountSetup: Bool
enum State { case setUpRecovery, recoveryOutOfSync }
let state: State
var context: HomeScreenViewModel.Context
var title: String { requiresExtraAccountSetup ? L10n.bannerSetUpRecoveryTitle : L10n.confirmRecoveryKeyBannerTitle }
var message: String { requiresExtraAccountSetup ? L10n.bannerSetUpRecoveryContent : L10n.confirmRecoveryKeyBannerMessage }
var actionTitle: String { requiresExtraAccountSetup ? L10n.bannerSetUpRecoverySubmit : L10n.confirmRecoveryKeyBannerPrimaryButtonTitle }
var primaryAction: HomeScreenViewAction { requiresExtraAccountSetup ? .setupRecovery : .confirmRecoveryKey }
var title: String {
switch state {
case .setUpRecovery: L10n.bannerSetUpRecoveryTitle
case .recoveryOutOfSync: L10n.confirmRecoveryKeyBannerTitle
}
}
var message: String {
switch state {
case .setUpRecovery: L10n.bannerSetUpRecoveryContent
case .recoveryOutOfSync: L10n.confirmRecoveryKeyBannerMessage
}
}
var actionTitle: String {
switch state {
case .setUpRecovery: L10n.bannerSetUpRecoverySubmit
case .recoveryOutOfSync: L10n.confirmRecoveryKeyBannerPrimaryButtonTitle
}
}
var primaryAction: HomeScreenViewAction {
switch state {
case .setUpRecovery: .setupRecovery
case .recoveryOutOfSync: .confirmRecoveryKey
}
}
var body: some View {
VStack(spacing: 16) {
@ -37,7 +61,7 @@ struct HomeScreenRecoveryKeyConfirmationBanner: View {
.foregroundColor(.compound.textPrimary)
.frame(maxWidth: .infinity, alignment: .leading)
if requiresExtraAccountSetup {
if state == .setUpRecovery {
Button {
context.send(viewAction: .skipRecoveryKeyConfirmation)
} label: {
@ -63,7 +87,7 @@ struct HomeScreenRecoveryKeyConfirmationBanner: View {
.buttonStyle(.compound(.primary, size: .medium))
.accessibilityIdentifier(A11yIdentifiers.homeScreen.recoveryKeyConfirmationBannerContinue)
if !requiresExtraAccountSetup {
if state == .recoveryOutOfSync {
Button {
context.send(viewAction: .resetEncryption)
} label: {
@ -81,10 +105,10 @@ struct HomeScreenRecoveryKeyConfirmationBanner_Previews: PreviewProvider, Testab
static let viewModel = buildViewModel()
static var previews: some View {
HomeScreenRecoveryKeyConfirmationBanner(requiresExtraAccountSetup: true,
HomeScreenRecoveryKeyConfirmationBanner(state: .setUpRecovery,
context: viewModel.context)
.previewDisplayName("Set up recovery")
HomeScreenRecoveryKeyConfirmationBanner(requiresExtraAccountSetup: false,
HomeScreenRecoveryKeyConfirmationBanner(state: .recoveryOutOfSync,
context: viewModel.context)
.previewDisplayName("Out of sync")
}

View File

@ -20,13 +20,6 @@ class HomeScreenViewModelTests: XCTestCase {
override func setUpWithError() throws {
cancellables.removeAll()
roomSummaryProvider = RoomSummaryProviderMock(.init(state: .loaded(.mockRooms)))
clientProxy = ClientProxyMock(.init(userID: "@mock:client.com", roomSummaryProvider: roomSummaryProvider))
viewModel = HomeScreenViewModel(userSession: UserSessionMock(.init(clientProxy: clientProxy)),
analyticsService: ServiceLocator.shared.analytics,
appSettings: ServiceLocator.shared.settings,
selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
userIndicatorController: ServiceLocator.shared.userIndicatorController)
}
override func tearDown() {
@ -34,6 +27,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testSelectRoom() async throws {
setupViewModel()
let mockRoomId = "mock_room_id"
var correctResult = false
var selectedRoomId = ""
@ -57,6 +52,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testTapUserAvatar() async throws {
setupViewModel()
var correctResult = false
viewModel.actions
@ -76,6 +73,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testLeaveRoomAlert() async throws {
setupViewModel()
let mockRoomId = "1"
clientProxy.roomForIdentifierClosure = { _ in .joined(JoinedRoomProxyMock(.init(id: mockRoomId, name: "Some room"))) }
@ -92,6 +91,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testLeaveRoomError() async throws {
setupViewModel()
let mockRoomId = "1"
let room = JoinedRoomProxyMock(.init(id: mockRoomId, name: "Some room"))
room.leaveRoomClosure = { .failure(.sdkError(ClientProxyMockError.generic)) }
@ -110,6 +111,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testLeaveRoomSuccess() async throws {
setupViewModel()
let mockRoomId = "1"
var correctResult = false
let expectation = expectation(description: #function)
@ -136,6 +139,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testShowRoomDetails() async throws {
setupViewModel()
let mockRoomId = "1"
var correctResult = false
viewModel.actions
@ -155,6 +160,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testFilters() async throws {
setupViewModel()
context.filtersState.activateFilter(.people)
try await Task.sleep(for: .milliseconds(100))
XCTAssertEqual(roomSummaryProvider.roomListPublisher.value.count, 2)
@ -162,6 +169,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testSearch() async throws {
setupViewModel()
context.isSearchFieldFocused = true
context.searchQuery = "lude to Found"
try await Task.sleep(for: .milliseconds(100))
@ -170,6 +179,8 @@ class HomeScreenViewModelTests: XCTestCase {
}
func testFiltersEmptyState() async throws {
setupViewModel()
context.filtersState.activateFilter(.people)
context.filtersState.activateFilter(.favourites)
try await Task.sleep(for: .milliseconds(100))
@ -177,4 +188,89 @@ class HomeScreenViewModelTests: XCTestCase {
context.isSearchFieldFocused = true
XCTAssertFalse(context.viewState.shouldShowEmptyFilterState)
}
func testSetUpRecoveryBannerState() async throws {
// Given a view model without a visible security banner.
let securityStateStateSubject = CurrentValueSubject<SessionSecurityState, Never>(.init(verificationState: .verified, recoveryState: .unknown))
setupViewModel(securityStatePublisher: securityStateStateSubject.asCurrentValuePublisher())
XCTAssertEqual(context.viewState.securityBannerMode, .none)
// When the recovery state comes through as disabled.
var deferred = deferFulfillment(context.$viewState) { $0.requiresExtraAccountSetup == true }
securityStateStateSubject.send(.init(verificationState: .verified, recoveryState: .disabled))
try await deferred.fulfill()
// Then the banner should be shown to set up recovery.
XCTAssertEqual(context.viewState.securityBannerMode, .show(.setUpRecovery))
// When the recovery is enabled.
deferred = deferFulfillment(context.$viewState) { $0.requiresExtraAccountSetup == false }
securityStateStateSubject.send(.init(verificationState: .verified, recoveryState: .enabled))
try await deferred.fulfill()
// Then the banner should no longer be shown.
XCTAssertEqual(context.viewState.securityBannerMode, .none)
}
func testDismissSetUpRecoveryBannerState() async throws {
// Given a view model with the setup recovery banner shown.
let securityStateStateSubject = CurrentValueSubject<SessionSecurityState, Never>(.init(verificationState: .verified, recoveryState: .unknown))
setupViewModel(securityStatePublisher: securityStateStateSubject.asCurrentValuePublisher())
var deferred = deferFulfillment(context.$viewState) { $0.securityBannerMode == .show(.setUpRecovery) }
securityStateStateSubject.send(.init(verificationState: .verified, recoveryState: .disabled))
try await deferred.fulfill()
// When the banner is dismissed.
deferred = deferFulfillment(context.$viewState) { $0.securityBannerMode == .dismissed }
context.send(viewAction: .skipRecoveryKeyConfirmation)
// Then the banner should no longer be shown.
try await deferred.fulfill()
// And when the recovery state comes through a second time the banner should still not be shown.
let failure = deferFailure(context.$viewState, timeout: 1) { $0.securityBannerMode != .dismissed }
securityStateStateSubject.send(.init(verificationState: .verified, recoveryState: .disabled))
try await failure.fulfill()
}
func testOutOfSyncRecoveryBannerState() async throws {
// Given a view model without a visible security banner.
let securityStateStateSubject = CurrentValueSubject<SessionSecurityState, Never>(.init(verificationState: .verified, recoveryState: .unknown))
setupViewModel(securityStatePublisher: securityStateStateSubject.asCurrentValuePublisher())
XCTAssertEqual(context.viewState.securityBannerMode, .none)
// When the recovery state comes through as incomplete.
var deferred = deferFulfillment(context.$viewState) { $0.requiresExtraAccountSetup == true }
securityStateStateSubject.send(.init(verificationState: .verified, recoveryState: .incomplete))
try await deferred.fulfill()
// Then the banner should be shown for out of sync recovery.
XCTAssertEqual(context.viewState.securityBannerMode, .show(.recoveryOutOfSync))
// When the recovery is enabled.
deferred = deferFulfillment(context.$viewState) { $0.requiresExtraAccountSetup == false }
securityStateStateSubject.send(.init(verificationState: .verified, recoveryState: .enabled))
try await deferred.fulfill()
// Then the banner should no longer be shown.
XCTAssertEqual(context.viewState.securityBannerMode, .none)
}
// MARK: - Helpers
private func setupViewModel(securityStatePublisher: CurrentValuePublisher<SessionSecurityState, Never>? = nil) {
roomSummaryProvider = RoomSummaryProviderMock(.init(state: .loaded(.mockRooms)))
clientProxy = ClientProxyMock(.init(userID: "@mock:client.com",
roomSummaryProvider: roomSummaryProvider))
let userSession = UserSessionMock(.init(clientProxy: clientProxy))
if let securityStatePublisher {
userSession.sessionSecurityStatePublisher = securityStatePublisher
}
viewModel = HomeScreenViewModel(userSession: userSession,
analyticsService: ServiceLocator.shared.analytics,
appSettings: ServiceLocator.shared.settings,
selectedRoomPublisher: CurrentValueSubject<String?, Never>(nil).asCurrentValuePublisher(),
userIndicatorController: ServiceLocator.shared.userIndicatorController)
}
}