Skip to content

Commit

Permalink
✨ (bank-sync) quality of life improvements for sync (#2416)
Browse files Browse the repository at this point in the history
  • Loading branch information
MatissJanis authored Mar 4, 2024
1 parent 6626164 commit 3b77609
Show file tree
Hide file tree
Showing 17 changed files with 121 additions and 142 deletions.
29 changes: 15 additions & 14 deletions packages/desktop-client/src/components/BankSyncStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/desktop-client/src/components/accounts/Header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
/>{' '}
Expand Down
10 changes: 7 additions & 3 deletions packages/desktop-client/src/components/sidebar/Account.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type AccountProps = {
query: Binding;
account?: AccountEntity;
connected?: boolean;
pending?: boolean;
failed?: boolean;
updated?: boolean;
style?: CSSProperties;
Expand All @@ -50,6 +51,7 @@ export function Account({
name,
account,
connected,
pending = false,
failed,
updated,
to,
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions packages/desktop-client/src/components/sidebar/Accounts.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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}`;

Expand Down Expand Up @@ -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)}
Expand All @@ -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)}
Expand Down
1 change: 1 addition & 0 deletions packages/desktop-client/src/style/themes/dark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/desktop-client/src/style/themes/development.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/desktop-client/src/style/themes/light.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/desktop-client/src/style/themes/midnight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
152 changes: 68 additions & 84 deletions packages/loot-core/src/client/actions/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import { send } from '../../platform/client/fetch';
import * as constants from '../constants';
import type {
AccountSyncFailuresAction,
AccountSyncStatusAction,
SetAccountsSyncingAction,
} from '../state-types/account';
Expand All @@ -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,
};
}

Expand All @@ -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) => {
Expand Down Expand Up @@ -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;
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/loot-core/src/client/actions/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion packages/loot-core/src/client/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Loading

0 comments on commit 3b77609

Please sign in to comment.