Fix ability to have duplicate account names (#3527)

* add duplicate name validation to sidebar, and both narrow/wide Account pages * unify error messages and styles * keep state for the current account name on the mobile account modal * add release notes * remove extra validation function * fix typo in AccountSyncSidebar params * don't set current account name to empty string and prevent further changes on AccountMenuModal * lint imports * update error message to Name x already exists * use proper translation functions --------- Co-authored-by: youngcw <[email protected]>
actualbudget · Oct 11, 2024 · f9c08a9 · f9c08a9
1 parent e37a42f
commit f9c08a9
Show file tree

Hide file tree

Showing 6 changed files with 243 additions and 119 deletions.
diff --git a/packages/desktop-client/src/components/accounts/Account.tsx b/packages/desktop-client/src/components/accounts/Account.tsx
@@ -69,6 +69,7 @@ import { Button } from '../common/Button2';
 import { Text } from '../common/Text';
 import { View } from '../common/View';
 import { TransactionList } from '../transactions/TransactionList';
+import { validateAccountName } from '../util/accountValidation';
 
 import { AccountHeader } from './Header';
 
@@ -296,6 +297,7 @@ type AccountInternalState = {
   prevShowCleared?: boolean;
   showReconciled: boolean;
   editingName: boolean;
+  nameError: string;
   isAdding: boolean;
   modalShowing?: boolean;
   sort: {
@@ -343,6 +345,7 @@ class AccountInternal extends PureComponent<
       showCleared: props.showCleared,
       showReconciled: props.showReconciled,
       editingName: false,
+      nameError: '',
       isAdding: false,
       sort: null,
       filteredAmount: null,
@@ -707,13 +710,19 @@ class AccountInternal extends PureComponent<
   };
 
   onSaveName = (name: string) => {
-    if (name.trim().length) {
-      const accountId = this.props.accountId;
+    const accountNameError = validateAccountName(
+      name,
+      this.props.accountId,
+      this.props.accounts,
+    );
+    if (accountNameError) {
+      this.setState({ nameError: accountNameError });
+    } else {
       const account = this.props.accounts.find(
-        account => account.id === accountId,
-      )!;
+        account => account.id === this.props.accountId,
+      );
       this.props.updateAccount({ ...account, name });
-      this.setState({ editingName: false });
+      this.setState({ editingName: false, nameError: '' });
     }
   };
 
@@ -1708,6 +1717,7 @@ class AccountInternal extends PureComponent<
                 onAddTransaction={this.onAddTransaction}
                 onToggleExtraBalances={this.onToggleExtraBalances}
                 onSaveName={this.onSaveName}
+                saveNameError={this.state.nameError}
                 onExposeName={this.onExposeName}
                 onReconcile={this.onReconcile}
                 onDoneReconciling={this.onDoneReconciling}

diff --git a/packages/desktop-client/src/components/accounts/Header.jsx b/packages/desktop-client/src/components/accounts/Header.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useRef } from 'react';
+import React, { useState, useRef, Fragment } from 'react';
 import { useHotkeys } from 'react-hotkeys-hook';
 import { Trans, useTranslation } from 'react-i18next';
 
@@ -67,6 +67,7 @@ export function AccountHeader({
   onCreateReconciliationTransaction,
   onToggleExtraBalances,
   onSaveName,
+  saveNameError,
   onExposeName,
   onSync,
   onImport,
@@ -90,7 +91,6 @@ export function AccountHeader({
   onMakeAsNonSplitTransactions,
 }) {
   const { t } = useTranslation();
-
   const [menuOpen, setMenuOpen] = useState(false);
   const searchInput = useRef(null);
   const triggerRef = useRef(null);
@@ -176,99 +176,21 @@ export function AccountHeader({
             }}
           >
             {!!account?.bank && (
-              <View
-                style={{
-                  backgroundColor: accountsSyncing.includes(account.id)
-                    ? theme.sidebarItemBackgroundPending
-                    : failedAccounts.has(account.id)
-                      ? theme.sidebarItemBackgroundFailed
-                      : theme.sidebarItemBackgroundPositive,
-                  marginRight: '4px',
-                  width: 8,
-                  height: 8,
-                  borderRadius: 8,
-                }}
+              <AccountSyncSidebar
+                account={account}
+                failedAccounts={failedAccounts}
+                accountsSyncing={accountsSyncing}
               />
             )}
-            {editingName ? (
-              <InitialFocus>
-                <Input
-                  defaultValue={accountName}
-                  onEnter={e => onSaveName(e.target.value)}
-                  onBlur={e => onSaveName(e.target.value)}
-                  onEscape={() => onExposeName(false)}
-                  style={{
-                    fontSize: 25,
-                    fontWeight: 500,
-                    marginTop: -3,
-                    marginBottom: -4,
-                    marginLeft: -6,
-                    paddingTop: 2,
-                    paddingBottom: 2,
-                    width: Math.max(20, accountName.length) + 'ch',
-                  }}
-                />
-              </InitialFocus>
-            ) : isNameEditable ? (
-              <View
-                style={{
-                  flexDirection: 'row',
-                  alignItems: 'center',
-                  gap: 3,
-                  '& .hover-visible': {
-                    opacity: 0,
-                    transition: 'opacity .25s',
-                  },
-                  '&:hover .hover-visible': {
-                    opacity: 1,
-                  },
-                }}
-              >
-                <View
-                  style={{
-                    fontSize: 25,
-                    fontWeight: 500,
-                    marginRight: 5,
-                    marginBottom: -1,
-                  }}
-                  data-testid="account-name"
-                >
-                  {account && account.closed
-                    ? t('Closed: {{ accountName }}', { accountName })
-                    : accountName}
-                </View>
-
-                {account && (
-                  <NotesButton
-                    id={`account-${account.id}`}
-                    defaultColor={theme.pageTextSubdued}
-                  />
-                )}
-                <Button
-                  variant="bare"
-                  aria-label={t('Edit account name')}
-                  className="hover-visible"
-                  onPress={() => onExposeName(true)}
-                >
-                  <SvgPencil1
-                    style={{
-                      width: 11,
-                      height: 11,
-                      color: theme.pageTextSubdued,
-                    }}
-                  />
-                </Button>
-              </View>
-            ) : (
-              <View
-                style={{ fontSize: 25, fontWeight: 500, marginBottom: -1 }}
-                data-testid="account-name"
-              >
-                {account && account.closed
-                  ? t('Closed: {{ accountName }}', { accountName })
-                  : accountName}
-              </View>
-            )}
+            <AccountNameField
+              account={account}
+              accountName={accountName}
+              isNameEditable={isNameEditable}
+              editingName={editingName}
+              saveNameError={saveNameError}
+              onSaveName={onSaveName}
+              onExposeName={onExposeName}
+            />
           </View>
         </View>
 
@@ -470,6 +392,129 @@ export function AccountHeader({
   );
 }
 
+function AccountSyncSidebar({ account, failedAccounts, accountsSyncing }) {
+  return (
+    <View
+      style={{
+        backgroundColor: accountsSyncing.includes(account.id)
+          ? theme.sidebarItemBackgroundPending
+          : failedAccounts.has(account.id)
+            ? theme.sidebarItemBackgroundFailed
+            : theme.sidebarItemBackgroundPositive,
+        marginRight: '4px',
+        width: 8,
+        height: 8,
+        borderRadius: 8,
+      }}
+    />
+  );
+}
+
+function AccountNameField({
+  account,
+  accountName,
+  isNameEditable,
+  editingName,
+  saveNameError,
+  onSaveName,
+  onExposeName,
+}) {
+  const { t } = useTranslation();
+
+  if (editingName) {
+    return (
+      <Fragment>
+        <InitialFocus>
+          <Input
+            defaultValue={accountName}
+            onEnter={e => onSaveName(e.target.value)}
+            onBlur={e => onSaveName(e.target.value)}
+            onEscape={() => onExposeName(false)}
+            style={{
+              fontSize: 25,
+              fontWeight: 500,
+              marginTop: -3,
+              marginBottom: -4,
+              marginLeft: -6,
+              paddingTop: 2,
+              paddingBottom: 2,
+              width: Math.max(20, accountName.length) + 'ch',
+            }}
+          />
+        </InitialFocus>
+        {saveNameError && (
+          <View style={{ color: theme.warningText }}>{saveNameError}</View>
+        )}
+      </Fragment>
+    );
+  } else {
+    if (isNameEditable) {
+      return (
+        <View
+          style={{
+            flexDirection: 'row',
+            alignItems: 'center',
+            gap: 3,
+            '& .hover-visible': {
+              opacity: 0,
+              transition: 'opacity .25s',
+            },
+            '&:hover .hover-visible': {
+              opacity: 1,
+            },
+          }}
+        >
+          <View
+            style={{
+              fontSize: 25,
+              fontWeight: 500,
+              marginRight: 5,
+              marginBottom: -1,
+            }}
+            data-testid="account-name"
+          >
+            {account && account.closed
+              ? t('Closed: {{ accountName }}', { accountName })
+              : accountName}
+          </View>
+
+          {account && (
+            <NotesButton
+              id={`account-${account.id}`}
+              defaultColor={theme.pageTextSubdued}
+            />
+          )}
+          <Button
+            variant="bare"
+            aria-label={t('Edit account name')}
+            className="hover-visible"
+            onPress={() => onExposeName(true)}
+          >
+            <SvgPencil1
+              style={{
+                width: 11,
+                height: 11,
+                color: theme.pageTextSubdued,
+              }}
+            />
+          </Button>
+        </View>
+      );
+    } else {
+      return (
+        <View
+          style={{ fontSize: 25, fontWeight: 500, marginBottom: -1 }}
+          data-testid="account-name"
+        >
+          {account && account.closed
+            ? t('Closed: {{ accountName }}', { accountName })
+            : accountName}
+        </View>
+      );
+    }
+  }
+}
+
 function AccountMenu({
   account,
   canSync,
@@ -483,7 +528,6 @@ function AccountMenu({
   onMenuSelect,
 }) {
   const { t } = useTranslation();
-
   const [tooltip, setTooltip] = useState('default');
   const syncServerStatus = useSyncServerStatus();