Skip to content

Commit

Permalink
[AC-2786] Remove unassigned items (#830)
Browse files Browse the repository at this point in the history
  • Loading branch information
@KatherineInCode
KatherineInCode authored Aug 15, 2024
1 parent 12d1998 commit 450808c
Show file tree
Hide file tree
Showing 21 changed files with 3 additions and 436 deletions.
4 changes: 2 additions & 2 deletions BitwardenShared/Core/Platform/Models/Domain/ServerConfigTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ final class ServerConfigTests: BitwardenTestCase {
environment: nil,
featureStates: [
"vault-onboarding": .bool(true),
"unassigned-items-banner": .bool(false),
"test-remote-feature-flag": .bool(false),
"not-a-real-feature-flag": .int(42),
],
gitHash: "123",
Expand All @@ -21,6 +21,6 @@ final class ServerConfigTests: BitwardenTestCase {
)

let subject = ServerConfig(date: Date(), responseModel: model)
XCTAssertEqual(subject.featureStates, [.unassignedItemsBanner: .bool(false)])
XCTAssertEqual(subject.featureStates, [.testRemoteFeatureFlag: .bool(false)])
}
}
6 changes: 1 addition & 5 deletions BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ enum FeatureFlag: String, Codable {
/// A feature flag for the intro carousel flow.
case nativeCarouselFlow = "native-carousel-flow"

/// A feature flag for showing the unassigned items banner.
case unassignedItemsBanner = "unassigned-items-banner"

// MARK: Test Flags

/// A test feature flag that isn't remotely configured.
Expand All @@ -32,8 +29,7 @@ enum FeatureFlag: String, Codable {
.nativeCarouselFlow,
.testLocalFeatureFlag:
false
case .testRemoteFeatureFlag,
.unassignedItemsBanner:
case .testRemoteFeatureFlag:
true
}
}
Expand Down
26 changes: 0 additions & 26 deletions BitwardenShared/Core/Platform/Services/StateService.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,13 +231,6 @@ protocol StateService: AnyObject {
///
func getServerConfig(userId: String?) async throws -> ServerConfig?

/// Gets whether we should check for unassigned items for the user.
///
/// - Parameter userId: The user ID associated with the flag.
/// - Returns: `false` if the user has seen and acknowledged the unassigned items alert.
///
func getShouldCheckOrganizationUnassignedItems(userId: String?) async throws -> Bool

/// Get whether the device should be trusted.
///
/// - Returns: Whether to trust the device.
Expand Down Expand Up @@ -501,15 +494,6 @@ protocol StateService: AnyObject {
///
func setServerConfig(_ config: ServerConfig?, userId: String?) async throws

/// Sets whether or not we should check for unassigned ciphers in an organization for
/// a particular user.
///
/// - Parameters:
/// - shouldCheck: Whether or not we should check for unassigned ciphers.
/// - userId: The user ID that acknowledged the alert.
///
func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String?) async throws

/// Set whether to trust the device.
///
/// - Parameter shouldTrustDevice: Whether to trust the device.
Expand Down Expand Up @@ -1221,11 +1205,6 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
return appSettingsStore.serverConfig(userId: userId)
}

func getShouldCheckOrganizationUnassignedItems(userId: String?) async throws -> Bool {
let userId = try userId ?? getActiveAccountUserId()
return appSettingsStore.shouldCheckOrganizationUnassignedItems(userId: userId) ?? true
}

func getShouldTrustDevice(userId: String) async -> Bool? {
appSettingsStore.shouldTrustDevice(userId: userId)
}
Expand Down Expand Up @@ -1445,11 +1424,6 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
appSettingsStore.setServerConfig(config, userId: userId)
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String?) async throws {
let userId = try userId ?? getActiveAccountUserId()
appSettingsStore.setShouldCheckOrganizationUnassignedItems(shouldCheck, userId: userId)
}

func setShouldTrustDevice(_ shouldTrustDevice: Bool?, userId: String) {
appSettingsStore.setShouldTrustDevice(shouldTrustDevice: shouldTrustDevice, userId: userId)
}
Expand Down
22 changes: 0 additions & 22 deletions BitwardenShared/Core/Platform/Services/StateServiceTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,22 +699,6 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(value, model)
}

