Apply coderabbit suggestions

actualbudget · Oct 15, 2024 · 31c80f9 · 31c80f9
1 parent 13d8a9e
commit 31c80f9
Show file tree

Hide file tree

Showing 12 changed files with 268 additions and 176 deletions.
diff --git a/packages/desktop-client/src/components/accounts/Account.tsx b/packages/desktop-client/src/components/accounts/Account.tsx
@@ -502,9 +502,8 @@ class AccountInternal extends PureComponent<
       query = query.filter({ reconciled: { $eq: false } });
     }
 
-    this.paged = pagedQuery(
-      query.select('*'),
-      async (groupedData, prevData) => {
+    this.paged = pagedQuery(query.select('*'), {
+      onData: async (groupedData, prevData) => {
         const data = ungroupTransactions([...groupedData]);
         const firstLoad = prevData == null;
 
@@ -547,11 +546,11 @@ class AccountInternal extends PureComponent<
           },
         );
       },
-      {
+      options: {
         pageCount: 150,
         onlySync: true,
       },
-    );
+    });
   }
 
   UNSAFE_componentWillReceiveProps(nextProps: AccountInternalProps) {

diff --git a/packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx b/packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
@@ -24,7 +24,7 @@ import {
   SchedulesProvider,
 } from 'loot-core/client/data-hooks/schedules';
 import {
-  usePreviewTransactions2,
+  usePreviewTransactions,
   useTransactions,
 } from 'loot-core/client/data-hooks/transactions';
 import * as queries from 'loot-core/client/queries';
@@ -238,7 +238,7 @@ function TransactionListWithPreviews({
   });
 
   const { data: previewTransactions, isLoading: isPreviewTransactionsLoading } =
-    usePreviewTransactions2({ options: { isDisabled: isSearching } });
+    usePreviewTransactions({ options: { isDisabled: isSearching } });
 
   const dateFormat = useDateFormat() || 'MM/dd/yyyy';
   const dispatch = useDispatch();
@@ -249,7 +249,9 @@ function TransactionListWithPreviews({
   }, [accountId, dispatch]);
 
   useEffect(() => {
-    dispatch(markAccountRead(accountId));
+    if (accountId) {
+      dispatch(markAccountRead(accountId));
+    }
   }, [accountId, dispatch]);
 
   useEffect(() => {

diff --git a/packages/desktop-client/src/components/mobile/budget/CategoryTransactions.jsx b/packages/desktop-client/src/components/mobile/budget/CategoryTransactions.jsx
@@ -85,7 +85,7 @@ export function CategoryTransactions({ category, month }) {
     [updateSearchQuery],
   );
 
-  const onOpenTranasction = useCallback(
+  const onOpenTransaction = useCallback(
     transaction => {
       // details of how the native app used to handle preview transactions here can be found at commit 05e58279
       if (!isPreviewId(transaction.id)) {
@@ -126,7 +126,7 @@ export function CategoryTransactions({ category, month }) {
         searchPlaceholder={`Search ${category.name}`}
         onSearch={onSearch}
         onLoadMore={loadMoreTransactions}
-        onOpenTransaction={onOpenTranasction}
+        onOpenTransaction={onOpenTransaction}
       />
     </Page>
   );

diff --git a/packages/desktop-client/src/components/schedules/ScheduleDetails.jsx b/packages/desktop-client/src/components/schedules/ScheduleDetails.jsx
@@ -298,7 +298,10 @@ export function ScheduleDetails({ id, transaction }) {
           .filter({ schedule: state.schedule.id })
           .select('*')
           .options({ splits: 'all' }),
-        data => dispatch({ type: 'set-transactions', transactions: data }),
+        {
+          onData: data =>
+            dispatch({ type: 'set-transactions', transactions: data }),
+        },
       );
       return live.unsubscribe;
     }
@@ -338,7 +341,10 @@ export function ScheduleDetails({ id, transaction }) {
               .filter({ $and: filters })
               .select('*')
               .options({ splits: 'all' }),
-            data => dispatch({ type: 'set-transactions', transactions: data }),
+            {
+              onData: data =>
+                dispatch({ type: 'set-transactions', transactions: data }),
+            },
           );
           unsubscribe = live.unsubscribe;
         }

diff --git a/packages/desktop-client/src/components/schedules/index.tsx b/packages/desktop-client/src/components/schedules/index.tsx
@@ -61,6 +61,7 @@ export function Schedules() {
           await send('schedule/delete', { id });
           break;
         default:
+          throw new Error(`Unknown action: ${name}`);
       }
     },
     [],

diff --git a/packages/desktop-client/src/hooks/usePreviewTransactions.ts b/packages/desktop-client/src/hooks/usePreviewTransactions.ts
@@ -6,68 +6,63 @@ import {
 } from 'loot-core/client/data-hooks/schedules';
 import { send } from 'loot-core/platform/client/fetch';
 import { ungroupTransactions } from 'loot-core/shared/transactions';
-import { type ScheduleEntity } from 'loot-core/types/models';
+import {
+  type TransactionEntity,
+  type ScheduleEntity,
+} from 'loot-core/types/models';
 
-import { type TransactionEntity } from '../../../loot-core/src/types/models/transaction.d';
+import { usePrevious } from './usePrevious';
 
 /**
- * @deprecated Please use `useTransactions` hook from `loot-core/client/data-hooks/transactions` instead.
+ * @deprecated Please use `usePreviewTransactions` hook from `loot-core/client/data-hooks/transactions` instead.
  */
 export function usePreviewTransactions(
   collapseTransactions?: (ids: string[]) => void,
 ) {
-  const [previousScheduleData, setPreviousScheduleData] = useState<ReturnType<
-    typeof useCachedSchedules
-  > | null>(null);
   const [previewTransactions, setPreviewTransactions] = useState<
     TransactionEntity[]
   >([]);
 
   const scheduleData = useCachedSchedules();
-  if (scheduleData?.isLoading) {
+  const previousScheduleData = usePrevious(scheduleData);
+
+  if (scheduleData.isLoading) {
     return [];
   }
 
-  if (scheduleData !== previousScheduleData) {
-    setPreviousScheduleData(scheduleData);
-
-    if (scheduleData) {
-      // Kick off an async rules application
-      const schedules =
-        scheduleData.schedules.filter(s =>
-          isForPreview(s, scheduleData.statuses),
-        ) || [];
+  if (scheduleData && scheduleData !== previousScheduleData) {
+    // Kick off an async rules application
+    const schedules = scheduleData.schedules.filter(s =>
+      isForPreview(s, scheduleData.statuses),
+    );
 
-      const baseTrans = schedules.map(schedule => ({
-        id: 'preview/' + schedule.id,
-        payee: schedule._payee,
-        account: schedule._account,
-        amount: schedule._amount,
-        date: schedule.next_date,
-        schedule: schedule.id,
-      }));
+    const baseTrans = schedules.map(schedule => ({
+      id: 'preview/' + schedule.id,
+      payee: schedule._payee,
+      account: schedule._account,
+      amount: schedule._amount,
+      date: schedule.next_date,
+      schedule: schedule.id,
+    }));
 
-      Promise.all(
-        baseTrans.map(transaction => send('rules-run', { transaction })),
-      ).then(newTrans => {
-        const withDefaults = newTrans.map(t => ({
-          ...t,
-          category: scheduleData.statuses.get(t.schedule),
+    Promise.all(
+      baseTrans.map(transaction => send('rules-run', { transaction })),
+    ).then(newTrans => {
+      const withDefaults = newTrans.map(t => ({
+        ...t,
+        category: scheduleData.statuses.get(t.schedule),
+        schedule: t.schedule,
+        subtransactions: t.subtransactions?.map((st: TransactionEntity) => ({
+          ...st,
+          id: 'preview/' + st.id,
           schedule: t.schedule,
-          subtransactions: t.subtransactions?.map((st: TransactionEntity) => ({
-            ...st,
-            id: 'preview/' + st.id,
-            schedule: t.schedule,
-          })),
-        }));
-        setPreviewTransactions(ungroupTransactions(withDefaults));
-        if (collapseTransactions) {
-          collapseTransactions(withDefaults.map(t => t.id));
-        }
-      });
-    }
-
-    return previewTransactions;
+        })),
+      }));
+      setPreviewTransactions(ungroupTransactions(withDefaults));
+      if (collapseTransactions) {
+        collapseTransactions(withDefaults.map(t => t.id));
+      }
+    });
   }
 
   return previewTransactions;

diff --git a/packages/loot-core/src/client/data-hooks/filters.ts b/packages/loot-core/src/client/data-hooks/filters.ts
@@ -5,7 +5,7 @@ import { q } from '../../shared/query';
 import { type TransactionFilterEntity } from '../../types/models';
 import { useQuery } from '../query-hooks';
 
-function toJS(rows) {
+function toJS(rows): TransactionFilterEntity[] {
   const filters = rows.map(row => {
     return {
       ...row.fields,
@@ -26,9 +26,11 @@ export function useFilters(): TransactionFilterEntity[] {
   );
 
   /** Sort filters by alphabetical order */
-  const sort = useCallback(filters => {
-    return filters.sort((a, b) =>
-      a.name.trim().localeCompare(b.name.trim(), { ignorePunctuation: true }),
+  const sort = useCallback((filters: TransactionFilterEntity[]) => {
+    return filters.toSorted((a, b) =>
+      a.name
+        .trim()
+        .localeCompare(b.name.trim(), undefined, { ignorePunctuation: true }),
     );
   }, []);
 

diff --git a/packages/loot-core/src/client/data-hooks/schedules.tsx b/packages/loot-core/src/client/data-hooks/schedules.tsx
@@ -29,11 +29,11 @@ export type ScheduleStatuses = Map<ScheduleEntity['id'], ScheduleStatusType>;
 function loadStatuses(
   schedules: readonly ScheduleEntity[],
   onData: (data: ScheduleStatuses) => void,
+  onError: (error: Error) => void,
   upcomingLength: string,
 ) {
-  return liveQuery<TransactionEntity>(
-    getHasTransactionsQuery(schedules),
-    data => {
+  return liveQuery<TransactionEntity>(getHasTransactionsQuery(schedules), {
+    onData: data => {
       const hasTrans = new Set(data.filter(Boolean).map(row => row.schedule));
 
       const scheduleStatuses = new Map(
@@ -50,7 +50,8 @@ function loadStatuses(
 
       onData?.(scheduleStatuses);
     },
-  );
+    onError,
+  });
 }
 
 type UseSchedulesProps = {
@@ -66,13 +67,15 @@ type ScheduleData = {
 type UseSchedulesResult = ScheduleData & {
   readonly isLoading: boolean;
   readonly isDisabled: boolean;
+  readonly error?: Error;
 };
 
 export function useSchedules({
   queryBuilder,
   options: { isDisabled } = { isDisabled: false },
 }: UseSchedulesProps = {}): UseSchedulesResult {
   const [isLoading, setIsLoading] = useState(true);
+  const [error, setError] = useState<Error | undefined>(undefined);
   const [data, setData] = useState<ScheduleData>({
     schedules: [],
     statuses: new Map(),
@@ -87,18 +90,12 @@ export function useSchedules({
   );
 
   useEffect(() => {
-    if (isDisabled) {
-      setData({ schedules: [], statuses: new Map() });
-      return;
-    }
-
     let isUnmounted = false;
 
-    setIsLoading(true);
+    setIsLoading(query !== null);
 
-    scheduleQueryRef.current = liveQuery<ScheduleEntity>(
-      query,
-      async schedules => {
+    scheduleQueryRef.current = liveQuery<ScheduleEntity>(query, {
+      onData: async schedules => {
         statusQueryRef.current = loadStatuses(
           schedules,
           (statuses: ScheduleStatuses) => {
@@ -107,22 +104,25 @@ export function useSchedules({
               setData({ schedules, statuses });
             }
           },
+          setError,
           upcomingLength,
         );
       },
-    );
+      onError: setError,
+    });
 
     return () => {
       isUnmounted = true;
       scheduleQueryRef.current?.unsubscribe();
       statusQueryRef.current?.unsubscribe();
     };
-  }, [isDisabled, query, upcomingLength]);
+  }, [query, upcomingLength]);
 
   return {
     isLoading,
     isDisabled,
-    ...data,
+    error,
+    ...(isDisabled ? { schedules: [], statuses: new Map() } : data),
   };
 }