[PM-9111] Extension: persist add/edit form (#12236)

* remove todo

* Retrieve cache cipher for add-edit form

* user prefilled cipher for add-edit form

* add listener for clearing view cache

* clear local cache when clearing global state

* track initial value of cache for down stream logic that should only occur on non-cached values

* add feature flag for edit form persistence

* add tests for cipher form cache service

* fix optional initialValues

* add services to cipher form storybook

* fix strict types

* rename variables to be platform agnostic

* use deconstructed collectionIds variable to avoid them be overwritten

* use the originalCipherView for initial values

* add comment about signal equality

* prevent events from being emitted when adding uris to the existing form

- This stops other values from being overwrote in the initialization process

* add check for cached cipher when adding initial uris

apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts

@@ -133,6 +133,7 @@ export class PopupViewCacheService implements ViewCacheService {
   }
 
   private clearState() {
+    this._cache = {}; // clear local cache
     this.messageSender.send(ClEAR_VIEW_CACHE_COMMAND, {});
   }
 }

apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts

@@ -196,13 +196,15 @@ describe("popup view cache", () => {
   });
 
   it("should clear on 2nd navigation", async () => {
-    await initServiceWithState({});
+    await initServiceWithState({ temp: "state" });
 
     await router.navigate(["a"]);
     expect(messageSenderMock.send).toHaveBeenCalledTimes(0);
+    expect(service["_cache"]).toEqual({ temp: "state" });
 
     await router.navigate(["b"]);
     expect(messageSenderMock.send).toHaveBeenCalledWith(ClEAR_VIEW_CACHE_COMMAND, {});
+    expect(service["_cache"]).toEqual({});
   });
 
   it("should ignore cached values when feature flag is off", async () => {

apps/browser/src/platform/services/popup-view-cache-background.service.ts

@@ -60,6 +60,11 @@ export class PopupViewCacheBackgroundService {
       )
       .subscribe();
 
+    this.messageListener
+      .messages$(ClEAR_VIEW_CACHE_COMMAND)
+      .pipe(concatMap(() => this.popupViewCacheState.update(() => null)))
+      .subscribe();
+
     merge(
       // on tab changed, excluding extension tabs
       fromChromeEvent(chrome.tabs.onActivated).pipe(

libs/common/src/enums/feature-flag.enum.ts

@@ -44,6 +44,7 @@ export enum FeatureFlag {
   NewDeviceVerificationPermanentDismiss = "new-device-permanent-dismiss",
   DisableFreeFamiliesSponsorship = "PM-12274-disable-free-families-sponsorship",
   MacOsNativeCredentialSync = "macos-native-credential-sync",
+  PM9111ExtensionPersistAddEditForm = "pm-9111-extension-persist-add-edit-form",
   PrivateKeyRegeneration = "pm-12241-private-key-regeneration",
   ResellerManagedOrgAlert = "PM-15814-alert-owners-of-reseller-managed-orgs",
 }
@@ -100,6 +101,7 @@ export const DefaultFeatureFlagValue = {
   [FeatureFlag.NewDeviceVerificationPermanentDismiss]: FALSE,
   [FeatureFlag.DisableFreeFamiliesSponsorship]: FALSE,
   [FeatureFlag.MacOsNativeCredentialSync]: FALSE,
+  [FeatureFlag.PM9111ExtensionPersistAddEditForm]: FALSE,
   [FeatureFlag.PrivateKeyRegeneration]: FALSE,
   [FeatureFlag.ResellerManagedOrgAlert]: FALSE,
 } satisfies Record<FeatureFlag, AllowedFeatureFlagTypes>;

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

@@ -14,7 +14,6 @@ import { SshKeySectionComponent } from "./components/sshkey-section/sshkey-secti
 
 /**
  * The complete form for a cipher. Includes all the sub-forms from their respective section components.
- * TODO: Add additional form sections as they are implemented.
  */
 export type CipherForm = {
   itemDetails?: ItemDetailsSectionComponent["itemDetailsForm"];
@@ -57,4 +56,12 @@ export abstract class CipherFormContainer {
    * @param updateFn - A function that takes the current cipherView and returns the updated cipherView
    */
   abstract patchCipher(updateFn: (current: CipherView) => CipherView): void;
+
+  /**
+   * Returns initial values for the CipherView, either from the config or the cached cipher
+   */
+  abstract getInitialCipherView(): CipherView | null;
+
+  /** Returns true when the `CipherFormContainer` was initialized with a cached cipher available. */
+  abstract initializedWithCachedCipher(): boolean;
 }

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

@@ -1,4 +1,6 @@
-import { importProvidersFrom } from "@angular/core";
+// FIXME: Update this file to be type safe and remove this and next line
+// @ts-strict-ignore
+import { importProvidersFrom, signal } from "@angular/core";
 import { action } from "@storybook/addon-actions";
 import {
   applicationConfig,
@@ -10,6 +12,7 @@ import {
 import { BehaviorSubject } from "rxjs";
 
 import { CollectionView } from "@bitwarden/admin-console/common";
+import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
@@ -18,6 +21,7 @@ import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/s
 import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
 import { ClientType } from "@bitwarden/common/enums";
 import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service";
+import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
@@ -39,6 +43,7 @@ import { CipherFormService } from "./abstractions/cipher-form.service";
 import { TotpCaptureService } from "./abstractions/totp-capture.service";
 import { CipherFormModule } from "./cipher-form.module";
 import { CipherFormComponent } from "./components/cipher-form.component";
+import { CipherFormCacheService } from "./services/default-cipher-form-cache.service";
 
 const defaultConfig: CipherFormConfig = {
   mode: "add",
@@ -192,6 +197,25 @@ export default {
             activeAccount$: new BehaviorSubject({ email: "[email protected]" }),
           },
         },
+        {
+          provide: CipherFormCacheService,
+          useValue: {
+            getCachedCipherView: (): null => null,
+            initializedWithValue: false,
+          },
+        },
+        {
+          provide: ViewCacheService,
+          useValue: {
+            signal: () => signal(null),
+          },
+        },
+        {
+          provide: ConfigService,
+          useValue: {
+            getFeatureFlag: () => Promise.resolve(false),
+          },
+        },
       ],
     }),
     componentWrapperDecorator(

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

@@ -27,9 +27,12 @@ describe("AdditionalOptionsSectionComponent", () => {
   let passwordRepromptService: MockProxy<PasswordRepromptService>;
   let passwordRepromptEnabled$: BehaviorSubject<boolean>;
 
+  const getInitialCipherView = jest.fn(() => null);
+
   beforeEach(async () => {
-    cipherFormProvider = mock<CipherFormContainer>();
+    getInitialCipherView.mockClear();
 
+    cipherFormProvider = mock<CipherFormContainer>({ getInitialCipherView });
     passwordRepromptService = mock<PasswordRepromptService>();
     passwordRepromptEnabled$ = new BehaviorSubject(true);
     passwordRepromptService.enabled$ = passwordRepromptEnabled$;
@@ -94,11 +97,11 @@ describe("AdditionalOptionsSectionComponent", () => {
     expect(component.additionalOptionsForm.disabled).toBe(true);
   });
 
-  it("initializes 'additionalOptionsForm' with original cipher view values", () => {
-    (cipherFormProvider.originalCipherView as any) = {
+  it("initializes 'additionalOptionsForm' from `getInitialCipherValue`", () => {
+    getInitialCipherView.mockReturnValueOnce({
       notes: "original notes",
       reprompt: 1,
-    } as CipherView;
+    } as CipherView);
 
     component.ngOnInit();
 

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

@@ -77,11 +77,12 @@ export class AdditionalOptionsSectionComponent implements OnInit {
   }
 
   ngOnInit() {
-    if (this.cipherFormContainer.originalCipherView) {
+    const prefillCipher = this.cipherFormContainer.getInitialCipherView();
+
+    if (prefillCipher) {
       this.additionalOptionsForm.patchValue({
-        notes: this.cipherFormContainer.originalCipherView.notes,
-        reprompt:
-          this.cipherFormContainer.originalCipherView.reprompt === CipherRepromptType.Password,
+        notes: prefillCipher.notes,
+        reprompt: prefillCipher.reprompt === CipherRepromptType.Password,
       });
     }
 

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

@@ -25,9 +25,11 @@ describe("AutofillOptionsComponent", () => {
   let domainSettingsService: MockProxy<DomainSettingsService>;
   let autofillSettingsService: MockProxy<AutofillSettingsServiceAbstraction>;
   let platformUtilsService: MockProxy<PlatformUtilsService>;
+  const getInitialCipherView = jest.fn(() => null);
 
   beforeEach(async () => {
-    cipherFormContainer = mock<CipherFormContainer>();
+    getInitialCipherView.mockClear();
+    cipherFormContainer = mock<CipherFormContainer>({ getInitialCipherView });
     liveAnnouncer = mock<LiveAnnouncer>();
     platformUtilsService = mock<PlatformUtilsService>();
     domainSettingsService = mock<DomainSettingsService>();
@@ -107,12 +109,14 @@ describe("AutofillOptionsComponent", () => {
     existingLogin.uri = "https://example.com";
     existingLogin.match = UriMatchStrategy.Exact;
 
-    (cipherFormContainer.originalCipherView as CipherView) = new CipherView();
-    cipherFormContainer.originalCipherView.login = {
+    const cipher = new CipherView();
+    cipher.login = {
       autofillOnPageLoad: true,
       uris: [existingLogin],
     } as LoginView;
 
+    getInitialCipherView.mockReturnValueOnce(cipher);
+
     fixture.detectChanges();
 
     expect(component.autofillOptionsForm.value.uris).toEqual([
@@ -138,12 +142,14 @@ describe("AutofillOptionsComponent", () => {
     existingLogin.uri = "https://example.com";
     existingLogin.match = UriMatchStrategy.Exact;
 
-    (cipherFormContainer.originalCipherView as CipherView) = new CipherView();
-    cipherFormContainer.originalCipherView.login = {
+    const cipher = new CipherView();
+    cipher.login = {
       autofillOnPageLoad: true,
       uris: [existingLogin],
     } as LoginView;
 
+    getInitialCipherView.mockReturnValueOnce(cipher);
+
     fixture.detectChanges();
 
     expect(component.autofillOptionsForm.value.uris).toEqual([
@@ -159,12 +165,14 @@ describe("AutofillOptionsComponent", () => {
     existingLogin.uri = "https://example.com";
     existingLogin.match = UriMatchStrategy.Exact;
 
-    (cipherFormContainer.originalCipherView as CipherView) = new CipherView();
-    cipherFormContainer.originalCipherView.login = {
+    const cipher = new CipherView();
+    cipher.login = {
       autofillOnPageLoad: true,
       uris: [existingLogin],
     } as LoginView;
 
+    getInitialCipherView.mockReturnValueOnce(cipher);
+
     fixture.detectChanges();
 
     expect(component.autofillOptionsForm.value.uris).toEqual([

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

@@ -130,8 +130,9 @@ export class AutofillOptionsComponent implements OnInit {
   }
 
   ngOnInit() {
-    if (this.cipherFormContainer.originalCipherView?.login) {
-      this.initFromExistingCipher(this.cipherFormContainer.originalCipherView.login);
+    const prefillCipher = this.cipherFormContainer.getInitialCipherView();
+    if (prefillCipher) {
+      this.initFromExistingCipher(prefillCipher.login);
     } else {
       this.initNewCipher();
     }
@@ -142,17 +143,29 @@ export class AutofillOptionsComponent implements OnInit {
   }
 
   private initFromExistingCipher(existingLogin: LoginView) {
+    // The `uris` control is a FormArray which needs to dynamically
+    // add controls to the form. Doing this will trigger the `valueChanges` observable on the form
+    // and overwrite the `autofillOnPageLoad` value before it is set in the following `patchValue` call.
+    // Pass `false` to `addUri` to stop events from emitting when adding the URIs.
     existingLogin.uris?.forEach((uri) => {
-      this.addUri({
-        uri: uri.uri,
-        matchDetection: uri.match,
-      });
+      this.addUri(
+        {
+          uri: uri.uri,
+          matchDetection: uri.match,
+        },
+        false,
+        false,
+      );
     });
     this.autofillOptionsForm.patchValue({
       autofillOnPageLoad: existingLogin.autofillOnPageLoad,
     });
 
-    if (this.cipherFormContainer.config.initialValues?.loginUri) {
+    // Only add the initial value when the cipher was not initialized from a cached state
+    if (
+      this.cipherFormContainer.config.initialValues?.loginUri &&
+      !this.cipherFormContainer.initializedWithCachedCipher()
+    ) {
       // Avoid adding the same uri again if it already exists
       if (
         existingLogin.uris?.findIndex(
@@ -197,9 +210,16 @@ export class AutofillOptionsComponent implements OnInit {
    * Adds a new URI input to the form.
    * @param uriFieldValue The initial value for the new URI input.
    * @param focusNewInput If true, the new URI input will be focused after being added.
+   * @param emitEvent When false, prevents the `valueChanges` & `statusChanges` observables from firing.
    */
-  addUri(uriFieldValue: UriField = { uri: null, matchDetection: null }, focusNewInput = false) {
-    this.autofillOptionsForm.controls.uris.push(this.formBuilder.control(uriFieldValue));
+  addUri(
+    uriFieldValue: UriField = { uri: null, matchDetection: null },
+    focusNewInput = false,
+    emitEvent = true,
+  ) {
+    this.autofillOptionsForm.controls.uris.push(this.formBuilder.control(uriFieldValue), {
+      emitEvent,
+    });
 
     if (focusNewInput) {
       this.focusOnNewInput$.next();

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

@@ -20,8 +20,10 @@ describe("CardDetailsSectionComponent", () => {
   let registerChildFormSpy: jest.SpyInstance;
   let patchCipherSpy: jest.SpyInstance;
 
+  const getInitialCipherView = jest.fn(() => null);
+
   beforeEach(async () => {
-    cipherFormProvider = mock<CipherFormContainer>();
+    cipherFormProvider = mock<CipherFormContainer>({ getInitialCipherView });
     registerChildFormSpy = jest.spyOn(cipherFormProvider, "registerChildForm");
     patchCipherSpy = jest.spyOn(cipherFormProvider, "patchCipher");
 
@@ -94,7 +96,7 @@ describe("CardDetailsSectionComponent", () => {
     expect(component.cardDetailsForm.disabled).toBe(true);
   });
 
-  it("initializes `cardDetailsForm` with current values", () => {
+  it("initializes `cardDetailsForm` from `getInitialCipherValue`", () => {
     const cardholderName = "Ron Burgundy";
     const number = "4242 4242 4242 4242";
     const code = "619";
@@ -105,9 +107,7 @@ describe("CardDetailsSectionComponent", () => {
     cardView.code = code;
     cardView.brand = "Visa";
 
-    component.originalCipherView = {
-      card: cardView,
-    } as CipherView;
+    getInitialCipherView.mockReturnValueOnce({ card: cardView });
 
     component.ngOnInit();
 

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

@@ -136,8 +136,10 @@ export class CardDetailsSectionComponent implements OnInit {
   }
 
   ngOnInit() {
-    if (this.originalCipherView?.card) {
-      this.setInitialValues();
+    const prefillCipher = this.cipherFormContainer.getInitialCipherView();
+
+    if (prefillCipher) {
+      this.setInitialValues(prefillCipher);
     }
 
     if (this.disabled) {
@@ -172,8 +174,8 @@ export class CardDetailsSectionComponent implements OnInit {
   }
 
   /** Set form initial form values from the current cipher */
-  private setInitialValues() {
-    const { cardholderName, number, brand, expMonth, expYear, code } = this.originalCipherView.card;
+  private setInitialValues(cipherView: CipherView) {
+    const { cardholderName, number, brand, expMonth, expYear, code } = cipherView.card;
 
     this.cardDetailsForm.setValue({
       cardholderName: cardholderName,