Skip to content

Commit

Permalink
♻️ (synced-prefs) moving the prefs from metadata.json to the db (#3423)
Browse files Browse the repository at this point in the history
  • Loading branch information
MatissJanis authored Sep 17, 2024
1 parent 5b685ec commit 6c87d85
Show file tree
Hide file tree
Showing 16 changed files with 202 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ vi.mock('loot-core/src/platform/client/fetch');
vi.mock('../../hooks/useFeatureFlag', () => ({
default: vi.fn().mockReturnValue(false),
}));
vi.mock('../../hooks/useSyncedPref', () => ({
useSyncedPref: vi.fn().mockReturnValue([undefined, vi.fn()]),
}));

const accounts = [generateAccount('Bank of America')];
const payees = [
Expand Down
7 changes: 0 additions & 7 deletions packages/desktop-client/src/hooks/useLocalPrefs.ts

This file was deleted.

27 changes: 22 additions & 5 deletions packages/desktop-client/src/hooks/useSyncedPref.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { type SyncedPrefs } from 'loot-core/src/types/prefs';
import { useCallback } from 'react';

import { useLocalPref } from './useLocalPref';
import { useQuery } from 'loot-core/client/query-hooks';
import { send } from 'loot-core/platform/client/fetch';
import { q } from 'loot-core/shared/query';
import { type SyncedPrefs } from 'loot-core/src/types/prefs';

type SetSyncedPrefAction<K extends keyof SyncedPrefs> = (
value: SyncedPrefs[K],
Expand All @@ -9,7 +12,21 @@ type SetSyncedPrefAction<K extends keyof SyncedPrefs> = (
export function useSyncedPref<K extends keyof SyncedPrefs>(
prefName: K,
): [SyncedPrefs[K], SetSyncedPrefAction<K>] {
// TODO: implement logic for fetching the pref exclusively from the
// database (in follow-up PR)
return useLocalPref(prefName);
const { data: queryData, overrideData: setQueryData } = useQuery<
[{ value: string | undefined }]
>(
() => q('preferences').filter({ id: prefName }).select('value'),
[prefName],
);

const setLocalPref = useCallback<SetSyncedPrefAction<K>>(
newValue => {
const value = String(newValue);
setQueryData([{ value }]);
send('preferences/save', { id: prefName, value });
},
[prefName, setQueryData],
);

return [queryData?.[0]?.value, setLocalPref];
}
43 changes: 30 additions & 13 deletions packages/desktop-client/src/hooks/useSyncedPrefs.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,39 @@
import { useCallback } from 'react';
import { useDispatch } from 'react-redux';
import { useCallback, useMemo } from 'react';

import { savePrefs } from 'loot-core/client/actions';
import { useQuery } from 'loot-core/client/query-hooks';
import { send } from 'loot-core/platform/client/fetch';
import { q } from 'loot-core/shared/query';
import { type SyncedPrefs } from 'loot-core/src/types/prefs';

import { useLocalPrefs } from './useLocalPrefs';

type SetSyncedPrefsAction = (value: Partial<SyncedPrefs>) => void;

/** @deprecated: please use `useSyncedPref` (singular) */
export function useSyncedPrefs(): [SyncedPrefs, SetSyncedPrefsAction] {
// TODO: implement real logic (follow-up PR)
const dispatch = useDispatch();
const setPrefs = useCallback<SetSyncedPrefsAction>(
newPrefs => {
dispatch(savePrefs(newPrefs));
},
[dispatch],
const { data: queryData } = useQuery<{ id: string; value: string }[]>(
() => q('preferences').select(['id', 'value']),
[],
);

const prefs = useMemo<SyncedPrefs>(
() =>
queryData.reduce(
(carry, { id, value }) => ({
...carry,
[id]: value,
}),
{},
),
[queryData],
);

return [useLocalPrefs(), setPrefs];
const setPrefs = useCallback<SetSyncedPrefsAction>(newValue => {
Object.entries(newValue).forEach(([id, value]) => {
send('preferences/save', {
id: id as keyof SyncedPrefs,
value: String(value),
});
});
}, []);

return [prefs, setPrefs];
}
59 changes: 59 additions & 0 deletions packages/loot-core/migrations/1723665565000_prefs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
const SYNCED_PREF_KEYS = [
'firstDayOfWeekIdx',
'dateFormat',
'numberFormat',
'hideFraction',
'isPrivacyEnabled',
/^show-extra-balances-/,
/^hide-cleared-/,
/^parse-date-/,
/^csv-mappings-/,
/^csv-delimiter-/,
/^csv-has-header-/,
/^ofx-fallback-missing-payee-/,
/^flip-amount-/,
// 'budgetType', // TODO: uncomment when `budgetType` moves from metadata to synced prefs
/^flags\./,
];

export default async function runMigration(db, { fs, fileId }) {
await db.execQuery(`
CREATE TABLE preferences
(id TEXT PRIMARY KEY,
value TEXT);
`);

try {
const budgetDir = fs.getBudgetDir(fileId);
const fullpath = fs.join(budgetDir, 'metadata.json');

const prefs = JSON.parse(await fs.readFile(fullpath));

if (typeof prefs !== 'object') {
return;
}

await Promise.all(
Object.keys(prefs).map(async key => {
// Check if the current key is of synced-keys type
if (
!SYNCED_PREF_KEYS.find(keyMatcher =>
keyMatcher instanceof RegExp
? keyMatcher.test(key)
: keyMatcher === key,
)
) {
return;
}

// insert the synced prefs in the new table
await db.runQuery('INSERT INTO preferences (id, value) VALUES (?, ?)', [
key,
String(prefs[key]),
]);
}),
);
} catch (e) {
// Do nothing
}
}
16 changes: 12 additions & 4 deletions packages/loot-core/src/client/query-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@ export async function runQuery(query) {
return send('query', query.serialize());
}

export function liveQuery(query, onData?, opts?): LiveQuery {
const q = new LiveQuery(query, onData, opts);
export function liveQuery<Response = unknown>(
query,
onData?: (response: Response) => void,
opts?,
): LiveQuery {
const q = new LiveQuery<Response>(query, onData, opts);
q.run();
return q;
}
Expand All @@ -20,7 +24,7 @@ export function pagedQuery(query, onData?, opts?): PagedQuery {
}

// Subscribe and refetch
export class LiveQuery {
export class LiveQuery<Response = unknown> {
_unsubscribe;
data;
dependencies;
Expand All @@ -36,7 +40,11 @@ export class LiveQuery {
inflightRequestId;
restart;

constructor(query, onData?, opts: { mapper?; onlySync?: boolean } = {}) {
constructor(
query,
onData?: (response: Response) => void,
opts: { mapper?; onlySync?: boolean } = {},
) {
this.error = new Error();
this.query = query;
this.data = null;
Expand Down
27 changes: 24 additions & 3 deletions packages/loot-core/src/client/query-hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,42 @@ export function useLiveQuery<Response = unknown>(
makeQuery: () => Query,
deps: DependencyList,
): Response {
const [data, setData] = useState(null);
const { data } = useQuery<Response>(makeQuery, deps);
return data;
}

export function useQuery<Response = unknown>(
makeQuery: () => Query,
deps: DependencyList,
): {
data: Response;
overrideData: (newData: Response) => void;
isLoading: boolean;
} {
const [data, setData] = useState<null | Response>(null);
const [isLoading, setIsLoading] = useState(true);
const query = useMemo(makeQuery, deps);

useEffect(() => {
let live = liveQuery(query, async data => {
setIsLoading(true);

let live = liveQuery<Response>(query, async data => {
if (live) {
setIsLoading(false);
setData(data);
}
});

return () => {
setIsLoading(false);
live.unsubscribe();
live = null;
};
}, [query]);

return data;
return {
data,
overrideData: setData,
isLoading,
};
}
4 changes: 4 additions & 0 deletions packages/loot-core/src/server/aql/schema/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ export const schema = {
id: f('id'),
note: f('string'),
},
preferences: {
id: f('id'),
value: f('string'),
},
transaction_filters: {
id: f('id'),
name: f('string'),
Expand Down
2 changes: 2 additions & 0 deletions packages/loot-core/src/server/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { mutator, runHandler } from './mutators';
import { app as notesApp } from './notes/app';
import * as Platform from './platform';
import { get, post } from './post';
import { app as preferencesApp } from './preferences/app';
import * as prefs from './prefs';
import { app as reportsApp } from './reports/app';
import { app as rulesApp } from './rules/app';
Expand Down Expand Up @@ -2079,6 +2080,7 @@ app.combine(
budgetApp,
dashboardApp,
notesApp,
preferencesApp,
toolsApp,
filtersApp,
reportsApp,
Expand Down
9 changes: 7 additions & 2 deletions packages/loot-core/src/server/migrate/migrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@
// them which doesn't play well with CSP. There isn't great, and eventually
// we can remove this migration.
import { Database } from '@jlongster/sql.js';
import { v4 as uuidv4 } from 'uuid';

import m1632571489012 from '../../../migrations/1632571489012_remove_cache';
import m1722717601000 from '../../../migrations/1722717601000_reports_move_selected_categories';
import m1722804019000 from '../../../migrations/1722804019000_create_dashboard_table';
import m1723665565000 from '../../../migrations/1723665565000_prefs';
import * as fs from '../../platform/server/fs';
import * as sqlite from '../../platform/server/sqlite';
import * as prefs from '../prefs';

let MIGRATIONS_DIR = fs.migrationsPath;

const javascriptMigrations = {
1632571489012: m1632571489012,
1722717601000: m1722717601000,
1722804019000: m1722804019000,
1723665565000: m1723665565000,
};

export async function withMigrationsDir(
Expand Down Expand Up @@ -107,7 +109,10 @@ async function applyJavaScript(db, id) {
}

const run = javascriptMigrations[id];
return run(dbInterface, () => uuidv4());
return run(dbInterface, {
fs,
fileId: prefs.getPrefs()?.id,
});
}

async function applySql(db, sql) {
Expand Down
21 changes: 21 additions & 0 deletions packages/loot-core/src/server/preferences/app.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { type SyncedPrefs } from '../../types/prefs';
import { createApp } from '../app';
import * as db from '../db';
import { mutator } from '../mutators';
import { undoable } from '../undo';

import { PreferencesHandlers } from './types/handlers';

export const app = createApp<PreferencesHandlers>();

const savePreferences = async ({
id,
value,
}: {
id: keyof SyncedPrefs;
value: string | undefined;
}) => {
await db.update('preferences', { id, value });
};

app.method('preferences/save', mutator(undoable(savePreferences)));
8 changes: 8 additions & 0 deletions packages/loot-core/src/server/preferences/types/handlers.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { type SyncedPrefs } from '../../../types/prefs';

export interface PreferencesHandlers {
'preferences/save': (arg: {
id: keyof SyncedPrefs;
value: string | undefined;
}) => Promise<void>;
}
13 changes: 0 additions & 13 deletions packages/loot-core/src/server/prefs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,6 @@ export async function loadPrefs(id?: string): Promise<MetadataPrefs> {
prefs = { id, budgetName: id };
}

// delete released feature flags
const releasedFeatures = ['syncAccount'];
for (const feature of releasedFeatures) {
delete prefs[`flags.${feature}`];
}

// delete legacy notifications
for (const key of Object.keys(prefs)) {
if (key.startsWith('notifications.')) {
delete prefs[key];
}
}

// No matter what is in `id` field, force it to be the current id.
// This makes it resilient to users moving around folders, etc
prefs.id = id;
Expand Down
2 changes: 2 additions & 0 deletions packages/loot-core/src/types/handlers.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { BudgetHandlers } from '../server/budget/types/handlers';
import type { DashboardHandlers } from '../server/dashboard/types/handlers';
import type { FiltersHandlers } from '../server/filters/types/handlers';
import type { NotesHandlers } from '../server/notes/types/handlers';
import type { PreferencesHandlers } from '../server/preferences/types/handlers';
import type { ReportsHandlers } from '../server/reports/types/handlers';
import type { RulesHandlers } from '../server/rules/types/handlers';
import type { SchedulesHandlers } from '../server/schedules/types/handlers';
Expand All @@ -17,6 +18,7 @@ export interface Handlers
DashboardHandlers,
FiltersHandlers,
NotesHandlers,
PreferencesHandlers,
ReportsHandlers,
RulesHandlers,
SchedulesHandlers,
Expand Down
5 changes: 2 additions & 3 deletions packages/loot-core/src/types/prefs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,10 @@ export type MetadataPrefs = Partial<{

/**
* Local preferences applicable to a single device. Stored in local storage.
* TODO: eventually `LocalPrefs` type should not use `SyncedPrefs` or `MetadataPrefs`;
* TODO: eventually `LocalPrefs` type should not use `MetadataPrefs`;
* this is only a stop-gap solution.
*/
export type LocalPrefs = SyncedPrefs &
MetadataPrefs &
export type LocalPrefs = MetadataPrefs &
Partial<{
'ui.showClosedAccounts': boolean;
'expand-splits': boolean;
Expand Down
Loading

0 comments on commit 6c87d85

Please sign in to comment.