[PM-9190] Use updateFn for patchCipher so that the current CipherView is available for context (#10258)

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

@@ -46,5 +46,9 @@ export abstract class CipherFormContainer {
     group: Exclude<CipherForm[K], undefined>,
   ): void;
 
-  abstract patchCipher(cipher: Partial<CipherView>): void;
+  /**
+   * Method to update the cipherView with the new values. This method should be called by the child form components
+   * @param updateFn - A function that takes the current cipherView and returns the updated cipherView
+   */
+  abstract patchCipher(updateFn: (current: CipherView) => CipherView): void;
 }

libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.spec.ts

@@ -4,6 +4,7 @@ import { mock, MockProxy } from "jest-mock-extended";
 import { BehaviorSubject } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { CipherRepromptType } from "@bitwarden/common/vault/enums";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
 import { PasswordRepromptService } from "../../../services/password-reprompt.service";
@@ -73,10 +74,16 @@ describe("AdditionalOptionsSectionComponent", () => {
       reprompt: true,
     });
 
-    expect(cipherFormProvider.patchCipher).toHaveBeenCalledWith({
+    const expectedCipher = new CipherView();
-      notes: "new notes",
+    expectedCipher.notes = "new notes";
-      reprompt: 1,
+    expectedCipher.reprompt = CipherRepromptType.Password;
-    });
+
+    expect(cipherFormProvider.patchCipher).toHaveBeenCalled();
+    const patchFn = cipherFormProvider.patchCipher.mock.lastCall[0];
+
+    const updated = patchFn(new CipherView());
+
+    expect(updated).toEqual(expectedCipher);
   });
 
   it("disables 'additionalOptionsForm' when in partial-edit mode", () => {

libs/vault/src/cipher-form/components/additional-options/additional-options-section.component.ts

@@ -66,9 +66,10 @@ export class AdditionalOptionsSectionComponent implements OnInit {
     this.cipherFormContainer.registerChildForm("additionalOptions", this.additionalOptionsForm);
 
     this.additionalOptionsForm.valueChanges.pipe(takeUntilDestroyed()).subscribe((value) => {
-      this.cipherFormContainer.patchCipher({
+      this.cipherFormContainer.patchCipher((cipher) => {
-        notes: value.notes,
+        cipher.notes = value.notes;
-        reprompt: value.reprompt ? CipherRepromptType.Password : CipherRepromptType.None,
+        cipher.reprompt = value.reprompt ? CipherRepromptType.Password : CipherRepromptType.None;
+        return cipher;
       });
     });
   }

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

@@ -62,9 +62,11 @@ describe("CardDetailsSectionComponent", () => {
     cardView.number = "4242 4242 4242 4242";
     cardView.brand = "Visa";
 
-    expect(patchCipherSpy).toHaveBeenCalledWith({
+    expect(patchCipherSpy).toHaveBeenCalled();
-      card: cardView,
+    const patchFn = patchCipherSpy.mock.lastCall[0];
-    });
+
+    const updateCipher = patchFn(new CipherView());
+    expect(updateCipher.card).toEqual(cardView);
   });
 
   it("it converts the year integer to a string", () => {
@@ -75,9 +77,11 @@ describe("CardDetailsSectionComponent", () => {
     const cardView = new CardView();
     cardView.expYear = "2022";
 
-    expect(patchCipherSpy).toHaveBeenCalledWith({
+    expect(patchCipherSpy).toHaveBeenCalled();
-      card: cardView,
+    const patchFn = patchCipherSpy.mock.lastCall[0];
-    });
+
+    const updatedCipher = patchFn(new CipherView());
+    expect(updatedCipher.card).toEqual(cardView);
   });
 
   it('disables `cardDetailsForm` when "disabled" is true', () => {

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

@@ -90,9 +90,6 @@ export class CardDetailsSectionComponent implements OnInit {
     { name: "12 - " + this.i18nService.t("december"), value: "12" },
   ];
 
-  /** Local CardView, either created empty or set to the existing card instance  */
-  private cardView: CardView;
-
   constructor(
     private cipherFormContainer: CipherFormContainer,
     private formBuilder: FormBuilder,
@@ -103,21 +100,21 @@ export class CardDetailsSectionComponent implements OnInit {
     this.cardDetailsForm.valueChanges
       .pipe(takeUntilDestroyed())
       .subscribe(({ cardholderName, number, brand, expMonth, expYear, code }) => {
-        // The input[type="number"] is returning a number, convert it to a string
+        this.cipherFormContainer.patchCipher((cipher) => {
-        // An empty field returns null, avoid casting `"null"` to a string
+          // The input[type="number"] is returning a number, convert it to a string
-        const expirationYear = expYear !== null ? `${expYear}` : null;
+          // An empty field returns null, avoid casting `"null"` to a string
+          const expirationYear = expYear !== null ? `${expYear}` : null;
 
-        const patchedCard = Object.assign(this.cardView, {
+          Object.assign(cipher.card, {
-          cardholderName,
+            cardholderName,
-          number,
+            number,
-          brand,
+            brand,
-          expMonth,
+            expMonth,
-          expYear: expirationYear,
+            expYear: expirationYear,
-          code,
+            code,
-        });
+          });
 
-        this.cipherFormContainer.patchCipher({
+          return cipher;
-          card: patchedCard,
         });
       });
 
@@ -133,9 +130,6 @@ export class CardDetailsSectionComponent implements OnInit {
   }
 
   ngOnInit() {
-    // If the original cipher has a card, use it. Otherwise, create a new card instance
-    this.cardView = this.originalCipherView?.card ?? new CardView();
-
     if (this.originalCipherView?.card) {
       this.setInitialValues();
     }

libs/vault/src/cipher-form/components/cipher-form.component.ts

@@ -143,12 +143,11 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci
   }
 
   /**
-   * Patches the updated cipher with the provided partial cipher. Used by child components to update the cipher
+   * Method to update the cipherView with the new values. This method should be called by the child form components
-   * as their form values change.
+   * @param updateFn - A function that takes the current cipherView and returns the updated cipherView
-   * @param cipher
    */
-  patchCipher(cipher: Partial<CipherView>): void {
+  patchCipher(updateFn: (current: CipherView) => CipherView): void {
-    this.updatedCipherView = Object.assign(this.updatedCipherView, cipher);
+    this.updatedCipherView = updateFn(this.updatedCipherView);
   }
 
   /**

libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.spec.ts

@@ -270,7 +270,11 @@ describe("CustomFieldsComponent", () => {
       fieldView.value = "new text value";
       fieldView.type = FieldType.Text;
 
-      expect(patchCipher).toHaveBeenCalledWith({ fields: [fieldView] });
+      expect(patchCipher).toHaveBeenCalled();
+      const patchFn = patchCipher.mock.lastCall[0];
+
+      const updatedCipher = patchFn(new CipherView());
+      expect(updatedCipher.fields).toEqual([fieldView]);
     });
 
     it("updates the label", () => {
@@ -281,7 +285,11 @@ describe("CustomFieldsComponent", () => {
       fieldView.value = "text value";
       fieldView.type = FieldType.Text;
 
-      expect(patchCipher).toHaveBeenCalledWith({ fields: [fieldView] });
+      expect(patchCipher).toHaveBeenCalled();
+      const patchFn = patchCipher.mock.lastCall[0];
+
+      const updatedCipher = patchFn(new CipherView());
+      expect(updatedCipher.fields).toEqual([fieldView]);
     });
   });
 
@@ -295,7 +303,11 @@ describe("CustomFieldsComponent", () => {
     it("removes the field", () => {
       component.removeField(0);
 
-      expect(patchCipher).toHaveBeenCalledWith({ fields: [] });
+      expect(patchCipher).toHaveBeenCalled();
+      const patchFn = patchCipher.mock.lastCall[0];
+
+      const updatedCipher = patchFn(new CipherView());
+      expect(updatedCipher.fields).toEqual([]);
     });
   });
 
@@ -325,9 +337,12 @@ describe("CustomFieldsComponent", () => {
       // Move second field to first
       component.drop({ previousIndex: 0, currentIndex: 1 } as CdkDragDrop<HTMLDivElement>);
 
-      const latestCallParams = patchCipher.mock.lastCall[0];
+      expect(patchCipher).toHaveBeenCalled();
+      const patchFn = patchCipher.mock.lastCall[0];
 
-      expect(latestCallParams.fields.map((f: FieldView) => f.name)).toEqual([
+      const updatedCipher = patchFn(new CipherView());
+
+      expect(updatedCipher.fields.map((f: FieldView) => f.name)).toEqual([
         "hidden label",
         "text label",
         "boolean label",
@@ -342,9 +357,12 @@ describe("CustomFieldsComponent", () => {
         preventDefault: jest.fn(),
       });
 
-      const latestCallParams = patchCipher.mock.lastCall[0];
+      expect(patchCipher).toHaveBeenCalled();
+      const patchFn = patchCipher.mock.lastCall[0];
 
-      expect(latestCallParams.fields.map((f: FieldView) => f.name)).toEqual([
+      const updatedCipher = patchFn(new CipherView());
+
+      expect(updatedCipher.fields.map((f: FieldView) => f.name)).toEqual([
         "text label",
         "hidden label",
         "linked label",
@@ -356,9 +374,12 @@ describe("CustomFieldsComponent", () => {
       // Move 2nd item (hidden label) up to 1st
       toggleItems[1].triggerEventHandler("keydown", { key: "ArrowUp", preventDefault: jest.fn() });
 
-      const latestCallParams = patchCipher.mock.lastCall[0];
+      expect(patchCipher).toHaveBeenCalled();
+      const patchFn = patchCipher.mock.lastCall[0];
 
-      expect(latestCallParams.fields.map((f: FieldView) => f.name)).toEqual([
+      const updatedCipher = patchFn(new CipherView());
+
+      expect(updatedCipher.fields.map((f: FieldView) => f.name)).toEqual([
         "hidden label",
         "text label",
         "boolean label",

libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.ts

@@ -8,11 +8,11 @@ import {
   DestroyRef,
   ElementRef,
   EventEmitter,
+  inject,
   OnInit,
   Output,
   QueryList,
   ViewChildren,
-  inject,
 } from "@angular/core";
 import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
 import { FormArray, FormBuilder, FormsModule, ReactiveFormsModule } from "@angular/forms";
@@ -26,16 +26,16 @@ import { FieldView } from "@bitwarden/common/vault/models/view/field.view";
 import { IdentityView } from "@bitwarden/common/vault/models/view/identity.view";
 import { LoginView } from "@bitwarden/common/vault/models/view/login.view";
 import {
+  CardComponent,
+  CheckboxModule,
   DialogService,
+  FormFieldModule,
+  IconButtonModule,
+  LinkModule,
   SectionComponent,
   SectionHeaderComponent,
-  FormFieldModule,
-  TypographyModule,
-  CardComponent,
-  IconButtonModule,
-  CheckboxModule,
   SelectModule,
-  LinkModule,
+  TypographyModule,
 } from "@bitwarden/components";
 
 import { CipherFormContainer } from "../../cipher-form-container";
@@ -344,8 +344,9 @@ export class CustomFieldsComponent implements OnInit, AfterViewInit {
 
     this.numberOfFieldsChange.emit(newFields.length);
 
-    this.cipherFormContainer.patchCipher({
+    this.cipherFormContainer.patchCipher((cipher) => {
-      fields: newFields,
+      cipher.fields = newFields;
+      return cipher;
     });
   }
 }

libs/vault/src/cipher-form/components/identity/identity.component.ts

@@ -9,11 +9,11 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { IdentityView } from "@bitwarden/common/vault/models/view/identity.view";
 import {
   ButtonModule,
-  SectionComponent,
-  SectionHeaderComponent,
   CardComponent,
   FormFieldModule,
   IconButtonModule,
+  SectionComponent,
+  SectionHeaderComponent,
   SelectModule,
   TypographyModule,
 } from "@bitwarden/components";
@@ -98,8 +98,9 @@ export class IdentitySectionComponent implements OnInit {
       data.postalCode = value.postalCode;
       data.country = value.country;
 
-      this.cipherFormContainer.patchCipher({
+      this.cipherFormContainer.patchCipher((cipher) => {
-        identity: data,
+        cipher.identity = data;
+        return cipher;
       });
     });
   }

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

@@ -62,13 +62,17 @@ describe("ItemDetailsSectionComponent", () => {
       component.config.organizations = [{ id: "org1" } as Organization];
       await component.ngOnInit();
       tick();
-      expect(cipherFormProvider.patchCipher).toHaveBeenLastCalledWith({
+
-        name: "",
+      expect(cipherFormProvider.patchCipher).toHaveBeenCalled();
-        organizationId: null,
+      const patchFn = cipherFormProvider.patchCipher.mock.lastCall[0];
-        folderId: null,
+
-        collectionIds: [],
+      const updatedCipher = patchFn(new CipherView());
-        favorite: false,
+
-      });
+      expect(updatedCipher.name).toBe("");
+      expect(updatedCipher.organizationId).toBeNull();
+      expect(updatedCipher.folderId).toBeNull();
+      expect(updatedCipher.collectionIds).toEqual([]);
+      expect(updatedCipher.favorite).toBe(false);
     }));
 
     it("should initialize form with values from originalCipher if provided", fakeAsync(async () => {
@@ -88,13 +92,16 @@ describe("ItemDetailsSectionComponent", () => {
       await component.ngOnInit();
       tick();
 
-      expect(cipherFormProvider.patchCipher).toHaveBeenLastCalledWith({
+      expect(cipherFormProvider.patchCipher).toHaveBeenCalled();
-        name: "cipher1",
+      const patchFn = cipherFormProvider.patchCipher.mock.lastCall[0];
-        organizationId: "org1",
+
-        folderId: "folder1",
+      const updatedCipher = patchFn(new CipherView());
-        collectionIds: ["col1"],
+
-        favorite: true,
+      expect(updatedCipher.name).toBe("cipher1");
-      });
+      expect(updatedCipher.organizationId).toBe("org1");
+      expect(updatedCipher.folderId).toBe("folder1");
+      expect(updatedCipher.collectionIds).toEqual(["col1"]);
+      expect(updatedCipher.favorite).toBe(true);
     }));
 
     it("should disable organizationId control if ownership change is not allowed", async () => {
@@ -294,11 +301,12 @@ describe("ItemDetailsSectionComponent", () => {
       fixture.detectChanges();
       await fixture.whenStable();
 
-      expect(cipherFormProvider.patchCipher).toHaveBeenLastCalledWith(
+      expect(cipherFormProvider.patchCipher).toHaveBeenCalled();
-        expect.objectContaining({
+      const patchFn = cipherFormProvider.patchCipher.mock.lastCall[0];
-          collectionIds: ["col1", "col2"],
+
-        }),
+      const updatedCipher = patchFn(new CipherView());
-      );
+
+      expect(updatedCipher.collectionIds).toEqual(["col1", "col2"]);
     });
 
     it("should automatically select the first collection if only one is available", async () => {

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

@@ -110,12 +110,15 @@ export class ItemDetailsSectionComponent implements OnInit {
         map(() => this.itemDetailsForm.getRawValue()),
       )
       .subscribe((value) => {
-        this.cipherFormContainer.patchCipher({
+        this.cipherFormContainer.patchCipher((cipher) => {
-          name: value.name,
+          Object.assign(cipher, {
-          organizationId: value.organizationId,
+            name: value.name,
-          folderId: value.folderId,
+            organizationId: value.organizationId,
-          collectionIds: value.collectionIds?.map((c) => c.id) || [],
+            folderId: value.folderId,
-          favorite: value.favorite,
+            collectionIds: value.collectionIds?.map((c) => c.id) || [],
+            favorite: value.favorite,
+          } as CipherView);
+          return cipher;
         });
       });
   }
@@ -212,7 +215,6 @@ export class ItemDetailsSectionComponent implements OnInit {
       this.itemDetailsForm.controls.favorite.enable();
       this.itemDetailsForm.controls.folderId.enable();
     } else if (this.config.mode === "edit") {
-      //
       this.readOnlyCollections = this.collections
         .filter(
           (c) => c.readOnly && this.originalCipherView.collectionIds.includes(c.id as CollectionId),

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

@@ -70,13 +70,14 @@ describe("LoginDetailsSectionComponent", () => {
       totp: "123456",
     });
 
-    expect(cipherFormContainer.patchCipher).toHaveBeenLastCalledWith({
+    expect(cipherFormContainer.patchCipher).toHaveBeenCalled();
-      login: expect.objectContaining({
+    const patchFn = cipherFormContainer.patchCipher.mock.lastCall[0];
-        username: "new-username",
+
-        password: "secret-password",
+    const updatedCipher = patchFn(new CipherView());
-        totp: "123456",
+
-      }),
+    expect(updatedCipher.login.username).toBe("new-username");
-    });
+    expect(updatedCipher.login.password).toBe("secret-password");
+    expect(updatedCipher.login.totp).toBe("123456");
   });
 
   it("disables 'loginDetailsForm' when in partial-edit mode", async () => {
@@ -154,12 +155,13 @@ describe("LoginDetailsSectionComponent", () => {
         username: "new-username",
       });
 
-      expect(cipherFormContainer.patchCipher).toHaveBeenLastCalledWith({
+      expect(cipherFormContainer.patchCipher).toHaveBeenCalled();
-        login: expect.objectContaining({
+      const patchFn = cipherFormContainer.patchCipher.mock.lastCall[0];
-          username: "new-username",
+
-          password: "original-password",
+      const updatedCipher = patchFn(new CipherView());
-        }),
+
-      });
+      expect(updatedCipher.login.username).toBe("new-username");
+      expect(updatedCipher.login.password).toBe("original-password");
     });
   });
 
@@ -493,11 +495,13 @@ describe("LoginDetailsSectionComponent", () => {
 
       tick();
 
-      expect(cipherFormContainer.patchCipher).toHaveBeenLastCalledWith({
+      expect(cipherFormContainer.patchCipher).toHaveBeenCalled();
-        login: expect.objectContaining({
+      const patchFn = cipherFormContainer.patchCipher.mock.lastCall[0];
-          fido2Credentials: null,
+
-        }),
+      const updatedCipher = patchFn(new CipherView());
-      });
+
+      expect(updatedCipher.login.fido2Credentials).toBeNull();
+      expect(component.hasPasskey).toBe(false);
     }));
   });
 });

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

@@ -7,6 +7,7 @@ import { map } from "rxjs";
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { Fido2CredentialView } from "@bitwarden/common/vault/models/view/fido2-credential.view";
 import { LoginView } from "@bitwarden/common/vault/models/view/login.view";
 import {
   AsyncActionsModule,
@@ -58,16 +59,21 @@ export class LoginDetailsSectionComponent implements OnInit {
 
   private datePipe = inject(DatePipe);
 
-  private loginView: LoginView;
+  /**
+   * A local reference to the Fido2 credentials for an existing login being edited.
+   * These cannot be created in the form and thus have no form control.
+   * @private
+   */
+  private existingFido2Credentials?: Fido2CredentialView[];
 
   get hasPasskey(): boolean {
-    return this.loginView?.hasFido2Credentials;
+    return this.existingFido2Credentials != null && this.existingFido2Credentials.length > 0;
   }
 
   get fido2CredentialCreationDateValue(): string {
     const dateCreated = this.i18nService.t("dateCreated");
     const creationDate = this.datePipe.transform(
-      this.loginView?.fido2Credentials?.[0]?.creationDate,
+      this.existingFido2Credentials?.[0]?.creationDate,
       "short",
     );
     return `${dateCreated} ${creationDate}`;
@@ -98,20 +104,19 @@ export class LoginDetailsSectionComponent implements OnInit {
         map(() => this.loginDetailsForm.getRawValue()),
       )
       .subscribe((value) => {
-        Object.assign(this.loginView, {
+        this.cipherFormContainer.patchCipher((cipher) => {
-          username: value.username,
+          Object.assign(cipher.login, {
-          password: value.password,
+            username: value.username,
-          totp: value.totp,
+            password: value.password,
-        } as LoginView);
+            totp: value.totp,
+          } as LoginView);
 
-        this.cipherFormContainer.patchCipher({
+          return cipher;
-          login: this.loginView,
         });
       });
   }
 
   async ngOnInit() {
-    this.loginView = new LoginView();
     if (this.cipherFormContainer.originalCipherView?.login) {
       this.initFromExistingCipher(this.cipherFormContainer.originalCipherView.login);
     } else {
@@ -124,15 +129,14 @@ export class LoginDetailsSectionComponent implements OnInit {
   }
 
   private initFromExistingCipher(existingLogin: LoginView) {
-    // Note: this.loginView will still contain references to the existing login's Uri and Fido2Credential arrays.
-    // We may need to deep clone these in the future.
-    Object.assign(this.loginView, existingLogin);
     this.loginDetailsForm.patchValue({
-      username: this.loginView.username,
+      username: existingLogin.username,
-      password: this.loginView.password,
+      password: existingLogin.password,
-      totp: this.loginView.totp,
+      totp: existingLogin.totp,
     });
 
+    this.existingFido2Credentials = existingLogin.fido2Credentials;
+
     if (!this.viewHiddenFields) {
       this.loginDetailsForm.controls.password.disable();
       this.loginDetailsForm.controls.totp.disable();
@@ -170,9 +174,10 @@ export class LoginDetailsSectionComponent implements OnInit {
 
   removePasskey = async () => {
     // Fido2Credentials do not have a form control, so update directly
-    this.loginView.fido2Credentials = null;
+    this.existingFido2Credentials = null;
-    this.cipherFormContainer.patchCipher({
+    this.cipherFormContainer.patchCipher((cipher) => {
-      login: this.loginView,
+      cipher.login.fido2Credentials = null;
+      return cipher;
     });
   };