Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove use of useActions #3911

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Remove use of useActions #3911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
abea8d6
Remove use of useActions
joel-jeremy Nov 26, 2024
9e07798
Release notes
joel-jeremy Nov 26, 2024
704ab01
Fix lint
joel-jeremy Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions packages/desktop-client/src/components/LoggedInUser.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// @ts-strict-ignore
import React, { useState, useEffect, useRef, type CSSProperties } from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { useSelector } from 'react-redux';
import { useDispatch, useSelector } from 'react-redux';

import { closeBudget, getUserData, signOut } from 'loot-core/client/actions';
import { type State } from 'loot-core/src/client/state-types';

import { useActions } from '../hooks/useActions';
import { useNavigate } from '../hooks/useNavigate';
import { theme, styles } from '../style';

Expand All @@ -27,22 +27,28 @@ export function LoggedInUser({
color,
}: LoggedInUserProps) {
const { t } = useTranslation();

const dispatch = useDispatch();
const navigate = useNavigate();
const userData = useSelector((state: State) => state.user.data);
const { getUserData, signOut, closeBudget } = useActions();
const [loading, setLoading] = useState(true);
const [menuOpen, setMenuOpen] = useState(false);
const serverUrl = useServerURL();
const triggerRef = useRef(null);

useEffect(() => {
getUserData().then(() => setLoading(false));
async function init() {
await dispatch(getUserData());
}

init().then(() => setLoading(false));
}, []);

const navigate = useNavigate();
async function onCloseBudget() {
await dispatch(closeBudget());
}

async function onChangePassword() {
await closeBudget();
await onCloseBudget();
navigate('/change-password');
}

Expand All @@ -54,14 +60,14 @@ export function LoggedInUser({
onChangePassword();
break;
case 'sign-in':
await closeBudget();
await onCloseBudget();
navigate('/login');
break;
case 'sign-out':
signOut();
dispatch(signOut());
break;
case 'config-server':
await closeBudget();
await onCloseBudget();
navigate('/config-server');
break;
default:
Expand Down
14 changes: 8 additions & 6 deletions packages/desktop-client/src/components/Titlebar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import React, { useState, useEffect, type CSSProperties } from 'react';
import { useHotkeys } from 'react-hotkeys-hook';
import { useDispatch } from 'react-redux';
import { Routes, Route, useLocation } from 'react-router-dom';

import { css } from '@emotion/css';
import { t } from 'i18next';

import { sync } from 'loot-core/client/actions';
import * as Platform from 'loot-core/src/client/platform';
import * as queries from 'loot-core/src/client/queries';
import { listen } from 'loot-core/src/platform/client/fetch';
Expand All @@ -13,7 +15,6 @@ import {
isElectron,
} from 'loot-core/src/shared/environment';

import { useActions } from '../hooks/useActions';
import { useGlobalPref } from '../hooks/useGlobalPref';
import { useMetadataPref } from '../hooks/useMetadataPref';
import { useNavigate } from '../hooks/useNavigate';
Expand Down Expand Up @@ -108,8 +109,7 @@ type SyncButtonProps = {
};
function SyncButton({ style, isMobile = false }: SyncButtonProps) {
const [cloudFileId] = useMetadataPref('cloudFileId');
const { sync } = useActions();

const dispatch = useDispatch();
const [syncing, setSyncing] = useState(false);
const [syncState, setSyncState] = useState<
null | 'offline' | 'local' | 'disabled' | 'error'
Expand Down Expand Up @@ -193,15 +193,17 @@ function SyncButton({ style, isMobile = false }: SyncButtonProps) {
marginRight: 5,
};

const onSync = () => dispatch(sync());

Comment on lines +196 to +197
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and loading state for sync action

The sync action should include error handling and potentially a loading state for better user feedback.

+const [isSyncing, setIsSyncing] = useState(false);
 const onSync = () => {
-  dispatch(sync());
+  setIsSyncing(true);
+  try {
+    dispatch(sync())
+      .then(() => {
+        // Consider showing success feedback
+      })
+      .catch((error) => {
+        console.error('Sync failed:', error);
+        // Consider showing error feedback
+      })
+      .finally(() => {
+        setIsSyncing(false);
+      });
+  } catch (error) {
+    console.error('Failed to initiate sync:', error);
+    setIsSyncing(false);
+  }
 };

Committable suggestion skipped: line range outside the PR's diff.

useHotkeys(
'ctrl+s, cmd+s, meta+s',
sync,
onSync,
{
enableOnFormTags: true,
preventDefault: true,
scopes: ['app'],
},
[sync],
[onSync],
);

return (
Expand All @@ -223,7 +225,7 @@ function SyncButton({ style, isMobile = false }: SyncButtonProps) {
'&[data-hovered]': hoveredStyle,
'&[data-pressed]': activeStyle,
})}
onPress={sync}
onPress={onSync}
>
{isMobile ? (
syncState === 'error' ? (
Expand Down
21 changes: 13 additions & 8 deletions packages/desktop-client/src/components/UpdateNotification.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from 'react';
import { useTranslation } from 'react-i18next';
import { useSelector } from 'react-redux';
import { useSelector, useDispatch } from 'react-redux';

import { setAppState, updateApp } from 'loot-core/client/actions';
import { type State } from 'loot-core/src/client/state-types';

import { useActions } from '../hooks/useActions';
import { SvgClose } from '../icons/v1';
import { theme } from '../style';

Expand All @@ -20,7 +20,10 @@ export function UpdateNotification() {
(state: State) => state.app.showUpdateNotification,
);

const { updateApp, setAppState } = useActions();
const dispatch = useDispatch();
const onRestart = () => {
dispatch(updateApp());
};

if (updateInfo && showUpdateNotification) {
const notes = updateInfo.releaseNotes;
Expand Down Expand Up @@ -51,7 +54,7 @@ export function UpdateNotification() {
<Text>
<Link
variant="text"
onClick={updateApp}
onClick={onRestart}
style={{
color: theme.buttonPrimaryText,
textDecoration: 'underline',
Expand Down Expand Up @@ -81,10 +84,12 @@ export function UpdateNotification() {
style={{ display: 'inline', padding: '1px 7px 2px 7px' }}
onPress={() => {
// Set a flag to never show an update notification again for this session
setAppState({
updateInfo: null,
showUpdateNotification: false,
});
dispatch(
setAppState({
updateInfo: null,
showUpdateNotification: false,
}),
);
}}
>
<SvgClose
Expand Down
12 changes: 7 additions & 5 deletions packages/desktop-client/src/components/manager/ConfigServer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// @ts-strict-ignore
import React, { useState, useEffect, useCallback } from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { useDispatch } from 'react-redux';

import { createBudget, loggedIn, signOut } from 'loot-core/client/actions';
import {
isNonProductionEnvironment,
isElectron,
} from 'loot-core/src/shared/environment';

import { useActions } from '../../hooks/useActions';
import { useGlobalPref } from '../../hooks/useGlobalPref';
import { useNavigate } from '../../hooks/useNavigate';
import { theme } from '../../style';
Expand All @@ -22,7 +23,7 @@ import { Title } from './subscribe/common';

export function ConfigServer() {
const { t } = useTranslation();
const { createBudget, signOut, loggedIn } = useActions();
const dispatch = useDispatch();
const navigate = useNavigate();
const [url, setUrl] = useState('');
const currentUrl = useServerURL();
Expand Down Expand Up @@ -77,7 +78,7 @@ export function ConfigServer() {
setError(error);
} else {
setLoading(false);
await signOut();
await dispatch(signOut());
navigate('/');
}
}
Expand All @@ -104,13 +105,14 @@ export function ConfigServer() {

async function onSkip() {
await setServerUrl(null);
await loggedIn();
await dispatch(loggedIn());
navigate('/');
}

async function onCreateTestFile() {
await setServerUrl(null);
await createBudget({ testMode: true });
await dispatch(createBudget({ testMode: true }));
navigate('/');
}

return (
Expand Down
16 changes: 11 additions & 5 deletions packages/desktop-client/src/components/manager/WelcomeScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { useDispatch } from 'react-redux';

import { createBudget, pushModal } from 'loot-core/client/actions';

import { useActions } from '../../hooks/useActions';
import { styles, theme } from '../../style';
import { Button } from '../common/Button2';
import { Link } from '../common/Link';
Expand All @@ -11,7 +13,7 @@ import { View } from '../common/View';

export function WelcomeScreen() {
const { t } = useTranslation();
const { createBudget, pushModal } = useActions();
const dispatch = useDispatch();

return (
<View
Expand Down Expand Up @@ -77,7 +79,7 @@ export function WelcomeScreen() {
flexShrink: 0,
}}
>
<Button onPress={() => pushModal('import')}>
<Button onPress={() => dispatch(pushModal('import'))}>
{t('Import my budget')}
</Button>
<View
Expand All @@ -87,10 +89,14 @@ export function WelcomeScreen() {
gap: 10,
}}
>
<Button onPress={() => createBudget({ testMode: true })}>
<Button onPress={() => dispatch(createBudget({ testMode: true }))}>
{t('View demo')}
</Button>
<Button variant="primary" autoFocus onPress={() => createBudget()}>
<Button
variant="primary"
autoFocus
onPress={() => dispatch(createBudget())}
>
{t('Start fresh')}
</Button>
</View>
Expand Down
38 changes: 18 additions & 20 deletions .../desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import React, { useState, useEffect, useCallback } from 'react';
import { useDispatch } from 'react-redux';

import deepEqual from 'deep-equal';
import { t } from 'i18next';

import {
getPayees,
importPreviewTransactions,
importTransactions,
parseTransactions,
} from 'loot-core/client/actions';
import { amountToInteger } from 'loot-core/src/shared/util';

import { useActions } from '../../../hooks/useActions';
import { useDateFormat } from '../../../hooks/useDateFormat';
import { useSyncedPrefs } from '../../../hooks/useSyncedPrefs';
import { theme } from '../../../style';
Expand Down Expand Up @@ -137,12 +143,7 @@ function parseCategoryFields(trans, categories) {
export function ImportTransactionsModal({ options }) {
const dateFormat = useDateFormat() || 'MM/dd/yyyy';
const [prefs, savePrefs] = useSyncedPrefs();
const {
parseTransactions,
importTransactions,
importPreviewTransactions,
getPayees,
} = useActions();
const dispatch = useDispatch();

const [multiplierAmount, setMultiplierAmount] = useState('');
const [loadingState, setLoadingState] = useState('parsing');
Expand Down Expand Up @@ -263,9 +264,8 @@ export function ImportTransactionsModal({ options }) {
}

// Retreive the transactions that would be updated (along with the existing trx)
const previewTrx = await importPreviewTransactions(
accountId,
previewTransactions,
const previewTrx = await dispatch(
importPreviewTransactions(accountId, previewTransactions),
Comment on lines +267 to +268
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error boundaries for dispatched actions

While the transition to useDispatch is correct, the error handling could be improved for the dispatched actions.

Apply this diff to add error handling:

-      const previewTrx = await dispatch(
-        importPreviewTransactions(accountId, previewTransactions),
-      );
+      let previewTrx;
+      try {
+        previewTrx = await dispatch(
+          importPreviewTransactions(accountId, previewTransactions),
+        );
+      } catch (error) {
+        console.error('Failed to preview transactions:', error);
+        setError({ parsed: false, message: 'Failed to preview transactions' });
+        setLoadingState(null);
+        return [];
+      }

-      const { errors, transactions: parsedTransactions = [] } = await dispatch(
-        parseTransactions(filename, options),
-      );
+      let result;
+      try {
+        result = await dispatch(parseTransactions(filename, options));
+      } catch (error) {
+        console.error('Failed to parse transactions:', error);
+        setError({ parsed: true, message: 'Failed to parse transactions' });
+        return { errors: [error], transactions: [] };
+      }
+      const { errors, transactions: parsedTransactions = [] } = result;

-    const didChange = await dispatch(
-      importTransactions(accountId, finalTransactions, reconcile),
-    );
-    if (didChange) {
-      await dispatch(getPayees());
-    }
+    try {
+      const didChange = await dispatch(
+        importTransactions(accountId, finalTransactions, reconcile),
+      );
+      if (didChange) {
+        await dispatch(getPayees());
+      }
+    } catch (error) {
+      console.error('Failed to import transactions:', error);
+      setError({ parsed: false, message: 'Failed to import transactions' });
+      setLoadingState(null);
+      return;
+    }

Also applies to: 323-325, 656-660

);
const matchedUpdateMap = previewTrx.reduce((map, entry) => {
map[entry.transaction.trx_id] = entry;
Expand Down Expand Up @@ -309,7 +309,7 @@ export function ImportTransactionsModal({ options }) {
return next;
}, []);
},
[accountId, categories.list, clearOnImport, importPreviewTransactions],
[accountId, categories.list, clearOnImport, dispatch],
);

const parse = useCallback(
Expand All @@ -320,8 +320,9 @@ export function ImportTransactionsModal({ options }) {
setFilename(filename);
setFileType(filetype);

const { errors, transactions: parsedTransactions = [] } =
await parseTransactions(filename, options);
const { errors, transactions: parsedTransactions = [] } = await dispatch(
parseTransactions(filename, options),
);

let index = 0;
const transactions = parsedTransactions.map(trans => {
Expand Down Expand Up @@ -399,11 +400,11 @@ export function ImportTransactionsModal({ options }) {
},
[
accountId,
dispatch,
getImportPreview,
inOutMode,
multiplierAmount,
outValue,
parseTransactions,
prefs,
],
);
Expand All @@ -427,7 +428,6 @@ export function ImportTransactionsModal({ options }) {

parse(options.filename, parseOptions);
}, [
parseTransactions,
options.filename,
delimiter,
hasHeaderRow,
Expand Down Expand Up @@ -653,13 +653,11 @@ export function ImportTransactionsModal({ options }) {
});
}

const didChange = await importTransactions(
accountId,
finalTransactions,
reconcile,
const didChange = await dispatch(
importTransactions(accountId, finalTransactions, reconcile),
);
if (didChange) {
await getPayees();
await dispatch(getPayees());
Comment on lines +656 to +660
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for dispatched actions

When dispatching importTransactions and getPayees, consider adding error handling to manage potential exceptions. This ensures that your application can handle failures gracefully.

Apply this diff to add error handling:

+try {
  const didChange = await dispatch(
    importTransactions(accountId, finalTransactions, reconcile),
  );
  if (didChange) {
    await dispatch(getPayees());
  }
+} catch (error) {
+  // Handle the error appropriately
+  console.error('Error importing transactions:', error);
+  setError({ parsed: false, message: 'Failed to import transactions.' });
+  setLoadingState(null);
+}

Committable suggestion skipped: line range outside the PR's diff.

}

if (onImported) {
Expand Down
Loading
Loading