1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-23 09:52:26 +01:00

Added navigation between multiple sign prompts and reject all sign prompts (#17093)

* Fixed navigation through multiple unapproved transactions for ERC20 tokens

* Fixed tx details activity-log currency

* Fixed e2e test failure

* Added navigation between multiple sign prompts and reject all sign prompts

* Resolving conflicts

* Creating SignatureRequestNavigation component and extracting the UI rendering part into a single component

* Fixing e2e tests and updating snapshot

* Using single component for navigation which shows both messages and transactions requests

* Fixing test-unit-jest-main

* Added more unit tests

* Fixing test-storybook

* Fixing test-storybook

---------

Co-authored-by: Filip Sekulic <filip.sekulic@consensys.net>
This commit is contained in:
Vladimir Saric 2023-01-31 16:29:23 +01:00 committed by GitHub
parent 6439d03075
commit 8aa3263b82
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 456 additions and 24 deletions

View File

@ -103,4 +103,4 @@
@import 'detected-token/detected-token-selection-popover/index';
@import 'network-account-balance-header/index';
@import 'approve-content-card/index';
@import 'transaction-alerts/transaction-alerts'
@import 'transaction-alerts/transaction-alerts';

View File

@ -2,14 +2,16 @@ import React, { useContext } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useHistory, useParams } from 'react-router-dom';
import {
getCurrentChainId,
getUnapprovedTransactions,
unconfirmedTransactionsHashSelector,
unapprovedDecryptMsgsSelector,
unapprovedEncryptionPublicKeyMsgsSelector,
} from '../../../../selectors';
import { transactionMatchesNetwork } from '../../../../../shared/modules/transaction.utils';
import { I18nContext } from '../../../../contexts/i18n';
import { CONFIRM_TRANSACTION_ROUTE } from '../../../../helpers/constants/routes';
import {
CONFIRM_TRANSACTION_ROUTE,
SIGNATURE_REQUEST_PATH,
} from '../../../../helpers/constants/routes';
import { clearConfirmTransaction } from '../../../../ducks/confirm-transaction/confirm-transaction.duck';
import { hexToDecimal } from '../../../../../shared/modules/conversion.utils';
const ConfirmPageContainerNavigation = () => {
const t = useContext(I18nContext);
@ -17,17 +19,26 @@ const ConfirmPageContainerNavigation = () => {
const history = useHistory();
const { id } = useParams();
const unapprovedTxs = useSelector(getUnapprovedTransactions);
const currentChainId = useSelector(getCurrentChainId);
const network = hexToDecimal(currentChainId);
const unapprovedDecryptMsgs = useSelector(unapprovedDecryptMsgsSelector);
const unapprovedEncryptionPublicKeyMsgs = useSelector(
unapprovedEncryptionPublicKeyMsgsSelector,
);
const uncofirmedTransactions = useSelector(
unconfirmedTransactionsHashSelector,
);
const currentNetworkUnapprovedTxs = Object.keys(unapprovedTxs)
.filter((key) =>
transactionMatchesNetwork(unapprovedTxs[key], currentChainId, network),
)
.reduce((acc, key) => ({ ...acc, [key]: unapprovedTxs[key] }), {});
const enumUnapprovedDecryptMsgsKey = Object.keys(unapprovedDecryptMsgs);
const enumUnapprovedEncryptMsgsKey = Object.keys(
unapprovedEncryptionPublicKeyMsgs,
);
const enumDecryptAndEncryptMsgs = [
...enumUnapprovedDecryptMsgsKey,
...enumUnapprovedEncryptMsgsKey,
];
const enumUnapprovedTxs = Object.keys(currentNetworkUnapprovedTxs);
const enumUnapprovedTxs = Object.keys(uncofirmedTransactions).filter(
(key) => enumDecryptAndEncryptMsgs.includes(key) === false,
);
const currentPosition = enumUnapprovedTxs.indexOf(id);
@ -42,7 +53,11 @@ const ConfirmPageContainerNavigation = () => {
const onNextTx = (txId) => {
if (txId) {
dispatch(clearConfirmTransaction());
history.push(`${CONFIRM_TRANSACTION_ROUTE}/${txId}`);
history.push(
uncofirmedTransactions[txId]?.msgParams
? `${CONFIRM_TRANSACTION_ROUTE}/${txId}${SIGNATURE_REQUEST_PATH}`
: `${CONFIRM_TRANSACTION_ROUTE}/${txId}`,
);
}
};

View File

@ -5,6 +5,76 @@ exports[`SignatureRequestOriginal should match snapshot 1`] = `
<div
class="request-signature__container"
>
<div
class="request-signature__navigation"
>
<div
class="confirm-page-container-navigation"
style="display: none;"
>
<div
class="confirm-page-container-navigation__container"
data-testid="navigation-container"
style="visibility: hidden;"
>
<button
class="confirm-page-container-navigation__arrow"
data-testid="first-page"
>
<i
class="fa fa-angle-double-left fa-2x"
/>
</button>
<button
class="confirm-page-container-navigation__arrow"
data-testid="previous-page"
>
<i
class="fa fa-angle-left fa-2x"
/>
</button>
</div>
<div
class="confirm-page-container-navigation__textcontainer"
>
<div
class="confirm-page-container-navigation__navtext"
>
0
of
0
</div>
<div
class="confirm-page-container-navigation__longtext"
>
requests waiting to be acknowledged
</div>
</div>
<div
class="confirm-page-container-navigation__container"
style="visibility: hidden;"
>
<button
class="confirm-page-container-navigation__arrow"
data-testid="next-page"
>
<i
class="fa fa-angle-right fa-2x"
/>
</button>
<button
class="confirm-page-container-navigation__arrow"
data-testid="last-page"
>
<i
class="fa fa-angle-double-right fa-2x"
/>
</button>
</div>
</div>
</div>
<div
class="request-signature__account"
>

View File

@ -20,7 +20,7 @@
}
@include screen-sm-min {
height: 620px;
height: 650px;
}
&__reject {
@ -56,7 +56,8 @@
z-index: 3;
}
&__account {
&__account,
&__navigation {
width: 100%;
}

View File

@ -21,6 +21,7 @@ import {
import { NETWORK_TYPES } from '../../../../shared/constants/network';
import { Numeric } from '../../../../shared/modules/Numeric';
import { EtherDenomination } from '../../../../shared/constants/common';
import ConfirmPageContainerNavigation from '../confirm-page-container/confirm-page-container-navigation';
import SignatureRequestOriginalWarning from './signature-request-original-warning';
export default class SignatureRequestOriginal extends Component {
@ -280,6 +281,9 @@ export default class SignatureRequestOriginal extends Component {
return (
<div className="request-signature__container">
<div className="request-signature__navigation">
<ConfirmPageContainerNavigation />
</div>
<div className="request-signature__account">
<NetworkAccountBalanceHeader
networkName={currentNetwork}

View File

@ -74,6 +74,12 @@ describe('SignatureRequestOriginal', () => {
expect(container).toMatchSnapshot();
});
it('should render navigation', () => {
render();
const navigationContainer = screen.queryByTestId('navigation-container');
expect(navigationContainer).toBeInTheDocument();
});
it('should render eth sign screen', () => {
render();
expect(screen.getByText('Signature request')).toBeInTheDocument();

View File

@ -5,6 +5,72 @@ exports[`Signature Request Component render should match snapshot 1`] = `
<div
class="signature-request"
>
<div
class="confirm-page-container-navigation"
style="display: none;"
>
<div
class="confirm-page-container-navigation__container"
data-testid="navigation-container"
style="visibility: hidden;"
>
<button
class="confirm-page-container-navigation__arrow"
data-testid="first-page"
>
<i
class="fa fa-angle-double-left fa-2x"
/>
</button>
<button
class="confirm-page-container-navigation__arrow"
data-testid="previous-page"
>
<i
class="fa fa-angle-left fa-2x"
/>
</button>
</div>
<div
class="confirm-page-container-navigation__textcontainer"
>
<div
class="confirm-page-container-navigation__navtext"
>
0
of
0
</div>
<div
class="confirm-page-container-navigation__longtext"
>
requests waiting to be acknowledged
</div>
</div>
<div
class="confirm-page-container-navigation__container"
style="visibility: hidden;"
>
<button
class="confirm-page-container-navigation__arrow"
data-testid="next-page"
>
<i
class="fa fa-angle-right fa-2x"
/>
</button>
<button
class="confirm-page-container-navigation__arrow"
data-testid="last-page"
>
<i
class="fa fa-angle-double-right fa-2x"
/>
</button>
</div>
</div>
<div
class="request-signature__account"
>

View File

@ -8,17 +8,22 @@
flex-direction: column;
min-width: 0;
width: 408px;
height: max-content;
background-color: var(--color-background-default);
box-shadow: var(--shadow-size-xs) var(--color-shadow-default);
border-radius: 8px;
@include screen-sm-min {
max-height: 55vh;
max-height: 80vh;
min-height: 570px;
flex: 0 0 auto;
margin-left: auto;
margin-right: auto;
}
&__reject-all-button {
margin-top: -15px;
}
}
.signature-request-header {

View File

@ -18,6 +18,7 @@ import NetworkAccountBalanceHeader from '../network-account-balance-header';
import { NETWORK_TYPES } from '../../../../shared/constants/network';
import { Numeric } from '../../../../shared/modules/Numeric';
import { EtherDenomination } from '../../../../shared/constants/common';
import ConfirmPageContainerNavigation from '../confirm-page-container/confirm-page-container-navigation';
import Footer from './signature-request-footer';
import Message from './signature-request-message';
@ -67,6 +68,12 @@ export default class SignatureRequest extends PureComponent {
nativeCurrency: PropTypes.string,
provider: PropTypes.object,
subjectMetadata: PropTypes.object,
unapprovedMessagesCount: PropTypes.number,
clearConfirmTransaction: PropTypes.func.isRequired,
history: PropTypes.object,
mostRecentOverviewPage: PropTypes.string,
showRejectTransactionsConfirmationModal: PropTypes.func.isRequired,
cancelAll: PropTypes.func.isRequired,
};
static contextTypes = {
@ -115,6 +122,26 @@ export default class SignatureRequest extends PureComponent {
return { sanitizedMessage, domain, primaryType };
});
handleCancelAll = () => {
const {
cancelAll,
clearConfirmTransaction,
history,
mostRecentOverviewPage,
showRejectTransactionsConfirmationModal,
unapprovedMessagesCount,
} = this.props;
showRejectTransactionsConfirmationModal({
unapprovedTxCount: unapprovedMessagesCount,
onSubmit: async () => {
await cancelAll();
clearConfirmTransaction();
history.push(mostRecentOverviewPage);
},
});
};
render() {
const {
txData: {
@ -133,13 +160,15 @@ export default class SignatureRequest extends PureComponent {
subjectMetadata,
conversionRate,
nativeCurrency,
unapprovedMessagesCount,
} = this.props;
const { trackEvent } = this.context;
const { t, trackEvent } = this.context;
const {
sanitizedMessage,
domain: { verifyingContract },
primaryType,
} = this.memoizedParseMessage(data);
const rejectNText = t('rejectRequestsN', [unapprovedMessagesCount]);
const currentNetwork = this.getNetworkName();
const balanceInBaseAsset = new Numeric(balance, 16, EtherDenomination.WEI)
@ -185,6 +214,7 @@ export default class SignatureRequest extends PureComponent {
return (
<div className="signature-request">
<ConfirmPageContainerNavigation />
<div className="request-signature__account">
<NetworkAccountBalanceHeader
networkName={currentNetwork}
@ -274,6 +304,19 @@ export default class SignatureRequest extends PureComponent {
isContractRequestingSignature
/>
)}
{unapprovedMessagesCount > 1 ? (
<Button
type="link"
className="signature-request__reject-all-button"
data-testid="signature-request-reject-all"
onClick={(e) => {
e.preventDefault();
this.handleCancelAll();
}}
>
{rejectNText}
</Button>
) : null}
</div>
);
}

View File

@ -1,4 +1,5 @@
import React from 'react';
import { fireEvent } from '@testing-library/react';
import configureMockStore from 'redux-mock-store';
import mockState from '../../../../test/data/mock-state.json';
import { renderWithProvider } from '../../../../test/lib/render-helpers';
@ -72,6 +73,10 @@ describe('Signature Request Component', () => {
hardwareWalletRequiresConnection={false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
cancelAll={() => undefined}
mostRecentOverviewPage="/"
showRejectTransactionsConfirmationModal={() => undefined}
history={{ push: '/' }}
sign={() => undefined}
txData={{
msgParams,
@ -85,6 +90,34 @@ describe('Signature Request Component', () => {
expect(container).toMatchSnapshot();
});
it('should render navigation', () => {
const msgParams = {
data: JSON.stringify(messageData),
version: 'V4',
origin: 'test',
};
const { queryByTestId } = renderWithProvider(
<SignatureRequest
hardwareWalletRequiresConnection={false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
cancelAll={() => undefined}
mostRecentOverviewPage="/"
showRejectTransactionsConfirmationModal={() => undefined}
history={{ push: '/' }}
sign={() => undefined}
txData={{
msgParams,
}}
fromAccount={{ address: fromAddress }}
provider={{ type: 'rpc' }}
/>,
store,
);
expect(queryByTestId('navigation-container')).toBeInTheDocument();
});
it('should render a div message parsed without typeless data', () => {
messageData.message.do_not_display = 'one';
messageData.message.do_not_display_2 = {
@ -100,6 +133,10 @@ describe('Signature Request Component', () => {
hardwareWalletRequiresConnection={false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
cancelAll={() => undefined}
mostRecentOverviewPage="/"
showRejectTransactionsConfirmationModal={() => undefined}
history={{ push: '/' }}
sign={() => undefined}
txData={{
msgParams,
@ -115,5 +152,128 @@ describe('Signature Request Component', () => {
expect(queryByText('do_not_display_2')).not.toBeInTheDocument();
expect(queryByText('two')).not.toBeInTheDocument();
});
it('should not render a reject multiple requests link if there is not multiple requests', () => {
const msgParams = {
data: JSON.stringify(messageData),
version: 'V4',
origin: 'test',
};
const { container } = renderWithProvider(
<SignatureRequest
hardwareWalletRequiresConnection={false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
cancelAll={() => undefined}
mostRecentOverviewPage="/"
showRejectTransactionsConfirmationModal={() => undefined}
history={{ push: '/' }}
sign={() => undefined}
txData={{
msgParams,
}}
fromAccount={{ address: fromAddress }}
provider={{ type: 'rpc' }}
/>,
store,
);
expect(
container.querySelector('.signature-request__reject-all-button'),
).not.toBeInTheDocument();
});
it('should render a reject multiple requests link if there is multiple requests (greater than 1)', () => {
const msgParams = {
data: JSON.stringify(messageData),
version: 'V4',
origin: 'test',
};
const { container } = renderWithProvider(
<SignatureRequest
hardwareWalletRequiresConnection={false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
cancelAll={() => undefined}
mostRecentOverviewPage="/"
showRejectTransactionsConfirmationModal={() => undefined}
history={{ push: '/' }}
sign={() => undefined}
txData={{
msgParams,
}}
fromAccount={{ address: fromAddress }}
provider={{ type: 'rpc' }}
unapprovedMessagesCount={2}
/>,
store,
);
expect(
container.querySelector('.signature-request__reject-all-button'),
).toBeInTheDocument();
});
it('should call reject all button when button is clicked', () => {
const msgParams = {
data: JSON.stringify(messageData),
version: 'V4',
origin: 'test',
};
const { container } = renderWithProvider(
<SignatureRequest
hardwareWalletRequiresConnection={false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
cancelAll={() => undefined}
mostRecentOverviewPage="/"
showRejectTransactionsConfirmationModal={() => undefined}
history={{ push: '/' }}
sign={() => undefined}
txData={{
msgParams,
}}
fromAccount={{ address: fromAddress }}
provider={{ type: 'rpc' }}
unapprovedMessagesCount={2}
/>,
store,
);
const rejectRequestsLink = container.querySelector(
'.signature-request__reject-all-button',
);
fireEvent.click(rejectRequestsLink);
expect(rejectRequestsLink).toBeDefined();
});
it('should render text of reject all button', () => {
const msgParams = {
data: JSON.stringify(messageData),
version: 'V4',
origin: 'test',
};
const { getByText } = renderWithProvider(
<SignatureRequest
hardwareWalletRequiresConnection={false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
cancelAll={() => undefined}
mostRecentOverviewPage="/"
showRejectTransactionsConfirmationModal={() => undefined}
history={{ push: '/' }}
sign={() => undefined}
txData={{
msgParams,
}}
fromAccount={{ address: fromAddress }}
provider={{ type: 'rpc' }}
unapprovedMessagesCount={2}
/>,
store,
);
expect(getByText('Reject 2 requests')).toBeInTheDocument();
});
});
});

View File

@ -6,13 +6,18 @@ import {
getRpcPrefsForCurrentProvider,
conversionRateSelector,
getSubjectMetadata,
unconfirmedMessagesHashSelector,
getTotalUnapprovedMessagesCount,
} from '../../../selectors';
import {
isAddressLedger,
getNativeCurrency,
} from '../../../ducks/metamask/metamask';
import { getAccountByAddress } from '../../../helpers/utils/util';
import { getAccountByAddress, valuesFor } from '../../../helpers/utils/util';
import { MESSAGE_TYPE } from '../../../../shared/constants/app';
import { cancelMsgs, showModal } from '../../../store/actions';
import { getMostRecentOverviewPage } from '../../../ducks/history/history';
import { clearConfirmTransaction } from '../../../ducks/confirm-transaction/confirm-transaction.duck';
import SignatureRequest from './signature-request.component';
function mapStateToProps(state, ownProps) {
@ -28,6 +33,8 @@ function mapStateToProps(state, ownProps) {
const chainId = getCurrentChainId(state);
const rpcPrefs = getRpcPrefsForCurrentProvider(state);
const subjectMetadata = getSubjectMetadata(state);
const unconfirmedMessagesList = unconfirmedMessagesHashSelector(state);
const unapprovedMessagesCount = getTotalUnapprovedMessagesCount(state);
const { iconUrl: siteImage = '' } =
subjectMetadata[txData.msgParams.origin] || {};
@ -39,6 +46,9 @@ function mapStateToProps(state, ownProps) {
chainId,
rpcPrefs,
siteImage,
unconfirmedMessagesList,
unapprovedMessagesCount,
mostRecentOverviewPage: getMostRecentOverviewPage(state),
conversionRate: conversionRateSelector(state),
nativeCurrency: getNativeCurrency(state),
subjectMetadata: getSubjectMetadata(state),
@ -47,6 +57,27 @@ function mapStateToProps(state, ownProps) {
};
}
function mapDispatchToProps(dispatch) {
return {
clearConfirmTransaction: () => dispatch(clearConfirmTransaction()),
showRejectTransactionsConfirmationModal: ({
onSubmit,
unapprovedTxCount: unapprovedMessagesCount,
}) => {
return dispatch(
showModal({
name: 'REJECT_TRANSACTIONS',
onSubmit,
unapprovedTxCount: unapprovedMessagesCount,
isRequestType: true,
}),
);
},
cancelAll: (unconfirmedMessagesList) =>
dispatch(cancelMsgs(unconfirmedMessagesList)),
};
}
function mergeProps(stateProps, dispatchProps, ownProps) {
const {
allAccounts,
@ -59,6 +90,9 @@ function mergeProps(stateProps, dispatchProps, ownProps) {
nativeCurrency,
provider,
subjectMetadata,
unconfirmedMessagesList,
unapprovedMessagesCount,
mostRecentOverviewPage,
} = stateProps;
const {
signPersonalMessage,
@ -70,6 +104,8 @@ function mergeProps(stateProps, dispatchProps, ownProps) {
txData,
} = ownProps;
const { cancelAll: dispatchCancelAll } = dispatchProps;
const {
type,
msgParams: { from },
@ -107,7 +143,14 @@ function mergeProps(stateProps, dispatchProps, ownProps) {
nativeCurrency,
provider,
subjectMetadata,
unapprovedMessagesCount,
mostRecentOverviewPage,
cancelAll: () => dispatchCancelAll(valuesFor(unconfirmedMessagesList)),
};
}
export default connect(mapStateToProps, null, mergeProps)(SignatureRequest);
export default connect(
mapStateToProps,
mapDispatchToProps,
mergeProps,
)(SignatureRequest);

View File

@ -58,6 +58,9 @@ describe('Signature Request', () => {
},
},
cachedBalances: {},
unapprovedDecryptMsgs: {},
unapprovedEncryptionPublicKeyMsgs: {},
uncofirmedTransactions: {},
selectedAddress: '0xd8f6a2ffb0fc5952d16c9768b71cfd35b6399aa5',
},
};
@ -71,12 +74,16 @@ describe('Signature Request', () => {
push: sinon.spy(),
},
hardwareWalletRequiresConnection: false,
mostRecentOverviewPage: '/',
clearConfirmTransaction: sinon.spy(),
cancelMessage: sinon.spy(),
cancel: sinon.stub().resolves(),
showRejectTransactionsConfirmationModal: sinon.stub().resolves(),
cancelAll: sinon.stub().resolves(),
provider: {
type: 'rpc',
},
unapprovedMessagesCount: 2,
sign: sinon.stub().resolves(),
txData: {
msgParams: {
@ -120,6 +127,14 @@ describe('Signature Request', () => {
expect(props.sign.calledOnce).toStrictEqual(true);
});
it('cancelAll', () => {
const cancelAll = screen.getByTestId('signature-request-reject-all');
fireEvent.click(cancelAll);
expect(props.cancelAll.calledOnce).toStrictEqual(false);
});
it('have user warning', () => {
const warningText = screen.getByText(
'Only sign this message if you fully understand the content and trust the requesting site.',

View File

@ -11,7 +11,10 @@ import {
getContractMethodData,
setDefaultHomeActiveTabName,
} from '../../store/actions';
import { unconfirmedTransactionsListSelector } from '../../selectors';
import {
unconfirmedTransactionsListSelector,
unconfirmedTransactionsHashSelector,
} from '../../selectors';
import { getMostRecentOverviewPage } from '../../ducks/history/history';
import { getSendTo } from '../../ducks/send';
import ConfirmTransaction from './confirm-transaction.component';
@ -27,9 +30,10 @@ const mapStateToProps = (state, ownProps) => {
const sendTo = getSendTo(state);
const unconfirmedTransactions = unconfirmedTransactionsListSelector(state);
const unconfirmedMessages = unconfirmedTransactionsHashSelector(state);
const totalUnconfirmed = unconfirmedTransactions.length;
const transaction = totalUnconfirmed
? unapprovedTxs[id] || unconfirmedTransactions[0]
? unapprovedTxs[id] || unconfirmedMessages[id] || unconfirmedTransactions[0]
: {};
const { id: transactionId, type } = transaction;