From 0324acaf7e88d3bc494ab1928a9d5a426ff597c2 Mon Sep 17 00:00:00 2001
From: Matt Gibson <mgibson@bitwarden.com>
Date: Wed, 4 Oct 2023 12:28:02 -0400
Subject: [PATCH] Prefer return value to parameter mutation

This also solves issues where JS deals with ref updates weird due
to cryptic variable capture rules.

Fixes: replace, clear methods
---
 .../src/platform/interfaces/active-user-state.ts      |  7 ++++++-
 .../services/default-active-user-state.provider.ts    | 11 +++++------
 .../src/vault/services/folder/folder.service.ts       |  6 ++++--
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/libs/common/src/platform/interfaces/active-user-state.ts b/libs/common/src/platform/interfaces/active-user-state.ts
index c5652719719..e1bb7a34c65 100644
--- a/libs/common/src/platform/interfaces/active-user-state.ts
+++ b/libs/common/src/platform/interfaces/active-user-state.ts
@@ -6,7 +6,12 @@ import { DerivedStateDefinition } from "../types/derived-state-definition";
 export interface ActiveUserState<T> {
   readonly state$: Observable<T>;
   readonly getFromState: () => Promise<T>;
-  readonly update: (configureState: (state: T) => void) => Promise<void>;
+  /**
+   *
+   * @param configureState function that takes the current state and returns the new state
+   * @returns The new state
+   */
+  readonly update: (configureState: (state: T) => T) => Promise<T>;
   createDerived: <TTo>(
     derivedStateDefinition: DerivedStateDefinition<T, TTo>
   ) => DerivedActiveUserState<T, TTo>;
diff --git a/libs/common/src/platform/services/default-active-user-state.provider.ts b/libs/common/src/platform/services/default-active-user-state.provider.ts
index 06f407adb9f..24792e07dc9 100644
--- a/libs/common/src/platform/services/default-active-user-state.provider.ts
+++ b/libs/common/src/platform/services/default-active-user-state.provider.ts
@@ -90,7 +90,6 @@ class DefaultActiveUserState<T> implements ActiveUserState<T> {
     );
     // startWith?
     this.formattedKey$ = this.accountService.activeAccount$.pipe(
-      tap((user) => console.log("user", user)), // Temp
       map((account) =>
         account != null && account.id != null
           ? userKeyBuilder(account.id, this.keyDefinition)
@@ -100,7 +99,6 @@ class DefaultActiveUserState<T> implements ActiveUserState<T> {
 
     const activeAccountData$ = this.formattedKey$.pipe(
       switchMap(async (key) => {
-        console.log("user emitted: ", key); // temp
         if (key == null) {
           return null;
         }
@@ -128,7 +126,7 @@ class DefaultActiveUserState<T> implements ActiveUserState<T> {
     });
   }
 
-  async update(configureState: (state: T) => void): Promise<void> {
+  async update(configureState: (state: T) => T): Promise<T> {
     const key = await this.createKey();
     if (key == null) {
       throw new Error("Attempting to active user state, when no user is active.");
@@ -137,10 +135,11 @@ class DefaultActiveUserState<T> implements ActiveUserState<T> {
       ? this.stateSubject.getValue()
       : await this.seedInitial(key);
 
-    configureState(currentState);
+    const newState = configureState(currentState);
 
-    await this.chosenStorageLocation.save(await this.createKey(), currentState);
-    this.stateSubject.next(currentState);
+    await this.chosenStorageLocation.save(await this.createKey(), newState);
+    this.stateSubject.next(newState);
+    return newState;
   }
 
   async getFromState(): Promise<T> {
diff --git a/libs/common/src/vault/services/folder/folder.service.ts b/libs/common/src/vault/services/folder/folder.service.ts
index f7d05914bf8..fade328163c 100644
--- a/libs/common/src/vault/services/folder/folder.service.ts
+++ b/libs/common/src/vault/services/folder/folder.service.ts
@@ -106,15 +106,16 @@ export class FolderService implements InternalFolderServiceAbstraction {
           folders[f.id] = f;
         });
       }
+      return folders;
     });
   }
 
   async replace(folders: { [id: string]: FolderData }): Promise<void> {
-    await this.folderState.update((f) => (f = folders));
+    await this.folderState.update((_) => folders);
   }
 
   async clear(userId?: string): Promise<any> {
-    await this.folderState.update((f) => (f = null));
+    await this.folderState.update((_) => null);
   }
 
   async delete(id: string | string[]): Promise<void> {
@@ -123,6 +124,7 @@ export class FolderService implements InternalFolderServiceAbstraction {
       folderIds.forEach((folderId) => {
         delete folders[folderId];
       });
+      return folders;
     });
 
     // Items in a deleted folder are re-assigned to "No Folder"