[AI] Fix missing patterns in Enable Banking integration

- Add SyncServerEnableBankingAccount to ExternalAccount union and getInstitutionName parameter type in SelectLinkedAccountsModal - Use BankSyncProviders type in mobile BankSyncAccountsList instead of hardcoded union missing enableBanking - Add getSecretsError handling to EnableBankingInitialiseModal for proper auth/permission error messages - Replace hardcoded #666 color with theme.pageTextSubdued - Wrap onConnectEnableBanking in try/catch with error notification and init modal re-open, matching SimpleFin/PluggyAI pattern - Translate hardcoded error string in enablebanking.ts - Add 60s timeout to downloadEnableBankingTransactions matching PluggyAI - Revert out-of-scope changes to del()/patch() in post.ts - Revert shared starting balance dedup logic back to master pattern
2026-05-07 12:28:57 -05:00 · 2026-03-31 01:15:00 +02:00
parent 33619dfc1d
commit 1cbe1efbf4
13 changed files with 128 additions and 80 deletions
--- a/packages/desktop-client/src/components/mobile/banksync/BankSyncAccountsList.tsx
+++ b/packages/desktop-client/src/components/mobile/banksync/BankSyncAccountsList.tsx
@@ -4,13 +4,13 @@ import { Text } from '@actual-app/components/text';
 import { theme } from '@actual-app/components/theme';
 import { View } from '@actual-app/components/view';

-import type { AccountEntity } from 'loot-core/types/models';
+import type { AccountEntity, BankSyncProviders } from 'loot-core/types/models';

 import { BankSyncAccountsListItem } from './BankSyncAccountsListItem';

 import { MOBILE_NAV_HEIGHT } from '@desktop-client/components/mobile/MobileNavTabs';

