[PM-22376] - [Vault] [Clients] Update cipher form component to default to My Items collections (#15356)

* fix tests

* remove unused code

* fix storybook

* fix storybook

* cleanup

* move observable to function. update tests

* fix type error

* move call to getDefaultCollectionId

* fix test

libs/vault/src/cipher-form/cipher-form.stories.ts

@@ -19,6 +19,7 @@ import { ViewCacheService } from "@bitwarden/angular/platform/view-cache";
 import { NudgeStatus, NudgesService } from "@bitwarden/angular/vault";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
+import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
@@ -243,6 +244,7 @@ export default {
           provide: ConfigService,
           useValue: {
             getFeatureFlag: () => Promise.resolve(false),
+            getFeatureFlag$: () => new BehaviorSubject(false),
           },
         },
         {
@@ -253,6 +255,12 @@ export default {
             },
           },
         },
+        {
+          provide: PolicyService,
+          useValue: {
+            policiesByType$: new BehaviorSubject([]),
+          },
+        },
       ],
     }),
     componentWrapperDecorator(

libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts

@@ -3,18 +3,24 @@ import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testin
 import { ReactiveFormsModule } from "@angular/forms";
 import { By } from "@angular/platform-browser";
 import { mock, MockProxy } from "jest-mock-extended";
-import { BehaviorSubject } from "rxjs";
+import { BehaviorSubject, of } from "rxjs";
 
 // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
 // eslint-disable-next-line no-restricted-imports
 import { CollectionTypes, CollectionView } from "@bitwarden/admin-console/common";
+import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { SelectComponent } from "@bitwarden/components";
 
-import { CipherFormConfig } from "../../abstractions/cipher-form-config.service";
+import {
+  CipherFormConfig,
+  OptionalInitialValues,
+} from "../../abstractions/cipher-form-config.service";
 import { CipherFormContainer } from "../../cipher-form-container";
 
 import { ItemDetailsSectionComponent } from "./item-details-section.component";
@@ -48,6 +54,8 @@ describe("ItemDetailsSectionComponent", () => {
   let fixture: ComponentFixture<ItemDetailsSectionComponent>;
   let cipherFormProvider: MockProxy<CipherFormContainer>;
   let i18nService: MockProxy<I18nService>;
+  let mockConfigService: MockProxy<ConfigService>;
+  let mockPolicyService: MockProxy<PolicyService>;
 
   const activeAccount$ = new BehaviorSubject<{ email: string }>({ email: "test@example.com" });
   const getInitialCipherView = jest.fn(() => null);
@@ -66,12 +74,19 @@ describe("ItemDetailsSectionComponent", () => {
       compare: (a: string, b: string) => a.localeCompare(b),
     } as Intl.Collator;
 
+    mockConfigService = mock<ConfigService>();
+    mockConfigService.getFeatureFlag$.mockReturnValue(of(true));
+    mockPolicyService = mock<PolicyService>();
+    mockPolicyService.policiesByType$.mockReturnValue(of([]));
+
     await TestBed.configureTestingModule({
       imports: [ItemDetailsSectionComponent, CommonModule, ReactiveFormsModule],
       providers: [
         { provide: CipherFormContainer, useValue: cipherFormProvider },
         { provide: I18nService, useValue: i18nService },
         { provide: AccountService, useValue: { activeAccount$ } },
+        { provide: ConfigService, useValue: mockConfigService },
+        { provide: PolicyService, useValue: mockPolicyService },
       ],
     }).compileComponents();
 
@@ -369,7 +384,7 @@ describe("ItemDetailsSectionComponent", () => {
       expect(collectionSelect).toBeNull();
     });
 
-    it("should enable/show collection control when an organization is selected", async () => {
+    it("should enable/show collection control when an organization is selected", fakeAsync(() => {
       component.config.organizationDataOwnershipDisabled = true;
       component.config.organizations = [{ id: "org1" } as Organization];
       component.config.collections = [
@@ -378,12 +393,12 @@ describe("ItemDetailsSectionComponent", () => {
       ];
 
       fixture.detectChanges();
-      await fixture.whenStable();
+      tick();
 
       component.itemDetailsForm.controls.organizationId.setValue("org1");
 
+      tick();
       fixture.detectChanges();
-      await fixture.whenStable();
 
       const collectionSelect = fixture.nativeElement.querySelector(
         "bit-multi-select[formcontrolname='collectionIds']",
@@ -391,7 +406,7 @@ describe("ItemDetailsSectionComponent", () => {
 
       expect(component.itemDetailsForm.controls.collectionIds.enabled).toBe(true);
       expect(collectionSelect).not.toBeNull();
-    });
+    }));
 
     it("should set collectionIds to originalCipher collections on first load", async () => {
       component.config.mode = "clone";
@@ -488,6 +503,9 @@ describe("ItemDetailsSectionComponent", () => {
 
       component.itemDetailsForm.controls.organizationId.setValue("org1");
 
+      fixture.detectChanges();
+      await fixture.whenStable();
+
       expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]);
     });
   });
@@ -548,4 +566,27 @@ describe("ItemDetailsSectionComponent", () => {
       expect(label).toBe("org1");
     });
   });
+
+  describe("getDefaultCollectionId", () => {
+    it("returns matching default when flag & policy match", async () => {
+      const def = createMockCollection("def1", "Def", "orgA");
+      component.config.collections = [def] as CollectionView[];
+      component.config.initialValues = { collectionIds: [] } as OptionalInitialValues;
+      mockConfigService.getFeatureFlag.mockResolvedValue(true);
+      mockPolicyService.policiesByType$.mockReturnValue(of([{ organizationId: "orgA" } as Policy]));
+
+      const id = await (component as any).getDefaultCollectionId("orgA");
+      expect(id).toEqual("def1");
+    });
+
+    it("returns undefined when no default found", async () => {
+      component.config.collections = [createMockCollection("c1", "C1", "orgB")] as CollectionView[];
+      component.config.initialValues = { collectionIds: [] } as OptionalInitialValues;
+      mockConfigService.getFeatureFlag.mockResolvedValue(true);
+      mockPolicyService.policiesByType$.mockReturnValue(of([{ organizationId: "orgA" } as Policy]));
+
+      const result = await (component as any).getDefaultCollectionId("orgA");
+      expect(result).toBeUndefined();
+    });
+  });
 });

libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts

@@ -4,15 +4,19 @@ import { CommonModule } from "@angular/common";
 import { Component, DestroyRef, Input, OnInit } from "@angular/core";
 import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
 import { FormBuilder, FormControl, ReactiveFormsModule, Validators } from "@angular/forms";
-import { concatMap, map } from "rxjs";
+import { concatMap, firstValueFrom, map } from "rxjs";
 
 // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
 // eslint-disable-next-line no-restricted-imports
-import { CollectionView } from "@bitwarden/admin-console/common";
+import { CollectionTypes, CollectionView } from "@bitwarden/admin-console/common";
 import { JslibModule } from "@bitwarden/angular/jslib.module";
-import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
+import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
+import { OrganizationUserType, PolicyType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { getUserId } from "@bitwarden/common/auth/services/account.service";
+import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
+import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
@@ -124,6 +128,8 @@ export class ItemDetailsSectionComponent implements OnInit {
     private i18nService: I18nService,
     private destroyRef: DestroyRef,
     private accountService: AccountService,
+    private configService: ConfigService,
+    private policyService: PolicyService,
   ) {
     this.cipherFormContainer.registerChildForm("itemDetails", this.itemDetailsForm);
     this.itemDetailsForm.valueChanges
@@ -200,30 +206,61 @@ export class ItemDetailsSectionComponent implements OnInit {
     if (prefillCipher) {
       await this.initFromExistingCipher(prefillCipher);
     } else {
+      const orgId = this.initialValues?.organizationId;
       this.itemDetailsForm.setValue({
         name: this.initialValues?.name || "",
-        organizationId: this.initialValues?.organizationId || this.defaultOwner,
+        organizationId: orgId || this.defaultOwner,
         folderId: this.initialValues?.folderId || null,
         collectionIds: [],
         favorite: false,
       });
-      await this.updateCollectionOptions(this.initialValues?.collectionIds || []);
+      await this.updateCollectionOptions(this.initialValues?.collectionIds);
     }
-
     if (!this.allowOwnershipChange) {
       this.itemDetailsForm.controls.organizationId.disable();
     }
-
     this.itemDetailsForm.controls.organizationId.valueChanges
       .pipe(
         takeUntilDestroyed(this.destroyRef),
-        concatMap(async () => {
+        concatMap(async () => await this.updateCollectionOptions()),
-          await this.updateCollectionOptions();
-        }),
       )
       .subscribe();
   }
 
+  /**
+   * Gets the default collection IDs for the selected organization.
+   * Returns null if any of the following apply:
+   * - the feature flag is disabled
+   * - no org is currently selected
+   * - the selected org doesn't have the "no private data policy" enabled
+   */
+  private async getDefaultCollectionId(orgId?: OrganizationId) {
+    if (!orgId) {
+      return;
+    }
+    const isFeatureEnabled = await this.configService.getFeatureFlag(
+      FeatureFlag.CreateDefaultLocation,
+    );
+    if (!isFeatureEnabled) {
+      return;
+    }
+    const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
+    const selectedOrgHasPolicyEnabled = (
+      await firstValueFrom(
+        this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, userId),
+      )
+    ).find((p) => p.organizationId);
+    if (!selectedOrgHasPolicyEnabled) {
+      return;
+    }
+    const defaultUserCollection = this.collections.find(
+      (c) => c.organizationId === orgId && c.type === CollectionTypes.DefaultUserCollection,
+    );
+    // If the user was added after the policy was enabled as they will not have any private data
+    // and will not have a default collection.
+    return defaultUserCollection?.id;
+  }
+
   private async initFromExistingCipher(prefillCipher: CipherView) {
     const { name, folderId, collectionIds } = prefillCipher;
 
@@ -332,6 +369,11 @@ export class ItemDetailsSectionComponent implements OnInit {
         // Non-admins can only select assigned collections that are not read only. (Non-AC)
         return c.assigned && !c.readOnly;
       })
+      .sort((a, b) => {
+        const aIsDefaultCollection = a.type === CollectionTypes.DefaultUserCollection ? -1 : 0;
+        const bIsDefaultCollection = b.type === CollectionTypes.DefaultUserCollection ? -1 : 0;
+        return aIsDefaultCollection - bIsDefaultCollection;
+      })
       .map((c) => ({
         id: c.id,
         name: c.name,
@@ -349,10 +391,17 @@ export class ItemDetailsSectionComponent implements OnInit {
       return;
     }
 
-    if (startingSelection.length > 0) {
+    if (startingSelection.filter(Boolean).length > 0) {
       collectionsControl.setValue(
         this.collectionOptions.filter((c) => startingSelection.includes(c.id as CollectionId)),
       );
+    } else {
+      const defaultCollectionId = await this.getDefaultCollectionId(orgId);
+      if (defaultCollectionId) {
+        collectionsControl.setValue(
+          this.collectionOptions.filter((c) => c.id === defaultCollectionId),
+        );
+      }
     }
   }
 }