Skip to content

Commit

Permalink
refactor: clean up profile sync hooks (#28132)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

These changes make it easier for us to extend and add other syncing
features.

Originally this was part of my local network sync feature, but decoupled
parts of it since the change are a lot.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28132?quickstart=1)

## **Related issues**

Fixes: N/A

## **Manual testing steps**

There should be no changes as this is a code refactor and the
interfaces/exports do not change.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
Prithpal-Sooriya authored Oct 29, 2024
1 parent 3a35385 commit 6eae9bc
Show file tree
Hide file tree
Showing 10 changed files with 331 additions and 285 deletions.
21 changes: 21 additions & 0 deletions test/lib/render-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,27 @@ export function renderHookWithProvider(hook, state, pathname = '/', Container) {
};
}

/**
* Renders a hook with a provider and optional container.
*
* @template {(...args: any) => any} Hook
* @template {Parameters<Hook>} HookParams
* @template {ReturnType<Hook>} HookReturn
* @template {import('@testing-library/react-hooks').RenderHookResult<HookParams, HookReturn>} RenderHookResult
* @template {import('history').History} History
* @param {Hook} hook - The hook to be rendered.
* @param [state] - The initial state for the store.
* @param [pathname] - The initial pathname for the history.
* @param [Container] - An optional container component.
* @returns {RenderHookResult & { history: History }} The result of the rendered hook and the history object.
*/
export const renderHookWithProviderTyped = (
hook,
state,
pathname = '/',
Container,
) => renderHookWithProvider(hook, state, pathname, Container);

export function renderWithLocalization(component) {
const Wrapper = ({ children }) => (
<I18nProvider currentLocale="en" current={en} en={en}>
Expand Down
178 changes: 0 additions & 178 deletions ui/hooks/metamask-notifications/useProfileSyncing.test.tsx

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { waitFor } from '@testing-library/react';
import { act } from '@testing-library/react-hooks';
import { renderHookWithProviderTyped } from '../../../../test/lib/render-helpers';
import * as actions from '../../../store/actions';
import {
useAccountSyncingEffect,
useDeleteAccountSyncingDataFromUserStorage,
} from './accountSyncing';
import * as ProfileSyncModule from './profileSyncing';

describe('useDeleteAccountSyncingDataFromUserStorage()', () => {
it('should dispatch account sync data deletion', async () => {
const mockDeleteAccountSyncAction = jest.spyOn(
actions,
'deleteAccountSyncingDataFromUserStorage',
);

const { result } = renderHookWithProviderTyped(
() => useDeleteAccountSyncingDataFromUserStorage(),
{},
);

await act(async () => {
await result.current.dispatchDeleteAccountData();
});

expect(mockDeleteAccountSyncAction).toHaveBeenCalled();
});
});

describe('useAccountSyncingEffect', () => {
const arrangeMocks = () => {
const mockUseShouldProfileSync = jest.spyOn(
ProfileSyncModule,
'useShouldDispatchProfileSyncing',
);
const mockSyncAccountsAction = jest.spyOn(
actions,
'syncInternalAccountsWithUserStorage',
);
return {
mockUseShouldProfileSync,
mockSyncAccountsAction,
};
};

const arrangeAndAct = (props: { profileSyncConditionsMet: boolean }) => {
const mocks = arrangeMocks();
mocks.mockUseShouldProfileSync.mockReturnValue(
props.profileSyncConditionsMet,
);

renderHookWithProviderTyped(() => useAccountSyncingEffect(), {});
return mocks;
};

it('should run effect if profile sync conditions are met', async () => {
const mocks = arrangeAndAct({ profileSyncConditionsMet: true });
await waitFor(() => {
expect(mocks.mockSyncAccountsAction).toHaveBeenCalled();
});
});

it('should not run effect if profile sync conditions are not met', async () => {
const mocks = arrangeAndAct({ profileSyncConditionsMet: false });
await waitFor(() => {
expect(mocks.mockSyncAccountsAction).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import log from 'loglevel';
import { useCallback, useEffect } from 'react';
import { useDispatch } from 'react-redux';
import {
deleteAccountSyncingDataFromUserStorage,
syncInternalAccountsWithUserStorage,
} from '../../../store/actions';
import { useShouldDispatchProfileSyncing } from './profileSyncing';

/**
* Custom hook to dispatch account syncing.
*
* @returns An object containing the `dispatchAccountSyncing` function, boolean `shouldDispatchAccountSyncing`,
* and error state.
*/
const useAccountSyncing = () => {
const dispatch = useDispatch();

const shouldDispatchAccountSyncing = useShouldDispatchProfileSyncing();

const dispatchAccountSyncing = useCallback(() => {
try {
if (!shouldDispatchAccountSyncing) {
return;
}
dispatch(syncInternalAccountsWithUserStorage());
} catch (e) {
log.error(e);
}
}, [dispatch, shouldDispatchAccountSyncing]);

return {
dispatchAccountSyncing,
shouldDispatchAccountSyncing,
};
};

/**
* Custom hook to apply account syncing effect.
*/
export const useAccountSyncingEffect = () => {
const shouldSync = useShouldDispatchProfileSyncing();
const { dispatchAccountSyncing } = useAccountSyncing();

useEffect(() => {
if (shouldSync) {
dispatchAccountSyncing();
}
}, [shouldSync, dispatchAccountSyncing]);
};

/**
* Custom hook to delete a user's account syncing data from user storage
*/
export const useDeleteAccountSyncingDataFromUserStorage = () => {
const dispatch = useDispatch();
const dispatchDeleteAccountData = useCallback(async () => {
try {
await dispatch(deleteAccountSyncingDataFromUserStorage());
} catch {
// Do Nothing
}
}, []);

return { dispatchDeleteAccountData };
};
9 changes: 9 additions & 0 deletions ui/hooks/metamask-notifications/useProfileSyncing/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export {
useDisableProfileSyncing,
useEnableProfileSyncing,
useSetIsProfileSyncingEnabled,
} from './profileSyncing';
export {
useAccountSyncingEffect,
useDeleteAccountSyncingDataFromUserStorage,
} from './accountSyncing';
Loading

0 comments on commit 6eae9bc

Please sign in to comment.