/// `getShouldCheckOrganizationUnassignedItems` returns the config value
func test_getShouldCheckOrganizationUnassignedItems() async throws {
appSettingsStore.shouldCheckOrganizationUnassignedItems["1"] = false
var shouldCheck = try await subject.getShouldCheckOrganizationUnassignedItems(userId: "1")
XCTAssertFalse(shouldCheck)
appSettingsStore.shouldCheckOrganizationUnassignedItems["1"] = true
shouldCheck = try await subject.getShouldCheckOrganizationUnassignedItems(userId: "1")
XCTAssertTrue(shouldCheck)
}

/// `getShouldCheckOrganizationUnassignedItems` returns true if it hasn't been set
func test_getShouldCheckOrganizationUnassignedItems_notSet() async throws {
let shouldCheck = try await subject.getShouldCheckOrganizationUnassignedItems(userId: "1")
XCTAssertTrue(shouldCheck)
}

/// `getShowWebIcons` gets the show web icons value.
func test_getShowWebIcons() async {
appSettingsStore.disableWebIcons = true
Expand Down Expand Up @@ -1527,12 +1511,6 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(appSettingsStore.serverConfig["1"], model)
}

/// `setShouldCheckOrganizationUnassignedItems` saves the should check value.
func test_setShouldCheckOrganizationUnassignedItems() async throws {
try await subject.setShouldCheckOrganizationUnassignedItems(true, userId: "1")
XCTAssertEqual(appSettingsStore.shouldCheckOrganizationUnassignedItems["1"], true)
}

