From 9794824a48e122801561689481ceb75f3c6d7fd5 Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Tue, 22 Oct 2024 14:03:40 +0200 Subject: [PATCH 1/3] fix: Disable transaction buttons until sdk is initialized --- src/components/common/CheckWallet/index.tsx | 7 +++++++ .../common/NetworkSelector/NetworkMultiSelector.tsx | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/common/CheckWallet/index.tsx b/src/components/common/CheckWallet/index.tsx index 65d6e2ab21..517e32575d 100644 --- 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 { useIsWalletDelegate } from '@/hooks/useDelegates' import { useMemo, type ReactElement } from 'react' import useIsOnlySpendingLimitBeneficiary from '@/hooks/useIsOnlySpendingLimitBeneficiary' @@ -19,6 +20,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', } @@ -37,6 +39,7 @@ const CheckWallet = ({ const connectWallet = useConnectWallet() const isWrongChain = useIsWrongChain() const isDelegate = useIsWalletDelegate() + const sdk = useSafeSDK() const { safe } = useSafeInfo() @@ -46,6 +49,9 @@ const CheckWallet = ({ if (!wallet) { return Message.WalletNotConnected } + if (!sdk) { + return Message.SDKNotInitialized + } if (isUndeployedSafe && !allowUndeployedSafe) { return Message.SafeNotActivated } @@ -61,6 +67,7 @@ const CheckWallet = ({ isOnlySpendingLimit, isSafeOwner, isUndeployedSafe, + sdk, wallet, ]) diff --git a/src/components/common/NetworkSelector/NetworkMultiSelector.tsx b/src/components/common/NetworkSelector/NetworkMultiSelector.tsx index 5a4fbdcc68..9406311836 100644 --- a/src/components/common/NetworkSelector/NetworkMultiSelector.tsx +++ b/src/components/common/NetworkSelector/NetworkMultiSelector.tsx @@ -128,7 +128,7 @@ const NetworkMultiSelector = ({ )) } renderOption={(props, chain, { selected }) => ( -
  • +
    • From 126e74c9a8c42b3ba7469566299fc2ed1500a08e Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Tue, 22 Oct 2024 14:44:27 +0200 Subject: [PATCH 2/3] fix: Failing tests --- src/components/common/CheckWallet/index.test.tsx | 5 +++++ .../tx-flow/flows/SignMessage/SignMessage.test.tsx | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/components/common/CheckWallet/index.test.tsx b/src/components/common/CheckWallet/index.test.tsx index 8455fa4c93..3032ad211f 100644 --- a/src/components/common/CheckWallet/index.test.tsx +++ b/src/components/common/CheckWallet/index.test.tsx @@ -61,6 +61,11 @@ jest.mock('@/hooks/useSafeInfo', () => ({ }), })) +jest.mock('@/hooks/coreSDK/safeCoreSDK', () => ({ + __esModule: true, + useSafeSDK: jest.fn(() => ({})), +})) + const renderButton = () => render({(isOk) => }) diff --git a/src/components/tx-flow/flows/SignMessage/SignMessage.test.tsx b/src/components/tx-flow/flows/SignMessage/SignMessage.test.tsx index 0699137ea8..542d53e056 100644 --- 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', () => { From 0b08f351ae9210d7f641bd78a60714ad9c6492c6 Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Tue, 22 Oct 2024 15:20:20 +0200 Subject: [PATCH 3/3] fix: Add test case and adjust other tests to use getByText --- .../common/CheckWallet/index.test.tsx | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/src/components/common/CheckWallet/index.test.tsx b/src/components/common/CheckWallet/index.test.tsx index 3032ad211f..4056f22b63 100644 --- 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' @@ -75,34 +76,31 @@ 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).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).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', () => { @@ -110,11 +108,11 @@ describe('CheckWallet', () => { ;(useIsSafeOwner as jest.MockedFunction).mockReturnValueOnce(true) const renderButtonWithNetworkCheck = () => - render({(isOk) => }) + render({(isOk) => }) - 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', () => { @@ -123,20 +121,20 @@ describe('CheckWallet', () => { useIsOnlySpendingLimitBeneficiary as jest.MockedFunction ).mockReturnValueOnce(true) - const { container: allowContainer } = render( + const { getByText } = render( {(isOk) => }, ) - expect(allowContainer.querySelector('button')).not.toBeDisabled() + expect(getByText('Continue')).not.toBeDisabled() }) it('should not disable the button for delegates', () => { ;(useIsSafeOwner as jest.MockedFunction).mockReturnValueOnce(false) ;(useIsWalletDelegate as jest.MockedFunction).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 counterfactual Safes', () => { @@ -155,10 +153,10 @@ describe('CheckWallet', () => { mockSafeInfo as unknown as ReturnType, ) - 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', () => { @@ -177,21 +175,21 @@ describe('CheckWallet', () => { mockSafeInfo as unknown as ReturnType, ) - const { container } = render( + const { getByText } = render( {(isOk) => }, ) - expect(container.querySelector('button')).toBeEnabled() + expect(getByText('Continue')).toBeEnabled() }) it('should allow non-owners if specified', () => { ;(useIsSafeOwner as jest.MockedFunction).mockReturnValueOnce(false) - const { container } = render( + const { getByText } = render( {(isOk) => }, ) - 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', () => { @@ -200,10 +198,19 @@ describe('CheckWallet', () => { useIsOnlySpendingLimitBeneficiary as jest.MockedFunction ).mockReturnValueOnce(true) - const { container: allowContainer } = render( + const { getByText } = render({(isOk) => }) + + expect(getByText('Continue')).toBeDisabled() + }) + + it('should disable the button if SDK is not initialized', () => { + ;(useSafeSDK as jest.MockedFunction).mockReturnValue(undefined) + + const { getByText, getByLabelText } = render( {(isOk) => }, ) - expect(allowContainer.querySelector('button')).toBeDisabled() + expect(getByText('Continue')).toBeDisabled() + expect(getByLabelText('SDK is not initialized yet')) }) })