Repository: joinmarket-webui/jam
Pull request #720 (2024-02-08 03:45:46)

636 check for existing wallet
This PR is a fix for https://github.com/joinmarket-webui/jam/issues/636. When creating/importing a new wallet, the wallet name will now be validated immediately for duplicates, instead of on form submission when the user clicks "Create".

https://github.com/joinmarket-webui/jam/assets/25010707/6443dcfb-875e-4293-8b92-ae8125c9cd2c

An attempted fix was made using react router loaders, but this caused many redundant api calls to be made on component mount and session refresh. For now, this fix follows existing patterns, but utilizing a loader is something that could/should be investigated in future refactors.

kristapsk: Concept ACK

Patch:
diff --git a/src/components/WalletCreationForm.tsx b/src/components/WalletCreationForm.tsx
index db7ba6e..413c500 100644
--- a/src/components/WalletCreationForm.tsx
+++ b/src/components/WalletCreationForm.tsx
@@ -1,10 +1,11 @@
-import { useCallback } from 'react'
+import { useCallback, useEffect, useState } from 'react'
 import * as rb from 'react-bootstrap'
 import { useTranslation } from 'react-i18next'
 import { Formik, FormikErrors } from 'formik'
 import Sprite from './Sprite'
 import { JM_WALLET_FILE_EXTENSION, sanitizeWalletName } from '../utils'
 import styles from './WalletCreationForm.module.css'
+import * as Api from '../libs/JmWalletApi'
 
 export interface CreateWalletFormValues {
   walletName: string
@@ -36,14 +37,34 @@ const WalletCreationForm = ({
   onCancel,
   onSubmit,
 }: WalletCreationFormProps) => {
+  const [walletList, setWalletList] = useState<Api.WalletFileName[] | null>(null)
   const { t, i18n } = useTranslation()
 
+  useEffect(() => {
+    const abortCtrl = new AbortController()
+
+    Api.getWalletAll({ signal: abortCtrl.signal })
+      .then((res) => (res.ok ? res.json() : Api.Helper.throwError(res, t('wallets.error_loading_failed'))))
+      .then((data) => {
+        if (abortCtrl.signal.aborted) return
+        setWalletList(data.wallets)
+      })
+      .catch(() => {
+        // do nothing on purpose
+      })
+
+    return () => abortCtrl.abort()
+  }, [t])
+
   const validate = useCallback(
     (values: CreateWalletFormValues) => {
       const errors = {} as FormikErrors<CreateWalletFormValues>
       if (!values.walletName || !validateWalletName(values.walletName)) {
         errors.walletName = t('create_wallet.feedback_invalid_wallet_name')
       }
+      if (walletList && walletList.includes(`${values.walletName}.jmdat`)) {
+        errors.walletName = t('create_wallet.feedback_wallet_name_already_exists')
+      }
       if (!values.password) {
         errors.password = t('create_wallet.feedback_invalid_password')
       }
@@ -52,7 +73,7 @@ const WalletCreationForm = ({
       }
       return errors
     },
-    [t],
+    [t, walletList],
   )
 
   return (
diff --git a/src/i18n/locales/en/translation.json b/src/i18n/locales/en/translation.json
index 4965e2c..861aa5f 100644
--- a/src/i18n/locales/en/translation.json
+++ b/src/i18n/locales/en/translation.json
@@ -131,6 +131,7 @@
     "label_wallet_name": "Wallet name",
     "placeholder_wallet_name": "Your Wallet...",
     "feedback_invalid_wallet_name": "Please choose a valid wallet name: Use only letters, numbers, underscores or hyphens.",
+    "feedback_wallet_name_already_exists": "Please choose another wallet name. This one is already in use.",
     "label_password": "Password to unlock the wallet",
     "placeholder_password": "Choose a secure password...",
     "feedback_invalid_password": "Please set a password.",


Test patch:
diff --git a/src/components/CreateWallet.test.tsx b/src/components/CreateWallet.test.tsx
index 967d688..96f9906 100644
--- a/src/components/CreateWallet.test.tsx
+++ b/src/components/CreateWallet.test.tsx
@@ -13,6 +13,7 @@ jest.mock('../libs/JmWalletApi', () => ({
   getGetinfo: jest.fn(),
   getSession: jest.fn(),
   postWalletCreate: jest.fn(),
+  getWalletAll: jest.fn(),
 }))
 
 const NOOP = () => {}
@@ -38,6 +39,7 @@ describe('<CreateWallet />', () => {
     const neverResolvingPromise = new Promise(() => {})
     ;(apiMock.getGetinfo as jest.Mock).mockReturnValue(neverResolvingPromise)
     ;(apiMock.getSession as jest.Mock).mockReturnValue(neverResolvingPromise)
+    ;(apiMock.getWalletAll as jest.Mock).mockReturnValue(neverResolvingPromise)
   })
 
   it('should display alert when rescanning is active', async () => {
@@ -87,6 +89,26 @@ describe('<CreateWallet />', () => {
     expect(await screen.findByText('create_wallet.feedback_invalid_password_confirm')).toBeVisible()
   })
 
+  it('should show validation message to user if duplicate wallet name', async () => {
+    ;(apiMock.getWalletAll as jest.Mock).mockReturnValue(
+      Promise.resolve({
+        ok: true,
+        json: () => Promise.resolve({ wallets: [`${testWalletName}.jmdat`] }),
+      }),
+    )
+    setup({})
+
+    expect(await screen.queryByText('create_wallet.feedback_wallet_name_already_exists')).not.toBeInTheDocument()
+
+    await user.type(screen.getByPlaceholderText('create_wallet.placeholder_wallet_name'), testWalletName)
+    await user.type(screen.getByPlaceholderText('create_wallet.placeholder_password'), testWalletPassword)
+    await user.type(screen.getByPlaceholderText('create_wallet.placeholder_password_confirm'), testWalletPassword)
+
+    await user.click(screen.getByText('create_wallet.button_create'))
+
+    expect(await screen.findByText('create_wallet.feedback_wallet_name_already_exists')).toBeVisible()
+  })
+
   it('should not submit form if wallet name contains invalid characters', async () => {
     setup({})