fix: Disable transaction buttons until sdk is initialized (#4405)

* fix: Disable transaction buttons until sdk is initialized * fix: Failing tests * fix: Add test case and adjust other tests to use getByText --------- Co-authored-by: katspaugh <[email protected]>
safe-global · Nov 25, 2024 · 5637ba7 · 5637ba7
1 parent 207dfd1
commit 5637ba7
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 28 deletions.
diff --git a/src/components/common/CheckWallet/index.test.tsx b/src/components/common/CheckWallet/index.test.tsx
@@ -1,4 +1,5 @@
-import { getByLabelText, render } from '@/tests/test-utils'
+import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
+import { render } from '@/tests/test-utils'
 import CheckWallet from '.'
 import useIsOnlySpendingLimitBeneficiary from '@/hooks/useIsOnlySpendingLimitBeneficiary'
 import useIsSafeOwner from '@/hooks/useIsSafeOwner'
@@ -61,6 +62,11 @@ jest.mock('@/hooks/useSafeInfo', () => ({
   }),
 }))
 
+jest.mock('@/hooks/coreSDK/safeCoreSDK', () => ({
+  __esModule: true,
+  useSafeSDK: jest.fn(() => ({})),
+}))
+
 const renderButton = () =>
   render(<CheckWallet checkNetwork={false}>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>)
 
