From a1bc66b10ab0cb2e84b972a8ee49de23a2f196d1 Mon Sep 17 00:00:00 2001
From: Matiss Janis Aboltins <matiss@mja.lv>
Date: Fri, 20 Sep 2024 20:01:51 +0100
Subject: [PATCH] :recycle: (synced-prefs) separate metadata and local prefs
 out (#3458)

---
 .../src/components/FinancesApp.tsx            | 50 ++++++++++++++++---
 .../desktop-client/src/hooks/useLocalPref.ts  | 41 ++++++++++-----
 .../src/hooks/useMetadataPref.ts              | 22 ++++++--
 .../loot-core/src/client/actions/prefs.ts     |  7 ++-
 .../src/client/state-types/prefs.d.ts         |  8 +--
 .../src/client/update-notification.ts         | 45 -----------------
 packages/loot-core/src/server/sync/index.ts   |  4 +-
 packages/loot-core/src/types/prefs.d.ts       | 39 +++++++--------
 .../loot-core/src/types/server-handlers.d.ts  |  4 +-
 upcoming-release-notes/3458.md                |  6 +++
 10 files changed, 122 insertions(+), 104 deletions(-)
 delete mode 100644 packages/loot-core/src/client/update-notification.ts
 create mode 100644 upcoming-release-notes/3458.md

diff --git a/packages/desktop-client/src/components/FinancesApp.tsx b/packages/desktop-client/src/components/FinancesApp.tsx
index a2ae37a50..73b8948ad 100644
--- a/packages/desktop-client/src/components/FinancesApp.tsx
+++ b/packages/desktop-client/src/components/FinancesApp.tsx
@@ -1,5 +1,6 @@
 // @ts-strict-ignore
 import React, { type ReactElement, useEffect } from 'react';
+import { useTranslation } from 'react-i18next';
 import { useDispatch, useSelector } from 'react-redux';
 import {
   Route,
@@ -9,12 +10,12 @@ import {
   useHref,
 } from 'react-router-dom';
 
-import { sync } from 'loot-core/client/actions';
+import { addNotification, sync } from 'loot-core/client/actions';
 import { type State } from 'loot-core/src/client/state-types';
-import { checkForUpdateNotification } from 'loot-core/src/client/update-notification';
 import * as undo from 'loot-core/src/platform/client/undo';
 
 import { useAccounts } from '../hooks/useAccounts';
+import { useLocalPref } from '../hooks/useLocalPref';
 import { useNavigate } from '../hooks/useNavigate';
 import { useResponsive } from '../ResponsiveProvider';
 import { theme } from '../style';
@@ -88,20 +89,53 @@ function RouterBehaviors() {
 
 export function FinancesApp() {
   const dispatch = useDispatch();
+  const { t } = useTranslation();
+
+  const [lastUsedVersion, setLastUsedVersion] = useLocalPref(
+    'flags.updateNotificationShownForVersion',
+  );
+
   useEffect(() => {
     // Wait a little bit to make sure the sync button will get the
     // sync start event. This can be improved later.
     setTimeout(async () => {
       await dispatch(sync());
-
-      await checkForUpdateNotification(
-        dispatch,
-        getIsOutdated,
-        getLatestVersion,
-      );
     }, 100);
   }, []);
 
+  useEffect(() => {
+    async function run() {
+      const latestVersion = await getLatestVersion();
+      const isOutdated = await getIsOutdated(latestVersion);
+
+      if (isOutdated && lastUsedVersion !== latestVersion) {
+        dispatch(
+          addNotification({
+            type: 'message',
+            title: t('A new version of Actual is available!'),
+            message: t(
+              'Version {{latestVersion}} of Actual was recently released.',
+              { latestVersion },
+            ),
+            sticky: true,
+            id: 'update-notification',
+            button: {
+              title: t('Open changelog'),
+              action: () => {
+                window.open('https://actualbudget.org/docs/releases');
+              },
+            },
+            onClose: () => {
+              setLastUsedVersion(latestVersion);
+            },
+          }),
+        );
+      }
+    }
+
+    run();
+  }, [lastUsedVersion, setLastUsedVersion]);
+
   return (
     <View style={{ height: '100%' }}>
       <RouterBehaviors />
diff --git a/packages/desktop-client/src/hooks/useLocalPref.ts b/packages/desktop-client/src/hooks/useLocalPref.ts
index 70a50cb55..3da16da93 100644
--- a/packages/desktop-client/src/hooks/useLocalPref.ts
+++ b/packages/desktop-client/src/hooks/useLocalPref.ts
@@ -1,27 +1,42 @@
-import { useCallback } from 'react';
-import { useDispatch, useSelector } from 'react-redux';
+import { useEffect } from 'react';
+
+import { useLocalStorage } from 'usehooks-ts';
 
-import { savePrefs } from 'loot-core/src/client/actions';
-import { type State } from 'loot-core/src/client/state-types';
 import { type LocalPrefs } from 'loot-core/src/types/prefs';
 
+import { useMetadataPref } from './useMetadataPref';
+
 type SetLocalPrefAction<K extends keyof LocalPrefs> = (
   value: LocalPrefs[K],
 ) => void;
 
+/**
+ * Local preferences are scoped to a specific budget file.
+ */
 export function useLocalPref<K extends keyof LocalPrefs>(
   prefName: K,
 ): [LocalPrefs[K], SetLocalPrefAction<K>] {
-  const dispatch = useDispatch();
-  const setLocalPref = useCallback<SetLocalPrefAction<K>>(
-    value => {
-      dispatch(savePrefs({ [prefName]: value } as LocalPrefs));
+  const [budgetId] = useMetadataPref('id');
+
+  const [value, setValue] = useLocalStorage<LocalPrefs[K]>(
+    `${budgetId}-${prefName}`,
+    undefined,
+    {
+      deserializer: JSON.parse,
+      serializer: JSON.stringify,
     },
-    [prefName, dispatch],
-  );
-  const localPref = useSelector(
-    (state: State) => state.prefs.local?.[prefName] as LocalPrefs[K],
   );
 
-  return [localPref, setLocalPref];
+  // Migrate from old pref storage location (metadata.json) to local storage
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const [metadataPref] = useMetadataPref(prefName as any);
+  useEffect(() => {
+    if (value !== undefined || metadataPref === undefined) {
+      return;
+    }
+
+    setValue(metadataPref);
+  }, [value, metadataPref, setValue]);
+
+  return [value, setValue];
 }
diff --git a/packages/desktop-client/src/hooks/useMetadataPref.ts b/packages/desktop-client/src/hooks/useMetadataPref.ts
index 32265fee1..57d4b7902 100644
--- a/packages/desktop-client/src/hooks/useMetadataPref.ts
+++ b/packages/desktop-client/src/hooks/useMetadataPref.ts
@@ -1,6 +1,9 @@
-import { type MetadataPrefs } from 'loot-core/src/types/prefs';
+import { useCallback } from 'react';
+import { useDispatch, useSelector } from 'react-redux';
 
-import { useLocalPref } from './useLocalPref';
+import { savePrefs } from 'loot-core/client/actions';
+import { type State } from 'loot-core/client/state-types';
+import { type MetadataPrefs } from 'loot-core/types/prefs';
 
 type SetMetadataPrefAction<K extends keyof MetadataPrefs> = (
   value: MetadataPrefs[K],
@@ -9,7 +12,16 @@ type SetMetadataPrefAction<K extends keyof MetadataPrefs> = (
 export function useMetadataPref<K extends keyof MetadataPrefs>(
   prefName: K,
 ): [MetadataPrefs[K], SetMetadataPrefAction<K>] {
-  // TODO: implement logic for fetching the pref exclusively from the
-  // metadata.json file (in follow-up PR)
-  return useLocalPref(prefName);
+  const dispatch = useDispatch();
+  const setLocalPref = useCallback<SetMetadataPrefAction<K>>(
+    value => {
+      dispatch(savePrefs({ [prefName]: value }));
+    },
+    [prefName, dispatch],
+  );
+  const localPref = useSelector(
+    (state: State) => state.prefs.local?.[prefName],
+  );
+
+  return [localPref, setLocalPref];
 }
diff --git a/packages/loot-core/src/client/actions/prefs.ts b/packages/loot-core/src/client/actions/prefs.ts
index 76b9272d6..c77be9c1e 100644
--- a/packages/loot-core/src/client/actions/prefs.ts
+++ b/packages/loot-core/src/client/actions/prefs.ts
@@ -1,6 +1,5 @@
-// @ts-strict-ignore
 import { send } from '../../platform/client/fetch';
-import type * as prefs from '../../types/prefs';
+import { type GlobalPrefs, type MetadataPrefs } from '../../types/prefs';
 import * as constants from '../constants';
 
 import { closeModal } from './modals';
@@ -26,7 +25,7 @@ export function loadPrefs() {
   };
 }
 
-export function savePrefs(prefs: prefs.LocalPrefs) {
+export function savePrefs(prefs: MetadataPrefs) {
   return async (dispatch: Dispatch) => {
     await send('save-prefs', prefs);
     dispatch({
@@ -49,7 +48,7 @@ export function loadGlobalPrefs() {
 }
 
 export function saveGlobalPrefs(
-  prefs: prefs.GlobalPrefs,
+  prefs: GlobalPrefs,
   onSaveGlobalPrefs?: () => void,
 ) {
   return async (dispatch: Dispatch) => {
diff --git a/packages/loot-core/src/client/state-types/prefs.d.ts b/packages/loot-core/src/client/state-types/prefs.d.ts
index 7f125962f..4c5e13c13 100644
--- a/packages/loot-core/src/client/state-types/prefs.d.ts
+++ b/packages/loot-core/src/client/state-types/prefs.d.ts
@@ -1,20 +1,20 @@
-import type { GlobalPrefs, LocalPrefs, MetadataPrefs } from '../../types/prefs';
+import type { GlobalPrefs, MetadataPrefs } from '../../types/prefs';
 import type * as constants from '../constants';
 
 export type PrefsState = {
-  local: LocalPrefs & MetadataPrefs;
+  local: MetadataPrefs;
   global: GlobalPrefs;
 };
 
 export type SetPrefsAction = {
   type: typeof constants.SET_PREFS;
-  prefs: LocalPrefs & MetadataPrefs;
+  prefs: MetadataPrefs;
   globalPrefs: GlobalPrefs;
 };
 
 export type MergeLocalPrefsAction = {
   type: typeof constants.MERGE_LOCAL_PREFS;
-  prefs: LocalPrefs & MetadataPrefs;
+  prefs: MetadataPrefs;
 };
 
 export type MergeGlobalPrefsAction = {
diff --git a/packages/loot-core/src/client/update-notification.ts b/packages/loot-core/src/client/update-notification.ts
deleted file mode 100644
index 217c78b89..000000000
--- a/packages/loot-core/src/client/update-notification.ts
+++ /dev/null
@@ -1,45 +0,0 @@
-import { t } from 'i18next';
-
-import { addNotification, loadPrefs, savePrefs } from './actions';
-import { type Dispatch } from './actions/types';
-
-export async function checkForUpdateNotification(
-  dispatch: Dispatch,
-  getIsOutdated: (latestVersion: string) => Promise<boolean>,
-  getLatestVersion: () => Promise<string>,
-) {
-  const latestVersion = await getLatestVersion();
-  const isOutdated = await getIsOutdated(latestVersion);
-  if (
-    !isOutdated ||
-    (await dispatch(loadPrefs()))['flags.updateNotificationShownForVersion'] ===
-      latestVersion
-  ) {
-    return;
-  }
-
-  dispatch(
-    addNotification({
-      type: 'message',
-      title: t('A new version of Actual is available!'),
-      message: t('Version {{latestVersion}} of Actual was recently released.', {
-        latestVersion,
-      }),
-      sticky: true,
-      id: 'update-notification',
-      button: {
-        title: t('Open changelog'),
-        action: () => {
-          window.open('https://actualbudget.org/docs/releases');
-        },
-      },
-      onClose: () => {
-        dispatch(
-          savePrefs({
-            'flags.updateNotificationShownForVersion': latestVersion,
-          }),
-        );
-      },
-    }),
-  );
-}
diff --git a/packages/loot-core/src/server/sync/index.ts b/packages/loot-core/src/server/sync/index.ts
index e50a1735c..7c4b03adb 100644
--- a/packages/loot-core/src/server/sync/index.ts
+++ b/packages/loot-core/src/server/sync/index.ts
@@ -13,7 +13,7 @@ import * as connection from '../../platform/server/connection';
 import { logger } from '../../platform/server/log';
 import { sequential, once } from '../../shared/async';
 import { setIn, getIn } from '../../shared/util';
-import { LocalPrefs } from '../../types/prefs';
+import { type MetadataPrefs } from '../../types/prefs';
 import { triggerBudgetChanges, setType as setBudgetType } from '../budget/base';
 import * as db from '../db';
 import { PostError, SyncError } from '../errors';
@@ -304,7 +304,7 @@ export const applyMessages = sequential(async (messages: Message[]) => {
     return data;
   }
 
-  const prefsToSet: LocalPrefs = {};
+  const prefsToSet: MetadataPrefs = {};
   const oldData = await fetchData();
 
   undo.appendMessages(messages, oldData);
diff --git a/packages/loot-core/src/types/prefs.d.ts b/packages/loot-core/src/types/prefs.d.ts
index 7ce798bc1..984225da3 100644
--- a/packages/loot-core/src/types/prefs.d.ts
+++ b/packages/loot-core/src/types/prefs.d.ts
@@ -53,28 +53,25 @@ export type MetadataPrefs = Partial<{
 
 /**
  * Local preferences applicable to a single device. Stored in local storage.
- * TODO: eventually `LocalPrefs` type should not use `MetadataPrefs`;
- * this is only a stop-gap solution.
  */
-export type LocalPrefs = MetadataPrefs &
-  Partial<{
-    'ui.showClosedAccounts': boolean;
-    'expand-splits': boolean;
-    'budget.collapsed': string[];
-    'budget.summaryCollapsed': boolean;
-    'budget.showHiddenCategories': boolean;
-    'budget.startMonth': string;
-    'flags.updateNotificationShownForVersion': string;
-    reportsViewLegend: boolean;
-    reportsViewSummary: boolean;
-    reportsViewLabel: boolean;
-    spendingReportFilter: string;
-    spendingReportMode: spendingReportModeType;
-    spendingReportCompare: string;
-    spendingReportCompareTo: string;
-    sidebarWidth: number;
-    'mobile.showSpentColumn': boolean;
-  }>;
+export type LocalPrefs = Partial<{
+  'ui.showClosedAccounts': boolean;
+  'expand-splits': boolean;
+  'budget.collapsed': string[];
+  'budget.summaryCollapsed': boolean;
+  'budget.showHiddenCategories': boolean;
+  'budget.startMonth': string;
+  'flags.updateNotificationShownForVersion': string;
+  reportsViewLegend: boolean;
+  reportsViewSummary: boolean;
+  reportsViewLabel: boolean;
+  spendingReportFilter: string;
+  spendingReportMode: spendingReportModeType;
+  spendingReportCompare: string;
+  spendingReportCompareTo: string;
+  sidebarWidth: number;
+  'mobile.showSpentColumn': boolean;
+}>;
 
 export type Theme = 'light' | 'dark' | 'auto' | 'midnight' | 'development';
 export type DarkTheme = 'dark' | 'midnight';
diff --git a/packages/loot-core/src/types/server-handlers.d.ts b/packages/loot-core/src/types/server-handlers.d.ts
index 5a5c589f6..0d367452e 100644
--- a/packages/loot-core/src/types/server-handlers.d.ts
+++ b/packages/loot-core/src/types/server-handlers.d.ts
@@ -17,7 +17,7 @@ import {
   RuleEntity,
   PayeeEntity,
 } from './models';
-import { GlobalPrefs, LocalPrefs } from './prefs';
+import { GlobalPrefs, MetadataPrefs } from './prefs';
 import { Query } from './query';
 import { EmptyObject } from './util';
 
@@ -239,7 +239,7 @@ export interface ServerHandlers {
 
   'save-prefs': (prefsToSet) => Promise<'ok'>;
 
-  'load-prefs': () => Promise<LocalPrefs | null>;
+  'load-prefs': () => Promise<MetadataPrefs | null>;
 
   'sync-reset': () => Promise<{ error?: { reason: string; meta?: unknown } }>;
 
diff --git a/upcoming-release-notes/3458.md b/upcoming-release-notes/3458.md
new file mode 100644
index 000000000..a13767e96
--- /dev/null
+++ b/upcoming-release-notes/3458.md
@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [MatissJanis]
+---
+
+SyncedPrefs: separate out MetadataPrefs and LocalPrefs in different storage locations.
-- 
GitLab