From 7abff9c7b2bf8e826ab1f2169642e6789bef2280 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 6 Dec 2024 15:34:27 +0000 Subject: [PATCH] fix: hide first time interaction alert if internal account (#28990) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Hide the first time interaction alert if the transaction `to` is an internal account. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28990?quickstart=1) ## **Related issues** Fixes: #28942 ## **Manual testing steps** See issue. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../useFirstTimeInteractionAlert.test.ts | 84 ++++++++++++------- .../useFirstTimeInteractionAlert.ts | 16 +++- 2 files changed, 68 insertions(+), 32 deletions(-) diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts index 6689d6610248..93da09a9674e 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts @@ -9,52 +9,53 @@ import { getMockConfirmState } from '../../../../../../test/data/confirmations/h import { renderHookWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; import { Severity } from '../../../../../helpers/constants/design-system'; import { RowAlertKey } from '../../../../../components/app/confirm/info/row/constants'; -import { genUnapprovedContractInteractionConfirmation } from '../../../../../../test/data/confirmations/contract-interaction'; import { useFirstTimeInteractionAlert } from './useFirstTimeInteractionAlert'; const ACCOUNT_ADDRESS = '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'; const TRANSACTION_ID_MOCK = '123-456'; -const CONFIRMATION_MOCK = genUnapprovedContractInteractionConfirmation({ - chainId: '0x5', -}) as TransactionMeta; - const TRANSACTION_META_MOCK = { id: TRANSACTION_ID_MOCK, chainId: '0x5', networkClientId: 'testNetworkClientId', - status: TransactionStatus.submitted, + status: TransactionStatus.unapproved, type: TransactionType.contractInteraction, txParams: { from: ACCOUNT_ADDRESS, }, time: new Date().getTime() - 10000, - firstTimeInteraction: true, } as TransactionMeta; function runHook({ currentConfirmation, - transactions = [], + internalAccountAddresses, }: { currentConfirmation?: TransactionMeta; - transactions?: TransactionMeta[]; + internalAccountAddresses?: string[]; } = {}) { - let pendingApprovals = {}; - if (currentConfirmation) { - pendingApprovals = { - [currentConfirmation.id as string]: { - id: currentConfirmation.id, - type: ApprovalType.Transaction, - }, - }; - transactions.push(currentConfirmation); - } + const pendingApprovals = currentConfirmation + ? { + [currentConfirmation.id as string]: { + id: currentConfirmation.id, + type: ApprovalType.Transaction, + }, + } + : {}; + + const transactions = currentConfirmation ? [currentConfirmation] : []; + + const internalAccounts = { + accounts: internalAccountAddresses?.map((address) => ({ address })) ?? [], + }; + const state = getMockConfirmState({ metamask: { + internalAccounts, pendingApprovals, transactions, }, }); + const response = renderHookWithConfirmContextProvider( useFirstTimeInteractionAlert, state, @@ -72,19 +73,22 @@ describe('useFirstTimeInteractionAlert', () => { expect(runHook()).toEqual([]); }); - it('returns no alerts if no transactions', () => { + it('returns no alerts if firstTimeInteraction is false', () => { + const notFirstTimeConfirmation = { + ...TRANSACTION_META_MOCK, + isFirstTimeInteraction: false, + }; expect( runHook({ - currentConfirmation: CONFIRMATION_MOCK, - transactions: [], + currentConfirmation: notFirstTimeConfirmation, }), ).toEqual([]); }); - it('returns no alerts if firstTimeInteraction is false', () => { + it('returns no alerts if firstTimeInteraction is undefined', () => { const notFirstTimeConfirmation = { ...TRANSACTION_META_MOCK, - firstTimeInteraction: false, + isFirstTimeInteraction: undefined, }; expect( runHook({ @@ -93,21 +97,43 @@ describe('useFirstTimeInteractionAlert', () => { ).toEqual([]); }); - it('returns no alerts if firstTimeInteraction is undefined', () => { - const notFirstTimeConfirmation = { + it('returns no alerts if transaction destination is internal account', () => { + const firstTimeConfirmation = { ...TRANSACTION_META_MOCK, - firstTimeInteraction: undefined, + isFirstTimeInteraction: true, + txParams: { + ...TRANSACTION_META_MOCK.txParams, + to: ACCOUNT_ADDRESS, + }, }; expect( runHook({ - currentConfirmation: notFirstTimeConfirmation, + currentConfirmation: firstTimeConfirmation, + internalAccountAddresses: [ACCOUNT_ADDRESS], + }), + ).toEqual([]); + }); + + it('returns no alerts if transaction destination is internal account with different case', () => { + const firstTimeConfirmation = { + ...TRANSACTION_META_MOCK, + isFirstTimeInteraction: true, + txParams: { + ...TRANSACTION_META_MOCK.txParams, + to: ACCOUNT_ADDRESS.toLowerCase(), + }, + }; + expect( + runHook({ + currentConfirmation: firstTimeConfirmation, + internalAccountAddresses: [ACCOUNT_ADDRESS.toUpperCase()], }), ).toEqual([]); }); it('returns alert if isFirstTimeInteraction is true', () => { const firstTimeConfirmation = { - ...CONFIRMATION_MOCK, + ...TRANSACTION_META_MOCK, isFirstTimeInteraction: true, }; const alerts = runHook({ diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts index 7e4a86c3802f..c74552575667 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts @@ -1,22 +1,32 @@ import { useMemo } from 'react'; import { TransactionMeta } from '@metamask/transaction-controller'; +import { useSelector } from 'react-redux'; import { Alert } from '../../../../../ducks/confirm-alerts/confirm-alerts'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; import { Severity } from '../../../../../helpers/constants/design-system'; import { RowAlertKey } from '../../../../../components/app/confirm/info/row/constants'; import { useConfirmContext } from '../../../context/confirm'; +import { getInternalAccounts } from '../../../../../selectors'; export function useFirstTimeInteractionAlert(): Alert[] { const t = useI18nContext(); const { currentConfirmation } = useConfirmContext(); + const internalAccounts = useSelector(getInternalAccounts); - const { isFirstTimeInteraction } = currentConfirmation ?? {}; + const { txParams, isFirstTimeInteraction } = currentConfirmation ?? {}; + const { to } = txParams ?? {}; + + const isInternalAccount = internalAccounts.some( + (account) => account.address?.toLowerCase() === to?.toLowerCase(), + ); + + const showAlert = !isInternalAccount && isFirstTimeInteraction; return useMemo(() => { // If isFirstTimeInteraction is undefined that means it's either disabled or error in accounts API // If it's false that means account relationship found - if (!isFirstTimeInteraction) { + if (!showAlert) { return []; } @@ -31,5 +41,5 @@ export function useFirstTimeInteractionAlert(): Alert[] { severity: Severity.Warning, }, ]; - }, [isFirstTimeInteraction, t]); + }, [showAlert, t]); }