Skip to content

Commit

Permalink
[WALL] wojtek/removed potential source of race hazards (#13079)
Browse files Browse the repository at this point in the history
* feat: fixed tests after fixing authorise

* feat: updated authorise to prevent requesting previous token

* feat: updated authorize in order to avoid duplicated calls

* feat: cancelled all the requests before switching to new account
  • Loading branch information
azmib-developer authored Jan 25, 2024
1 parent e089089 commit 1fe5b37
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 11 deletions.
2 changes: 2 additions & 0 deletions packages/api/src/APIProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type APIContextData = {
send: TSendFunction;
subscribe: TSubscribeFunction;
unsubscribe: TUnsubscribeFunction;
queryClient: QueryClient;
};

const APIContext = createContext<APIContextData | null>(null);
Expand Down Expand Up @@ -229,6 +230,7 @@ const APIProvider = ({ children, standalone = false }: PropsWithChildren<TAPIPro
send,
subscribe,
unsubscribe,
queryClient,
}}
>
<QueryClientProvider client={queryClient}>
Expand Down
15 changes: 11 additions & 4 deletions packages/api/src/hooks/useAuthorize.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useMemo } from 'react';
import { useCallback, useMemo, useState } from 'react';
import { getActiveAuthTokenIDFromLocalStorage, getActiveLoginIDFromLocalStorage } from '@deriv/utils';
import useInvalidateQuery from '../useInvalidateQuery';
import useQuery from '../useQuery';
Expand All @@ -10,7 +10,9 @@ import { useAPIContext } from '../APIProvider';
const useAuthorize = () => {
const current_token = getActiveAuthTokenIDFromLocalStorage();
const invalidate = useInvalidateQuery();
const { switchEnvironment } = useAPIContext();
const { switchEnvironment, queryClient } = useAPIContext();

const [currentLoginID, setCurrentLoginID] = useState(getActiveLoginIDFromLocalStorage());

const { data, ...rest } = useQuery('authorize', {
payload: { authorize: current_token || '' },
Expand All @@ -33,17 +35,22 @@ const useAuthorize = () => {
if (active_loginid !== loginid) {
localStorage.setItem('active_loginid', loginid);
switchEnvironment(active_loginid);
invalidate('authorize');
// whenever we change the loginid, we need to invalidate all queries
// as there might be ongoing queries against previous loginid
// and we really do not want data from previous loginid, to be mixed with current loginid
queryClient.cancelQueries();
setCurrentLoginID(loginid);
}
},
[invalidate, switchEnvironment]
[invalidate, switchEnvironment, currentLoginID]
);

return {
/** The authorize response. */
data: modified_authorize,
/** Function to switch to another account */
switchAccount,
currentLoginID,
...rest,
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@ test.describe('Wallets - Crypto withdrawal', () => {
page,
state: {
accounts: DEFAULT_WALLET_ACCOUNTS,
currentToken: 'a1-x0000000000000000000000000004',
currentToken: 'a1-x0000000000000000000000000001',
},
});

await page.goto(`${baseURL}/wallets`);

await page.click('.wallets-accordion:nth-child(2) .wallets-accordion__dropdown');
});

test('render withdrawal form with all elements', async ({ baseURL, page }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ export default function mockWalletsAuthorize(context: Context) {
throw new Error('authorize has not been prepopulated');
}

const accountByToken = context.state.accounts.find(account => account.token === context.state.currentToken);
const accountByAuthorise = context.state.accounts.find(account => account.token === context.request.authorize);
context.state.currentToken = context.request.authorize || context.state.currentToken;
const currentAccount = context.state.accounts.find(account => account.token === context.state.currentToken);

context.response.authorize.account_list = ACCOUNTS_LIST;
context.response.authorize.loginid = accountByToken?.id || accountByAuthorise.id;
context.response.authorize.loginid = currentAccount?.id;
}
}
14 changes: 11 additions & 3 deletions packages/wallets/component-tests/wallets-carousel-content.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { mockProposalOpenContract } from './mocks/mockProposalOpenContract';
import mockWalletsAuthorize, { DEFAULT_WALLET_ACCOUNTS } from './mocks/mockWalletsAuthorize';
import mockWalletsLoggedIn from './mocks/mockWalletsLoggedIn';

const CAROUSEL_SELECTOR = '.wallets-carousel-content__container';
const CAROUSEL_SELECTOR = '.wallets-carousel-content__container .wallets-card:nth-child(1)';

// swipe function
async function swipeLeft(mobilePage: Page) {
Expand Down Expand Up @@ -91,17 +91,25 @@ test.describe('Wallets - Mobile carousel', () => {

test('renders progress bar with active item and updates it when swiping', async ({ baseURL }) => {
await mobilePage.goto(`${baseURL}/wallets`);
const activeProgressBarItem = await mobilePage.locator('.wallets-progress-bar div:nth-child(2)');
const activeProgressBarItem = await mobilePage.locator('.wallets-progress-bar div:nth-child(1)');
const progressBarItemClass = await activeProgressBarItem.getAttribute('class');

expect(progressBarItemClass).toContain('wallets-progress-bar-active');

// swipe left
await swipeLeft(mobilePage);

const activeProgressBarItem2 = await mobilePage.locator('.wallets-progress-bar div:nth-child(3)');
const activeProgressBarItem2 = await mobilePage.locator('.wallets-progress-bar div:nth-child(2)');
const progressBarItemClass2 = await activeProgressBarItem2.getAttribute('class');

expect(progressBarItemClass2).toContain('wallets-progress-bar-active');

await swipeLeft(mobilePage);

const activeProgressBarItem3 = await mobilePage.locator('.wallets-progress-bar div:nth-child(3)');
const progressBarItemClass3 = await activeProgressBarItem3.getAttribute('class');

expect(progressBarItemClass3).toContain('wallets-progress-bar-active');
});

test('switches account when clicking on progress bar', async ({ baseURL }) => {
Expand Down

1 comment on commit 1fe5b37

@vercel
Copy link

@vercel vercel bot commented on 1fe5b37 Jan 25, 2024

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

deriv-app – ./

deriv-app.vercel.app
deriv-app-git-master.binary.sx
deriv-app.binary.sx
binary.sx

Please sign in to comment.