-type SyncProviders = 'goCardless' | 'simpleFin' | 'pluggyai' | 'unlinked';
+type SyncProviders = BankSyncProviders | 'unlinked';

 type BankSyncAccountsListProps = {
  groupedAccounts: Record<SyncProviders, AccountEntity[]>;
--- a/packages/desktop-client/src/components/modals/CreateAccountModal.tsx
+++ b/packages/desktop-client/src/components/modals/CreateAccountModal.tsx
@@ -223,7 +223,7 @@ export function CreateAccountModal({
    }
  };

-  const onConnectEnableBanking = () => {
+  const onConnectEnableBanking = async () => {
    if (isEnableBankingSetupComplete === null) {
      // Still loading — ignore click
      return;
@@ -232,7 +232,30 @@ export function CreateAccountModal({
      onEnableBankingInit();
      return;
    }
-    void authorizeEnableBanking(dispatch);
+
+    try {
+      await authorizeEnableBanking(dispatch);
+    } catch (err) {
+      console.error(err);
+      addNotification({
+        notification: {
+          type: 'error',
+          title: t('Error when trying to contact Enable Banking'),
+          message: (err as Error).message,
+          timeout: 5000,
+        },
+      });
+      dispatch(
+        pushModal({
+          modal: {
+            name: 'enablebanking-init',
+            options: {
+              onSuccess: () => setIsEnableBankingSetupComplete(true),
+            },
+          },
+        }),
+      );
+    }
  };

  const onGoCardlessInit = () => {
--- a/packages/desktop-client/src/components/modals/EnableBankingInitialiseModal.tsx
+++ b/packages/desktop-client/src/components/modals/EnableBankingInitialiseModal.tsx
@@ -3,12 +3,15 @@ import type { ChangeEvent } from 'react';
 import { Trans, useTranslation } from 'react-i18next';

 import { ButtonWithLoading } from '@actual-app/components/button';
+import { SvgCheckCircle1 } from '@actual-app/components/icons/v2';
 import { InitialFocus } from '@actual-app/components/initial-focus';
 import { Input } from '@actual-app/components/input';
 import { Text } from '@actual-app/components/text';
+import { theme } from '@actual-app/components/theme';
 import { View } from '@actual-app/components/view';

 import { send } from 'loot-core/platform/client/connection';
+import { getSecretsError } from 'loot-core/shared/errors';

 import { Error as ErrorAlert } from '@desktop-client/components/alerts';
 import { Link } from '@desktop-client/components/common/Link';
@@ -34,6 +37,7 @@ export function EnableBankingInitialiseModal({
  const [secretKey, setSecretKey] = useState('');
  const [isValid, setIsValid] = useState(true);
  const [isLoading, setIsLoading] = useState(false);
+  const [keyFileName, setKeyFileName] = useState('');
  const [error, setError] = useState(
    t('It is required to provide both the Application ID and the secret key.'),
  );
@@ -42,9 +46,17 @@ export function EnableBankingInitialiseModal({
    const file = e.target.files?.[0];
    if (!file) return;

-    const text = await file.text();
-    setSecretKey(text);
-    setIsValid(true);
+    try {
+      const text = await file.text();
+      setSecretKey(text);
+      setKeyFileName(file.name);
+      setIsValid(true);
+    } catch {
+      setSecretKey('');
+      setKeyFileName('');
+      setIsValid(false);
+      setError(t('Failed to read the key file. Please try again.'));
+    }
  }

  async function onSubmit(close: () => void) {
@@ -66,6 +78,12 @@ export function EnableBankingInitialiseModal({
        secretKey,
      });

+      if (result?.error) {
+        setIsValid(false);
+        setError(getSecretsError(result.error, result.reason));
+        return;
+      }
+
      if (result?.data?.error_code) {
        setIsValid(false);
        setError(
@@ -95,7 +113,7 @@ export function EnableBankingInitialiseModal({
  return (
    <Modal
      name="enablebanking-init"
-      containerProps={{ style: { width: '30vw' } }}
+      containerProps={{ style: { width: '30vw', minWidth: 420 } }}
    >
      {({ state }) => (
        <>
@@ -169,9 +187,20 @@ export function EnableBankingInitialiseModal({
            </FormField>

            {secretKey && (
-              <Text style={{ fontSize: 12, color: '#666' }}>
-                <Trans>Secret key loaded successfully.</Trans>
-              </Text>
+              <View
+                style={{
+                  flexDirection: 'row',
+                  alignItems: 'center',
+                  gap: 4,
+                }}
+              >
+                <SvgCheckCircle1
+                  style={{ width: 14, height: 14, color: theme.noticeText }}
+                />
+                <Text style={{ fontSize: 12, color: theme.pageTextSubdued }}>
+                  {keyFileName}
+                </Text>
+              </View>
            )}

            {!isValid && <ErrorAlert>{error}</ErrorAlert>}
--- a/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.tsx
+++ b/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.tsx
@@ -509,7 +509,8 @@ export function SelectLinkedAccountsModal({
 type ExternalAccount =
  | SyncServerGoCardlessAccount
  | SyncServerSimpleFinAccount
-  | SyncServerPluggyAiAccount;
+  | SyncServerPluggyAiAccount
+  | SyncServerEnableBankingAccount;

 type StartingBalanceInfo = {
  date: string;
@@ -747,7 +748,8 @@ function getInstitutionName(
  externalAccount:
    | SyncServerGoCardlessAccount
    | SyncServerSimpleFinAccount
-    | SyncServerPluggyAiAccount,
+    | SyncServerPluggyAiAccount
+    | SyncServerEnableBankingAccount,
 ) {
  if (typeof externalAccount?.institution === 'string') {
    return externalAccount?.institution ?? '';
--- a/packages/desktop-client/src/enablebanking.ts
+++ b/packages/desktop-client/src/enablebanking.ts
@@ -1,3 +1,5 @@
+import { t } from 'i18next';
+
 import { sendCatch } from 'loot-core/platform/client/connection';
 import type { SyncServerEnableBankingAccount } from 'loot-core/types/models';

@@ -51,7 +53,7 @@ function _authorize(
            if (!authUrl || !state) {
              return {
                error: 'unknown' as const,
-                message: 'Missing auth URL or state',
+                message: t('Missing auth URL or state'),
              };
            }

--- a/packages/loot-core/src/server/accounts/sync.ts
+++ b/packages/loot-core/src/server/accounts/sync.ts
@@ -333,6 +333,7 @@ async function downloadEnableBankingTransactions(
    {
      'X-ACTUAL-TOKEN': userToken,
    },
+    60000,
  );

  if (res.error_code) {
@@ -1050,8 +1051,7 @@ async function processBankSyncDownload(
    if (customStartingDate) {
      startingBalanceDate = customStartingDate;
    } else if (transactions.length > 0) {
-      startingBalanceDate =
-        oldestTransaction.date ?? oldestTransaction.bookingDate;
+      startingBalanceDate = oldestTransaction.date;
    } else {
      startingBalanceDate = monthUtils.currentDay();
    }
@@ -1059,23 +1059,15 @@ async function processBankSyncDownload(
    const payee = await getStartingBalancePayee();

    return runMutator(async () => {
-      // Check if a starting balance transaction already exists for this account
-      const existingStartingBalance = await db.first(
-        'SELECT id FROM transactions WHERE acct = ? AND starting_balance_flag = 1 AND tombstone = 0',
-        [id],
-      );
-
-      const initialId = existingStartingBalance
-        ? existingStartingBalance.id
-        : await db.insertTransaction({
-            account: id,
-            amount: balanceToUse,
-            category: acctRow.offbudget === 0 ? payee.category : null,
-            payee: payee.id,
-            date: startingBalanceDate,
-            cleared: true,
-            starting_balance_flag: true,
-          });
+      const initialId = await db.insertTransaction({
+        account: id,
+        amount: balanceToUse,
+        category: acctRow.offbudget === 0 ? payee.category : null,
+        payee: payee.id,
+        date: startingBalanceDate,
+        cleared: true,
+        starting_balance_flag: true,
+      });

      const result = await reconcileTransactions(
        id,
--- a/packages/loot-core/src/server/post.ts
+++ b/packages/loot-core/src/server/post.ts
@@ -70,7 +70,11 @@ export async function post(
    });
    text = await res.text();
  } catch (err) {
-    if (err instanceof Error && err.name === 'AbortError' && externalSignal?.aborted) {
+    if (
+      err instanceof Error &&
+      err.name === 'AbortError' &&
+      externalSignal?.aborted
+    ) {
      throw new PostError('aborted');
    }
    throw new PostError('network-failure');
@@ -112,12 +116,10 @@ export async function del(url, data, headers = {}, timeout = null) {
  let text;
  let res;

-  const controller = new AbortController();
-  const timeoutId =
-    timeout != null ? setTimeout(() => controller.abort(), timeout) : undefined;
-
  try {
-    const signal = timeout != null ? controller.signal : null;
+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), timeout);
+    const signal = timeout ? controller.signal : null;
    res = await fetch(url, {
      method: 'DELETE',
      body: JSON.stringify(data),
@@ -127,11 +129,10 @@ export async function del(url, data, headers = {}, timeout = null) {
        'Content-Type': 'application/json',
      },
    });
+    clearTimeout(timeoutId);
    text = await res.text();
  } catch {
    throw new PostError('network-failure');
-  } finally {
-    if (timeoutId != null) clearTimeout(timeoutId);
  }

  throwIfNot200(res, text);
@@ -163,12 +164,10 @@ export async function patch(url, data, headers = {}, timeout = null) {
  let text;
  let res;

-  const controller = new AbortController();
-  const timeoutId =
-    timeout != null ? setTimeout(() => controller.abort(), timeout) : undefined;
-
  try {
-    const signal = timeout != null ? controller.signal : null;
+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), timeout);
+    const signal = timeout ? controller.signal : null;
    res = await fetch(url, {
      method: 'PATCH',
      body: JSON.stringify(data),
@@ -178,11 +177,10 @@ export async function patch(url, data, headers = {}, timeout = null) {
        'Content-Type': 'application/json',
      },
    });
+    clearTimeout(timeoutId);
    text = await res.text();
  } catch {
    throw new PostError('network-failure');
-  } finally {
-    if (timeoutId != null) clearTimeout(timeoutId);
  }

  throwIfNot200(res, text);
--- a/packages/sync-server/src/app-enablebanking/app-enablebanking.ts
+++ b/packages/sync-server/src/app-enablebanking/app-enablebanking.ts
@@ -69,8 +69,9 @@ async function buildSessionResult(session: {
 // Auth callback from bank redirect — must be before validateSessionMiddleware
 // since the bank redirects here directly (no auth token available)
 app.get('/auth_callback', async (req: Request, res: Response) => {
-  const code = req.query.code as string | undefined;
-  const state = req.query.state as string | undefined;
+  const code = typeof req.query.code === 'string' ? req.query.code : undefined;
+  const state =
+    typeof req.query.state === 'string' ? req.query.state : undefined;

  if (!code) {
    res
--- a/packages/sync-server/src/app-enablebanking/services/enablebanking-service.ts
+++ b/packages/sync-server/src/app-enablebanking/services/enablebanking-service.ts
@@ -155,6 +155,7 @@ async function request<T>(
    throw handleEnableBankingError(response.status, responseBody);
  }

+  // eslint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- generic API wrapper, type is validated by caller
  return (await response.json()) as T;
 }

--- a/packages/sync-server/src/app-enablebanking/services/tests/enablebanking-service.spec.ts
+++ b/packages/sync-server/src/app-enablebanking/services/tests/enablebanking-service.spec.ts
@@ -8,7 +8,19 @@ import {
  vi,
 } from 'vitest';

+import { secretsService } from '../../../services/secrets-service';
 import { EnableBankingError } from '../../utils/errors';
+import { enableBankingService } from '../enablebanking-service';
+
+import {
+  mockAspspList,
+  mockAuthResponse,
+  mockBalance,
+  mockCreditTransaction,
+  mockDebitTransaction,
+  mockSession,
+  mockSessionAccount,
+} from './fixtures';

 // Mock dependencies before importing the service
 vi.mock('../../../services/secrets-service', () => ({
@@ -30,20 +42,6 @@ vi.mock('../../utils/jwt', () => ({
 const mockFetch = vi.fn();
 vi.stubGlobal('fetch', mockFetch);

-import { secretsService } from '../../../services/secrets-service';
-import { enableBankingService } from '../enablebanking-service';
-
-import {
-  mockAspsp,
-  mockAspspList,
-  mockAuthResponse,
-  mockBalance,
-  mockCreditTransaction,
-  mockDebitTransaction,
-  mockSession,
-  mockSessionAccount,
-} from './fixtures';
-
 function mockFetchResponse(data: unknown, ok = true, status = 200) {
  mockFetch.mockResolvedValueOnce({
    ok,
@@ -186,7 +184,7 @@ describe('enableBankingService', () => {
        }),
      );

-      const body = JSON.parse(mockFetch.mock.calls[0][1].body as string);
+      const body = JSON.parse(String(mockFetch.mock.calls[0][1].body));
      expect(body.aspsp).toEqual({ name: 'Nordea', country: 'FI' });
      expect(body.redirect_url).toBe('https://app.example.com/callback');
      expect(body.state).toBe('test-state-uuid');
@@ -206,7 +204,7 @@ describe('enableBankingService', () => {
        'state',
      );

-      const body = JSON.parse(mockFetch.mock.calls[0][1].body as string);
+      const body = JSON.parse(String(mockFetch.mock.calls[0][1].body));
      const validUntil = new Date(body.access.valid_until);
      const now = new Date();
      const diffDays = Math.round(
--- a/packages/sync-server/src/app-enablebanking/tests/poll-auth.spec.ts
+++ b/packages/sync-server/src/app-enablebanking/tests/poll-auth.spec.ts
@@ -1,3 +1,5 @@
+import express from 'express';
+import request from 'supertest';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

 // Mock all external dependencies before importing the app
@@ -27,7 +29,7 @@ vi.mock('../../app-gocardless/util/handle-error', () => ({
  handleError:
    (fn: Function) =>
    (req: unknown, res: { send: (data: unknown) => void }) => {
-      (fn(req, res) as Promise<void>).catch((err: Error) => {
+      Promise.resolve(fn(req, res)).catch((err: Error) => {
        res.send({
          status: 'ok',
          data: {
@@ -43,9 +45,6 @@ vi.mock('../../app-gocardless/util/handle-error', () => ({
 const mockFetch = vi.fn();
 vi.stubGlobal('fetch', mockFetch);

-import express from 'express';
-import request from 'supertest';
-
 // We need to dynamically import the handlers after mocks are set up
 const { handlers } = await import('../app-enablebanking');

--- a/packages/sync-server/src/app-enablebanking/utils/errors.ts
+++ b/packages/sync-server/src/app-enablebanking/utils/errors.ts
@@ -22,9 +22,9 @@ export function handleEnableBankingError(
    typeof body === 'string' ? body : JSON.stringify(body ?? 'unknown');
  debug('Enable Banking API error: status=%d body=%s', statusCode, bodyStr);

-  const parsed =
+  const parsed: Record<string, unknown> =
    typeof body === 'object' && body !== null
-      ? (body as Record<string, unknown>)
+      ? Object.fromEntries(Object.entries(body))
      : {};
  const message = typeof parsed.message === 'string' ? parsed.message : bodyStr;
  const errorType = typeof parsed.error === 'string' ? parsed.error : 'UNKNOWN';
--- a/packages/sync-server/src/app-enablebanking/utils/tests/jwt.spec.ts
+++ b/packages/sync-server/src/app-enablebanking/utils/tests/jwt.spec.ts
@@ -1,5 +1,8 @@
+import { sign } from 'jws';
 import { beforeEach, describe, expect, it, vi } from 'vitest';

+import { getJWT } from '../jwt';
+
 // Mock jws to avoid needing real RSA keys
 vi.mock('jws', () => ({
  sign: vi.fn(({ header, payload }) => {
@@ -7,10 +10,6 @@ vi.mock('jws', () => ({
  }),
 }));

-import { sign } from 'jws';
-
-import { getJWT } from '../jwt';
-
 describe('getJWT', () => {
  beforeEach(() => {
    vi.clearAllMocks();
@@ -33,22 +32,26 @@ describe('getJWT', () => {
    getJWT('my-app-id', 'my-secret-key');

    const callArgs = vi.mocked(sign).mock.calls[0][0];
-    const payload = callArgs.payload as Record<string, unknown>;
+    const rawPayload = callArgs.payload;
+    const payload: { iss: string; aud: string; iat: number; exp: number } =
+      typeof rawPayload === 'string' ? JSON.parse(rawPayload) : rawPayload;

    expect(payload.iss).toBe('enablebanking.com');
    expect(payload.aud).toBe('api.enablebanking.com');
    expect(typeof payload.iat).toBe('number');
    expect(typeof payload.exp).toBe('number');
-    expect((payload.exp as number) - (payload.iat as number)).toBe(3600);
+    expect(payload.exp - payload.iat).toBe(3600);
  });

  it('should use custom expiry', () => {
    getJWT('my-app-id', 'my-secret-key', 7200);

    const callArgs = vi.mocked(sign).mock.calls[0][0];
-    const payload = callArgs.payload as Record<string, unknown>;
+    const rawPayload = callArgs.payload;
+    const payload: { iat: number; exp: number } =
+      typeof rawPayload === 'string' ? JSON.parse(rawPayload) : rawPayload;

-    expect((payload.exp as number) - (payload.iat as number)).toBe(7200);
+    expect(payload.exp - payload.iat).toBe(7200);
  });

  it('should pass the secret key to jws.sign', () => {