@@ -70,46 +76,43 @@ describe('CheckWallet', () => {
   })
 
   it('renders correctly when the wallet is connected to the right chain and is an owner', () => {
-    const { container } = renderButton()
+    const { getByText } = renderButton()
 
     // Check that the button is enabled
-    expect(container.querySelector('button')).not.toBeDisabled()
+    expect(getByText('Continue')).not.toBeDisabled()
   })
 
   it('should disable the button when the wallet is not connected', () => {
     ;(useWallet as jest.MockedFunction<typeof useWallet>).mockReturnValueOnce(null)
 
-    const { container } = renderButton()
+    const { getByText, getByLabelText } = renderButton()
 
     // Check that the button is disabled
-    expect(container.querySelector('button')).toBeDisabled()
+    expect(getByText('Continue')).toBeDisabled()
 
     // Check the tooltip text
-    getByLabelText(container, 'Please connect your wallet')
+    expect(getByLabelText('Please connect your wallet')).toBeInTheDocument()
   })
 
   it('should disable the button when the wallet is connected to the right chain but is not an owner', () => {
     ;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)
 
-    const { container } = renderButton()
+    const { getByText, getByLabelText } = renderButton()
 
-    expect(container.querySelector('button')).toBeDisabled()
-    expect(container.querySelector('span[aria-label]')).toHaveAttribute(
-      'aria-label',
-      `Your connected wallet is not a signer of this Safe Account`,
-    )
+    expect(getByText('Continue')).toBeDisabled()
+    expect(getByLabelText('Your connected wallet is not a signer of this Safe Account')).toBeInTheDocument()
   })
 
   it('should be disabled when connected to the wrong network', () => {
     ;(useIsWrongChain as jest.MockedFunction<typeof useIsWrongChain>).mockReturnValue(true)
     ;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(true)
 
     const renderButtonWithNetworkCheck = () =>
-      render(<CheckWallet checkNetwork={true}>{(isOk) => <button disabled={!isOk}></button>}</CheckWallet>)
+      render(<CheckWallet checkNetwork={true}>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>)
 
-    const { container } = renderButtonWithNetworkCheck()
+    const { getByText } = renderButtonWithNetworkCheck()
 
-    expect(container.querySelector('button')).toBeDisabled()
+    expect(getByText('Continue')).toBeDisabled()
   })
 
   it('should not disable the button for non-owner spending limit benificiaries', () => {
@@ -118,20 +121,20 @@ describe('CheckWallet', () => {
       useIsOnlySpendingLimitBeneficiary as jest.MockedFunction<typeof useIsOnlySpendingLimitBeneficiary>
     ).mockReturnValueOnce(true)
 
-    const { container: allowContainer } = render(
+    const { getByText } = render(
       <CheckWallet allowSpendingLimit>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
     )
 
-    expect(allowContainer.querySelector('button')).not.toBeDisabled()
+    expect(getByText('Continue')).not.toBeDisabled()
   })
 
   it('should not disable the button for proposers', () => {
     ;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)
     ;(useIsWalletProposer as jest.MockedFunction<typeof useIsWalletProposer>).mockReturnValueOnce(true)
 
-    const { container } = renderButton()
+    const { getByText } = renderButton()
 
-    expect(container.querySelector('button')).not.toBeDisabled()
+    expect(getByText('Continue')).not.toBeDisabled()
   })
 
   it('should disable the button for proposers if specified via flag', () => {
@@ -172,10 +175,10 @@ describe('CheckWallet', () => {
       mockSafeInfo as unknown as ReturnType<typeof useSafeInfo>,
     )
 
-    const { container } = renderButton()
+    const { getByText, getByLabelText } = renderButton()
 
-    expect(container.querySelector('button')).toBeDisabled()
-    getByLabelText(container, 'You need to activate the Safe before transacting')
+    expect(getByText('Continue')).toBeDisabled()
+    expect(getByLabelText('You need to activate the Safe before transacting')).toBeInTheDocument()
   })
 
   it('should enable the button for counterfactual Safes if allowed', () => {
@@ -194,21 +197,21 @@ describe('CheckWallet', () => {
       mockSafeInfo as unknown as ReturnType<typeof useSafeInfo>,
     )
 
-    const { container } = render(
+    const { getByText } = render(
       <CheckWallet allowUndeployedSafe>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
     )
 
-    expect(container.querySelector('button')).toBeEnabled()
+    expect(getByText('Continue')).toBeEnabled()
   })
 
   it('should allow non-owners if specified', () => {
     ;(useIsSafeOwner as jest.MockedFunction<typeof useIsSafeOwner>).mockReturnValueOnce(false)
 
-    const { container } = render(
+    const { getByText } = render(
       <CheckWallet allowNonOwner>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
     )
 
-    expect(container.querySelector('button')).not.toBeDisabled()
+    expect(getByText('Continue')).not.toBeDisabled()
   })
 
   it('should not allow non-owners that have a spending limit without allowing spending limits', () => {
@@ -217,10 +220,19 @@ describe('CheckWallet', () => {
       useIsOnlySpendingLimitBeneficiary as jest.MockedFunction<typeof useIsOnlySpendingLimitBeneficiary>
     ).mockReturnValueOnce(true)
 
-    const { container: allowContainer } = render(
+    const { getByText } = render(<CheckWallet>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>)
+
+    expect(getByText('Continue')).toBeDisabled()
+  })
+
+  it('should disable the button if SDK is not initialized', () => {
+    ;(useSafeSDK as jest.MockedFunction<typeof useSafeSDK>).mockReturnValue(undefined)
+
+    const { getByText, getByLabelText } = render(
       <CheckWallet>{(isOk) => <button disabled={!isOk}>Continue</button>}</CheckWallet>,
     )
 
-    expect(allowContainer.querySelector('button')).toBeDisabled()
+    expect(getByText('Continue')).toBeDisabled()
+    expect(getByLabelText('SDK is not initialized yet'))
   })
 })
diff --git a/src/components/common/CheckWallet/index.tsx b/src/components/common/CheckWallet/index.tsx
@@ -1,3 +1,4 @@
+import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
 import { useIsWalletProposer } from '@/hooks/useProposers'
 import { useMemo, type ReactElement } from 'react'
 import useIsOnlySpendingLimitBeneficiary from '@/hooks/useIsOnlySpendingLimitBeneficiary'
@@ -20,6 +21,7 @@ type CheckWalletProps = {
 
 enum Message {
   WalletNotConnected = 'Please connect your wallet',
+  SDKNotInitialized = 'SDK is not initialized yet',
   NotSafeOwner = 'Your connected wallet is not a signer of this Safe Account',
   SafeNotActivated = 'You need to activate the Safe before transacting',
 }
@@ -38,6 +40,7 @@ const CheckWallet = ({
   const isOnlySpendingLimit = useIsOnlySpendingLimitBeneficiary()
   const connectWallet = useConnectWallet()
   const isWrongChain = useIsWrongChain()
+  const sdk = useSafeSDK()
   const isProposer = useIsWalletProposer()
 
   const { safe } = useSafeInfo()
@@ -48,6 +51,9 @@ const CheckWallet = ({
     if (!wallet) {
       return Message.WalletNotConnected
     }
+    if (!sdk) {
+      return Message.SDKNotInitialized
+    }
 
     if (isUndeployedSafe && !allowUndeployedSafe) {
       return Message.SafeNotActivated
@@ -69,6 +75,7 @@ const CheckWallet = ({
     isOnlySpendingLimit,
     isSafeOwner,
     isUndeployedSafe,
+    sdk,
     wallet,
   ])
 

diff --git a/src/components/tx-flow/flows/SignMessage/SignMessage.test.tsx b/src/components/tx-flow/flows/SignMessage/SignMessage.test.tsx
@@ -1,3 +1,4 @@
+import type Safe from '@safe-global/protocol-kit'
 import { act } from 'react'
 import { extendedSafeInfoBuilder } from '@/tests/builders/safe'
 import { hexlify, zeroPadValue, toUtf8Bytes } from 'ethers'
@@ -13,6 +14,7 @@ import * as useChainsHook from '@/hooks/useChains'
 import * as sender from '@/services/safe-messages/safeMsgSender'
 import * as onboard from '@/hooks/wallets/useOnboard'
 import * as useSafeMessage from '@/hooks/messages/useSafeMessage'
+import * as sdk from '@/hooks/coreSDK/safeCoreSDK'
 import { render, fireEvent, waitFor } from '@/tests/test-utils'
 import type { ConnectedWallet } from '@/hooks/wallets/useOnboard'
 import type { EIP1193Provider, WalletState, AppState, OnboardAPI } from '@web3-onboard/core'
@@ -102,6 +104,8 @@ describe('SignMessage', () => {
     }))
 
     jest.spyOn(useIsWrongChainHook, 'default').mockImplementation(() => false)
+
+    jest.spyOn(sdk, 'useSafeSDK').mockReturnValue({} as unknown as Safe)
   })
 
   describe('EIP-191 messages', () => {