From 200fc71c5cb4d0042da5c3fbcb6f09ceed4dda64 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Wed, 17 Sep 2025 13:58:46 -0500 Subject: [PATCH] [PM-24305] Only enable the cipher form when it is disabled (#16259) * only enable the cipher form when it is disabled * switch to tracking the last emitted state for disabled rather than the form status. The form status can be changed if any child control changes --- .../components/cipher-form.component.spec.ts | 29 +++++++++++++++++++ .../components/cipher-form.component.ts | 16 +++++++--- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts b/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts index da687f33ef9..970e6b1fb9a 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts @@ -122,4 +122,33 @@ describe("CipherFormComponent", () => { expect(component["updatedCipherView"]?.login.fido2Credentials).toBeNull(); }); }); + + describe("enableFormFields", () => { + beforeEach(() => { + // disable the form so enabling occurs + component.disableFormFields(); + }); + + it("only enables the form when it is disabled", () => { + jest.spyOn(component["cipherForm"], "enable"); + component.enableFormFields(); + + expect(component["cipherForm"].enable).toHaveBeenCalled(); + + component.enableFormFields(); + component.enableFormFields(); + + // enable is only called once + expect(component["cipherForm"].enable).toHaveBeenCalledTimes(1); + }); + + it("emits formStatusChange$", (done) => { + component.formStatusChange$.subscribe((status) => { + expect(status).toBe("enabled"); + done(); + }); + + component.enableFormFields(); + }); + }); }); diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index be3092861f7..19e7f9d2d90 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -17,7 +17,7 @@ import { } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormBuilder, FormGroup, ReactiveFormsModule } from "@angular/forms"; -import { Subject } from "rxjs"; +import { BehaviorSubject, Subject } from "rxjs"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherType, SecureNoteType } from "@bitwarden/common/vault/enums"; @@ -115,7 +115,7 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci /** * Emitted when the form is enabled */ - private formStatusChangeSubject = new Subject<"enabled" | "disabled">(); + private formStatusChangeSubject = new BehaviorSubject<"enabled" | "disabled" | null>(null); @Output() formStatusChange$ = this.formStatusChangeSubject.asObservable(); /** @@ -161,9 +161,17 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci this.formStatusChangeSubject.next("disabled"); } + /** + * Enables the form, only when it is currently disabled. + * Child forms could have disabled some of their controls based on + * other factors. Enabling the form from this level should only occur + * when the form was disabled at this level. + */ enableFormFields(): void { - this.cipherForm.enable({ emitEvent: false }); - this.formStatusChangeSubject.next("enabled"); + if (this.formStatusChangeSubject.getValue() === "disabled") { + this.cipherForm.enable({ emitEvent: false }); + this.formStatusChangeSubject.next("enabled"); + } } /**