[PM-12770] Assign to Collections Hint (#14529)

* allow use of common spec in lib/vault tests

* pass readonly collections to the assign collection component

- The assign to collections component filters them out already.
-They're also needed to display copy within the component

* add hint to assign to collections component when there are read only collections assigned to a cipher already

* add readonly hint to desktop

* only show collection hint for collections that are assigned to the provided ciphers

* consider admin/owner edit everything permission when assigning to collections

* fix icon in test

apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts

@@ -74,7 +74,7 @@ export class AssignCollections {
     combineLatest([cipher$, this.collectionService.decryptedCollections$])
       .pipe(takeUntilDestroyed(), first())
       .subscribe(([cipherView, collections]) => {
-        let availableCollections = collections.filter((c) => !c.readOnly);
+        let availableCollections = collections;
         const organizationId = (cipherView?.organizationId as OrganizationId) ?? null;
 
         // If the cipher is already a part of an organization,

apps/desktop/src/locales/en/messages.json

@@ -3703,6 +3703,15 @@
   "changeAtRiskPassword": {
     "message": "Change at-risk password"
   },
+  "cannotRemoveViewOnlyCollections": {
+    "message": "You cannot remove collections with View only permissions: $COLLECTIONS$",
+    "placeholders": {
+      "collections": {
+        "content": "$1",
+        "example": "Work, Personal"
+      }
+    }
+  },
   "move": {
     "message": "Move"
   },

apps/web/src/app/admin-console/organizations/collections/vault.component.ts

@@ -362,8 +362,7 @@ export class VaultComponent implements OnInit, OnDestroy {
         if (this.organization.canEditAllCiphers) {
           return collections;
         }
-        // The user is only allowed to add/edit items to assigned collections that are not readonly
+        return collections.filter((c) => c.assigned);
-        return collections.filter((c) => c.assigned && !c.readOnly);
       }),
       shareReplay({ refCount: true, bufferSize: 1 }),
     );

apps/web/src/app/vault/individual-vault/vault.component.ts

@@ -933,7 +933,7 @@ export class VaultComponent implements OnInit, OnDestroy {
     if (orgId && orgId !== "MyVault") {
       const organization = this.allOrganizations.find((o) => o.id === orgId);
       availableCollections = this.allCollections.filter(
-        (c) => c.organizationId === organization.id && !c.readOnly,
+        (c) => c.organizationId === organization.id,
       );
     }
 

libs/vault/jest.config.js

@@ -10,7 +10,11 @@ module.exports = {
   displayName: "libs/vault tests",
   preset: "jest-preset-angular",
   setupFilesAfterEnv: ["<rootDir>/test.setup.ts"],
-  moduleNameMapper: pathsToModuleNameMapper(compilerOptions?.paths || {}, {
+  moduleNameMapper: pathsToModuleNameMapper(
-    prefix: "<rootDir>/",
+    // lets us use @bitwarden/common/spec in tests
-  }),
+    { "@bitwarden/common/spec": ["../common/spec"], ...(compilerOptions?.paths ?? {}) },
+    {
+      prefix: "<rootDir>/",
+    },
+  ),
 };

libs/vault/src/components/assign-collections.component.html

@@ -37,13 +37,16 @@
   </div>
 
   <div class="tw-flex">
-    <bit-form-field class="tw-grow tw-max-w-full">
+    <bit-form-field class="tw-grow tw-max-w-full" disableMargin>
       <bit-label>{{ "selectCollectionsToAssign" | i18n }}</bit-label>
       <bit-multi-select
         class="tw-w-full"
         formControlName="collections"
         [baseItems]="availableCollections"
       ></bit-multi-select>
+      <bit-hint *ngIf="readOnlyCollectionNames.length > 0" data-testid="view-only-hint">
+        {{ "cannotRemoveViewOnlyCollections" | i18n: readOnlyCollectionNames.join(", ") }}
+      </bit-hint>
     </bit-form-field>
   </div>
 </form>

libs/vault/src/components/assign-collections.component.spec.ts

@@ -0,0 +1,113 @@
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { By } from "@angular/platform-browser";
+import { mock } from "jest-mock-extended";
+import { of } from "rxjs";
+
+import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
+import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
+import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { ProductTierType } from "@bitwarden/common/billing/enums";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
+import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid";
+import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
+import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
+import { ToastService } from "@bitwarden/components";
+
+import {
+  AssignCollectionsComponent,
+  CollectionAssignmentParams,
+} from "./assign-collections.component";
+
+describe("AssignCollectionsComponent", () => {
+  let component: AssignCollectionsComponent;
+  let fixture: ComponentFixture<AssignCollectionsComponent>;
+
+  const mockUserId = "mock-user-id" as UserId;
+  const accountService: FakeAccountService = mockAccountServiceWith(mockUserId);
+
+  const editCollection = new CollectionView();
+  editCollection.id = "collection-id" as CollectionId;
+  editCollection.organizationId = "org-id" as OrganizationId;
+  editCollection.name = "Editable Collection";
+  editCollection.readOnly = false;
+  editCollection.manage = true;
+
+  const readOnlyCollection1 = new CollectionView();
+  readOnlyCollection1.id = "read-only-collection-id" as CollectionId;
+  readOnlyCollection1.organizationId = "org-id" as OrganizationId;
+  readOnlyCollection1.name = "Read Only Collection";
+  readOnlyCollection1.readOnly = true;
+
+  const readOnlyCollection2 = new CollectionView();
+  readOnlyCollection2.id = "read-only-collection-id-2" as CollectionId;
+  readOnlyCollection2.organizationId = "org-id" as OrganizationId;
+  readOnlyCollection2.name = "Read Only Collection 2";
+  readOnlyCollection2.readOnly = true;
+
+  const params = {
+    organizationId: "org-id" as OrganizationId,
+    ciphers: [
+      {
+        id: "cipher-id",
+        name: "Cipher Name",
+        collectionIds: [readOnlyCollection1.id],
+        edit: true,
+      } as unknown as CipherView,
+    ],
+    availableCollections: [editCollection, readOnlyCollection1, readOnlyCollection2],
+  } as CollectionAssignmentParams;
+
+  const org = {
+    id: "org-id",
+    name: "Test Org",
+    productTierType: ProductTierType.Enterprise,
+  } as Organization;
+
+  const organizations$ = jest.fn().mockReturnValue(of([org]));
+
+  beforeEach(async () => {
+    await TestBed.configureTestingModule({
+      providers: [
+        { provide: CipherService, useValue: mock<CipherService>() },
+        { provide: OrganizationService, useValue: mock<OrganizationService>({ organizations$ }) },
+        { provide: CollectionService, useValue: mock<CollectionService>() },
+        { provide: ToastService, useValue: mock<ToastService>() },
+        { provide: AccountService, useValue: accountService },
+        { provide: I18nService, useValue: { t: (...keys: string[]) => keys.join(" ") } },
+      ],
+    }).compileComponents();
+
+    fixture = TestBed.createComponent(AssignCollectionsComponent);
+    component = fixture.componentInstance;
+    component.params = params;
+    fixture.detectChanges();
+  });
+
+  describe("read only collections", () => {
+    beforeEach(async () => {
+      await component.ngOnInit();
+      fixture.detectChanges();
+    });
+
+    it("shows read-only hint for assigned collections", () => {
+      const hint = fixture.debugElement.query(By.css('[data-testid="view-only-hint"]'));
+
+      expect(hint.nativeElement.textContent.trim()).toBe(
+        "cannotRemoveViewOnlyCollections Read Only Collection",
+      );
+    });
+
+    it("does not show read only collections in the list", () => {
+      expect(component["availableCollections"]).toEqual([
+        {
+          icon: "bwi-collection-shared",
+          id: editCollection.id,
+          labelName: editCollection.name,
+          listName: editCollection.name,
+        },
+      ]);
+    });
+  });
+});

libs/vault/src/components/assign-collections.component.ts

@@ -126,6 +126,12 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
     collections: [<SelectItemView[]>[], [Validators.required]],
   });
 
+  /**
+   * Collections that are already assigned to the cipher and are read-only. These cannot be removed.
+   * @protected
+   */
+  protected readOnlyCollectionNames: string[] = [];
+
   protected totalItemCount: number;
   protected editableItemCount: number;
   protected readonlyItemCount: number;
@@ -301,6 +307,8 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
       this.organizationService.organizations$(userId).pipe(getOrganizationById(organizationId)),
     );
 
+    await this.setReadOnlyCollectionNames();
+
     this.availableCollections = this.params.availableCollections
       .filter((collection) => {
         return collection.canEditItems(org);
@@ -503,4 +511,25 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
       await this.cipherService.saveCollectionsWithServer(cipher, userId);
     }
   }
+
+  /**
+   * Only display collections that are read-only and are assigned to the ciphers.
+   */
+  private async setReadOnlyCollectionNames() {
+    const { availableCollections, ciphers } = this.params;
+
+    const organization = await firstValueFrom(
+      this.organizations$.pipe(map((orgs) => orgs.find((o) => o.id === this.selectedOrgId))),
+    );
+
+    this.readOnlyCollectionNames = availableCollections
+      .filter((c) => {
+        return (
+          c.readOnly &&
+          ciphers.some((cipher) => cipher.collectionIds.includes(c.id)) &&
+          !c.canEditItems(organization)
+        );
+      })
+      .map((c) => c.name);
+  }
 }