Skip to content

[PM-13428] Remove ownership option #1405

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

Merged
merged 24 commits into from
Jun 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
64cec21
pm-13428 Add validation to check if user belongs to any org
LRNcardozoWDF Feb 25, 2025
7589d70
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option-local
LRNcardozoWDF Feb 28, 2025
49c5320
pm-13428 Add ui test
LRNcardozoWDF Feb 28, 2025
1aa4586
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Feb 28, 2025
7d5ddb5
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Mar 4, 2025
dc5f086
pm-13428 Add missing tests
LRNcardozoWDF Mar 4, 2025
4d07dd7
pm-13428 Remove changes
LRNcardozoWDF Mar 11, 2025
ecfe06f
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Mar 11, 2025
c3168a4
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Mar 24, 2025
f9044cc
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Apr 18, 2025
e174bd2
pm-13428 Fix PR comments
LRNcardozoWDF Apr 18, 2025
447e703
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF May 26, 2025
c27b6cb
pm-13428 Add tests for hasOrganizations
LRNcardozoWDF May 27, 2025
7a1732d
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 3, 2025
a32c0e7
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 4, 2025
081d243
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 5, 2025
fc78ddf
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 5, 2025
23b6d5d
pm-13428 Fix PR comments
LRNcardozoWDF Jun 10, 2025
b668b9d
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 10, 2025
6940927
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 11, 2025
979a3ab
pm-13428 Fix PR comment
LRNcardozoWDF Jun 12, 2025
9f85d3a
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 12, 2025
8e7977e
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 17, 2025
2f9ec57
Merge branch 'main' into cmcg/pm-13428-remove-ownership-option
LRNcardozoWDF Jun 17, 2025
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 @@ -45,6 +45,9 @@ protocol AddEditItemState: Sendable {
/// The state for guided tour view.
var guidedTourViewState: GuidedTourViewState { get set }

/// Whether the user belongs to any organization.
var hasOrganizations: Bool { get }

/// The state for a identity type item.
var identityState: IdentityItemState { get set }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ struct AddEditItemView: View {
)
.accessibilityIdentifier("FolderPicker")

if store.state.configuration.isAdding, let owner = store.state.owner {
if store.state.configuration.isAdding, store.state.hasOrganizations, let owner = store.state.owner {
ContentBlock(dividerLeadingPadding: 16) {
BitwardenMenuField(
title: Localizations.owner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,12 @@ class AddEditItemViewTests: BitwardenTestCase { // swiftlint:disable:this type_b

@MainActor
func test_snapshot_add_personalOwnershipPolicy() {
processor.state.ownershipOptions.append(.organization(id: "1", name: "Organization"))
processor.state.owner = .organization(id: "1", name: "Organization")
processor.state.isPersonalOwnershipDisabled = true
processor.state.allUserCollections = [
.fixture(id: "1", name: "Default collection", organizationId: "1"),
]
assertSnapshot(of: subject.navStackWrapped, as: .defaultPortrait)
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions BitwardenShared/UI/Vault/VaultItem/CipherItemState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ struct CipherItemState: Equatable { // swiftlint:disable:this type_body_length
self
}

var hasOrganizations: Bool {
cipher.organizationId != nil || ownershipOptions.contains { !$0.isPersonal }
}
Comment on lines +140 to +142
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โ›๏ธ Add tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, ty! Tests added c27b6cb.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โ›๏ธ Nice! Could you add an additional test when organizationId is nil and state.ownershipOptions is empty? It would be the intermediate state while the view is fetching the cipher options.


/// Whether or not this item can be assigned to collections.
var canAssignToCollection: Bool {
guard !collectionIds.isEmpty else { return true }
Expand Down
35 changes: 35 additions & 0 deletions BitwardenShared/UI/Vault/VaultItem/CipherItemStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,41 @@ class CipherItemStateTests: BitwardenTestCase { // swiftlint:disable:this type_b
XCTAssertFalse(state.canBeRestored)
}

/// `hasOrganizations` is true when the cipher has a non-nil organizationId.
func test_hasOrganizations_whenCipherBelongsToAnOrg_returnsTrue() throws {
let cipher = CipherView.fixture(organizationId: "org123")

let state = try XCTUnwrap(CipherItemState(existing: cipher, hasPremium: true))
XCTAssertTrue(state.hasOrganizations)
}

/// `hasOrganizations` is false when ownership options are only personal and organizationId is nil.
func test_hasOrganizations_whenCipherBelongsToPersonal_returnsFalse() throws {
let cipher = CipherView.fixture()
var state = try XCTUnwrap(CipherItemState(existing: cipher, hasPremium: true))
state.ownershipOptions = [CipherOwner.personal(email: "user@bitwarden")]

XCTAssertFalse(state.hasOrganizations)
}

/// `hasOrganizations` is true when ownership options include at least one non-personal option.
func test_hasOrganizations_whenOwnershipIncludesNonPersonal_returnsTrue() throws {
let cipher = CipherView.fixture()
var state = try XCTUnwrap(CipherItemState(existing: cipher, hasPremium: true))
state.ownershipOptions = [CipherOwner.organization(id: "org123", name: "Organization")]

XCTAssertTrue(state.hasOrganizations)
}

/// `hasOrganizations` is false when ownership options is empty (not yet fetched) and organizationId is nil.
func test_hasOrganizations_withEmptyOwnershiptOptionsAndOrgIdIsNil_returnsFalse() throws {
let cipher = CipherView.fixture()
var state = try XCTUnwrap(CipherItemState(existing: cipher, hasPremium: true))
state.ownershipOptions = []

XCTAssertFalse(state.hasOrganizations)
}

/// `restrictCipherItemDeletionFlagEnable` default value is false
func test_restrictCipherItemDeletionFlagValue() throws {
let cipher = CipherView.loginFixture(
Expand Down