-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(NOTIFY-1096): add account syncing #11190
Open
mathieuartu
wants to merge
57
commits into
main
Choose a base branch
from
feat/add_account_syncing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+354
−80
Open
Changes from 4 commits
Commits
Show all changes
57 commits
Select commit
Hold shift + click to select a range
631dbe9
feat(NOTIFY-1096): add account syncing
mathieuartu 04780e0
fix: import path
mathieuartu 771ad50
fix: useEffect
mathieuartu 2f20cdb
fix: temporarily remove keyring-controller patch
mathieuartu 249a1ba
fix: yarn.lock
mathieuartu 6e2b5b5
fix: remove Gemfile.lock
mathieuartu 888f01d
fix: add @ts-expect-error for controller version mismatch
mathieuartu b9bb37a
fix: remove unused import
mathieuartu 6c9b2ce
feat: group profile-sync effects under a layout effect + AppState cha…
mathieuartu 80af81c
add layouteffect deps
mathieuartu ab17435
Merge branch 'main' into feat/add_account_syncing
mathieuartu 95563d9
Merge branch 'main' into feat/add_account_syncing
mathieuartu c8e8e91
chore: remove preview packages and use latest deps
mathieuartu c3a3cb6
fix: yarn.lock
mathieuartu c0be57e
Merge branch 'main' into feat/add_account_syncing
mathieuartu 9aca646
Merge branch 'main' into feat/add_account_syncing
mathieuartu 8efc079
fix: update package.json & yarn.lock
mathieuartu 8836b0f
Merge branch 'main' into feat/add_account_syncing
mathieuartu 970f65e
fix: update yarn.lock
mathieuartu 63708ef
feat: add analytics
mathieuartu b6d891e
Merge branch 'main' into feat/add_account_syncing
mathieuartu c821134
Merge branch 'main' into feat/add_account_syncing
mathieuartu 5d17b5d
fix: yarn.lock
mathieuartu b7af253
fix: bump snaps packages
mathieuartu 91fa607
Merge branch 'main' into feat/add_account_syncing
mathieuartu 01d2304
Merge branch 'main' into feat/add_account_syncing
mathieuartu a3fc2cc
fix: remove unused ts-error statements
mathieuartu a971856
fix: snaps imports
mathieuartu fd5615d
Merge branch 'main' into feat/add_account_syncing
mathieuartu cdca605
Merge branch 'main' into feat/add_account_syncing
mathieuartu 92d9839
fix: yarn.lock
mathieuartu 40e8294
Merge branch 'main' into feat/add_account_syncing
mathieuartu c157056
fix: yarn.lock
mathieuartu 5d708da
fix: yarn.lock
mathieuartu 77c9c31
Merge branch 'main' into feat/add_account_syncing
mathieuartu 0210192
fix: yarn.lock
mathieuartu 2be206f
Merge branch 'main' into feat/add_account_syncing
mathieuartu f5a2aa9
fix: bump profile-sync-controller version
mathieuartu 1050e86
Merge branch 'main' into feat/add_account_syncing
mathieuartu 66aff4c
Merge branch 'main' into feat/add_account_syncing
mathieuartu 22cfc1a
fix: update analytics events names
mathieuartu bcfd1bb
Merge branch 'main' into feat/add_account_syncing
mathieuartu 307b449
Merge branch 'feat/add_account_syncing' of github.com:MetaMask/metama…
mathieuartu 7a4dc64
fix: update yarn.lock
mathieuartu b03fd6a
fix: build issues
mathieuartu ddf4c9e
Merge branch 'main' into feat/add_account_syncing
mathieuartu 2fee02f
fix: update yarn.lock
mathieuartu 0fa03b7
fix: yarn dedupe
mathieuartu 730fb38
Merge branch 'main' into feat/add_account_syncing
mathieuartu c953e4f
Merge branch 'main' into feat/add_account_syncing
mathieuartu 1ba5618
Merge branch 'main' into feat/add_account_syncing
mathieuartu 5752f14
fix: add missing semi-colon
mathieuartu d859175
fix: move account syncing to its own hook file
mathieuartu 250f1e9
fix: add tests
mathieuartu b0786f6
Merge branch 'main' into feat/add_account_syncing
mathieuartu 3885490
feat: use new event builder
mathieuartu 9ca93ba
fix: formatting
mathieuartu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,13 +1,15 @@ | ||||||||||||||
import React from 'react'; | ||||||||||||||
import Wallet from './'; | ||||||||||||||
import { renderScreen } from '../../../util/test/renderWithProvider'; | ||||||||||||||
import { screen } from '@testing-library/react-native'; | ||||||||||||||
import { act, screen } from '@testing-library/react-native'; | ||||||||||||||
import ScrollableTabView from 'react-native-scrollable-tab-view'; | ||||||||||||||
import Routes from '../../../constants/navigation/Routes'; | ||||||||||||||
import { backgroundState } from '../../../util/test/initial-root-state'; | ||||||||||||||
import { createMockAccountsControllerState } from '../../../util/test/accountsControllerTestUtils'; | ||||||||||||||
import { WalletViewSelectorsIDs } from '../../../../e2e/selectors/wallet/WalletView.selectors'; | ||||||||||||||
import { CommonSelectorsIDs } from '../../../../e2e/selectors/Common.selectors'; | ||||||||||||||
import { useAccountSyncing } from '../../../util/notifications/hooks/useAccountSyncing'; | ||||||||||||||
import { AppState } from 'react-native'; | ||||||||||||||
|
||||||||||||||
const MOCK_ADDRESS = '0xc4955c0d639d99699bfd7ec54d9fafee40e4d272'; | ||||||||||||||
|
||||||||||||||
|
@@ -108,6 +110,13 @@ jest.mock('react-native-scrollable-tab-view', () => { | |||||||||||||
return ScrollableTabViewMock; | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
jest.mock('../../../util/notifications/hooks/useAccountSyncing', () => ({ | ||||||||||||||
useAccountSyncing: jest.fn().mockReturnValue({ | ||||||||||||||
dispatchAccountSyncing: jest.fn(), | ||||||||||||||
error: undefined, | ||||||||||||||
}), | ||||||||||||||
})); | ||||||||||||||
|
||||||||||||||
const render = (Component: React.ComponentType) => | ||||||||||||||
renderScreen( | ||||||||||||||
Component, | ||||||||||||||
|
@@ -144,4 +153,36 @@ describe('Wallet', () => { | |||||||||||||
const foxIcon = screen.getByTestId(CommonSelectorsIDs.FOX_ICON); | ||||||||||||||
expect(foxIcon).toBeDefined(); | ||||||||||||||
}); | ||||||||||||||
it('should dispatch account syncing on mount', () => { | ||||||||||||||
jest.clearAllMocks(); | ||||||||||||||
//@ts-expect-error we are ignoring the navigation params on purpose because we do not want to mock setOptions to test the navbar | ||||||||||||||
render(Wallet); | ||||||||||||||
expect(useAccountSyncing().dispatchAccountSyncing).toHaveBeenCalledTimes(1); | ||||||||||||||
}); | ||||||||||||||
it('should dispatch account syncing when appState switches from inactive|background to active', () => { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
jest.clearAllMocks(); | ||||||||||||||
|
||||||||||||||
const addEventListener = jest.spyOn(AppState, 'addEventListener'); | ||||||||||||||
|
||||||||||||||
//@ts-expect-error we are ignoring the navigation params on purpose because we do not want to mock setOptions to test the navbar | ||||||||||||||
render(Wallet); | ||||||||||||||
|
||||||||||||||
expect(addEventListener).toHaveBeenCalledTimes(1); | ||||||||||||||
|
||||||||||||||
const handleAppStateChange = addEventListener.mock.calls[0][1]; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C: Not fond of this as if we add another listener in the code, this will break this test even if the feature works.
Suggested change
|
||||||||||||||
|
||||||||||||||
act(() => { | ||||||||||||||
handleAppStateChange('background'); | ||||||||||||||
handleAppStateChange('active'); | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
expect(useAccountSyncing().dispatchAccountSyncing).toHaveBeenCalledTimes(2); | ||||||||||||||
|
||||||||||||||
act(() => { | ||||||||||||||
handleAppStateChange('inactive'); | ||||||||||||||
handleAppStateChange('active'); | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
expect(useAccountSyncing().dispatchAccountSyncing).toHaveBeenCalledTimes(3); | ||||||||||||||
}); | ||||||||||||||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.