Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Hide redesigned contract interaction confirmation #27346

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import FixtureBuilder from '../fixture-builder';
import { SMART_CONTRACTS } from '../seeder/smart-contracts';
import { installSnapSimpleKeyring, importKeyAndSwitch } from './common';

describe('Snap Account Contract interaction', function () {
describe.skip('Snap Account Contract interaction', function () {
const smartContract = SMART_CONTRACTS.PIGGYBANK;

it('deposits to piggybank contract', async function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const FixtureBuilder = require('../../../fixture-builder');
const { SMART_CONTRACTS } = require('../../../seeder/smart-contracts');
const { CHAIN_IDS } = require('../../../../../shared/constants/network');

describe('Confirmation Redesign Contract Interaction Component', function () {
describe.skip('Confirmation Redesign Contract Interaction Component', function () {
const smartContract = SMART_CONTRACTS.PIGGYBANK;

describe('Create a deposit transaction @no-mmi', function () {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/tests/confirmations/transactions/metrics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const {
} = require('../../../helpers');
const FixtureBuilder = require('../../../fixture-builder');

describe('Metrics @no-mmi', function () {
describe.skip('Metrics @no-mmi', function () {
it('Sends a contract interaction type 2 transaction (EIP1559) with the right properties in the metric events', async function () {
await withFixtures(
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ const getMetaMaskStateWithMaliciousUnapprovedContractInteraction = (
};
};

describe('Contract Interaction Confirmation', () => {
describe.skip('Contract Interaction Confirmation', () => {
beforeEach(() => {
jest.resetAllMocks();
setupSubmitRequestToBackgroundMocks();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,20 +216,6 @@ exports[`Header should match snapshot with transaction confirmation 1`] = `
</button>
</div>
</div>
<div
class="mm-box mm-box--background-color-transparent mm-box--rounded-md"
>
<button
aria-label="Advanced tx details"
class="mm-box mm-button-icon mm-button-icon--size-md mm-box--display-inline-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-icon-default mm-box--background-color-transparent mm-box--rounded-lg"
data-testid="header-advanced-details-button"
>
<span
class="mm-box mm-icon mm-icon--size-md mm-box--display-inline-block mm-box--color-inherit"
style="mask-image: url('./images/icons/customize.svg');"
/>
</button>
</div>
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@ import {
TransactionStatus,
TransactionType,
} from '@metamask/transaction-controller';
import { Severity } from '../../../../../helpers/constants/design-system';
import { renderHookWithProvider } from '../../../../../../test/lib/render-helpers';
import mockState from '../../../../../../test/data/mock-state.json';
import { RowAlertKey } from '../../../../../components/app/confirm/info/row/constants';
import { renderHookWithProvider } from '../../../../../../test/lib/render-helpers';
import { usePendingTransactionAlerts } from './usePendingTransactionAlerts';

const ACCOUNT_ADDRESS = '0x123';
Expand Down Expand Up @@ -120,21 +118,12 @@ describe('usePendingTransactionAlerts', () => {
).toEqual([]);
});

it('returns alert if submitted transaction', () => {
it('returns no alert if submitted transaction because transaction type is not valid', () => {
const alerts = runHook({
currentConfirmation: CONFIRMATION_MOCK,
transactions: [TRANSACTION_META_MOCK],
});

expect(alerts).toEqual([
{
field: RowAlertKey.Speed,
key: 'pendingTransactions',
message:
'This transaction won’t go through until a previous transaction is complete. Learn how to cancel or speed up a transaction.',
reason: 'Pending transaction',
severity: Severity.Warning,
},
]);
expect(alerts).toEqual([]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,15 @@ import {
TransactionStatus,
TransactionType,
} from '@metamask/transaction-controller';
import { Severity } from '../../../../../helpers/constants/design-system';
import { renderHookWithProvider } from '../../../../../../test/lib/render-helpers';
import mockState from '../../../../../../test/data/mock-state.json';
import { renderHookWithProvider } from '../../../../../../test/lib/render-helpers';
import { useSigningOrSubmittingAlerts } from './useSigningOrSubmittingAlerts';

const TRANSACTION_META_MOCK: Partial<TransactionMeta> = {
id: '123-456',
chainId: '0x5',
};

const EXPECTED_ALERT = {
isBlocking: true,
key: 'signingOrSubmitting',
message:
'This transaction will only go through once your previous transaction is complete.',
severity: Severity.Warning,
};

const CONFIRMATION_MOCK = {
type: TransactionType.contractInteraction,
};
Expand Down Expand Up @@ -77,7 +68,7 @@ describe('useSigningOrSubmittingAlerts', () => {
).toEqual([]);
});

it('returns alerts if transaction on different chain', () => {
it('doesnt return alerts if transaction on different chain because transaction type is not valid', () => {
expect(
runHook({
currentConfirmation: CONFIRMATION_MOCK,
Expand All @@ -89,7 +80,7 @@ describe('useSigningOrSubmittingAlerts', () => {
},
],
}),
).toEqual([EXPECTED_ALERT]);
).toEqual([]);
});

it('returns no alerts if transaction has alternate status', () => {
Expand All @@ -112,25 +103,25 @@ describe('useSigningOrSubmittingAlerts', () => {
).toEqual([]);
});

it('returns alert if signed transaction', () => {
it('doesnt return alert if signed transaction because type is not valid', () => {
const alerts = runHook({
currentConfirmation: CONFIRMATION_MOCK,
transactions: [
{ ...TRANSACTION_META_MOCK, status: TransactionStatus.signed },
],
});

expect(alerts).toEqual([EXPECTED_ALERT]);
expect(alerts).toEqual([]);
});

it('returns alert if approved transaction', () => {
it('doesnt return alert if approved transaction because type is not valid', () => {
const alerts = runHook({
currentConfirmation: CONFIRMATION_MOCK,
transactions: [
{ ...TRANSACTION_META_MOCK, status: TransactionStatus.approved },
],
});

expect(alerts).toEqual([EXPECTED_ALERT]);
expect(alerts).toEqual([]);
});
});
111 changes: 3 additions & 108 deletions ui/pages/confirmations/hooks/useConfirmationAlertMetrics.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import { act } from '@testing-library/react-hooks';
import { TransactionType } from '@metamask/transaction-controller';
import { renderHookWithProvider } from '../../../../test/lib/render-helpers';
import mockState from '../../../../test/data/mock-state.json';
import { renderHookWithProvider } from '../../../../test/lib/render-helpers';
import { Severity } from '../../../helpers/constants/design-system';
import {
useConfirmationAlertMetrics,
ALERTS_NAME_METRICS,
} from './useConfirmationAlertMetrics';
import * as transactionEventFragmentHook from './useTransactionEventFragment';
import { AlertsName } from './alerts/constants';
import { useConfirmationAlertMetrics } from './useConfirmationAlertMetrics';
import * as transactionEventFragmentHook from './useTransactionEventFragment';

jest.mock('./useTransactionEventFragment');

Expand All @@ -17,7 +13,6 @@ const mockUpdateTransactionEventFragment = jest.fn();
const OWNER_ID_MOCK = '123';
const KEY_ALERT_KEY_MOCK = 'Key';
const ALERT_MESSAGE_MOCK = 'Alert 1';
const ALERT_NAME_METRICS_MOCK = ALERTS_NAME_METRICS[AlertsName.GasFeeLow];
const UUID_ALERT_KEY_MOCK = '550e8400-e29b-41d4-a716-446655440000';
const alertsMock = [
{
Expand Down Expand Up @@ -58,20 +53,6 @@ const STATE_MOCK = {
},
};

const EXPECTED_PROPERTIES_BASE = {
alert_action_clicked: [],
alert_key_clicked: [],
alert_resolved: [],
alert_resolved_count: 0,
alert_triggered: [
ALERT_NAME_METRICS_MOCK,
ALERTS_NAME_METRICS[AlertsName.Blockaid],
],
alert_triggered_count: 2,
alert_visualized: [],
alert_visualized_count: 0,
};

beforeEach(() => {
jest.clearAllMocks();
(
Expand All @@ -92,90 +73,4 @@ describe('useConfirmationAlertMetrics', () => {
expect(result.current.trackInlineAlertClicked).toBeInstanceOf(Function);
expect(result.current.trackAlertActionClicked).toBeInstanceOf(Function);
});

it('calls updateTransactionEventFragment with correct properties on initialization', () => {
renderHookWithProvider(() => useConfirmationAlertMetrics(), STATE_MOCK);

expect(mockUpdateTransactionEventFragment).toHaveBeenCalledWith(
{ properties: EXPECTED_PROPERTIES_BASE },
OWNER_ID_MOCK,
);
});

const testCases = [
{
description: 'updates metrics properties when trackAlertRender is called',
alertKey: AlertsName.GasFeeLow,
action: 'trackAlertRender',
expectedProperties: {
alert_visualized: [ALERT_NAME_METRICS_MOCK],
alert_visualized_count: 1,
},
},
{
description:
'updates metrics properties when trackInlineAlertClicked is called',
alertKey: AlertsName.GasFeeLow,
action: 'trackInlineAlertClicked',
expectedProperties: {
alert_key_clicked: [ALERT_NAME_METRICS_MOCK],
},
},
{
description:
'updates metrics properties when trackAlertActionClicked is called',
alertKey: AlertsName.GasFeeLow,
action: 'trackAlertActionClicked',
expectedProperties: {
alert_action_clicked: [ALERT_NAME_METRICS_MOCK],
},
},
{
description:
'updates metrics properties when receives alertKey as a valid UUID',
alertKey: UUID_ALERT_KEY_MOCK,
action: 'trackAlertRender',
expectedProperties: {
alert_visualized: [ALERTS_NAME_METRICS[AlertsName.Blockaid]],
alert_visualized_count: 1,
},
},
];

// @ts-expect-error This is missing from the Mocha type definitions
it.each(testCases)(
'$description',
({
alertKey,
action,
expectedProperties,
}: {
description: string;
alertKey: string;
action:
| 'trackAlertRender'
| 'trackInlineAlertClicked'
| 'trackAlertActionClicked';
expectedProperties: Record<string, unknown>;
}) => {
const finalExpectedProperties = {
...EXPECTED_PROPERTIES_BASE,
...expectedProperties,
};

const { result } = renderHookWithProvider(
() => useConfirmationAlertMetrics(),
STATE_MOCK,
);

act(() => {
result.current[action](alertKey);
});

expect(mockUpdateTransactionEventFragment).toHaveBeenCalledWith(
{ properties: finalExpectedProperties },
OWNER_ID_MOCK,
);
},
);
});
8 changes: 4 additions & 4 deletions ui/pages/confirmations/hooks/useCurrentConfirmation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ describe('useCurrentConfirmation', () => {
isRedesignedConfirmationsDeveloperEnabled: true,
});

expect(currentConfirmation).toStrictEqual(TRANSACTION_MOCK);
expect(currentConfirmation).toBeUndefined();
});

it('returns message matching ID param', () => {
Expand Down Expand Up @@ -266,10 +266,10 @@ describe('useCurrentConfirmation', () => {
isRedesignedConfirmationsDeveloperEnabled: true,
});

expect(currentConfirmation).toStrictEqual(TRANSACTION_MOCK);
expect(currentConfirmation).toBeUndefined();
});

it('returns if env var and user settings are enabled and transaction has correct type', () => {
it('returns undefined if env var and user settings are enabled and transaction type is not supported', () => {
const currentConfirmation = runHook({
pendingApprovals: [{ ...APPROVAL_MOCK, type: ApprovalType.Transaction }],
transaction: {
Expand All @@ -280,7 +280,7 @@ describe('useCurrentConfirmation', () => {
isRedesignedConfirmationsDeveloperEnabled: true,
});

expect(currentConfirmation).toStrictEqual(TRANSACTION_MOCK);
expect(currentConfirmation).toStrictEqual(undefined);
});

describe('useCurrentConfirmation with env var', () => {
Expand Down
2 changes: 1 addition & 1 deletion ui/pages/confirmations/utils/confirm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const REDESIGN_APPROVAL_TYPES = [
ApprovalType.PersonalSign,
];

export const REDESIGN_TRANSACTION_TYPES = [TransactionType.contractInteraction];
export const REDESIGN_TRANSACTION_TYPES: TransactionType[] = [];

const SIGNATURE_APPROVAL_TYPES = [
ApprovalType.PersonalSign,
Expand Down