/// `setShouldTrustDevice` saves the should trust device value.
func test_setShouldTrustDevice() async {
await subject.setShouldTrustDevice(true, userId: "1")
Expand Down
27 changes: 0 additions & 27 deletions BitwardenShared/Core/Platform/Services/Stores/AppSettingsStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,15 +334,6 @@ protocol AppSettingsStore: AnyObject {
///
func setServerConfig(_ config: ServerConfig?, userId: String)

/// Sets whether or not we should check for unassigned ciphers in an organization for
/// a particular user.
///
/// - Parameters:
/// - shouldCheck: Whether or not we should check for unassigned ciphers.
/// - userId: The user ID to track.
///
func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String)

/// Set whether to trust the device.
///
/// - Parameter shouldTrustDevice: Whether to trust the device.
Expand Down Expand Up @@ -389,13 +380,6 @@ protocol AppSettingsStore: AnyObject {
///
func setUsernameGenerationOptions(_ options: UsernameGenerationOptions?, userId: String)

/// Gets whether or not we should check for unassigned ciphers in an organization for
/// a particular user.
///
/// - Parameter userId: The user ID to check.
///
func shouldCheckOrganizationUnassignedItems(userId: String) -> Bool?

/// Get whether the device should be trusted.
///
/// - Returns: Whether to trust the device.
Expand Down Expand Up @@ -592,7 +576,6 @@ extension DefaultAppSettingsStore: AppSettingsStore {
case rememberedEmail
case rememberedOrgIdentifier
case serverConfig(userId: String)
case shouldCheckOrganizationUnassignedItems(userId: String)
case shouldTrustDevice(userId: String)
case state
case twoFactorToken(email: String)
Expand Down Expand Up @@ -667,8 +650,6 @@ extension DefaultAppSettingsStore: AppSettingsStore {
key = "rememberedOrgIdentifier"
case let .serverConfig(userId):
key = "serverConfig_\(userId)"
case let .shouldCheckOrganizationUnassignedItems(userId):
key = "shouldCheckOrganizationUnassignedItems_\(userId)"
case let .shouldTrustDevice(userId):
key = "shouldTrustDevice_\(userId)"
case .state:
Expand Down Expand Up @@ -920,10 +901,6 @@ extension DefaultAppSettingsStore: AppSettingsStore {
store(config, for: .serverConfig(userId: userId))
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String) {
store(shouldCheck, for: .shouldCheckOrganizationUnassignedItems(userId: userId))
}

func setShouldTrustDevice(shouldTrustDevice: Bool?, userId: String) {
store(shouldTrustDevice, for: .shouldTrustDevice(userId: userId))
}
Expand All @@ -944,10 +921,6 @@ extension DefaultAppSettingsStore: AppSettingsStore {
store(minutes, for: .vaultTimeout(userId: userId))
}

func shouldCheckOrganizationUnassignedItems(userId: String) -> Bool? {
fetch(for: .shouldCheckOrganizationUnassignedItems(userId: userId))
}

func timeoutAction(userId: String) -> Int? {
fetch(for: .vaultTimeoutAction(userId: userId))
}
Expand Down
13 changes: 0 additions & 13 deletions BitwardenShared/Core/Platform/Services/Stores/AppSettingsStoreTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,19 +642,6 @@ class AppSettingsStoreTests: BitwardenTestCase { // swiftlint:disable:this type_
)
}

/// `shouldCheckOrganizationUnassignedItems(:)` is initially nil {
func test_shouldCheckOrganizationUnassignedItems_isInitiallyNil() {
XCTAssertNil(subject.shouldCheckOrganizationUnassignedItems(userId: "1"))
}

/// `shouldCheckOrganizationUnassignedItems(:)` can be used to get and set the persisted value.
func test_shouldCheckOrganizationUnassignedItems_withValue() {
subject.setShouldCheckOrganizationUnassignedItems(true, userId: "1")
XCTAssertEqual(subject.shouldCheckOrganizationUnassignedItems(userId: "1"), true)
subject.setShouldCheckOrganizationUnassignedItems(false, userId: "1")
XCTAssertEqual(subject.shouldCheckOrganizationUnassignedItems(userId: "1"), false)
}

/// `twoFactorToken(email:)` returns `nil` if there isn't a previously stored value.
func test_twoFactorToken_isInitiallyNil() {
XCTAssertNil(subject.twoFactorToken(email: "[email protected]"))
Expand Down
9 changes: 0 additions & 9 deletions BitwardenShared/Core/Platform/Services/Stores/TestHelpers/MockAppSettingsStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class MockAppSettingsStore: AppSettingsStore {
var passwordGenerationOptions = [String: PasswordGenerationOptions]()
var pinProtectedUserKey = [String: String]()
var serverConfig = [String: ServerConfig]()
var shouldCheckOrganizationUnassignedItems = [String: Bool?]()
var shouldTrustDevice = [String: Bool?]()
var timeoutAction = [String: Int]()
var twoFactorTokens = [String: String]()
Expand Down Expand Up @@ -196,10 +195,6 @@ class MockAppSettingsStore: AppSettingsStore {
serverConfig[userId] = config
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String) {
shouldCheckOrganizationUnassignedItems[userId] = shouldCheck
}

func setShouldTrustDevice(shouldTrustDevice: Bool?, userId: String) {
self.shouldTrustDevice[userId] = shouldTrustDevice
}
Expand Down Expand Up @@ -228,10 +223,6 @@ class MockAppSettingsStore: AppSettingsStore {
vaultTimeout[userId] = minutes
}

func shouldCheckOrganizationUnassignedItems(userId: String) -> Bool? {
shouldCheckOrganizationUnassignedItems[userId] ?? nil
}

func shouldTrustDevice(userId: String) -> Bool? {
shouldTrustDevice[userId] ?? false
}
Expand Down
11 changes: 0 additions & 11 deletions BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
var setAccountHasBeenUnlockedInteractivelyResult: Result<Void, Error> = .success(())
var setBiometricAuthenticationEnabledResult: Result<Void, Error> = .success(())
var setBiometricIntegrityStateError: Error?
var shouldCheckOrganizationUnassignedItems = [String: Bool?]()
var shouldTrustDevice = [String: Bool?]()
var twoFactorTokens = [String: String]()
var unsuccessfulUnlockAttempts = [String: Int]()
Expand Down Expand Up @@ -250,11 +249,6 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
return serverConfig[userId]
}

func getShouldCheckOrganizationUnassignedItems(userId: String?) async throws -> Bool {
let userId = try unwrapUserId(userId)
return (shouldCheckOrganizationUnassignedItems[userId] ?? false) ?? false
}

func getShouldTrustDevice(userId: String) async -> Bool? {
shouldTrustDevice[userId] ?? false
}
Expand Down Expand Up @@ -457,11 +451,6 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
serverConfig[userId] = config
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String?) async throws {
let userId = try unwrapUserId(userId)
shouldCheckOrganizationUnassignedItems[userId] = shouldCheck
}

func setShouldTrustDevice(_ shouldTrustDevice: Bool?, userId: String) async {
self.shouldTrustDevice[userId] = shouldTrustDevice
}
Expand Down
12 changes: 0 additions & 12 deletions BitwardenShared/Core/Vault/Repositories/TestHelpers/MockVaultRepository.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ class MockVaultRepository: VaultRepository {

var getDisableAutoTotpCopyResult: Result<Bool, Error> = .success(false)

var hasUnassignedCiphersResult: Result<Bool, Error> = .success(false)

var needsSyncCalled = false
var needsSyncResult: Result<Bool, Error> = .success(false)

Expand Down Expand Up @@ -85,8 +83,6 @@ class MockVaultRepository: VaultRepository {
var shareCipherCiphers = [CipherView]()
var shareCipherResult: Result<Void, Error> = .success(())

var shouldShowUnassignedCiphersAlert = false

var softDeletedCipher = [CipherView]()
var softDeleteCipherResult: Result<Void, Error> = .success(())

Expand Down Expand Up @@ -193,10 +189,6 @@ class MockVaultRepository: VaultRepository {
try getDisableAutoTotpCopyResult.get()
}

func hasUnassignedCiphers() async throws -> Bool {
try hasUnassignedCiphersResult.get()
}

func needsSync() async throws -> Bool {
needsSyncCalled = true
return try needsSyncResult.get()
Expand Down Expand Up @@ -263,10 +255,6 @@ class MockVaultRepository: VaultRepository {
try shareCipherResult.get()
}

func shouldShowUnassignedCiphersAlert() async -> Bool {
shouldShowUnassignedCiphersAlert
}

func softDeleteCipher(_ cipher: CipherView) async throws {
softDeletedCipher.append(cipher)
try softDeleteCipherResult.get()
Expand Down
20 changes: 0 additions & 20 deletions BitwardenShared/Core/Vault/Repositories/VaultRepository.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,6 @@ public protocol VaultRepository: AnyObject {
///
func shareCipher(_ cipher: CipherView, newOrganizationId: String, newCollectionIds: [String]) async throws

/// Whether or not we should show the unassigned ciphers alert based on properties of the account.
///
/// - Returns: `true` if we should show the unassigned ciphers alert
///
func shouldShowUnassignedCiphersAlert() async -> Bool

/// Soft delete a cipher from the user's vault.
///
/// - Parameter cipher: The cipher that the user is soft deleting.
Expand Down Expand Up @@ -1157,20 +1151,6 @@ extension DefaultVaultRepository: VaultRepository {
try await cipherService.shareCipherWithServer(encryptedOrganizationCipher)
}

func shouldShowUnassignedCiphersAlert() async -> Bool {
do {
guard await configService.getFeatureFlag(.unassignedItemsBanner, defaultValue: false),
try await stateService.getShouldCheckOrganizationUnassignedItems(userId: nil),
try await !organizationService.fetchAllOrganizations().isEmpty,
try await cipherService.hasUnassignedCiphers()
else { return false }
return true
} catch {
errorReporter.log(error: error)
return false
}
}

func softDeleteCipher(_ cipher: CipherView) async throws {
guard let id = cipher.id else { throw CipherAPIServiceError.updateMissingId }
let softDeletedCipher = cipher.update(deletedDate: timeProvider.presentTime)
Expand Down
Loading

0 comments on commit 450808c

Please sign in to comment.