From 3b776091596a9ac6e0baf6dc5ceb01eee4c8e5cb Mon Sep 17 00:00:00 2001
From: Matiss Janis Aboltins <matiss@mja.lv>
Date: Mon, 4 Mar 2024 18:42:34 +0000
Subject: [PATCH] :sparkles: (bank-sync) quality of life improvements for sync
(#2416)
---
.../src/components/BankSyncStatus.tsx | 29 ++--
.../src/components/accounts/Account.jsx | 2 +-
.../src/components/accounts/Header.jsx | 5 +-
.../src/components/sidebar/Account.tsx | 10 +-
.../src/components/sidebar/Accounts.tsx | 7 +
.../desktop-client/src/style/themes/dark.ts | 1 +
.../src/style/themes/development.ts | 1 +
.../desktop-client/src/style/themes/light.ts | 1 +
.../src/style/themes/midnight.ts | 1 +
.../loot-core/src/client/actions/account.ts | 152 ++++++++----------
packages/loot-core/src/client/actions/sync.ts | 2 +-
packages/loot-core/src/client/constants.ts | 1 -
.../loot-core/src/client/reducers/account.ts | 15 +-
.../src/client/state-types/account.d.ts | 18 +--
packages/loot-core/src/server/main.ts | 10 +-
.../loot-core/src/types/server-handlers.d.ts | 2 +-
upcoming-release-notes/2416.md | 6 +
17 files changed, 121 insertions(+), 142 deletions(-)
create mode 100644 upcoming-release-notes/2416.md
diff --git a/packages/desktop-client/src/components/BankSyncStatus.tsx b/packages/desktop-client/src/components/BankSyncStatus.tsx
index f49b92007..77ae6457d 100644
--- a/packages/desktop-client/src/components/BankSyncStatus.tsx
+++ b/packages/desktop-client/src/components/BankSyncStatus.tsx
@@ -14,19 +14,17 @@ export function BankSyncStatus() {
const accountsSyncing = useSelector(
(state: State) => state.account.accountsSyncing,
);
+ const accountsSyncingCount = accountsSyncing.length;
- const name = accountsSyncing
- ? accountsSyncing === '__all'
- ? 'accounts'
- : accountsSyncing
- : null;
-
- const transitions = useTransition(name, {
- from: { opacity: 0, transform: 'translateY(-100px)' },
- enter: { opacity: 1, transform: 'translateY(0)' },
- leave: { opacity: 0, transform: 'translateY(-100px)' },
- unique: true,
- });
+ const transitions = useTransition(
+ accountsSyncingCount > 0 ? 'syncing' : null,
+ {
+ from: { opacity: 0, transform: 'translateY(-100px)' },
+ enter: { opacity: 1, transform: 'translateY(0)' },
+ leave: { opacity: 0, transform: 'translateY(-100px)' },
+ unique: true,
+ },
+ );
return (
<View
@@ -56,10 +54,13 @@ export function BankSyncStatus() {
}}
>
<AnimatedRefresh
- animating={true}
+ animating
iconStyle={{ color: theme.pillTextSelected }}
/>
- <Text>Syncing {item}</Text>
+ <Text style={{ marginLeft: 5 }}>
+ Syncing... {accountsSyncingCount} account
+ {accountsSyncingCount > 1 && 's'} remaining
+ </Text>
</View>
</animated.div>
),
diff --git a/packages/desktop-client/src/components/accounts/Account.jsx b/packages/desktop-client/src/components/accounts/Account.jsx
index 4db7abb7c..62f2a7f6f 100644
--- a/packages/desktop-client/src/components/accounts/Account.jsx
+++ b/packages/desktop-client/src/components/accounts/Account.jsx
@@ -462,7 +462,7 @@ class AccountInternal extends PureComponent {
const accountId = this.props.accountId;
const account = this.props.accounts.find(acct => acct.id === accountId);
- await this.props.syncAndDownload(account ? account.id : null);
+ await this.props.syncAndDownload(account ? account.id : undefined);
};
onImport = async () => {
diff --git a/packages/desktop-client/src/components/accounts/Header.jsx b/packages/desktop-client/src/components/accounts/Header.jsx
index eaade96bf..93cd45115 100644
--- a/packages/desktop-client/src/components/accounts/Header.jsx
+++ b/packages/desktop-client/src/components/accounts/Header.jsx
@@ -232,8 +232,9 @@ export function AccountHeader({
width={13}
height={13}
animating={
- (account && accountsSyncing === account.name) ||
- accountsSyncing === '__all'
+ account
+ ? accountsSyncing.includes(account.id)
+ : accountsSyncing.length > 0
}
style={{ marginRight: 4 }}
/>{' '}
diff --git a/packages/desktop-client/src/components/sidebar/Account.tsx b/packages/desktop-client/src/components/sidebar/Account.tsx
index 3cd28ba2e..2c232a73a 100644
--- a/packages/desktop-client/src/components/sidebar/Account.tsx
+++ b/packages/desktop-client/src/components/sidebar/Account.tsx
@@ -38,6 +38,7 @@ type AccountProps = {
query: Binding;
account?: AccountEntity;
connected?: boolean;
+ pending?: boolean;
failed?: boolean;
updated?: boolean;
style?: CSSProperties;
@@ -50,6 +51,7 @@ export function Account({
name,
account,
connected,
+ pending = false,
failed,
updated,
to,
@@ -125,9 +127,11 @@ export function Account({
width: 5,
height: 5,
borderRadius: 5,
- backgroundColor: failed
- ? theme.sidebarItemBackgroundFailed
- : theme.sidebarItemBackgroundPositive,
+ backgroundColor: pending
+ ? theme.sidebarItemBackgroundPending
+ : failed
+ ? theme.sidebarItemBackgroundFailed
+ : theme.sidebarItemBackgroundPositive,
marginLeft: 2,
transition: 'transform .3s',
opacity: connected ? 1 : 0,
diff --git a/packages/desktop-client/src/components/sidebar/Accounts.tsx b/packages/desktop-client/src/components/sidebar/Accounts.tsx
index e85e32e4a..d85cb537b 100644
--- a/packages/desktop-client/src/components/sidebar/Accounts.tsx
+++ b/packages/desktop-client/src/components/sidebar/Accounts.tsx
@@ -1,7 +1,9 @@
// @ts-strict-ignore
import React, { useState } from 'react';
+import { useSelector } from 'react-redux';
import * as queries from 'loot-core/src/client/queries';
+import { type State } from 'loot-core/src/client/state-types';
import { useBudgetedAccounts } from '../../hooks/useBudgetedAccounts';
import { useClosedAccounts } from '../../hooks/useClosedAccounts';
@@ -35,6 +37,9 @@ export function Accounts({
const offbudgetAccounts = useOffBudgetAccounts();
const budgetedAccounts = useBudgetedAccounts();
const closedAccounts = useClosedAccounts();
+ const syncingAccountIds = useSelector(
+ (state: State) => state.account.accountsSyncing,
+ );
const getAccountPath = account => `/accounts/${account.id}`;
@@ -78,6 +83,7 @@ export function Accounts({
name={account.name}
account={account}
connected={!!account.bank}
+ pending={syncingAccountIds.includes(account.id)}
failed={failedAccounts && failedAccounts.has(account.id)}
updated={updatedAccounts && updatedAccounts.includes(account.id)}
to={getAccountPath(account)}
@@ -103,6 +109,7 @@ export function Accounts({
name={account.name}
account={account}
connected={!!account.bank}
+ pending={syncingAccountIds.includes(account.id)}
failed={failedAccounts && failedAccounts.has(account.id)}
updated={updatedAccounts && updatedAccounts.includes(account.id)}
to={getAccountPath(account)}
diff --git a/packages/desktop-client/src/style/themes/dark.ts b/packages/desktop-client/src/style/themes/dark.ts
index a2a2f3526..4d2800587 100644
--- a/packages/desktop-client/src/style/themes/dark.ts
+++ b/packages/desktop-client/src/style/themes/dark.ts
@@ -39,6 +39,7 @@ export const tableRowHeaderBackground = colorPalette.navy700;
export const tableRowHeaderText = colorPalette.navy150;
export const sidebarBackground = colorPalette.navy900;
+export const sidebarItemBackgroundPending = colorPalette.orange200;
export const sidebarItemBackgroundPositive = colorPalette.green500;
export const sidebarItemBackgroundFailed = colorPalette.red300;
export const sidebarItemAccentSelected = colorPalette.purple200;
diff --git a/packages/desktop-client/src/style/themes/development.ts b/packages/desktop-client/src/style/themes/development.ts
index 68e190b6b..9ce9db879 100644
--- a/packages/desktop-client/src/style/themes/development.ts
+++ b/packages/desktop-client/src/style/themes/development.ts
@@ -39,6 +39,7 @@ export const tableRowHeaderBackground = colorPalette.navy50;
export const tableRowHeaderText = colorPalette.navy800;
export const sidebarBackground = colorPalette.navy900;
+export const sidebarItemBackgroundPending = colorPalette.orange200;
export const sidebarItemBackgroundPositive = colorPalette.green500;
export const sidebarItemBackgroundFailed = colorPalette.red300;
export const sidebarItemBackgroundHover = colorPalette.navy800;
diff --git a/packages/desktop-client/src/style/themes/light.ts b/packages/desktop-client/src/style/themes/light.ts
index a442f746a..124bb671e 100644
--- a/packages/desktop-client/src/style/themes/light.ts
+++ b/packages/desktop-client/src/style/themes/light.ts
@@ -39,6 +39,7 @@ export const tableRowHeaderBackground = colorPalette.navy50;
export const tableRowHeaderText = colorPalette.navy800;
export const sidebarBackground = colorPalette.navy900;
+export const sidebarItemBackgroundPending = colorPalette.orange200;
export const sidebarItemBackgroundPositive = colorPalette.green500;
export const sidebarItemBackgroundFailed = colorPalette.red300;
export const sidebarItemBackgroundHover = colorPalette.navy800;
diff --git a/packages/desktop-client/src/style/themes/midnight.ts b/packages/desktop-client/src/style/themes/midnight.ts
index 6e6dea4bd..1fc41c202 100644
--- a/packages/desktop-client/src/style/themes/midnight.ts
+++ b/packages/desktop-client/src/style/themes/midnight.ts
@@ -39,6 +39,7 @@ export const tableRowHeaderBackground = colorPalette.gray700;
export const tableRowHeaderText = colorPalette.gray150;
export const sidebarBackground = colorPalette.gray900;
+export const sidebarItemBackgroundPending = colorPalette.orange200;
export const sidebarItemBackgroundPositive = colorPalette.green400;
export const sidebarItemBackgroundFailed = colorPalette.red300;
export const sidebarItemAccentSelected = colorPalette.purple200;
diff --git a/packages/loot-core/src/client/actions/account.ts b/packages/loot-core/src/client/actions/account.ts
index e7f7f269b..f6448ce9e 100644
--- a/packages/loot-core/src/client/actions/account.ts
+++ b/packages/loot-core/src/client/actions/account.ts
@@ -2,7 +2,6 @@
import { send } from '../../platform/client/fetch';
import * as constants from '../constants';
import type {
- AccountSyncFailuresAction,
AccountSyncStatusAction,
SetAccountsSyncingAction,
} from '../state-types/account';
@@ -17,11 +16,11 @@ import { getPayees, getAccounts } from './queries';
import type { Dispatch, GetState } from './types';
export function setAccountsSyncing(
- name: SetAccountsSyncingAction['name'],
+ ids: SetAccountsSyncingAction['ids'],
): SetAccountsSyncingAction {
return {
type: constants.SET_ACCOUNTS_SYNCING,
- name,
+ ids,
};
}
@@ -47,14 +46,6 @@ export function markAccountSuccess(
failed: false,
};
}
-export function setFailedAccounts(
- syncErrors: AccountSyncFailuresAction['syncErrors'],
-): AccountSyncFailuresAction {
- return {
- type: constants.ACCOUNT_SYNC_FAILURES,
- syncErrors,
- };
-}
export function unlinkAccount(id: string) {
return async (dispatch: Dispatch) => {
@@ -107,96 +98,89 @@ export function connectAccounts(
};
}
-// TODO: type correctly or remove (unused)
-export function connectGoCardlessAccounts(
- institution,
- publicToken,
- accountIds,
- offbudgetIds,
-) {
- return async (dispatch: Dispatch) => {
- const ids = await send('gocardless-accounts-connect', {
- institution,
- publicToken,
- accountIds,
- offbudgetIds,
- });
- await dispatch(getPayees());
- await dispatch(getAccounts());
- return ids;
- };
-}
-
-export function syncAccounts(id: string) {
+export function syncAccounts(id?: string) {
return async (dispatch: Dispatch, getState: GetState) => {
- if (getState().account.accountsSyncing) {
+ // Disallow two parallel sync operations
+ if (getState().account.accountsSyncing.length > 0) {
return false;
}
- if (id) {
- const account = getState().queries.accounts.find(a => a.id === id);
- dispatch(setAccountsSyncing(account.name));
- } else {
- dispatch(setAccountsSyncing('__all'));
- }
+ // Build an array of IDs for accounts to sync.. if no `id` provided
+ // then we assume that all accounts should be synced
+ const accountIdsToSync = id
+ ? [id]
+ : getState()
+ .queries.accounts.filter(
+ ({ bank, closed, tombstone }) => !!bank && !closed && !tombstone,
+ )
+ .map(({ id }) => id);
+
+ dispatch(setAccountsSyncing(accountIdsToSync));
- const { errors, newTransactions, matchedTransactions, updatedAccounts } =
- await send('gocardless-accounts-sync', { id });
- dispatch(setAccountsSyncing(null));
+ let isSyncSuccess = false;
- if (id) {
- const error = errors.find(error => error.accountId === id);
+ // Loop through the accounts and perform sync operation.. one by one
+ for (let idx = 0; idx < accountIdsToSync.length; idx++) {
+ const accountId = accountIdsToSync[idx];
+ // Perform sync operation
+ const { errors, newTransactions, matchedTransactions, updatedAccounts } =
+ await send('gocardless-accounts-sync', {
+ id: accountId,
+ });
+
+ // Mark the account as failed or succeeded (depending on sync output)
+ const [error] = errors;
if (error) {
// We only want to mark the account as having problem if it
// was a real syncing error.
if (error.type === 'SyncError') {
- dispatch(markAccountFailed(id, error.category, error.code));
+ dispatch(markAccountFailed(accountId, error.category, error.code));
}
} else {
- dispatch(markAccountSuccess(id));
+ dispatch(markAccountSuccess(accountId));
}
- } else {
- dispatch(
- setFailedAccounts(
- errors
- .filter(error => error.type === 'SyncError')
- .map(error => ({
- id: error.accountId,
- type: error.category,
- code: error.code,
- })),
- ),
- );
- }
- errors.forEach(error => {
- if (error.type === 'SyncError') {
- dispatch(
- addNotification({
- type: 'error',
- message: error.message,
- }),
- );
- } else {
- dispatch(
- addNotification({
- type: 'error',
- message: error.message,
- internal: error.internal,
- }),
- );
- }
- });
+ // Dispatch errors (if any)
+ errors.forEach(error => {
+ if (error.type === 'SyncError') {
+ dispatch(
+ addNotification({
+ type: 'error',
+ message: error.message,
+ }),
+ );
+ } else {
+ dispatch(
+ addNotification({
+ type: 'error',
+ message: error.message,
+ internal: error.internal,
+ }),
+ );
+ }
+ });
- dispatch({
- type: constants.SET_NEW_TRANSACTIONS,
- newTransactions,
- matchedTransactions,
- updatedAccounts,
- });
+ // Set new transactions
+ dispatch({
+ type: constants.SET_NEW_TRANSACTIONS,
+ newTransactions,
+ matchedTransactions,
+ updatedAccounts,
+ });
+
+ // Dispatch the ids for the accounts that are yet to be synced
+ dispatch(setAccountsSyncing(accountIdsToSync.slice(idx + 1)));
+
+ if (newTransactions.length > 0 || matchedTransactions.length > 0) {
+ isSyncSuccess = true;
+ }
+ }
- return newTransactions.length > 0 || matchedTransactions.length > 0;
+ // Rest the sync state back to empty (fallback in case something breaks
+ // in the logic above)
+ dispatch(setAccountsSyncing([]));
+ return isSyncSuccess;
};
}
diff --git a/packages/loot-core/src/client/actions/sync.ts b/packages/loot-core/src/client/actions/sync.ts
index 94aa3f7db..77838897f 100644
--- a/packages/loot-core/src/client/actions/sync.ts
+++ b/packages/loot-core/src/client/actions/sync.ts
@@ -50,7 +50,7 @@ export function sync() {
};
}
-export function syncAndDownload(accountId) {
+export function syncAndDownload(accountId?: string) {
return async (dispatch: Dispatch) => {
// It is *critical* that we sync first because of transaction
// reconciliation. We want to get all transactions that other
diff --git a/packages/loot-core/src/client/constants.ts b/packages/loot-core/src/client/constants.ts
index c107dea1c..03f0e5786 100644
--- a/packages/loot-core/src/client/constants.ts
+++ b/packages/loot-core/src/client/constants.ts
@@ -26,5 +26,4 @@ export const SET_LAST_UNDO_STATE = 'SET_LAST_UNDO_STATE';
export const SET_LAST_SPLIT_STATE = 'SET_LAST_SPLIT_STATE';
export const SET_ACCOUNTS_SYNCING = 'SET_ACCOUNTS_SYNCING';
export const ACCOUNT_SYNC_STATUS = 'ACCOUNT_SYNC_STATUS';
-export const ACCOUNT_SYNC_FAILURES = 'ACCOUNT_SYNC_FAILURES';
export const SIGN_OUT = 'SIGN_OUT';
diff --git a/packages/loot-core/src/client/reducers/account.ts b/packages/loot-core/src/client/reducers/account.ts
index 7c36ee5f7..37387c5e5 100644
--- a/packages/loot-core/src/client/reducers/account.ts
+++ b/packages/loot-core/src/client/reducers/account.ts
@@ -4,7 +4,7 @@ import type { AccountState } from '../state-types/account';
const initialState: AccountState = {
failedAccounts: new Map(),
- accountsSyncing: null,
+ accountsSyncing: [],
};
export function update(state = initialState, action: Action): AccountState {
@@ -12,7 +12,7 @@ export function update(state = initialState, action: Action): AccountState {
case constants.SET_ACCOUNTS_SYNCING:
return {
...state,
- accountsSyncing: action.name,
+ accountsSyncing: action.ids,
};
case constants.ACCOUNT_SYNC_STATUS: {
const failedAccounts = new Map(state.failedAccounts);
@@ -27,17 +27,6 @@ export function update(state = initialState, action: Action): AccountState {
return { ...state, failedAccounts };
}
- case constants.ACCOUNT_SYNC_FAILURES: {
- const failures = new Map();
- action.syncErrors.forEach(error => {
- failures.set(error.id, {
- type: error.type,
- code: error.code,
- });
- });
-
- return { ...state, failedAccounts: failures };
- }
default:
}
return state;
diff --git a/packages/loot-core/src/client/state-types/account.d.ts b/packages/loot-core/src/client/state-types/account.d.ts
index 2b119bc27..5abad2a6d 100644
--- a/packages/loot-core/src/client/state-types/account.d.ts
+++ b/packages/loot-core/src/client/state-types/account.d.ts
@@ -2,12 +2,12 @@ import type * as constants from '../constants';
export type AccountState = {
failedAccounts: Map<string, { type: string; code: string }>;
- accountsSyncing: string | null;
+ accountsSyncing: string[];
};
export type SetAccountsSyncingAction = {
type: typeof constants.SET_ACCOUNTS_SYNCING;
- name: string | null;
+ ids: string[];
};
export type AccountSyncStatusAction = {
@@ -24,16 +24,4 @@ export type AccountSyncStatusAction = {
}
);
-export type AccountSyncFailuresAction = {
- type: typeof constants.ACCOUNT_SYNC_FAILURES;
- syncErrors: Array<{
- id: string;
- type: string;
- code: string;
- }>;
-};
-
-export type AccountActions =
- | SetAccountsSyncingAction
- | AccountSyncStatusAction
- | AccountSyncFailuresAction;
+export type AccountActions = SetAccountsSyncingAction | AccountSyncStatusAction;
diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts
index 21c3b7420..64cd29938 100644
--- a/packages/loot-core/src/server/main.ts
+++ b/packages/loot-core/src/server/main.ts
@@ -1266,18 +1266,14 @@ handlers['gocardless-accounts-sync'] = async function ({ id }) {
'user-id',
'user-key',
]);
- let accounts = await db.runQuery(
+ const accounts = await db.runQuery(
`SELECT a.*, b.bank_id as bankId FROM accounts a
LEFT JOIN banks b ON a.bank = b.id
- WHERE a.tombstone = 0 AND a.closed = 0`,
- [],
+ WHERE a.tombstone = 0 AND a.closed = 0 AND a.id = ?`,
+ [id],
true,
);
- if (id) {
- accounts = accounts.filter(acct => acct.id === id);
- }
-
const errors = [];
let newTransactions = [];
let matchedTransactions = [];
diff --git a/packages/loot-core/src/types/server-handlers.d.ts b/packages/loot-core/src/types/server-handlers.d.ts
index 1883a36cc..eff46e2f6 100644
--- a/packages/loot-core/src/types/server-handlers.d.ts
+++ b/packages/loot-core/src/types/server-handlers.d.ts
@@ -246,7 +246,7 @@ export interface ServerHandlers {
| { error: 'failed' }
>;
- 'gocardless-accounts-sync': (arg: { id }) => Promise<{
+ 'gocardless-accounts-sync': (arg: { id: string }) => Promise<{
errors;
newTransactions;
matchedTransactions;
diff --git a/upcoming-release-notes/2416.md b/upcoming-release-notes/2416.md
new file mode 100644
index 000000000..eb28989aa
--- /dev/null
+++ b/upcoming-release-notes/2416.md
@@ -0,0 +1,6 @@
+---
+category: Enhancements
+authors: [MatissJanis]
+---
+
+Bank sync quality of life improvements: show "pending" status on accounts, progressively import new transactions instead of waiting for all account sync to finish before adding them to the ledger.
--
GitLab