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

feat: add alerts for redesigned contract interaction confirmation #25174

Merged
merged 55 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
327cfe7
Implement initial transaction alerts
matthewwalsh0 Jun 5, 2024
a215a7b
Add paymaster alert
matthewwalsh0 Jun 5, 2024
3b2a14a
Support transaction blockaid alerts
matthewwalsh0 Jun 5, 2024
be66b21
Add signing or submitting alert
matthewwalsh0 Jun 5, 2024
616fb54
Add gas too low alert
matthewwalsh0 Jun 5, 2024
8b30fcd
Add no gas price alert
matthewwalsh0 Jun 6, 2024
c97e962
Fix no gas price alert
matthewwalsh0 Jun 6, 2024
4dd53bf
Add unit tests for gas estimate failed alert
matthewwalsh0 Jun 6, 2024
1d4f81b
Update gas estimate alert tests to integration tests
matthewwalsh0 Jun 6, 2024
27b0e91
Add unit tests for gas fee low alert
matthewwalsh0 Jun 7, 2024
666871a
Add unit tests for gas too low alert
matthewwalsh0 Jun 7, 2024
23de612
Add unit tests for insufficient balance alert
matthewwalsh0 Jun 7, 2024
7c1ec03
Add unit tests for no gas price alert
matthewwalsh0 Jun 7, 2024
a8f9dd5
Add unit tests for paymaster alert
matthewwalsh0 Jun 7, 2024
37f6c86
Add unit tests for pending transaction alert
matthewwalsh0 Jun 7, 2024
161691b
Add unit tests for signing or submitting alert
matthewwalsh0 Jun 10, 2024
210d128
Ensure alerts validate specific transaction or type
matthewwalsh0 Jun 10, 2024
4cd607a
Move buy action to confirmation alert actions hook
matthewwalsh0 Jun 10, 2024
5d5b5ab
Use confirmation selector in blockaid hook
matthewwalsh0 Jun 10, 2024
ad2fce0
Add unit test for transaction blockaid alert
matthewwalsh0 Jun 10, 2024
1d925c9
Add unit test for buy action
matthewwalsh0 Jun 10, 2024
600d31b
Remove test row
matthewwalsh0 Jun 10, 2024
3d87ef3
Merge branch 'develop' into feat/redesign-transaction-alerts
matthewwalsh0 Jun 10, 2024
c9025ec
Fix linting
matthewwalsh0 Jun 10, 2024
ffbd54e
Fix linting
matthewwalsh0 Jun 10, 2024
27f9cc6
Fix unit tests
matthewwalsh0 Jun 10, 2024
abee921
Fix unit tests
matthewwalsh0 Jun 10, 2024
e78cf18
Merge branch 'develop' into feat/redesign-transaction-alerts
matthewwalsh0 Jun 10, 2024
8c0eeb5
Add field to paymaster alert
matthewwalsh0 Jun 11, 2024
09a1cc9
Merge branch 'feat/redesign-transaction-alerts' of github.com:MetaMas…
matthewwalsh0 Jun 11, 2024
05d51c3
Add translations
matthewwalsh0 Jun 11, 2024
13cec31
Merge branch 'develop' into feat/redesign-transaction-alerts
matthewwalsh0 Jun 11, 2024
6c54f9f
Simplify useCurrentConfirmation hook
matthewwalsh0 Jun 12, 2024
395e655
Extend unit tests for current confirmation hook
matthewwalsh0 Jun 12, 2024
6cc412a
Merge branch 'develop' into feat/redesign-transaction-alerts
matthewwalsh0 Jun 12, 2024
67c94f2
Fix no gas price alerts
matthewwalsh0 Jun 12, 2024
325991f
Add action to update gas
matthewwalsh0 Jun 12, 2024
cb23cfb
Fix unit tests
matthewwalsh0 Jun 13, 2024
19533e5
Add unit test for update gas alert action
matthewwalsh0 Jun 13, 2024
5278ff5
Rename full property to recursive
matthewwalsh0 Jun 13, 2024
e58702c
Remove unnecessary metadata selection
matthewwalsh0 Jun 13, 2024
9f43cbb
Add alert action to update gas fee
matthewwalsh0 Jun 13, 2024
dad1bfa
Add network busy alert
matthewwalsh0 Jun 13, 2024
f07eb63
Rename confirm alerts component
matthewwalsh0 Jun 13, 2024
04229ba
Fix unit tests and E2E tests
matthewwalsh0 Jun 14, 2024
500e1ba
Update handler names
matthewwalsh0 Jun 14, 2024
ff442cf
Remove paymaster alert
matthewwalsh0 Jun 20, 2024
2c44cba
Merge branch 'develop' into feat/redesign-transaction-alerts
matthewwalsh0 Jun 20, 2024
6729dd4
Fix linting
matthewwalsh0 Jun 20, 2024
0bd3d0c
Update translations
matthewwalsh0 Jun 20, 2024
956e494
Update actions
matthewwalsh0 Jun 20, 2024
0dde6d0
Update severity
matthewwalsh0 Jun 20, 2024
3441fe1
Remove unused translation
matthewwalsh0 Jun 20, 2024
675d90a
Merge branch 'develop' into feat/redesign-transaction-alerts
matthewwalsh0 Jun 21, 2024
e2138dc
Fix snapshot
matthewwalsh0 Jun 21, 2024
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
57 changes: 57 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ process.env.NOTIFICATIONS_SERVICE_URL =
'https://mock-test-notifications-api.metamask.io';
process.env.PUSH_NOTIFICATIONS_SERVICE_URL =
'https://mock-test-push-notifications-api.metamask.io';
process.env.ENABLE_CONFIRMATION_REDESIGN = 'true';
process.env.PORTFOLIO_URL = 'https://portfolio.test';
19 changes: 16 additions & 3 deletions test/lib/render-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,24 @@ export function renderWithProvider(component, store, pathname = '/') {
};
}

