From 59980f1b2714b2050284634b819495ef565f6e2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Oliv=C3=A9?= Date: Fri, 30 Jun 2023 12:41:28 +0200 Subject: [PATCH] Fixing connect custody flow (#19802) * Fixing connect custody flow * Finished fixing mmi connect flow * Fixed test * updated snapshot * Finished fixing tests --- app/scripts/metamask-controller.js | 43 ++++-- .../confirm-add-custodian-token.js | 129 +++++++++--------- .../__snapshots__/custody.test.js.snap | 127 +++-------------- ui/pages/institutional/custody/custody.js | 82 +++++------ .../institutional/custody/custody.test.js | 26 ++-- .../institution-background.test.js | 1 - .../institutional/institution-background.ts | 46 ++++--- 7 files changed, 186 insertions(+), 268 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e3a0a704a..7c40d960a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2368,19 +2368,38 @@ export default class MetamaskController extends EventEmitter { ...getPermissionBackgroundApiMethods(permissionController), ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) - connectCustodyAddresses: - this.mmiController.connectCustodyAddresses.bind(this), - getCustodianAccounts: this.mmiController.getCustodianAccounts.bind(this), + connectCustodyAddresses: this.mmiController.connectCustodyAddresses.bind( + this.mmiController, + ), + getCustodianAccounts: this.mmiController.getCustodianAccounts.bind( + this.mmiController, + ), getCustodianAccountsByAddress: - this.mmiController.getCustodianAccountsByAddress.bind(this), + this.mmiController.getCustodianAccountsByAddress.bind( + this.mmiController, + ), getCustodianTransactionDeepLink: - this.mmiController.getCustodianTransactionDeepLink.bind(this), + this.mmiController.getCustodianTransactionDeepLink.bind( + this.mmiController, + ), getCustodianConfirmDeepLink: - this.mmiController.getCustodianConfirmDeepLink.bind(this), + this.mmiController.getCustodianConfirmDeepLink.bind(this.mmiController), getCustodianSignMessageDeepLink: - this.mmiController.getCustodianSignMessageDeepLink.bind(this), - getCustodianToken: this.mmiController.getCustodianToken.bind(this), - getCustodianJWTList: this.mmiController.getCustodianJWTList.bind(this), + this.mmiController.getCustodianSignMessageDeepLink.bind( + this.mmiController, + ), + getCustodianToken: this.mmiController.getCustodianToken.bind( + this.mmiController, + ), + getCustodianJWTList: this.mmiController.getCustodianJWTList.bind( + this.mmiController, + ), + getAllCustodianAccountsWithToken: + this.mmiController.getAllCustodianAccountsWithToken.bind( + this.mmiController, + ), + setCustodianNewRefreshToken: + this.mmiController.setCustodianNewRefreshToken.bind(this.mmiController), setWaitForConfirmDeepLinkDialog: this.custodyController.setWaitForConfirmDeepLinkDialog.bind( this.custodyController, @@ -2393,8 +2412,6 @@ export default class MetamaskController extends EventEmitter { this.custodyController.getCustodianConnectRequest.bind( this.custodyController, ), - getAllCustodianAccountsWithToken: - this.mmiController.getAllCustodianAccountsWithToken.bind(this), getMmiConfiguration: this.mmiConfigurationController.getConfiguration.bind( this.mmiConfigurationController, @@ -2415,12 +2432,10 @@ export default class MetamaskController extends EventEmitter { this.institutionalFeaturesController.syncReportsInProgress.bind( this.institutionalFeaturesController, ), - removeConnectInstitutionalFeature: this.institutionalFeaturesController.removeConnectInstitutionalFeature.bind( this.institutionalFeaturesController, ), - getComplianceHistoricalReportsByAddress: this.institutionalFeaturesController.getComplianceHistoricalReportsByAddress.bind( this.institutionalFeaturesController, @@ -2429,8 +2444,6 @@ export default class MetamaskController extends EventEmitter { this.institutionalFeaturesController.removeAddTokenConnectRequest.bind( this.institutionalFeaturesController, ), - setCustodianNewRefreshToken: - this.mmiController.setCustodianNewRefreshToken.bind(this), ///: END:ONLY_INCLUDE_IN ///: BEGIN:ONLY_INCLUDE_IN(snaps) diff --git a/ui/pages/institutional/confirm-add-custodian-token/confirm-add-custodian-token.js b/ui/pages/institutional/confirm-add-custodian-token/confirm-add-custodian-token.js index 15c879463..e353f2285 100644 --- a/ui/pages/institutional/confirm-add-custodian-token/confirm-add-custodian-token.js +++ b/ui/pages/institutional/confirm-add-custodian-token/confirm-add-custodian-token.js @@ -5,10 +5,10 @@ import PulseLoader from '../../../components/ui/pulse-loader'; import { CUSTODY_ACCOUNT_ROUTE } from '../../../helpers/constants/routes'; import { AlignItems, - DISPLAY, + Display, TextColor, - TEXT_ALIGN, - FLEX_DIRECTION, + TextAlign, + FlexDirection, } from '../../../helpers/constants/design-system'; import { BUILT_IN_NETWORKS } from '../../../../shared/constants/network'; import { I18nContext } from '../../../contexts/i18n'; @@ -23,8 +23,12 @@ import { Button, BUTTON_SIZES, BUTTON_VARIANT, + Box, } from '../../../components/component-library'; -import Box from '../../../components/ui/box'; +import { + complianceActivated, + getInstitutionalConnectRequests, +} from '../../../ducks/institutional/institutional'; const ConfirmAddCustodianToken = () => { const t = useContext(I18nContext); @@ -34,59 +38,12 @@ const ConfirmAddCustodianToken = () => { const mmiActions = mmiActionsFactory(); const mostRecentOverviewPage = useSelector(getMostRecentOverviewPage); - const connectRequests = useSelector( - (state) => state.metamask.institutionalFeatures?.connectRequests, - ); - const complianceActivated = useSelector((state) => - Boolean(state.metamask.institutionalFeatures?.complianceProjectId), - ); + const connectRequests = useSelector(getInstitutionalConnectRequests); + const isComplianceActivated = useSelector(complianceActivated); const [showMore, setShowMore] = useState(false); const [isLoading, setIsLoading] = useState(false); const [connectError, setConnectError] = useState(''); - const handleConnectError = (e) => { - let errorMessage = e.message; - - if (!errorMessage) { - errorMessage = 'Connection error'; - } - - setConnectError(errorMessage); - setIsLoading(false); - }; - - const renderSelectedToken = () => { - const connectRequest = connectRequests ? connectRequests[0] : undefined; - - return ( - - - {showMore && connectRequest?.token - ? connectRequest?.token - : `...${connectRequest?.token.slice(-9)}`} - - {!showMore && ( - - { - setShowMore(true); - }} - > - {t('showMore')} - - - )} - - ); - }; - const connectRequest = connectRequests ? connectRequests[0] : undefined; if (!connectRequest) { @@ -148,7 +105,31 @@ const ConfirmAddCustodianToken = () => { marginLeft={4} className="add_custodian_token_confirm__token" > - {renderSelectedToken()} + + + {showMore && connectRequest?.token + ? connectRequest?.token + : `...${connectRequest?.token.slice(-9)}`} + + {!showMore && ( + + { + setShowMore(true); + }} + > + {t('showMore')} + + + )} + {connectRequest.apiUrl && ( @@ -168,9 +149,9 @@ const ConfirmAddCustodianToken = () => { )} - {!complianceActivated && ( + {!isComplianceActivated && ( - + {connectError} @@ -180,19 +161,19 @@ const ConfirmAddCustodianToken = () => { {isLoading ? ( ) : ( - + -

- Back -

- -

-
-

-

-

-

- Enter your token or add a new token -

-
-
-
-
-

- Paste or drop your token here: -

- -
-
-
-

- API URL -

-
- -
-
-
-
- - -
-
- `; diff --git a/ui/pages/institutional/custody/custody.js b/ui/pages/institutional/custody/custody.js index 7ab0efb5f..1257e1c57 100644 --- a/ui/pages/institutional/custody/custody.js +++ b/ui/pages/institutional/custody/custody.js @@ -44,6 +44,7 @@ import { getCurrentChainId } from '../../../selectors'; import { getMMIConfiguration } from '../../../selectors/institutional/selectors'; import CustodyAccountList from '../connect-custody/account-list'; import JwtUrlForm from '../../../components/institutional/jwt-url-form'; +import PulseLoader from '../../../components/ui/pulse-loader/pulse-loader'; const CustodyPage = () => { const t = useI18nContext(); @@ -55,6 +56,7 @@ const CustodyPage = () => { const currentChainId = useSelector(getCurrentChainId); const { custodians } = useSelector(getMMIConfiguration); + const [loading, setLoading] = useState(true); const [selectedAccounts, setSelectedAccounts] = useState({}); const [selectedCustodianName, setSelectedCustodianName] = useState(''); const [selectedCustodianImage, setSelectedCustodianImage] = useState(null); @@ -200,12 +202,12 @@ const CustodyPage = () => { ); const getCustodianAccounts = useCallback( - async (token, custody, getNonImportedAccounts) => { + async (token, getNonImportedAccounts) => { return await dispatch( mmiActions.getCustodianAccounts( token, apiUrl, - custody || selectedCustodianType, + selectedCustodianType, getNonImportedAccounts, ), ); @@ -218,12 +220,8 @@ const CustodyPage = () => { // If you have one JWT already, but no dropdown yet, currentJwt is null! const jwt = currentJwt || jwtList[0]; setConnectError(''); - const accountsValue = await getCustodianAccounts( - jwt, - apiUrl, - selectedCustodianType, - true, - ); + const accountsValue = await getCustodianAccounts(jwt, true); + setAccounts(accountsValue); trackEvent({ category: 'MMI', @@ -245,15 +243,16 @@ const CustodyPage = () => { handleConnectError, jwtList, selectedCustodianName, - selectedCustodianType, trackEvent, ]); useEffect(() => { + setLoading(true); const fetchConnectRequest = async () => { const connectRequestValue = await dispatch( mmiActions.getCustodianConnectRequest(), ); + setChainId(parseInt(currentChainId, 16)); // check if it's empty object @@ -266,15 +265,22 @@ const CustodyPage = () => { setSelectedCustodianType(connectRequestValue.custodianType); setSelectedCustodianName(connectRequestValue.custodianName); setApiUrl(connectRequestValue.apiUrl); - connect(); } }; - // call the function - fetchConnectRequest() - // make sure to catch any error - .catch(console.error); - }, [dispatch, connect, currentChainId, mmiActions]); + const handleFetchConnectRequest = async () => { + try { + await fetchConnectRequest(); + setLoading(false); + } catch (error) { + console.error(error); + setLoading(false); + } + }; + + handleFetchConnectRequest(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); useEffect(() => { const handleNetworkChange = async () => { @@ -282,14 +288,7 @@ const CustodyPage = () => { const jwt = currentJwt || jwtList[0]; if (jwt && jwt.length) { - setAccounts( - await getCustodianAccounts( - jwt, - apiUrl, - selectedCustodianType, - true, - ), - ); + setAccounts(await getCustodianAccounts(jwt, true)); } } }; @@ -341,6 +340,10 @@ const CustodyPage = () => { } }; + if (loading) { + return ; + } + return ( <> {connectError && ( @@ -348,7 +351,6 @@ const CustodyPage = () => { {connectError} )} - {selectError && ( {selectError} @@ -397,7 +399,6 @@ const CustodyPage = () => { ) : null} - {!accounts && selectedCustodianType && ( <> { )} - {accounts && accounts.length > 0 && ( <> { selectedAccounts={selectedAccounts} onAddAccounts={async () => { try { + const selectedCustodian = custodians.find( + (custodian) => custodian.name === selectedCustodianName, + ); + await dispatch( mmiActions.connectCustodyAddresses( selectedCustodianType, @@ -548,17 +552,7 @@ const CustodyPage = () => { selectedAccounts, ), ); - const selectedCustodian = custodians.find( - (custodian) => custodian.name === selectedCustodianName, - ); - history.push({ - pathname: CUSTODY_ACCOUNT_DONE_ROUTE, - state: { - imgSrc: selectedCustodian.iconUrl, - title: t('custodianAccountAddedTitle'), - description: t('custodianAccountAddedDesc'), - }, - }); + trackEvent({ category: 'MMI', event: 'Custodial accounts connected', @@ -568,6 +562,15 @@ const CustodyPage = () => { chainId, }, }); + + history.push({ + pathname: CUSTODY_ACCOUNT_DONE_ROUTE, + state: { + imgSrc: selectedCustodian.iconUrl, + title: t('custodianAccountAddedTitle'), + description: t('custodianAccountAddedDesc'), + }, + }); } catch (e) { setSelectError(e.message); } @@ -598,7 +601,6 @@ const CustodyPage = () => { /> )} - {accounts && accounts.length === 0 && ( { marginBottom={2} fontWeight={FontWeight.Bold} color={TextColor.textDefault} - variant={TextVariant.bodySm} + variant={TextVariant.bodyLgMedium} > {t('allCustodianAccountsConnectedTitle')} - + {t('allCustodianAccountsConnectedSubtitle')} diff --git a/ui/pages/institutional/custody/custody.test.js b/ui/pages/institutional/custody/custody.test.js index f944d33aa..22612a456 100644 --- a/ui/pages/institutional/custody/custody.test.js +++ b/ui/pages/institutional/custody/custody.test.js @@ -1,6 +1,6 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; -import { fireEvent, waitFor, screen } from '@testing-library/react'; +import { fireEvent, waitFor, screen, act } from '@testing-library/react'; import thunk from 'redux-thunk'; import { renderWithProvider } from '../../../../test/lib/render-helpers'; import CustodyPage from '.'; @@ -16,8 +16,8 @@ const mockedGetCustodianConnectRequest = jest.fn().mockReturnValue({ custodian: 'saturn', token: 'token', apiUrl: 'url', - custodianType: 'JSON-RPC', - custodianName: 'Saturn', + custodianType: undefined, + custodianName: 'saturn', }); jest.mock('../../../store/institutional/institution-background', () => ({ @@ -85,29 +85,19 @@ describe('CustodyPage', function () { }); }); - it('calls getCustodianJwtList on custody select when connect btn is click', async () => { - const { getByTestId } = renderWithProvider(, store); - - const custodyBtn = getByTestId('custody-connect-button'); - await waitFor(() => { - fireEvent.click(custodyBtn); + it('calls getCustodianJwtList on custody select when connect btn is click and clicks connect button and shows the jwt form', async () => { + act(() => { + renderWithProvider(, store); }); await waitFor(() => { - expect(mockedGetCustodianJWTList).toHaveBeenCalled(); - }); - }); - - it('clicks connect button and shows the jwt form', async () => { - const { getByTestId } = renderWithProvider(, store); - const custodyBtn = getByTestId('custody-connect-button'); - - await waitFor(() => { + const custodyBtn = screen.getByTestId('custody-connect-button'); fireEvent.click(custodyBtn); }); await waitFor(() => { expect(screen.getByTestId('jwt-form-connect-button')).toBeInTheDocument(); + expect(mockedGetCustodianJWTList).toHaveBeenCalled(); }); }); }); diff --git a/ui/store/institutional/institution-background.test.js b/ui/store/institutional/institution-background.test.js index 878ea24ce..7d9f0869f 100644 --- a/ui/store/institutional/institution-background.test.js +++ b/ui/store/institutional/institution-background.test.js @@ -117,7 +117,6 @@ describe('Institution Actions', () => { 'newApiUrl', ); connectCustodyAddresses(jest.fn()); - setWaitForConfirmDeepLinkDialog(jest.fn()); expect(connectCustodyAddresses).toBeDefined(); expect(setWaitForConfirmDeepLinkDialog).toBeDefined(); }); diff --git a/ui/store/institutional/institution-background.ts b/ui/store/institutional/institution-background.ts index 22b0c0818..5a6156e29 100644 --- a/ui/store/institutional/institution-background.ts +++ b/ui/store/institutional/institution-background.ts @@ -91,14 +91,17 @@ export function mmiActionsFactory() { }; } - function createAction(name: string, payload: any) { - return () => { - callBackgroundMethod(name, [payload], (err) => { - if (isErrorWithMessage(err)) { - throw new Error(err.message); + function createAction(name: string, payload: any): Promise { + return new Promise((resolve, reject) => { + callBackgroundMethod(name, [payload], (error) => { + if (error) { + reject(error); + return; } + + resolve(); }); - }; + }); } return { @@ -190,18 +193,27 @@ export function mmiActionsFactory() { createAction('syncReportsInProgress', { address, historicalReports }), removeConnectInstitutionalFeature: (origin: string, projectId: string) => createAction('removeConnectInstitutionalFeature', { origin, projectId }), - removeAddTokenConnectRequest: ( - origin: string, - apiUrl: string, - token: string, - ) => + removeAddTokenConnectRequest: ({ + origin, + apiUrl, + token, + }: { + origin: string; + apiUrl: string; + token: string; + }) => createAction('removeAddTokenConnectRequest', { origin, apiUrl, token }), - setCustodianConnectRequest: ( - token: string, - apiUrl: string, - custodianType: string, - custodianName: string, - ) => + setCustodianConnectRequest: ({ + token, + apiUrl, + custodianType, + custodianName, + }: { + token: string; + apiUrl: string; + custodianType: string; + custodianName: string; + }) => createAsyncAction('setCustodianConnectRequest', [ { token, apiUrl, custodianType, custodianName }, ]),