Repository navigation
perf: pre-warm user location in ShareRootPage and read via Onyx.get #104479
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
Draft
jmgraa
wants to merge
2
commits into
Expensify:main
Choose a base branch
from
software-mansion-labs:jmgraa/fix-share-gps-prewarm
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.
Draft
Changes from all commits
Commits
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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| import {render} from '@testing-library/react-native'; | ||
|
|
||
| import {LocaleContextProvider} from '@components/LocaleContextProvider'; | ||
|
|
||
| import getCurrentPosition from '@libs/getCurrentPosition'; | ||
|
|
||
| import ShareRootPage from '@pages/Share/ShareRootPage'; | ||
|
|
||
| import CONST from '@src/CONST'; | ||
| import IntlStore from '@src/languages/IntlStore'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import type {UserLocation} from '@src/types/onyx'; | ||
|
|
||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
|
|
||
| import React from 'react'; | ||
| import Onyx from 'react-native-onyx'; | ||
| import {check} from 'react-native-permissions'; | ||
|
|
||
| import type * as FileUtilsModule from '../../src/libs/fileDownload/FileUtils'; | ||
|
|
||
| import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; | ||
|
|
||
| type SharedFile = {id: string; content: string; mimeType: string; processedAt: string; aspectRatio: number}; | ||
|
|
||
| // The file the mocked native share handler hands back. The `mock` prefix lets the hoisted jest.mock factory read it. | ||
| let mockSharedFiles: SharedFile[] = []; | ||
|
|
||
| jest.mock('@libs/ShareActionHandlerModule', () => ({ | ||
| __esModule: true, | ||
| default: { | ||
| processFiles: jest.fn((callback: (files: SharedFile[]) => void) => callback(mockSharedFiles)), | ||
| }, | ||
| })); | ||
|
|
||
| jest.mock('@libs/getCurrentPosition', () => jest.fn()); | ||
|
|
||
| jest.mock('@libs/fileDownload/FileUtils', () => { | ||
| const actual = jest.requireActual<typeof FileUtilsModule>('@libs/fileDownload/FileUtils'); | ||
| return { | ||
| ...actual, | ||
| validateImageForCorruption: jest.fn(() => Promise.resolve()), | ||
| }; | ||
| }); | ||
|
|
||
| jest.mock('@pages/Share/getFileSize', () => jest.fn(() => Promise.resolve(10000))); | ||
|
|
||
| jest.mock('@hooks/useFilesValidation', () => jest.fn(() => ({validateFiles: jest.fn()}))); | ||
|
|
||
| // Isolate the page from the tab contents and the screen chrome: these tests only cover what processing a shared file triggers. | ||
| jest.mock( | ||
| '@components/ScreenWrapper', | ||
| () => | ||
| ({children}: {children: React.ReactNode}) => | ||
| children, | ||
| ); | ||
| jest.mock('@components/HeaderWithBackButton', () => () => null); | ||
| jest.mock('@components/Skeletons/TabNavigatorSkeleton', () => () => null); | ||
| jest.mock('@libs/Navigation/OnyxTabNavigator', () => ({ | ||
| __esModule: true, | ||
| default: () => null, | ||
| TopTab: {Screen: () => null}, | ||
| })); | ||
| jest.mock('@pages/Share/ShareTab', () => () => null); | ||
| jest.mock('@pages/Share/SubmitTab', () => () => null); | ||
|
|
||
| const DEVICE_POSITION = {latitude: 52.2297, longitude: 21.0122}; | ||
|
|
||
| function createSharedFile(content: string, mimeType: string): SharedFile { | ||
| return {id: 'shared-file-1', content, mimeType, processedAt: '0', aspectRatio: 1}; | ||
| } | ||
|
|
||
| function mockPositionAnswer() { | ||
| jest.mocked(getCurrentPosition).mockImplementation(async (success) => { | ||
| success({ | ||
| coords: { | ||
| latitude: DEVICE_POSITION.latitude, | ||
| longitude: DEVICE_POSITION.longitude, | ||
| altitude: null, | ||
| accuracy: null, | ||
| altitudeAccuracy: null, | ||
| heading: null, | ||
| speed: null, | ||
| }, | ||
| timestamp: 0, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function getUserLocationFromOnyx(): Promise<OnyxEntry<UserLocation>> { | ||
| return new Promise((resolve) => { | ||
| const connection = Onyx.connect({ | ||
| key: ONYXKEYS.USER_LOCATION, | ||
| callback: (value) => { | ||
| Onyx.disconnect(connection); | ||
| resolve(value); | ||
| }, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| async function renderShareRootPage() { | ||
| render( | ||
| <LocaleContextProvider> | ||
| <ShareRootPage /> | ||
| </LocaleContextProvider>, | ||
| ); | ||
| await waitForBatchedUpdatesWithAct(); | ||
| } | ||
|
|
||
| describe('ShareRootPage location snapshot', () => { | ||
| beforeAll(() => { | ||
| Onyx.init({keys: ONYXKEYS}); | ||
| return IntlStore.load(CONST.LOCALES.EN); | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| mockPositionAnswer(); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| await Onyx.clear(); | ||
| }); | ||
|
|
||
| it('caches the device position as soon as a receipt file is shared', async () => { | ||
| // Given a shared image that can become a receipt, and location permission already granted | ||
| mockSharedFiles = [createSharedFile('file:///shared/receipt.jpg', CONST.RECEIPT_ALLOWED_FILE_TYPES.JPEG)]; | ||
|
|
||
| // When the share root processes the file | ||
| await renderShareRootPage(); | ||
|
|
||
| // Then the position is cached before the user even picks a recipient, so the Submit details page never has to | ||
| // read the device on open, which would land inside the ShareExtensionOpenSubmitFlow span | ||
| expect(getCurrentPosition).toHaveBeenCalledTimes(1); | ||
| expect(await getUserLocationFromOnyx()).toEqual(DEVICE_POSITION); | ||
| }); | ||
|
|
||
| it('does not read the location for a file that cannot become a receipt', async () => { | ||
| // Given a shared video, which only offers the Share tab, so the Submit flow is unreachable | ||
| mockSharedFiles = [createSharedFile('file:///shared/clip.mp4', CONST.SHARE_FILE_MIMETYPE.MP4)]; | ||
|
|
||
| // When the share root processes the file | ||
| await renderShareRootPage(); | ||
|
|
||
| // Then the device location is never read, since no expense can use it | ||
| expect(check).not.toHaveBeenCalled(); | ||
| expect(getCurrentPosition).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('does not read the location when the permission is not granted, so sharing never prompts for it', async () => { | ||
| // Given a shared receipt image and a location permission the user has not granted | ||
| mockSharedFiles = [createSharedFile('file:///shared/receipt.jpg', CONST.RECEIPT_ALLOWED_FILE_TYPES.JPEG)]; | ||
| jest.mocked(check).mockResolvedValueOnce('denied'); | ||
|
|
||
| // When the share root processes the file | ||
| await renderShareRootPage(); | ||
|
|
||
| // Then the device location is never read, because reading it would request the permission and show the system prompt | ||
| expect(getCurrentPosition).not.toHaveBeenCalled(); | ||
| expect(await getUserLocationFromOnyx()).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('clears stale cached location when the permission is not granted', async () => { | ||
| // Given a stale cached location and a location permission the user has not granted | ||
| await Onyx.set(ONYXKEYS.USER_LOCATION, DEVICE_POSITION); | ||
| mockSharedFiles = [createSharedFile('file:///shared/receipt.jpg', CONST.RECEIPT_ALLOWED_FILE_TYPES.JPEG)]; | ||
| jest.mocked(check).mockResolvedValueOnce('denied'); | ||
|
|
||
| // When the share root processes the file | ||
| await renderShareRootPage(); | ||
|
|
||
| // Then the device location is never read and the stale position is cleared from cache | ||
| expect(getCurrentPosition).not.toHaveBeenCalled(); | ||
| expect(await getUserLocationFromOnyx()).toBeUndefined(); | ||
| }); | ||
|
jmgraa marked this conversation as resolved.
|
||
|
|
||
| it('caches nothing and raises nothing when the permission check fails', async () => { | ||
| // Given a shared receipt image and a device whose location permission check fails outright | ||
| mockSharedFiles = [createSharedFile('file:///shared/receipt.jpg', CONST.RECEIPT_ALLOWED_FILE_TYPES.JPEG)]; | ||
| jest.mocked(check).mockRejectedValueOnce(new Error('permission check failed')); | ||
| const unhandledRejection = jest.fn(); | ||
| process.on('unhandledRejection', unhandledRejection); | ||
|
|
||
| // When the share root processes the file | ||
| await renderShareRootPage(); | ||
| process.off('unhandledRejection', unhandledRejection); | ||
|
|
||
| // Then no position is cached, and the failed check never surfaces as an unhandled rejection | ||
| expect(getCurrentPosition).not.toHaveBeenCalled(); | ||
| expect(await getUserLocationFromOnyx()).toBeUndefined(); | ||
| expect(unhandledRejection).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
This file contains hidden or 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.
Uh oh!
There was an error while loading. Please reload this page.