diff --git a/app/_locales/de/messages.json b/app/_locales/de/messages.json index 85e776155793..b160e822ff16 100644 --- a/app/_locales/de/messages.json +++ b/app/_locales/de/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Ich habe die Benachrichtigung zur Kenntnis genommen und möchte trotzdem fortfahren" }, - "confirmAlertModalDetails": { - "message": "Wenn Sie sich anmelden, könnten Dritte, die für Betrügereien bekannt sind, all Ihre Assets an sich reißen. Lesen Sie bitte die Benachrichtigungen, bevor Sie fortfahren." - }, - "confirmAlertModalTitle": { - "message": "Ihre Assets könnten gefährdet sein" - }, "confirmConnectCustodianRedirect": { "message": "Sobald Sie auf „Weiter“ klicken, leiten wir Sie weiter zu $1." }, diff --git a/app/_locales/el/messages.json b/app/_locales/el/messages.json index a8cee7a6aab1..66064c3eddf3 100644 --- a/app/_locales/el/messages.json +++ b/app/_locales/el/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Έχω ενημερωθεί για την ειδοποίηση και εξακολουθώ να θέλω να συνεχίσω" }, - "confirmAlertModalDetails": { - "message": "Εάν συνδεθείτε, ένας τρίτος που είναι γνωστός για απάτες μπορεί να αποκτήσει όλα τα περιουσιακά σας στοιχεία. Ελέγξτε τις ειδοποιήσεις πριν συνεχίσετε." - }, - "confirmAlertModalTitle": { - "message": "Τα περιουσιακά σας στοιχεία μπορεί να κινδυνεύουν" - }, "confirmConnectCustodianRedirect": { "message": "Θα σας ανακατευθύνουμε στο $1 όταν κάνετε κλικ για να συνεχίσετε." }, diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 119719b97090..f6e5c830e876 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1014,12 +1014,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "I have acknowledged the alert and still want to proceed" }, - "confirmAlertModalDetails": { - "message": "If you sign in, a third party known for scams might take all your assets. Please review the alerts before you proceed." - }, - "confirmAlertModalTitle": { - "message": "Your assets may be at risk" - }, "confirmConnectCustodianRedirect": { "message": "We will redirect you to $1 upon clicking continue." }, @@ -1077,6 +1071,12 @@ "confirmTitleTransaction": { "message": "Transaction request" }, + "confirmationAlertModalDetails": { + "message": "To protect your assets and login information, we suggest you reject the request." + }, + "confirmationAlertModalTitle": { + "message": "This request is suspicious" + }, "confirmed": { "message": "Confirmed" }, @@ -4474,6 +4474,9 @@ "revealTheSeedPhrase": { "message": "Reveal seed phrase" }, + "reviewAlert": { + "message": "Review alert" + }, "reviewAlerts": { "message": "Review alerts" }, diff --git a/app/_locales/en_GB/messages.json b/app/_locales/en_GB/messages.json index e67b9f0e4fcb..ccff2b96819d 100644 --- a/app/_locales/en_GB/messages.json +++ b/app/_locales/en_GB/messages.json @@ -997,12 +997,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "I have acknowledged the alert and still want to proceed" }, - "confirmAlertModalDetails": { - "message": "If you sign in, a third party known for scams might take all your assets. Please review the alerts before you proceed." - }, - "confirmAlertModalTitle": { - "message": "Your assets may be at risk" - }, "confirmConnectCustodianRedirect": { "message": "We will redirect you to $1 upon clicking continue." }, diff --git a/app/_locales/es/messages.json b/app/_locales/es/messages.json index 2346fda97d16..d76415fb6727 100644 --- a/app/_locales/es/messages.json +++ b/app/_locales/es/messages.json @@ -969,12 +969,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Soy consciente de la alerta y aun así deseo continuar" }, - "confirmAlertModalDetails": { - "message": "Si inicia sesión, un tercero conocido por estafas podría quedarse con todos sus activos. Revise las alertas antes de continuar." - }, - "confirmAlertModalTitle": { - "message": "Sus activos podrían estar en riesgo" - }, "confirmConnectCustodianRedirect": { "message": "Lo redirigiremos a $1 al hacer clic en continuar." }, diff --git a/app/_locales/fr/messages.json b/app/_locales/fr/messages.json index eff9261237fa..ab4b35446b84 100644 --- a/app/_locales/fr/messages.json +++ b/app/_locales/fr/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "J’ai pris connaissance de l’alerte, mais je souhaite quand même continuer" }, - "confirmAlertModalDetails": { - "message": "Si vous vous connectez, un tiers connu pour ses activités frauduleuses pourrait s’emparer de tous vos actifs. Veuillez examiner les alertes avant de continuer." - }, - "confirmAlertModalTitle": { - "message": "Vous risquez de perdre tout ou partie de vos actifs" - }, "confirmConnectCustodianRedirect": { "message": "Nous vous redirigerons vers $1 une fois que vous cliquerez sur « Continuer »." }, diff --git a/app/_locales/hi/messages.json b/app/_locales/hi/messages.json index a41bda8439cc..51b80d7420ce 100644 --- a/app/_locales/hi/messages.json +++ b/app/_locales/hi/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "मैंने एलर्ट को स्वीकार कर लिया है और इसके बावजूद आगे बढ़ना चाहता/चाहती हूं" }, - "confirmAlertModalDetails": { - "message": "यदि आप साइन इन करते हैं, तो स्कैम के लिए मशहूर कोई थर्ड पार्टी आपके सारे एसेट चुरा सकती है। कृपया आगे बढ़ने से पहले एलर्ट की समीक्षा करें।" - }, - "confirmAlertModalTitle": { - "message": "आपके एसेट खतरे में हो सकते हैं" - }, "confirmConnectCustodianRedirect": { "message": "'जारी रखें' पर क्लिक करने पर हम आपको $1 पर रीडायरेक्ट कर देंगे।" }, diff --git a/app/_locales/id/messages.json b/app/_locales/id/messages.json index b13602011b99..05f88b78d0bb 100644 --- a/app/_locales/id/messages.json +++ b/app/_locales/id/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Saya telah mengetahui peringatannya dan tetap ingin melanjutkan" }, - "confirmAlertModalDetails": { - "message": "Jika masuk, pihak ketiga yang terdeteksi melakukan penipuan dapat mengambil semua aset Anda. Tinjau peringatannya sebelum melanjutkan." - }, - "confirmAlertModalTitle": { - "message": "Aset Anda mungkin berisiko" - }, "confirmConnectCustodianRedirect": { "message": "Kami akan mengarahkan Anda ke $1 setelah mengeklik lanjutkan." }, diff --git a/app/_locales/ja/messages.json b/app/_locales/ja/messages.json index 8d006fef2257..e730f058911a 100644 --- a/app/_locales/ja/messages.json +++ b/app/_locales/ja/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "アラートを確認したうえで続行します" }, - "confirmAlertModalDetails": { - "message": "サインインすると、詐欺が判明しているサードパーティにすべての資産を奪われる可能性があります。続ける前にアラートを確認してください。" - }, - "confirmAlertModalTitle": { - "message": "資産が危険にさらされている可能性があります" - }, "confirmConnectCustodianRedirect": { "message": "「続行」をクリックすると、$1にリダイレクトされます。" }, diff --git a/app/_locales/ko/messages.json b/app/_locales/ko/messages.json index 666d99d90b00..12dbc92ae552 100644 --- a/app/_locales/ko/messages.json +++ b/app/_locales/ko/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "경고를 인지했으며, 계속 진행합니다" }, - "confirmAlertModalDetails": { - "message": "로그인하면 스캠을 목적으로 하는 제3자가 회원님의 자산을 모두 가져갈 수 있습니다. 계속하기 전에 경고를 검토하세요." - }, - "confirmAlertModalTitle": { - "message": "자산이 위험할 수 있습니다" - }, "confirmConnectCustodianRedirect": { "message": "계속을 클릭하면 $1(으)로 리디렉션됩니다." }, diff --git a/app/_locales/pt/messages.json b/app/_locales/pt/messages.json index a5de21cb0889..4c44c67d31d1 100644 --- a/app/_locales/pt/messages.json +++ b/app/_locales/pt/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Reconheço o alerta e quero prosseguir mesmo assim" }, - "confirmAlertModalDetails": { - "message": "Se você fizer login, um terceiro conhecido por aplicar golpes poderá se apropriar de todos os seus ativos. Confira os alertas antes de prosseguir." - }, - "confirmAlertModalTitle": { - "message": "Seus ativos podem estar em risco" - }, "confirmConnectCustodianRedirect": { "message": "Você será redirecionado para $1 ao clicar em continuar." }, diff --git a/app/_locales/ru/messages.json b/app/_locales/ru/messages.json index aba23b0b3284..f0cdb837d6a4 100644 --- a/app/_locales/ru/messages.json +++ b/app/_locales/ru/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Я подтвердил(-а) получение предупреждения и все еще хочу продолжить" }, - "confirmAlertModalDetails": { - "message": "Если вы войдете, третья сторона, которая, как известно, совершала мошеннические действия, может похитиь твсе ваши активы. Прежде чем продолжить, просмотрите оповещения." - }, - "confirmAlertModalTitle": { - "message": "Ваши активы могут быть в опасности" - }, "confirmConnectCustodianRedirect": { "message": "Мы перенаправим вас на $1 после нажатия кнопки «Продолжить»." }, diff --git a/app/_locales/tl/messages.json b/app/_locales/tl/messages.json index 9089ff123db1..03544a48369f 100644 --- a/app/_locales/tl/messages.json +++ b/app/_locales/tl/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Kinikilala ko ang mga alerto at nais ko pa rin magpatuloy" }, - "confirmAlertModalDetails": { - "message": "Kung mag-sign in ka, maaaring kunin ng third party na kilala sa mga panloloko ang lahat ng iyong mga asset. Mangyaring suriin ang mga alerto bago ka magpatuloy." - }, - "confirmAlertModalTitle": { - "message": "Maaaring nasa panganib ang iyong mga asset" - }, "confirmConnectCustodianRedirect": { "message": "Ire-redirect ka namin sa $1 sa pagpindot ng magpatuloy." }, diff --git a/app/_locales/tr/messages.json b/app/_locales/tr/messages.json index 7537fd19b1fe..8bebc1b4a104 100644 --- a/app/_locales/tr/messages.json +++ b/app/_locales/tr/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Uyarıyı kabul ediyor ve yine de ilerlemek istiyorum" }, - "confirmAlertModalDetails": { - "message": "Oturum açarsanız dolandırıcıklarla bilinen üçüncü bir taraf tüm varlıklarınızı ele geçirebilir. İlerlemeden önce lütfen uyarıları inceleyin." - }, - "confirmAlertModalTitle": { - "message": "Varlıklarınız risk altında olabilir" - }, "confirmConnectCustodianRedirect": { "message": "Devam et düğmesine tıkladığınızda sizi şuraya yönlendireceğiz: $1." }, diff --git a/app/_locales/vi/messages.json b/app/_locales/vi/messages.json index 81e8c008a46b..40c2bce428c8 100644 --- a/app/_locales/vi/messages.json +++ b/app/_locales/vi/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "Tôi đã hiểu rõ cảnh báo và vẫn muốn tiếp tục" }, - "confirmAlertModalDetails": { - "message": "Nếu bạn đăng nhập, một bên thứ ba nổi tiếng là lừa đảo có thể lấy tất cả tài sản của bạn. Vui lòng xem lại các cảnh báo trước khi tiếp tục." - }, - "confirmAlertModalTitle": { - "message": "Tài sản của bạn có thể gặp rủi ro" - }, "confirmConnectCustodianRedirect": { "message": "Chúng tôi sẽ chuyển hướng bạn đến $1 sau khi nhấn vào tiếp tục." }, diff --git a/app/_locales/zh_CN/messages.json b/app/_locales/zh_CN/messages.json index 5b556663ac31..ecf0553942c5 100644 --- a/app/_locales/zh_CN/messages.json +++ b/app/_locales/zh_CN/messages.json @@ -972,12 +972,6 @@ "confirmAlertModalAcknowledgeSingle": { "message": "我已知晓提醒并仍想继续" }, - "confirmAlertModalDetails": { - "message": "如果您登录，以欺诈闻名的第三方可能会拿走您的所有资产。在继续操作之前，请查看提醒。" - }, - "confirmAlertModalTitle": { - "message": "您的资产可能面临风险" - }, "confirmConnectCustodianRedirect": { "message": "点击“继续”后，我们会将您重定向到 $1。" }, diff --git a/test/e2e/tests/confirmations/alerts/insufficient-funds.spec.ts b/test/e2e/tests/confirmations/alerts/insufficient-funds.spec.ts index f8d70b8e6568..b473dabe8478 100644 --- a/test/e2e/tests/confirmations/alerts/insufficient-funds.spec.ts +++ b/test/e2e/tests/confirmations/alerts/insufficient-funds.spec.ts @@ -7,7 +7,6 @@ import { WINDOW_TITLES, } from '../../../helpers'; import { SMART_CONTRACTS } from '../../../seeder/smart-contracts'; -import { scrollAndConfirmAndAssertConfirm } from '../helpers'; import { TestSuiteArguments, openDAppWithContract, @@ -47,37 +46,19 @@ describe('Alert for insufficient funds @no-mmi', function () { await mintNft(driver); await verifyAlertForInsufficientBalance(driver); - - await scrollAndConfirmAndAssertConfirm(driver); - - await verifyConfirmationIsDisabled(driver); }, ); }); }); -async function verifyConfirmationIsDisabled(driver: Driver) { - const confirmButton = await driver.findElement( - '[data-testid="confirm-alert-modal-submit-button"]', - ); - assert.equal(await confirmButton.isEnabled(), false); -} - async function verifyAlertForInsufficientBalance(driver: Driver) { const alert = await driver.findElement('[data-testid="inline-alert"]'); assert.equal(await alert.getText(), 'Alert'); await driver.clickElementSafe('.confirm-scroll-to-bottom__button'); await driver.clickElement('[data-testid="inline-alert"]'); - const alertDescription = await driver.findElement( - '[data-testid="alert-modal__selected-alert"]', - ); - const alertDescriptionText = await alertDescription.getText(); - assert.equal( - alertDescriptionText, - 'You do not have enough ETH in your account to pay for transaction fees.', - ); - await driver.clickElement('[data-testid="alert-modal-close-button"]'); + await displayAlertForInsufficientBalance(driver); + await driver.clickElement('[data-testid="alert-modal-button"]'); } async function mintNft(driver: Driver) { @@ -87,3 +68,14 @@ async function mintNft(driver: Driver) { await driver.waitUntilXWindowHandles(3); await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); } + +async function displayAlertForInsufficientBalance(driver: Driver) { + const alertDescription = await driver.findElement( + '[data-testid="alert-modal__selected-alert"]', + ); + const alertDescriptionText = await alertDescription.getText(); + assert.equal( + alertDescriptionText, + 'You do not have enough ETH in your account to pay for transaction fees.', + ); +} diff --git a/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts b/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts index 7e4568d326f0..c332e9451005 100644 --- a/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts +++ b/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts @@ -95,6 +95,8 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this await scrollAndConfirmAndAssertConfirm(driver); + await acknowledgeAlert(driver); + await driver.clickElement( '[data-testid="confirm-alert-modal-cancel-button"]', ); @@ -118,8 +120,8 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this expectedProps: { alert_action_clicked: [], alert_key_clicked: [], - alert_resolved: [], - alert_resolved_count: 0, + alert_resolved: ['requestFrom'], + alert_resolved_count: 1, alert_triggered: ['requestFrom'], alert_triggered_count: 1, alert_visualized: ['requestFrom'], diff --git a/ui/components/app/alert-system/alert-modal/alert-modal.test.tsx b/ui/components/app/alert-system/alert-modal/alert-modal.test.tsx index 23d70b213edd..1c54dddbae55 100644 --- a/ui/components/app/alert-system/alert-modal/alert-modal.test.tsx +++ b/ui/components/app/alert-system/alert-modal/alert-modal.test.tsx @@ -147,11 +147,14 @@ describe('AlertModal', () => { it('sets the alert as confirmed when checkbox is called', () => { const setAlertConfirmedMock = jest.fn(); + const dangerAlertMock = alertsMock.find( + (alert) => alert.key === DATA_ALERT_KEY_MOCK, + ); const useAlertsSpy = jest.spyOn(useAlertsModule, 'default'); const newMockStore = configureMockStore([])({ ...STATE_MOCK, confirmAlerts: { - alerts: { [OWNER_ID_MOCK]: [alertsMock[1]] }, + alerts: { [OWNER_ID_MOCK]: [dangerAlertMock] }, confirmed: { [OWNER_ID_MOCK]: { [DATA_ALERT_KEY_MOCK]: false, @@ -162,10 +165,10 @@ describe('AlertModal', () => { (useAlertsSpy as jest.Mock).mockReturnValue({ setAlertConfirmed: setAlertConfirmedMock, - alerts: [alertsMock[1]], + alerts: [dangerAlertMock], generalAlerts: [], - fieldAlerts: [alertsMock[1]], - getFieldAlerts: () => [], + fieldAlerts: [dangerAlertMock], + getFieldAlerts: () => [dangerAlertMock], isAlertConfirmed: () => false, }); const { getByTestId } = renderWithProvider( @@ -233,11 +236,11 @@ describe('AlertModal', () => { ); expect(queryByTestId('alert-modal-acknowledge-checkbox')).toBeNull(); - expect(queryByTestId('alert-modal-button')).toBeNull(); + expect(queryByTestId('alert-modal-button')).toBeInTheDocument(); expect(getByText(ACTION_LABEL_MOCK)).toBeInTheDocument(); }); - it('renders acknowledge button and checkbox for non-blocking alerts', () => { + it('renders checkbox for non-blocking alerts', () => { const { getByTestId } = renderWithProvider( {t('gotIt')} @@ -313,7 +308,7 @@ export function AlertModal({ customDetails, customAcknowledgeCheckbox, customAcknowledgeButton, - enableProvider = true, + showCloseIcon = true, }: AlertModalProps) { const { isAlertConfirmed, setAlertConfirmed, alerts } = useAlerts(ownerId); const { trackAlertRender } = useAlertMetrics(); @@ -348,13 +343,14 @@ export function AlertModal({ @@ -373,13 +369,6 @@ export function AlertModal({ onCheckboxClick={handleCheckboxClick} /> )} - {enableProvider ? ( - - ) : null} { mockStore, ); - expect(getByText('Your assets may be at risk')).toBeInTheDocument(); + expect(getByText('This request is suspicious')).toBeInTheDocument(); }); it('disables submit button when confirm modal is not acknowledged', () => { @@ -101,41 +101,37 @@ describe('ConfirmAlertModal', () => { expect(onSubmitMock).toHaveBeenCalledTimes(1); }); - // todo: following 2 tests have been temporarily commented out - // we can un-comment as we add more alert providers - - // it('calls open multiple alert modal when review alerts link is clicked', () => { - // const { getByTestId } = renderWithProvider( - // , - // mockStore, - // ); - - // fireEvent.click(getByTestId('confirm-alert-modal-review-all-alerts')); - // expect(getByTestId('alert-modal-button')).toBeInTheDocument(); - // }); - - // describe('when there are multiple alerts', () => { - // it('renders the next alert when the "Got it" button is clicked', () => { - // const mockStoreAcknowledgeAlerts = configureMockStore([])({ - // ...STATE_MOCK, - // confirmAlerts: { - // alerts: { [OWNER_ID_MOCK]: alertsMock }, - // confirmed: { - // [OWNER_ID_MOCK]: { - // [FROM_ALERT_KEY_MOCK]: true, - // [DATA_ALERT_KEY_MOCK]: false, - // }, - // }, - // }, - // }); - // const { getByTestId, getByText } = renderWithProvider( - // , - // mockStoreAcknowledgeAlerts, - // ); - // fireEvent.click(getByTestId('confirm-alert-modal-review-all-alerts')); - // fireEvent.click(getByTestId('alert-modal-button')); - - // expect(getByText(DATA_ALERT_MESSAGE_MOCK)).toBeInTheDocument(); - // }); - // }); + it('calls open multiple alert modal when review alerts link is clicked', () => { + const { getByTestId } = renderWithProvider( + , + mockStore, + ); + + fireEvent.click(getByTestId('confirm-alert-modal-review-all-alerts')); + expect(getByTestId('alert-modal-button')).toBeInTheDocument(); + }); + + describe('when there are multiple alerts', () => { + it('renders the next alert when the "Got it" button is clicked', () => { + const mockStoreAcknowledgeAlerts = configureMockStore([])({ + ...STATE_MOCK, + confirmAlerts: { + alerts: { [OWNER_ID_MOCK]: alertsMock }, + confirmed: { + [OWNER_ID_MOCK]: { + [FROM_ALERT_KEY_MOCK]: true, + [DATA_ALERT_KEY_MOCK]: false, + }, + }, + }, + }); + const { getByTestId, getByText } = renderWithProvider( + , + mockStoreAcknowledgeAlerts, + ); + fireEvent.click(getByTestId('alert-modal-button')); + + expect(getByText(DATA_ALERT_MESSAGE_MOCK)).toBeInTheDocument(); + }); + }); }); diff --git a/ui/components/app/alert-system/confirm-alert-modal/confirm-alert-modal.tsx b/ui/components/app/alert-system/confirm-alert-modal/confirm-alert-modal.tsx index d46595e6b6be..8f5c04e3491c 100644 --- a/ui/components/app/alert-system/confirm-alert-modal/confirm-alert-modal.tsx +++ b/ui/components/app/alert-system/confirm-alert-modal/confirm-alert-modal.tsx @@ -1,6 +1,5 @@ import React, { useCallback, useState } from 'react'; -import { SecurityProvider } from '../../../../../shared/constants/security-provider'; import { Box, Button, @@ -15,6 +14,7 @@ import { } from '../../../component-library'; import { AlignItems, + Severity, TextAlign, TextVariant, } from '../../../../helpers/constants/design-system'; @@ -87,7 +87,7 @@ function ConfirmDetails({ <> - {t('confirmAlertModalDetails')} + {t('confirmationAlertModalDetails')} (false); - // if there are multiple alerts, show the multiple alert modal + const unconfirmedDangerAlerts = fieldAlerts.filter( + (alert) => + !isAlertConfirmed(alert.key) && alert.severity === Severity.Danger, + ); + + const hasDangerBlockingAlerts = fieldAlerts.some( + (alert) => alert.severity === Severity.Danger && alert.isBlocking, + ); + const hasUnconfirmedDangerAlerts = unconfirmedDangerAlerts.length > 0; + + // if there are unconfirmed danger alerts, show the multiple alert modal const [multipleAlertModalVisible, setMultipleAlertModalVisible] = - useState(unconfirmedDangerAlerts.length > 1); + useState(hasUnconfirmedDangerAlerts); const handleCloseMultipleAlertModal = useCallback( (request?: { recursive?: boolean }) => { setMultipleAlertModalVisible(false); - if (request?.recursive) { + if ( + request?.recursive || + hasUnconfirmedDangerAlerts || + hasDangerBlockingAlerts + ) { onClose(); } }, - [onClose], + [onClose, hasUnconfirmedDangerAlerts, hasDangerBlockingAlerts], ); const handleOpenMultipleAlertModal = useCallback(() => { @@ -155,6 +169,7 @@ export function ConfirmAlertModal({ ownerId={ownerId} onFinalAcknowledgeClick={handleCloseMultipleAlertModal} onClose={handleCloseMultipleAlertModal} + showCloseIcon={false} /> ); } @@ -171,13 +186,9 @@ export function ConfirmAlertModal({ onAcknowledgeClick={onClose} alertKey={selectedAlert.key} onClose={onClose} - customTitle={t('confirmAlertModalTitle')} + customTitle={t('confirmationAlertModalTitle')} customDetails={ - selectedAlert.provider === SecurityProvider.Blockaid ? ( - SecurityProvider.Blockaid - ) : ( - - ) + } customAcknowledgeCheckbox={ } - enableProvider={false} /> ); } diff --git a/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.test.tsx b/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.test.tsx index 0c3e810a5657..c4b79fb28b7c 100644 --- a/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.test.tsx +++ b/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.test.tsx @@ -70,6 +70,20 @@ describe('MultipleAlertModal', () => { onClose: onCloseMock, }; + const mockStoreAcknowledgeAlerts = configureMockStore([])({ + ...STATE_MOCK, + confirmAlerts: { + alerts: { [OWNER_ID_MOCK]: alertsMock }, + confirmed: { + [OWNER_ID_MOCK]: { + [FROM_ALERT_KEY_MOCK]: true, + [DATA_ALERT_KEY_MOCK]: true, + [CONTRACT_ALERT_KEY_MOCK]: false, + }, + }, + }, + }); + it('renders the multiple alert modal', () => { const { getByTestId } = renderWithProvider( , @@ -80,19 +94,6 @@ describe('MultipleAlertModal', () => { }); it('invokes the onFinalAcknowledgeClick when the button is clicked', () => { - const mockStoreAcknowledgeAlerts = configureMockStore([])({ - ...STATE_MOCK, - confirmAlerts: { - alerts: { [OWNER_ID_MOCK]: alertsMock }, - confirmed: { - [OWNER_ID_MOCK]: { - [FROM_ALERT_KEY_MOCK]: true, - [DATA_ALERT_KEY_MOCK]: true, - [CONTRACT_ALERT_KEY_MOCK]: true, - }, - }, - }, - }); const { getByTestId } = renderWithProvider( { }); it('render the next alert when the "Got it" button is clicked', () => { - const mockStoreAcknowledgeAlerts = configureMockStore([])({ - ...STATE_MOCK, - confirmAlerts: { - alerts: { [OWNER_ID_MOCK]: alertsMock }, - confirmed: { - [OWNER_ID_MOCK]: { - [FROM_ALERT_KEY_MOCK]: true, - [DATA_ALERT_KEY_MOCK]: true, - [CONTRACT_ALERT_KEY_MOCK]: false, - }, - }, - }, - }); const { getByTestId, getByText } = renderWithProvider( , mockStoreAcknowledgeAlerts, @@ -127,7 +115,23 @@ describe('MultipleAlertModal', () => { fireEvent.click(getByTestId('alert-modal-button')); - expect(getByText(alertsMock[2].message)).toBeInTheDocument(); + expect(getByText(alertsMock[1].message)).toBeInTheDocument(); + }); + + it('closes modal when the "Got it" button is clicked', () => { + onAcknowledgeClickMock.mockReset(); + const { getByTestId } = renderWithProvider( + , + mockStoreAcknowledgeAlerts, + ); + + fireEvent.click(getByTestId('alert-modal-button')); + + expect(onAcknowledgeClickMock).toHaveBeenCalledTimes(1); }); describe('Navigation', () => { @@ -139,11 +143,14 @@ describe('MultipleAlertModal', () => { fireEvent.click(getByTestId('alert-modal-next-button')); - expect(getByText(alertsMock[1].message)).toBeInTheDocument(); + expect(getByText(alertsMock[2].message)).toBeInTheDocument(); }); it('calls previous alert when the previous button is clicked', () => { - const selectSecondAlertMock = { ...defaultProps, alertKey: 'data' }; + const selectSecondAlertMock = { + ...defaultProps, + alertKey: CONTRACT_ALERT_KEY_MOCK, + }; const { getByTestId, getByText } = renderWithProvider( , mockStore, @@ -151,7 +158,7 @@ describe('MultipleAlertModal', () => { fireEvent.click(getByTestId('alert-modal-back-button')); - expect(getByText(alertsMock[0].message)).toBeInTheDocument(); + expect(getByText(alertsMock[1].message)).toBeInTheDocument(); }); }); }); diff --git a/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.tsx b/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.tsx index ae3e285efa00..d3b289343d00 100644 --- a/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.tsx +++ b/ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.tsx @@ -30,6 +30,10 @@ export type MultipleAlertModalProps = { onClose: (request?: { recursive?: boolean }) => void; /** The unique identifier of the entity that owns the alert. */ ownerId: string; + /** Whether to show the close icon in the modal header. */ + showCloseIcon?: boolean; + /** Whether to skip the unconfirmed alerts validation and close the modal directly. */ + skipAlertNavigation?: boolean; }; function PreviousButton({ @@ -145,8 +149,10 @@ export function MultipleAlertModal({ onClose, onFinalAcknowledgeClick, ownerId, + showCloseIcon = true, + skipAlertNavigation = false, }: MultipleAlertModalProps) { - const { isAlertConfirmed, alerts } = useAlerts(ownerId); + const { isAlertConfirmed, fieldAlerts: alerts } = useAlerts(ownerId); const initialAlertIndex = alerts.findIndex( (alert: Alert) => alert.key === alertKey, @@ -173,6 +179,11 @@ export function MultipleAlertModal({ }, []); const handleAcknowledgeClick = useCallback(() => { + if (skipAlertNavigation) { + onFinalAcknowledgeClick(); + return; + } + if (selectedIndex + 1 === alerts.length) { if (!hasUnconfirmedAlerts) { onFinalAcknowledgeClick(); @@ -189,6 +200,7 @@ export function MultipleAlertModal({ selectedIndex, alerts.length, hasUnconfirmedAlerts, + skipAlertNavigation, ]); return ( @@ -205,6 +217,7 @@ export function MultipleAlertModal({ selectedIndex={selectedIndex} /> } + showCloseIcon={showCloseIcon} /> ); } diff --git a/ui/components/app/confirm/info/row/alert-row/alert-row.tsx b/ui/components/app/confirm/info/row/alert-row/alert-row.tsx index 532bd5987c7c..3956cc3095eb 100644 --- a/ui/components/app/confirm/info/row/alert-row/alert-row.tsx +++ b/ui/components/app/confirm/info/row/alert-row/alert-row.tsx @@ -85,6 +85,8 @@ export const ConfirmInfoAlertRow = ({ ownerId={ownerId} onFinalAcknowledgeClick={handleModalClose} onClose={handleModalClose} + showCloseIcon={false} + skipAlertNavigation={true} /> )} diff --git a/ui/components/app/confirm/info/row/row.tsx b/ui/components/app/confirm/info/row/row.tsx index 0dd0b9b110b8..c61abbadac70 100644 --- a/ui/components/app/confirm/info/row/row.tsx +++ b/ui/components/app/confirm/info/row/row.tsx @@ -117,7 +117,7 @@ export const ConfirmInfoRow: React.FC = ({ {label} {labelChildren} - {tooltip && tooltip.length > 0 && ( + {!labelChildren && tooltip?.length && ( { const ownerId2Mock = '321'; const fromAlertKeyMock = 'from'; const dataAlertKeyMock = 'data'; + const toAlertKeyMock = 'to'; const alertsMock = [ { - key: fromAlertKeyMock, - field: fromAlertKeyMock, - severity: Severity.Danger as AlertSeverity, - message: 'Alert 1', + key: toAlertKeyMock, + field: toAlertKeyMock, + severity: Severity.Info as AlertSeverity, + message: 'Alert 3', }, { key: dataAlertKeyMock, severity: Severity.Warning as AlertSeverity, message: 'Alert 2', }, + { + key: fromAlertKeyMock, + field: fromAlertKeyMock, + severity: Severity.Danger as AlertSeverity, + message: 'Alert 1', + }, ]; const mockState = { confirmAlerts: { alerts: { [ownerIdMock]: alertsMock, [ownerId2Mock]: [alertsMock[0]] }, confirmed: { - [ownerIdMock]: { [fromAlertKeyMock]: true, [dataAlertKeyMock]: false }, + [ownerIdMock]: { + [fromAlertKeyMock]: true, + [dataAlertKeyMock]: false, + [toAlertKeyMock]: false, + }, [ownerId2Mock]: { [fromAlertKeyMock]: false }, }, }, @@ -54,6 +65,11 @@ describe('useAlerts', () => { expect(result.current.hasDangerAlerts).toEqual(true); expect(result.current.hasUnconfirmedDangerAlerts).toEqual(false); }); + + it('returns alerts ordered by severity', () => { + const orderedAlerts = result.current.alerts; + expect(orderedAlerts[0].severity).toEqual(Severity.Danger); + }); }); describe('unconfirmedDangerAlerts', () => { @@ -103,10 +119,10 @@ describe('useAlerts', () => { describe('fieldAlerts', () => { it('returns all alerts with field property', () => { - const expectedFieldAlerts = alertsMock.find( - (alert) => alert.field === fromAlertKeyMock, - ); - expect(result.current.fieldAlerts).toEqual([expectedFieldAlerts]); + expect(result.current.fieldAlerts).toEqual([ + alertsMock[0], + alertsMock[2], + ]); }); it('returns empty array if no alerts with field property', () => { diff --git a/ui/hooks/useAlerts.ts b/ui/hooks/useAlerts.ts index 92440822bbdd..e2d204fbdb8f 100644 --- a/ui/hooks/useAlerts.ts +++ b/ui/hooks/useAlerts.ts @@ -16,8 +16,8 @@ import { Severity } from '../helpers/constants/design-system'; const useAlerts = (ownerId: string) => { const dispatch = useDispatch(); - const alerts: Alert[] = useSelector((state) => - selectAlerts(state as AlertsState, ownerId), + const alerts: Alert[] = sortAlertsBySeverity( + useSelector((state) => selectAlerts(state as AlertsState, ownerId)), ); const confirmedAlertKeys = useSelector((state) => @@ -28,8 +28,8 @@ const useAlerts = (ownerId: string) => { selectGeneralAlerts(state as AlertsState, ownerId), ); - const fieldAlerts = useSelector((state) => - selectFieldAlerts(state as AlertsState, ownerId), + const fieldAlerts = sortAlertsBySeverity( + useSelector((state) => selectFieldAlerts(state as AlertsState, ownerId)), ); const getFieldAlerts = useCallback( @@ -82,4 +82,16 @@ const useAlerts = (ownerId: string) => { }; }; +function sortAlertsBySeverity(alerts: Alert[]): Alert[] { + const severityOrder = { + [Severity.Danger]: 3, + [Severity.Warning]: 2, + [Severity.Info]: 1, + }; + + return alerts.sort( + (a, b) => severityOrder[b.severity] - severityOrder[a.severity], + ); +} + export default useAlerts; diff --git a/ui/pages/confirmations/components/confirm/footer/footer.test.tsx b/ui/pages/confirmations/components/confirm/footer/footer.test.tsx index 2c735a508422..a98a0ce60f4c 100644 --- a/ui/pages/confirmations/components/confirm/footer/footer.test.tsx +++ b/ui/pages/confirmations/components/confirm/footer/footer.test.tsx @@ -27,6 +27,7 @@ import { Severity } from '../../../../../helpers/constants/design-system'; import { SignatureRequestType } from '../../../types/confirm'; import * as confirmContext from '../../../context/confirm'; +import { Alert } from '../../../../../ducks/confirm-alerts/confirm-alerts'; import Footer from './footer'; jest.mock('react-redux', () => ({ @@ -247,7 +248,8 @@ describe('ConfirmFooter', () => { const OWNER_ID_MOCK = '123'; const KEY_ALERT_KEY_MOCK = 'Key'; const ALERT_MESSAGE_MOCK = 'Alert 1'; - const alertsMock = [ + + const alertsMock: Alert[] = [ { key: KEY_ALERT_KEY_MOCK, field: KEY_ALERT_KEY_MOCK, @@ -257,30 +259,82 @@ describe('ConfirmFooter', () => { alertDetails: ['Detail 1', 'Detail 2'], }, ]; - const stateWithAlertsMock = getMockPersonalSignConfirmStateForRequest( - { - ...unapprovedPersonalSignMsg, - id: OWNER_ID_MOCK, - msgParams: { - from: '0xc42edfcc21ed14dda456aa0756c153f7985d8813', - }, - } as SignatureRequestType, - { - confirmAlerts: { - alerts: { [OWNER_ID_MOCK]: alertsMock }, - confirmed: { - [OWNER_ID_MOCK]: { [KEY_ALERT_KEY_MOCK]: false }, + + const createStateWithAlerts = ( + alerts: Alert[], + confirmed: Record, + ) => { + return getMockPersonalSignConfirmStateForRequest( + { + ...unapprovedPersonalSignMsg, + id: OWNER_ID_MOCK, + msgParams: { + from: '0xc42edfcc21ed14dda456aa0756c153f7985d8813', + }, + } as SignatureRequestType, + { + confirmAlerts: { + alerts: { [OWNER_ID_MOCK]: alerts }, + confirmed: { [OWNER_ID_MOCK]: confirmed }, }, + metamask: {}, }, - metamask: {}, - }, - ); - it('renders the review alerts button when there are unconfirmed alerts', () => { + ); + }; + + const stateWithAlertsMock = createStateWithAlerts(alertsMock, { + [KEY_ALERT_KEY_MOCK]: false, + }); + + it('renders the "review alerts" button when there are unconfirmed alerts', () => { + const stateWithMultipleDangerAlerts = createStateWithAlerts( + [ + alertsMock[0], + { + ...alertsMock[0], + key: 'From', + }, + ], + { [KEY_ALERT_KEY_MOCK]: false }, + ); + const { getByText } = render(stateWithMultipleDangerAlerts); + expect(getByText('Review alerts')).toBeInTheDocument(); + }); + + it('renders the "review alerts" button disabled when there are blocking alerts', () => { + const stateWithMultipleDangerAlerts = createStateWithAlerts( + [ + alertsMock[0], + { + ...alertsMock[0], + key: 'From', + isBlocking: true, + }, + ], + { [KEY_ALERT_KEY_MOCK]: false }, + ); + const { getByText } = render(stateWithMultipleDangerAlerts); + expect(getByText('Review alerts')).toBeInTheDocument(); + expect(getByText('Review alerts')).toBeDisabled(); + }); + + it('renders the "review alert" button when there are unconfirmed alerts', () => { const { getByText } = render(stateWithAlertsMock); + expect(getByText('Review alert')).toBeInTheDocument(); + }); + + it('renders the "confirm" button when there are confirmed danger alerts', () => { + const stateWithConfirmedDangerAlertMock = createStateWithAlerts( + alertsMock, + { + [KEY_ALERT_KEY_MOCK]: true, + }, + ); + const { getByText } = render(stateWithConfirmedDangerAlertMock); expect(getByText('Confirm')).toBeInTheDocument(); }); - it('renders the confirm button when there are no unconfirmed alerts', () => { + it('renders the "confirm" button when there are no alerts', () => { const { getByText } = render(); expect(getByText('Confirm')).toBeInTheDocument(); }); @@ -288,7 +342,7 @@ describe('ConfirmFooter', () => { it('sets the alert modal visible when the review alerts button is clicked', () => { const { getByTestId } = render(stateWithAlertsMock); fireEvent.click(getByTestId('confirm-footer-button')); - expect(getByTestId('confirm-alert-modal-submit-button')).toBeDefined(); + expect(getByTestId('alert-modal-button')).toBeDefined(); }); }); }); diff --git a/ui/pages/confirmations/components/confirm/footer/footer.tsx b/ui/pages/confirmations/components/confirm/footer/footer.tsx index b7951c299255..22404d8a2b4e 100644 --- a/ui/pages/confirmations/components/confirm/footer/footer.tsx +++ b/ui/pages/confirmations/components/confirm/footer/footer.tsx @@ -38,6 +38,8 @@ import { import { useConfirmContext } from '../../../context/confirm'; import { getConfirmationSender } from '../utils'; import { MetaMetricsEventLocation } from '../../../../../../shared/constants/metametrics'; +import { Alert } from '../../../../../ducks/confirm-alerts/confirm-alerts'; +import { Severity } from '../../../../../helpers/constants/design-system'; export type OnCancelHandler = ({ location, @@ -45,6 +47,37 @@ export type OnCancelHandler = ({ location: MetaMetricsEventLocation; }) => void; +function reviewAlertButtonText( + unconfirmedDangerAlerts: Alert[], + t: ReturnType, +) { + if (unconfirmedDangerAlerts.length === 1) { + return t('reviewAlert'); + } + + if (unconfirmedDangerAlerts.length > 1) { + return t('reviewAlerts'); + } + + return t('confirm'); +} + +function getButtonDisabledState( + hasUnconfirmedDangerAlerts: boolean, + hasBlockingAlerts: boolean, + disabled: boolean, +) { + if (hasBlockingAlerts) { + return true; + } + + if (hasUnconfirmedDangerAlerts) { + return false; + } + + return disabled; +} + const ConfirmButton = ({ alertOwnerId = '', disabled, @@ -61,8 +94,21 @@ const ConfirmButton = ({ const [confirmModalVisible, setConfirmModalVisible] = useState(false); - const { dangerAlerts, hasDangerAlerts, hasUnconfirmedDangerAlerts } = - useAlerts(alertOwnerId); + const { + hasDangerAlerts, + hasUnconfirmedDangerAlerts, + fieldAlerts, + isAlertConfirmed, + } = useAlerts(alertOwnerId); + + const unconfirmedDangerAlerts = fieldAlerts.filter( + (alert) => + !isAlertConfirmed(alert.key) && alert.severity === Severity.Danger, + ); + + const hasDangerBlockingAlerts = fieldAlerts.some( + (alert) => alert.severity === Severity.Danger && alert.isBlocking, + ); const handleCloseConfirmModal = useCallback(() => { setConfirmModalVisible(false); @@ -87,12 +133,20 @@ const ConfirmButton = ({ block danger data-testid="confirm-footer-button" - disabled={hasUnconfirmedDangerAlerts ? false : disabled} + disabled={getButtonDisabledState( + hasUnconfirmedDangerAlerts, + hasDangerBlockingAlerts, + disabled, + )} onClick={handleOpenConfirmModal} size={ButtonSize.Lg} - startIconName={IconName.Danger} + startIconName={ + unconfirmedDangerAlerts.length > 0 + ? IconName.SecuritySearch + : IconName.Danger + } > - {dangerAlerts?.length > 1 ? t('reviewAlerts') : t('confirm')} + {reviewAlertButtonText(unconfirmedDangerAlerts, t)} ) : (