diff --git a/app/scripts/controllers/metametrics.js b/app/scripts/controllers/metametrics.js index 14b49dd7dad6..394a69e5800d 100644 --- a/app/scripts/controllers/metametrics.js +++ b/app/scripts/controllers/metametrics.js @@ -20,7 +20,7 @@ import { import { SECOND } from '../../../shared/constants/time'; import { isManifestV3 } from '../../../shared/modules/mv3.utils'; import { METAMETRICS_FINALIZE_EVENT_FRAGMENT_ALARM } from '../../../shared/constants/alarms'; -import { checkAlarmExists } from '../lib/util'; +import { checkAlarmExists, generateRandomId, isValidDate } from '../lib/util'; const EXTENSION_UNINSTALL_URL = 'https://metamask.io/uninstalled'; @@ -110,6 +110,7 @@ export default class MetaMetricsController { this.environment = environment; const abandonedFragments = omitBy(initState?.fragments, 'persist'); + const segmentApiCalls = initState?.segmentApiCalls || {}; this.store = new ObservableStore({ participateInMetaMetrics: null, @@ -120,6 +121,9 @@ export default class MetaMetricsController { fragments: { ...initState?.fragments, }, + segmentApiCalls: { + ...segmentApiCalls, + }, }); preferencesStore.subscribe(({ currentLocale }) => { @@ -142,6 +146,15 @@ export default class MetaMetricsController { this.finalizeEventFragment(fragment.id, { abandoned: true }); }); + // Code below submits any pending segmentApiCalls to Segment if/when the controller is re-instantiated + if (isManifestV3) { + Object.values(segmentApiCalls).forEach( + ({ eventType, payload, callback }) => { + this._submitSegmentAPICall(eventType, payload, callback); + }, + ); + } + // Close out event fragments that were created but not progressed. An // interval is used to routinely check if a fragment has not been updated // within the fragment's timeout window. When creating a new event fragment @@ -455,7 +468,7 @@ export default class MetaMetricsController { const { metaMetricsId } = this.state; const idTrait = metaMetricsId ? 'userId' : 'anonymousId'; const idValue = metaMetricsId ?? METAMETRICS_ANONYMOUS_ID; - this.segment.page({ + this._submitSegmentAPICall('page', { [idTrait]: idValue, name, properties: { @@ -808,7 +821,7 @@ export default class MetaMetricsController { } try { - this.segment.identify({ + this._submitSegmentAPICall('identify', { userId: metaMetricsId, traits: userTraits, }); @@ -937,10 +950,49 @@ export default class MetaMetricsController { return resolve(); }; - this.segment.track(payload, callback); + this._submitSegmentAPICall('track', payload, callback); if (flushImmediately) { this.segment.flush(); } }); } + + // Method below submits the request to analytics SDK. + // It will also add event to controller store + // and pass a callback to remove it from store once request is submitted to segment + // Saving segmentApiCalls in controller store in MV3 ensures that events are tracked + // even if service worker terminates before events are submiteed to segment. + _submitSegmentAPICall(eventType, payload, callback) { + const messageId = payload.messageId || generateRandomId(); + let timestamp = new Date(); + if (payload.timestamp) { + const payloadDate = new Date(payload.timestamp); + if (isValidDate(payloadDate)) { + timestamp = payloadDate; + } + } + const modifiedPayload = { ...payload, messageId, timestamp }; + this.store.updateState({ + segmentApiCalls: { + ...this.store.getState().segmentApiCalls, + [messageId]: { + eventType, + payload: { + ...modifiedPayload, + timestamp: modifiedPayload.timestamp.toString(), + }, + callback, + }, + }, + }); + const modifiedCallback = (result) => { + const { segmentApiCalls } = this.store.getState(); + delete segmentApiCalls[messageId]; + this.store.updateState({ + segmentApiCalls, + }); + return callback?.(result); + }; + this.segment[eventType](modifiedPayload, modifiedCallback); + } } diff --git a/app/scripts/controllers/metametrics.test.js b/app/scripts/controllers/metametrics.test.js index 1925da7a9141..6d41a77ea849 100644 --- a/app/scripts/controllers/metametrics.test.js +++ b/app/scripts/controllers/metametrics.test.js @@ -9,6 +9,7 @@ import { } from '../../../shared/constants/metametrics'; import waitUntilCalled from '../../../test/lib/wait-until-called'; import { CHAIN_IDS, CURRENCY_SYMBOLS } from '../../../shared/constants/network'; +import * as Utils from '../lib/util'; import MetaMetricsController from './metametrics'; import { NETWORK_EVENTS } from './network'; @@ -124,9 +125,10 @@ function getMetaMetricsController({ metaMetricsId = TEST_META_METRICS_ID, preferencesStore = getMockPreferencesStore(), networkController = getMockNetworkController(), + segmentInstance, } = {}) { return new MetaMetricsController({ - segment, + segment: segmentInstance || segment, getNetworkIdentifier: networkController.getNetworkIdentifier.bind(networkController), getCurrentChainId: @@ -145,10 +147,17 @@ function getMetaMetricsController({ testid: SAMPLE_PERSISTED_EVENT, testid2: SAMPLE_NON_PERSISTED_EVENT, }, + events: {}, }, }); } describe('MetaMetricsController', function () { + const now = new Date(); + let clock; + beforeEach(function () { + clock = sinon.useFakeTimers(now.getTime()); + sinon.stub(Utils, 'generateRandomId').returns('DUMMY_RANDOM_ID'); + }); describe('constructor', function () { it('should properly initialize', function () { const mock = sinon.mock(segment); @@ -163,6 +172,8 @@ describe('MetaMetricsController', function () { ...DEFAULT_EVENT_PROPERTIES, test: true, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); const metaMetricsController = getMetaMetricsController(); assert.strictEqual(metaMetricsController.version, VERSION); @@ -233,15 +244,18 @@ describe('MetaMetricsController', function () { }); const mock = sinon.mock(segment); - mock - .expects('identify') - .once() - .withArgs({ userId: TEST_META_METRICS_ID, traits: MOCK_TRAITS }); + mock.expects('identify').once().withArgs({ + userId: TEST_META_METRICS_ID, + traits: MOCK_TRAITS, + messageId: Utils.generateRandomId(), + timestamp: new Date(), + }); metaMetricsController.identify({ ...MOCK_TRAITS, ...MOCK_INVALID_TRAITS, }); + mock.verify(); }); @@ -263,6 +277,8 @@ describe('MetaMetricsController', function () { traits: { test_date: mockDateISOString, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); metaMetricsController.identify({ @@ -358,6 +374,8 @@ describe('MetaMetricsController', function () { test: 1, ...DEFAULT_EVENT_PROPERTIES, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); metaMetricsController.submitEvent( { @@ -388,6 +406,8 @@ describe('MetaMetricsController', function () { test: 1, ...DEFAULT_EVENT_PROPERTIES, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); metaMetricsController.submitEvent( { @@ -417,6 +437,8 @@ describe('MetaMetricsController', function () { legacy_event: true, ...DEFAULT_EVENT_PROPERTIES, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); metaMetricsController.submitEvent( { @@ -439,12 +461,14 @@ describe('MetaMetricsController', function () { .once() .withArgs({ event: 'Fake Event', - userId: TEST_META_METRICS_ID, - context: DEFAULT_TEST_CONTEXT, properties: { test: 1, ...DEFAULT_EVENT_PROPERTIES, }, + context: DEFAULT_TEST_CONTEXT, + userId: TEST_META_METRICS_ID, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); metaMetricsController.submitEvent({ event: 'Fake Event', @@ -519,6 +543,8 @@ describe('MetaMetricsController', function () { foo: 'bar', ...DEFAULT_EVENT_PROPERTIES, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }), ); assert.ok( @@ -527,6 +553,8 @@ describe('MetaMetricsController', function () { userId: TEST_META_METRICS_ID, context: DEFAULT_TEST_CONTEXT, properties: DEFAULT_EVENT_PROPERTIES, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }), ); }); @@ -547,6 +575,8 @@ describe('MetaMetricsController', function () { params: null, ...DEFAULT_PAGE_PROPERTIES, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); metaMetricsController.trackPage({ name: 'home', @@ -590,6 +620,8 @@ describe('MetaMetricsController', function () { params: null, ...DEFAULT_PAGE_PROPERTIES, }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), }); metaMetricsController.trackPage( { @@ -788,9 +820,35 @@ describe('MetaMetricsController', function () { }); }); + describe('submitting segmentApiCalls to segment SDK', function () { + it('should add event to store when submitting to SDK', function () { + const metaMetricsController = getMetaMetricsController({}); + metaMetricsController.trackPage({}, { isOptIn: true }); + const { segmentApiCalls } = metaMetricsController.store.getState(); + assert(Object.keys(segmentApiCalls).length > 0); + }); + + it('should remove event from store when callback is invoked', function () { + const segmentInstance = createSegmentMock(2, 10000); + const stubFn = (_, cb) => { + cb(); + }; + sinon.stub(segmentInstance, 'track').callsFake(stubFn); + sinon.stub(segmentInstance, 'page').callsFake(stubFn); + + const metaMetricsController = getMetaMetricsController({ + segmentInstance, + }); + metaMetricsController.trackPage({}, { isOptIn: true }); + const { segmentApiCalls } = metaMetricsController.store.getState(); + assert(Object.keys(segmentApiCalls).length === 0); + }); + }); + afterEach(function () { // flush the queues manually after each test segment.flush(); + clock.restore(); sinon.restore(); }); }); diff --git a/app/scripts/lib/segment/analytics.js b/app/scripts/lib/segment/analytics.js index c93275f9408f..8966c8eebf7c 100644 --- a/app/scripts/lib/segment/analytics.js +++ b/app/scripts/lib/segment/analytics.js @@ -2,21 +2,10 @@ import removeSlash from 'remove-trailing-slash'; import looselyValidate from '@segment/loosely-validate-event'; import { isString } from 'lodash'; import isRetryAllowed from 'is-retry-allowed'; +import { generateRandomId } from '../util'; const noop = () => ({}); -// Taken from https://stackoverflow.com/a/1349426/3696652 -const characters = - 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; -const generateRandomId = () => { - let result = ''; - const charactersLength = characters.length; - for (let i = 0; i < 20; i++) { - result += characters.charAt(Math.floor(Math.random() * charactersLength)); - } - return result; -}; - // Method below is inspired from axios-retry https://github.com/softonic/axios-retry function isNetworkError(error) { return ( diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js index 6539e1f23f84..c3f761a6ca35 100644 --- a/app/scripts/lib/util.js +++ b/app/scripts/lib/util.js @@ -174,3 +174,19 @@ export { getChainType, checkAlarmExists, }; + +// Taken from https://stackoverflow.com/a/1349426/3696652 +const characters = + 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; +export const generateRandomId = () => { + let result = ''; + const charactersLength = characters.length; + for (let i = 0; i < 20; i++) { + result += characters.charAt(Math.floor(Math.random() * charactersLength)); + } + return result; +}; + +export const isValidDate = (d) => { + return d instanceof Date && !isNaN(d); +}; diff --git a/test/e2e/tests/send-eth.spec.js b/test/e2e/tests/send-eth.spec.js index 0833a1f8594e..a0c254dcd75f 100644 --- a/test/e2e/tests/send-eth.spec.js +++ b/test/e2e/tests/send-eth.spec.js @@ -135,7 +135,7 @@ describe('Send ETH non-contract address with data that matches ERC20 transfer da await driver.clickElement({ text: 'Next', tag: 'button' }); - await driver.clickElement({ text: '0xc42...cd28' }); + await driver.clickElement({ text: 'New contract' }); const recipientAddress = await driver.findElements({ text: '0xc427D562164062a23a5cFf596A4a3208e72Acd28', diff --git a/test/e2e/tests/send-hex-address.spec.js b/test/e2e/tests/send-hex-address.spec.js index ade462eb3aa6..315b5565d6be 100644 --- a/test/e2e/tests/send-hex-address.spec.js +++ b/test/e2e/tests/send-hex-address.spec.js @@ -57,7 +57,7 @@ describe('Send ETH to a 40 character hexadecimal address', function () { ); await sendTransactionListItem.click(); await driver.clickElement({ text: 'Activity log', tag: 'summary' }); - await driver.clickElement('.sender-to-recipient__name:nth-of-type(2)'); + await driver.clickElement('[data-testid="sender-to-recipient__name"]'); // Verify address in activity log const publicAddress = await driver.findElement( @@ -108,7 +108,7 @@ describe('Send ETH to a 40 character hexadecimal address', function () { ); await sendTransactionListItem.click(); await driver.clickElement({ text: 'Activity log', tag: 'summary' }); - await driver.clickElement('.sender-to-recipient__name:nth-of-type(2)'); + await driver.clickElement('[data-testid="sender-to-recipient__name"]'); // Verify address in activity log const publicAddress = await driver.findElement( @@ -212,7 +212,7 @@ describe('Send ERC20 to a 40 character hexadecimal address', function () { ); await sendTransactionListItem.click(); await driver.clickElement({ text: 'Activity log', tag: 'summary' }); - await driver.clickElement('.sender-to-recipient__name:nth-of-type(2)'); + await driver.clickElement('[data-testid="sender-to-recipient__name"]'); // Verify address in activity log const publicAddress = await driver.findElement( @@ -302,7 +302,7 @@ describe('Send ERC20 to a 40 character hexadecimal address', function () { ); await sendTransactionListItem.click(); await driver.clickElement({ text: 'Activity log', tag: 'summary' }); - await driver.clickElement('.sender-to-recipient__name:nth-of-type(2)'); + await driver.clickElement('[data-testid="sender-to-recipient__name"]'); // Verify address in activity log const publicAddress = await driver.findElement( diff --git a/ui/components/app/confirm-page-container/confirm-page-container-container.test.js b/ui/components/app/confirm-page-container/confirm-page-container-container.test.js index 2256f6e9c681..7964f06455b3 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container-container.test.js +++ b/ui/components/app/confirm-page-container/confirm-page-container-container.test.js @@ -92,7 +92,7 @@ describe('Confirm Page Container Container Test', () => { expect(senderRecipient).toBeInTheDocument(); }); it('should render recipient as address', () => { - const recipientName = screen.queryByText(shortenAddress(props.toAddress)); + const recipientName = screen.queryByText('New contract'); expect(recipientName).toBeInTheDocument(); }); @@ -118,7 +118,7 @@ describe('Confirm Page Container Container Test', () => { describe('Contact/AddressBook name should appear in recipient header', () => { it('should not show add to address dialog if recipient is in contact list and should display contact name', () => { - const addressBookName = 'test save name'; + const addressBookName = 'New contract'; const addressBook = { '0x5': { diff --git a/ui/components/app/confirm-page-container/confirm-page-container.component.js b/ui/components/app/confirm-page-container/confirm-page-container.component.js index a972aee17e78..70a3335f33e2 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container.component.js +++ b/ui/components/app/confirm-page-container/confirm-page-container.component.js @@ -63,6 +63,7 @@ export default class ConfirmPageContainer extends Component { fromName: PropTypes.string, toAddress: PropTypes.string, toName: PropTypes.string, + toMetadataName: PropTypes.string, toEns: PropTypes.string, toNickname: PropTypes.string, // Content @@ -118,6 +119,7 @@ export default class ConfirmPageContainer extends Component { fromName, fromAddress, toName, + toMetadataName, toEns, toNickname, toAddress, @@ -231,6 +233,7 @@ export default class ConfirmPageContainer extends Component { senderName={fromName} senderAddress={fromAddress} recipientName={toName} + recipientMetadataName={toMetadataName} recipientAddress={toAddress} recipientEns={toEns} recipientNickname={toNickname} diff --git a/ui/components/app/confirm-page-container/confirm-page-container.container.js b/ui/components/app/confirm-page-container/confirm-page-container.container.js index 567192cb1645..6323c32cf877 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container.container.js +++ b/ui/components/app/confirm-page-container/confirm-page-container.container.js @@ -5,6 +5,8 @@ import { getIsBuyableChain, getNetworkIdentifier, getSwapsDefaultToken, + getMetadataContractName, + getAccountName, } from '../../../selectors'; import { showModal } from '../../../store/actions'; import ConfirmPageContainer from './confirm-page-container.component'; @@ -16,11 +18,14 @@ function mapStateToProps(state, ownProps) { const networkIdentifier = getNetworkIdentifier(state); const defaultToken = getSwapsDefaultToken(state); const accountBalance = defaultToken.string; + const toName = getAccountName(state, to); + const toMetadataName = getMetadataContractName(to); return { isBuyableChain, contact, - toName: contact?.name || ownProps.toName, + toName, + toMetadataName, isOwnedAccount: getAccountsWithLabels(state) .map((accountWithLabel) => accountWithLabel.address) .includes(to), diff --git a/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js b/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js index 92592a5b0d22..7f80e2db791d 100644 --- a/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js +++ b/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js @@ -41,6 +41,8 @@ export default class TransactionListItemDetails extends PureComponent { onClose: PropTypes.func.isRequired, recipientEns: PropTypes.string, recipientAddress: PropTypes.string, + recipientName: PropTypes.string, + recipientMetadataName: PropTypes.string, rpcPrefs: PropTypes.object, senderAddress: PropTypes.string.isRequired, tryReverseResolveAddress: PropTypes.func.isRequired, @@ -139,6 +141,8 @@ export default class TransactionListItemDetails extends PureComponent { showRetry, recipientEns, recipientAddress, + recipientName, + recipientMetadataName, senderAddress, isEarliestNonce, senderNickname, @@ -238,6 +242,8 @@ export default class TransactionListItemDetails extends PureComponent { recipientEns={recipientEns} recipientAddress={recipientAddress} recipientNickname={recipientNickname} + recipientName={recipientName} + recipientMetadataName={recipientMetadataName} senderName={senderNickname} senderAddress={senderAddress} onRecipientClick={() => { diff --git a/ui/components/app/transaction-list-item-details/transaction-list-item-details.container.js b/ui/components/app/transaction-list-item-details/transaction-list-item-details.container.js index 61731c09218c..6d8e29756229 100644 --- a/ui/components/app/transaction-list-item-details/transaction-list-item-details.container.js +++ b/ui/components/app/transaction-list-item-details/transaction-list-item-details.container.js @@ -8,6 +8,8 @@ import { getIsCustomNetwork, getRpcPrefsForCurrentProvider, getEnsResolutionByAddress, + getAccountName, + getMetadataContractName, } from '../../../selectors'; import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; import TransactionListItemDetails from './transaction-list-item-details.component'; @@ -20,6 +22,8 @@ const mapStateToProps = (state, ownProps) => { recipientEns = getEnsResolutionByAddress(state, address); } const addressBook = getAddressBook(state); + const recipientName = getAccountName(state, recipientAddress); + const recipientMetadataName = getMetadataContractName(recipientAddress); const getNickName = (address) => { const entry = addressBook.find((contact) => { @@ -38,6 +42,8 @@ const mapStateToProps = (state, ownProps) => { recipientNickname: recipientAddress ? getNickName(recipientAddress) : null, isCustomNetwork, blockExplorerLinkText: getBlockExplorerLinkText(state), + recipientName, + recipientMetadataName, }; }; diff --git a/ui/components/component-library/help-text/help-text.js b/ui/components/component-library/help-text/help-text.js index c3cc34e5b992..2eeb66841705 100644 --- a/ui/components/component-library/help-text/help-text.js +++ b/ui/components/component-library/help-text/help-text.js @@ -38,7 +38,7 @@ HelpText.propTypes = { * The color of the HelpText will be overridden if error is true * Defaults to COLORS.TEXT_DEFAULT */ - color: PropTypes.oneOf(Object.values[TEXT_COLORS]), + color: PropTypes.oneOf(Object.values(TEXT_COLORS)), /** * The content of the help-text */ diff --git a/ui/components/ui/sender-to-recipient/sender-to-recipient.component.js b/ui/components/ui/sender-to-recipient/sender-to-recipient.component.js index 417b36e84a6a..6aec343bec93 100644 --- a/ui/components/ui/sender-to-recipient/sender-to-recipient.component.js +++ b/ui/components/ui/sender-to-recipient/sender-to-recipient.component.js @@ -103,34 +103,69 @@ export function RecipientWithAddress({ recipientNickname, recipientEns, recipientName, + recipientMetadataName, }) { const t = useI18nContext(); const [showNicknamePopovers, setShowNicknamePopovers] = useState(false); + const [addressCopied, setAddressCopied] = useState(false); + + let tooltipHtml =

{t('copiedExclamation')}

; + if (!addressCopied) { + tooltipHtml = addressOnly ? ( +

{t('copyAddress')}

+ ) : ( +

+ {shortenAddress(checksummedRecipientAddress)} +
+ {t('copyAddress')} +

+ ); + } return ( <>
{ - setShowNicknamePopovers(true); - if (onRecipientClick) { - onRecipientClick(); + if (recipientName) { + setAddressCopied(true); + copyToClipboard(checksummedRecipientAddress); + } else { + setShowNicknamePopovers(true); + if (onRecipientClick) { + onRecipientClick(); + } } }} >
-
- {addressOnly - ? recipientNickname || - recipientEns || - shortenAddress(checksummedRecipientAddress) - : recipientNickname || - recipientEns || - recipientName || - t('newContract')} -
+ setAddressCopied(false)} + > +
+ {addressOnly + ? recipientName || + recipientNickname || + recipientMetadataName || + recipientEns || + shortenAddress(checksummedRecipientAddress) + : recipientName || + recipientNickname || + recipientMetadataName || + recipientEns || + t('newContract')} +
+
{showNicknamePopovers ? ( ) : (
@@ -228,6 +266,7 @@ SenderToRecipient.propTypes = { senderName: PropTypes.string, senderAddress: PropTypes.string, recipientName: PropTypes.string, + recipientMetadataName: PropTypes.string, recipientEns: PropTypes.string, recipientAddress: PropTypes.string, recipientNickname: PropTypes.string, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 614dadc82524..42fda351a8e0 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -412,6 +412,20 @@ export function getAddressBookEntryOrAccountName(state, address) { return entry && entry.name !== '' ? entry.name : address; } +export function getAccountName(state, address) { + const entry = Object.values(state.metamask.identities).find((identity) => + isEqualCaseInsensitive(identity.address, toChecksumHexAddress(address)), + ); + return entry && entry.name !== '' ? entry.name : ''; +} + +export function getMetadataContractName(address) { + const entry = Object.values(STATIC_MAINNET_TOKEN_LIST).find((identity) => + isEqualCaseInsensitive(identity.address, toChecksumHexAddress(address)), + ); + return entry && entry.name !== '' ? entry.name : ''; +} + export function accountsWithSendEtherInfoSelector(state) { const accounts = getMetaMaskAccounts(state); const identities = getMetaMaskIdentities(state);