export function renderHookWithProvider(hook, state, pathname = '/') {
export function renderHookWithProvider(hook, state, pathname = '/', Container) {
const store = state ? configureStore(state) : undefined;
const { history, Wrapper } = createProviderWrapper(store, pathname);

const { history, Wrapper: ProviderWrapper } = createProviderWrapper(
store,
pathname,
);

const wrapper = Container
? ({ children }) => (
<ProviderWrapper>
<Container>{children}</Container>
</ProviderWrapper>
)
: ProviderWrapper;

return {
...renderHook(hook, { wrapper: Wrapper }),
...renderHook(hook, { wrapper }),
history,
};
}
Expand Down
36 changes: 29 additions & 7 deletions ui/components/app/alert-system/alert-modal/alert-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export type AlertModalProps = {
/**
* The function to be executed when the modal needs to be closed.
*/
onClose: () => void;
onClose: (request?: { recursive?: boolean }) => void;
/**
* The owner ID of the relevant alert from the `confirmAlerts` reducer.
*/
Expand Down Expand Up @@ -254,9 +254,24 @@ function AcknowledgeButton({
);
}

function ActionButton({ action }: { action?: { key: string; label: string } }) {
function ActionButton({
action,
onClose,
}: {
action?: { key: string; label: string };
onClose: (request: { recursive?: boolean } | void) => void;
}) {
const { processAction } = useAlertActionHandler();

const handleClick = useCallback(() => {
if (!action) {
return;
}

processAction(action.key);
onClose({ recursive: true });
}, [action, onClose, processAction]);

if (!action) {
return null;
}
Expand All @@ -269,7 +284,7 @@ function ActionButton({ action }: { action?: { key: string; label: string } }) {
variant={ButtonVariant.Primary}
width={BlockSize.Full}
size={ButtonSize.Lg}
onClick={() => processAction(key)}
onClick={handleClick}
>
{label}
</Button>
Expand All @@ -290,9 +305,12 @@ export function AlertModal({
}: AlertModalProps) {
const { isAlertConfirmed, setAlertConfirmed, alerts } = useAlerts(ownerId);

const handleClose = useCallback(() => {
onClose();
}, [onClose]);
const handleClose = useCallback(
(...args) => {
onClose(...args);
},
[onClose],
);

const selectedAlert = alerts.find((alert: Alert) => alert.key === alertKey);

Expand Down Expand Up @@ -358,7 +376,11 @@ export function AlertModal({
/>
{(selectedAlert.actions ?? []).map(
(action: { key: string; label: string }) => (
<ActionButton key={action.key} action={action} />
<ActionButton
key={action.key}
action={action}
onClose={handleClose}
/>
),
)}
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ describe('ConfirmAlertModal', () => {
const onCancelMock = jest.fn();
const onSubmitMock = jest.fn();
const alertsMock = [
{
key: DATA_ALERT_KEY_MOCK,
field: DATA_ALERT_KEY_MOCK,
severity: Severity.Danger,
message: DATA_ALERT_MESSAGE_MOCK,
},
{
key: FROM_ALERT_KEY_MOCK,
field: FROM_ALERT_KEY_MOCK,
Expand All @@ -25,12 +31,6 @@ describe('ConfirmAlertModal', () => {
reason: 'Reason 1',
alertDetails: ['Detail 1', 'Detail 2'],
},
{
key: DATA_ALERT_KEY_MOCK,
field: DATA_ALERT_KEY_MOCK,
severity: Severity.Danger,
message: DATA_ALERT_MESSAGE_MOCK,
},
];

const STATE_MOCK = {
Expand All @@ -44,11 +44,11 @@ describe('ConfirmAlertModal', () => {
},
},
};

const mockStore = configureMockStore([])(STATE_MOCK);

const defaultProps: ConfirmAlertModalProps = {
ownerId: OWNER_ID_MOCK,
alertKey: FROM_ALERT_KEY_MOCK,
onClose: onCloseMock,
onCancel: onCancelMock,
onSubmit: onSubmitMock,
Expand All @@ -65,7 +65,7 @@ describe('ConfirmAlertModal', () => {

it('disables submit button when confirm modal is not acknowledged', () => {
const { getByTestId } = renderWithProvider(
<ConfirmAlertModal {...defaultProps} alertKey={DATA_ALERT_KEY_MOCK} />,
<ConfirmAlertModal {...defaultProps} />,
mockStore,
);

Expand All @@ -84,7 +84,7 @@ describe('ConfirmAlertModal', () => {

it('calls onSubmit when the button is clicked', () => {
const { getByTestId } = renderWithProvider(
<ConfirmAlertModal {...defaultProps} alertKey={DATA_ALERT_KEY_MOCK} />,
<ConfirmAlertModal {...defaultProps} />,
mockStore,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import { AcknowledgeCheckboxBase } from '../alert-modal/alert-modal';
import { MultipleAlertModal } from '../multiple-alert-modal';

export type ConfirmAlertModalProps = {
/** The unique key representing the specific alert field. */
alertKey: string;
/** Callback function that is called when the cancel button is clicked. */
onCancel: () => void;
/** The function to be executed when the modal needs to be closed. */
Expand Down Expand Up @@ -112,7 +110,6 @@ function ConfirmDetails({
}

export function ConfirmAlertModal({
alertKey,
onCancel,
onClose,
onSubmit,
Expand All @@ -121,49 +118,56 @@ export function ConfirmAlertModal({
const t = useI18nContext();
const { alerts, unconfirmedDangerAlerts } = useAlerts(ownerId);

const selectedAlert = alerts.find((alert) => alert.key === alertKey);

const [confirmCheckbox, setConfirmCheckbox] = useState<boolean>(false);

// if there are multiple alerts, show the multiple alert modal
const [multipleAlertModalVisible, setMultipleAlertModalVisible] =
useState<boolean>(unconfirmedDangerAlerts.length > 1);

const handleCloseMultipleAlertModal = useCallback(() => {
setMultipleAlertModalVisible(false);
}, []);
const handleCloseMultipleAlertModal = useCallback(
(request?: { recursive?: boolean }) => {
setMultipleAlertModalVisible(false);

if (request?.recursive) {
onClose();
}
},
[onClose],
);

const handleOpenMultipleAlertModal = useCallback(() => {
setMultipleAlertModalVisible(true);
}, []);

const handleConfirmCheckbox = useCallback(() => {
setConfirmCheckbox(!confirmCheckbox);
}, [confirmCheckbox, selectedAlert]);

if (!selectedAlert) {
return null;
}
}, [confirmCheckbox]);

if (multipleAlertModalVisible) {
return (
<MultipleAlertModal
alertKey={alertKey}
ownerId={ownerId}
onFinalAcknowledgeClick={handleCloseMultipleAlertModal}
onClose={handleCloseMultipleAlertModal}
/>
);
}

const selectedAlert = alerts[0];

if (!selectedAlert) {
return null;
}

return (
<AlertModal
ownerId={ownerId}
onAcknowledgeClick={onClose}
alertKey={alertKey}
alertKey={selectedAlert.key}
onClose={onClose}
customTitle={t('confirmAlertModalTitle')}
customDetails={
selectedAlert?.provider === SecurityProvider.Blockaid ? (
selectedAlert.provider === SecurityProvider.Blockaid ? (
SecurityProvider.Blockaid
) : (
<ConfirmDetails onAlertLinkClick={handleOpenMultipleAlertModal} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ import { Alert } from '../../../../ducks/confirm-alerts/confirm-alerts';

export type MultipleAlertModalProps = {
/** The key of the initial alert to display. */
alertKey: string;
alertKey?: string;
/** The function to be executed when the button in the alert modal is clicked. */
onFinalAcknowledgeClick: () => void;
/** The function to be executed when the modal needs to be closed. */
onClose: () => void;
onClose: (request?: { recursive?: boolean }) => void;
/** The unique identifier of the entity that owns the alert. */
ownerId: string;
};
Expand Down Expand Up @@ -148,8 +148,12 @@ export function MultipleAlertModal({
}: MultipleAlertModalProps) {
const { isAlertConfirmed, alerts } = useAlerts(ownerId);

const initialAlertIndex = alerts.findIndex(
(alert: Alert) => alert.key === alertKey,
);

const [selectedIndex, setSelectedIndex] = useState(
alerts.findIndex((alert: Alert) => alert.key === alertKey),
initialAlertIndex === -1 ? 0 : initialAlertIndex,
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be we move this to hook useAlerts. Hook can be passed selectedAlertKey.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was simply to make it more readable, plus handle if the alert key isn't found instead of throwing.


const selectedAlert = alerts[selectedIndex];
Expand Down
15 changes: 9 additions & 6 deletions ui/components/app/confirm/info/row/alert-row/alert-row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ export const ConfirmInfoAlertRow = ({

const [alertModalVisible, setAlertModalVisible] = useState<boolean>(false);

const handleCloseModal = () => {
const handleModalClose = () => {
setAlertModalVisible(false);
};

const handleOpenModal = () => {
const handleInlineAlertClick = () => {
setAlertModalVisible(true);
};

Expand All @@ -66,18 +66,21 @@ export const ConfirmInfoAlertRow = ({

const inlineAlert = hasFieldAlert ? (
<Box marginLeft={1}>
<InlineAlert onClick={handleOpenModal} severity={selectedAlertSeverity} />
<InlineAlert
onClick={handleInlineAlertClick}
severity={selectedAlertSeverity}
/>
</Box>
) : null;

return (
<>
{alertModalVisible && (
<MultipleAlertModal
alertKey={alertKey}
alertKey={fieldAlerts[0].key}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am bit confused here, is it ok to hardcode key to always alert at index 0 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is safe, since the modal isn't visible unless the alert is clicked, which is only shown if there are field alerts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly renaming alertKeyinitialAlertKey in MultipleAlertModal could be helpful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that, but perhaps we could do that in a separate PR to limit the scope of this big one?

ownerId={ownerId}
onFinalAcknowledgeClick={handleCloseModal}
onClose={handleCloseModal}
onFinalAcknowledgeClick={handleModalClose}
onClose={handleModalClose}
/>
)}
<ConfirmInfoRow {...confirmInfoRowProps} labelChildren={inlineAlert} />
Expand Down
Loading