From 9a22907e276b0452aa81f0d88dd1e87757f576d6 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Thu, 15 Jan 2026 11:08:18 -0600 Subject: [PATCH] [PM-30296] Assign to Collections for Archived Ciphers (#18223) * allow for archived ciphers to be assigned to a collection via the more options menu * reference `userId$` directly --- .../item-more-options.component.spec.ts | 38 +++++ .../popup/settings/archive.component.html | 9 ++ .../popup/settings/archive.component.spec.ts | 135 ++++++++++++++++++ .../vault/popup/settings/archive.component.ts | 32 ++++- .../vault-cipher-row.component.spec.ts | 41 ++++++ .../vault-items/vault-cipher-row.component.ts | 4 - 6 files changed, 254 insertions(+), 5 deletions(-) create mode 100644 apps/browser/src/vault/popup/settings/archive.component.spec.ts diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts index 6728249b788..b999d8db35a 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts @@ -405,4 +405,42 @@ describe("ItemMoreOptionsComponent", () => { }); }); }); + + describe("canAssignCollections$", () => { + it("emits true when user has organizations and editable collections", (done) => { + jest.spyOn(component["organizationService"], "hasOrganizations").mockReturnValue(of(true)); + jest + .spyOn(component["collectionService"], "decryptedCollections$") + .mockReturnValue(of([{ id: "col-1", readOnly: false }] as any)); + + component["canAssignCollections$"].subscribe((result) => { + expect(result).toBe(true); + done(); + }); + }); + + it("emits false when user has no organizations", (done) => { + jest.spyOn(component["organizationService"], "hasOrganizations").mockReturnValue(of(false)); + jest + .spyOn(component["collectionService"], "decryptedCollections$") + .mockReturnValue(of([{ id: "col-1", readOnly: false }] as any)); + + component["canAssignCollections$"].subscribe((result) => { + expect(result).toBe(false); + done(); + }); + }); + + it("emits false when all collections are read-only", (done) => { + jest.spyOn(component["organizationService"], "hasOrganizations").mockReturnValue(of(true)); + jest + .spyOn(component["collectionService"], "decryptedCollections$") + .mockReturnValue(of([{ id: "col-1", readOnly: true }] as any)); + + component["canAssignCollections$"].subscribe((result) => { + expect(result).toBe(false); + done(); + }); + }); + }); }); diff --git a/apps/browser/src/vault/popup/settings/archive.component.html b/apps/browser/src/vault/popup/settings/archive.component.html index 3273ca612fe..16afab4384b 100644 --- a/apps/browser/src/vault/popup/settings/archive.component.html +++ b/apps/browser/src/vault/popup/settings/archive.component.html @@ -63,6 +63,15 @@ + @if (canAssignCollections$ | async) { + + } diff --git a/apps/browser/src/vault/popup/settings/archive.component.spec.ts b/apps/browser/src/vault/popup/settings/archive.component.spec.ts new file mode 100644 index 00000000000..6ad5c2c2907 --- /dev/null +++ b/apps/browser/src/vault/popup/settings/archive.component.spec.ts @@ -0,0 +1,135 @@ +import { TestBed } from "@angular/core/testing"; +import { Router } from "@angular/router"; +import { mock } from "jest-mock-extended"; +import { BehaviorSubject, of } from "rxjs"; + +import { CollectionService } from "@bitwarden/admin-console/common"; +import { PopupRouterCacheService } from "@bitwarden/browser/platform/popup/view-cache/popup-router-cache.service"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { CipherViewLike } from "@bitwarden/common/vault/utils/cipher-view-like-utils"; +import { DialogService, ToastService } from "@bitwarden/components"; +import { LogService } from "@bitwarden/logging"; +import { PasswordRepromptService } from "@bitwarden/vault"; + +import { ArchiveComponent } from "./archive.component"; + +describe("ArchiveComponent", () => { + let component: ArchiveComponent; + + let hasOrganizations: jest.Mock; + let decryptedCollections$: jest.Mock; + let navigate: jest.Mock; + let showPasswordPrompt: jest.Mock; + + beforeAll(async () => { + navigate = jest.fn(); + showPasswordPrompt = jest.fn().mockResolvedValue(true); + hasOrganizations = jest.fn(); + decryptedCollections$ = jest.fn(); + + await TestBed.configureTestingModule({ + providers: [ + { provide: Router, useValue: { navigate } }, + { + provide: AccountService, + useValue: { activeAccount$: new BehaviorSubject({ id: "user-id" }) }, + }, + { provide: PasswordRepromptService, useValue: { showPasswordPrompt } }, + { provide: OrganizationService, useValue: { hasOrganizations } }, + { provide: CollectionService, useValue: { decryptedCollections$ } }, + { provide: DialogService, useValue: mock() }, + { provide: CipherService, useValue: mock() }, + { provide: CipherArchiveService, useValue: mock() }, + { provide: ToastService, useValue: mock() }, + { provide: PopupRouterCacheService, useValue: mock() }, + { provide: PlatformUtilsService, useValue: mock() }, + { provide: LogService, useValue: mock() }, + { provide: I18nService, useValue: { t: (key: string) => key } }, + ], + }).compileComponents(); + + const fixture = TestBed.createComponent(ArchiveComponent); + component = fixture.componentInstance; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("canAssignCollections$", () => { + it("emits true when user has organizations and editable collections", (done) => { + hasOrganizations.mockReturnValue(of(true)); + decryptedCollections$.mockReturnValue(of([{ id: "col-1", readOnly: false }] as any)); + + component["canAssignCollections$"].subscribe((result) => { + expect(result).toBe(true); + done(); + }); + }); + + it("emits false when user has no organizations", (done) => { + hasOrganizations.mockReturnValue(of(false)); + decryptedCollections$.mockReturnValue(of([{ id: "col-1", readOnly: false }] as any)); + + component["canAssignCollections$"].subscribe((result) => { + expect(result).toBe(false); + done(); + }); + }); + + it("emits false when all collections are read-only", (done) => { + hasOrganizations.mockReturnValue(of(true)); + decryptedCollections$.mockReturnValue(of([{ id: "col-1", readOnly: true }] as any)); + + component["canAssignCollections$"].subscribe((result) => { + expect(result).toBe(false); + done(); + }); + }); + }); + + describe("conditionallyNavigateToAssignCollections", () => { + const mockCipher = { + id: "cipher-1", + reprompt: 0, + } as CipherViewLike; + + it("navigates to assign-collections when reprompt is not required", async () => { + await component.conditionallyNavigateToAssignCollections(mockCipher); + + expect(navigate).toHaveBeenCalledWith(["/assign-collections"], { + queryParams: { cipherId: "cipher-1" }, + }); + }); + + it("prompts for password when reprompt is required", async () => { + const cipherWithReprompt = { ...mockCipher, reprompt: 1 }; + + await component.conditionallyNavigateToAssignCollections( + cipherWithReprompt as CipherViewLike, + ); + + expect(showPasswordPrompt).toHaveBeenCalled(); + expect(navigate).toHaveBeenCalledWith(["/assign-collections"], { + queryParams: { cipherId: "cipher-1" }, + }); + }); + + it("does not navigate when password prompt is cancelled", async () => { + const cipherWithReprompt = { ...mockCipher, reprompt: 1 }; + showPasswordPrompt.mockResolvedValueOnce(false); + + await component.conditionallyNavigateToAssignCollections( + cipherWithReprompt as CipherViewLike, + ); + + expect(showPasswordPrompt).toHaveBeenCalled(); + expect(navigate).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/browser/src/vault/popup/settings/archive.component.ts b/apps/browser/src/vault/popup/settings/archive.component.ts index 2b151116e20..2a46ac0c46e 100644 --- a/apps/browser/src/vault/popup/settings/archive.component.ts +++ b/apps/browser/src/vault/popup/settings/archive.component.ts @@ -1,9 +1,11 @@ import { CommonModule } from "@angular/common"; import { Component, inject } from "@angular/core"; import { Router } from "@angular/router"; -import { firstValueFrom, map, Observable, startWith, switchMap } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable, startWith, switchMap } from "rxjs"; +import { CollectionService } from "@bitwarden/admin-console/common"; import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -71,6 +73,9 @@ export class ArchiveComponent { private i18nService = inject(I18nService); private cipherArchiveService = inject(CipherArchiveService); private passwordRepromptService = inject(PasswordRepromptService); + private organizationService = inject(OrganizationService); + private collectionService = inject(CollectionService); + private userId$: Observable = this.accountService.activeAccount$.pipe(getUserId); protected archivedCiphers$ = this.userId$.pipe( @@ -87,6 +92,20 @@ export class ArchiveComponent { startWith(true), ); + protected canAssignCollections$ = this.userId$.pipe( + switchMap((userId) => { + return combineLatest([ + this.organizationService.hasOrganizations(userId), + this.collectionService.decryptedCollections$(userId), + ]).pipe( + map(([hasOrgs, collections]) => { + const canEditCollections = collections.some((c) => !c.readOnly); + return hasOrgs && canEditCollections; + }), + ); + }), + ); + protected showSubscriptionEndedMessaging$ = this.userId$.pipe( switchMap((userId) => this.cipherArchiveService.showSubscriptionEndedMessaging$(userId)), ); @@ -187,6 +206,17 @@ export class ArchiveComponent { }); } + /** Prompts for password when necessary then navigates to the assign collections route */ + async conditionallyNavigateToAssignCollections(cipher: CipherViewLike) { + if (cipher.reprompt && !(await this.passwordRepromptService.showPasswordPrompt())) { + return; + } + + await this.router.navigate(["/assign-collections"], { + queryParams: { cipherId: cipher.id }, + }); + } + /** * Check if the user is able to interact with the cipher * (password re-prompt / decryption failure checks). diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts index 9378ee54e51..49c9df8d582 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts @@ -142,4 +142,45 @@ describe("VaultCipherRowComponent", () => { expect(overlayContent).not.toContain('appcopyfield="password"'); }); }); + + describe("showAssignToCollections", () => { + let archivedCipher: CipherView; + + beforeEach(() => { + archivedCipher = new CipherView(); + archivedCipher.id = "cipher-1"; + archivedCipher.name = "Test Cipher"; + archivedCipher.type = CipherType.Login; + archivedCipher.organizationId = "org-1"; + archivedCipher.deletedDate = null; + archivedCipher.archivedDate = new Date(); + + component.cipher = archivedCipher; + component.organizations = [{ id: "org-1" } as any]; + component.canAssignCollections = true; + component.disabled = false; + }); + + it("returns true when cipher is archived and conditions are met", () => { + expect(component["showAssignToCollections"]).toBe(true); + }); + + it("returns false when cipher is deleted", () => { + archivedCipher.deletedDate = new Date(); + + expect(component["showAssignToCollections"]).toBe(false); + }); + + it("returns false when user cannot assign collections", () => { + component.canAssignCollections = false; + + expect(component["showAssignToCollections"]).toBe(false); + }); + + it("returns false when there are no organizations", () => { + component.organizations = []; + + expect(component["showAssignToCollections"]).toBeFalsy(); + }); + }); }); diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts index df1e70723ca..ec0fe42f927 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts @@ -217,11 +217,7 @@ export class VaultCipherRowComponent implements OnInit return CipherViewLikeUtils.decryptionFailure(this.cipher); } - // Do Not show Assign to Collections option if item is archived protected get showAssignToCollections() { - if (CipherViewLikeUtils.isArchived(this.cipher)) { - return false; - } return ( this.organizations?.length && this.canAssignCollections &&