IOURequestStepScan clean-up, phase 2: Extract shared business logic into useReceiptScan hook #82130
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Refactors IOURequestStepScan by extracting shared “receipt scan / upload” business logic into a new useReceiptScan hook to reduce duplication between web and native implementations.
Changes:
- Added
useReceiptScanhook to centralize Onyx subscriptions, confirmation navigation, multi-scan toggling, and file validation/receipt processing. - Updated
IOURequestStepScanweb and native screens to consume the hook and remove duplicated logic. - Kept platform-specific camera capture flows in the respective screen files while delegating shared upload/flow behavior to the hook.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts |
New shared hook encapsulating scan/upload state + navigation + multi-scan logic. |
src/pages/iou/request/step/IOURequestStepScan/index.tsx |
Web scan step updated to use useReceiptScan for shared business logic. |
src/pages/iou/request/step/IOURequestStepScan/index.native.tsx |
Native scan step updated to use useReceiptScan for shared business logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
Outdated
Show resolved
Hide resolved
src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
Outdated
Show resolved
Hide resolved
samranahm
changed the title
IOURequestStepScan clean-up, phase 2: Extract shared business logic into useReceiptScan Hook IOURequestStepScan clean-up, phase 2: Extract shared business logic into useReceiptScan hook
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
PR doesn’t need product input as a refactor PR. Unassigning and unsubscribing myself. |
…pScan-clean-up-p2
|
Resolving merge conflicts. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14f5883d50
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
Outdated
Show resolved
Hide resolved
|
Ready for review 👍 |
| getSource, | ||
| }: UseReceiptScanParams) { | ||
| const {isBetaEnabled} = usePermissions(); | ||
| const {shouldStartLocationPermissionFlow} = useIOUUtils(); |
There was a problem hiding this comment.
useIOUUtils hook was added in #66740 to make it reusable between index.ts and index.native files. Since we now consolidate the logic in useReceiptScan, I think we can just move the logic here and remove useIOUUtils
There was a problem hiding this comment.
This blinkStyle and showBlink can be moved to the hook too. The only difference is we call HaptiFeedback.press on the native file, but we already no-op it on the web.
There was a problem hiding this comment.
This can be moved to the hook too.
There was a problem hiding this comment.
Native file pass file.uri to the source param of navigateToConfirmationStep, but the source param from setTestReceipt already contains the correct source. So, let's copy the index.ts logic.
src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
Outdated
Show resolved
Hide resolved
src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
Outdated
Show resolved
Hide resolved
|
@samranahm I keep having infinite loading when submitting the scan expense (happens on main too). Do you experience it too? If yes, I think we should report this. 
|
|
On iOS, the image selector is unresponsive. ios.e.mp4 |
|
@bernhardoj I’m not able to reproduce this on either dev or staging. Kapture.2026-02-19.at.22.30.10.mp4 |
@bernhardoj Please let me know if you still encountering this we should post this in slack to get more eyes. |
IOS.mp4It’s working as expected on my end. Try building the app on latest that will potentially resolve it. |
|
The infinite loading in my Android mWeb is caused by App/src/libs/getCurrentPosition/index.ts Line 16 in 677ec78 I've removed the get position logic to be able to test this. |
|
@samranahm may I know the iOS simulator that you use? |
|
@bernhardoj It's iPhone 16 with iOS 26.2 |
|
It keep failing when I tried to run with iOS 26 |
|
I can use the lower version. |
| const [shouldShowMultiScanEducationalPopup, setShouldShowMultiScanEducationalPopup] = useState(false); | ||
|
|
||
| // Clear receipt files when multi-scan is disabled | ||
| useEffect(() => { |
There was a problem hiding this comment.
I think this is a misuse of useEffect - it fits somewhere between Updating state based on props or state and Resetting all state when a prop changes.
I believe that this is essentially a symptom that shows that isMultiScanEnabled should belong to this hook, rather than the parent.
- Move
isMultiScanEnabledstate intouseReceiptScanhook (was in parent) - Remove the
useEffectthat reactively clearsreceiptFileson prop change - Clear
receiptFilesdirectly intoggleMultiScanwhen disabling - Add
key={transactionRequestType}on<IOURequestStepScan>to reset all state on tab switch - Remove
isMultiScanEnabled/setIsMultiScanEnabledprops from component and type interfaces
diff
diff --git a/src/pages/iou/request/IOURequestStartPage.tsx b/src/pages/iou/request/IOURequestStartPage.tsx
index 677d618a822..f02d50628bd 100644
--- a/src/pages/iou/request/IOURequestStartPage.tsx
+++ b/src/pages/iou/request/IOURequestStartPage.tsx
@@ -86,7 +86,6 @@ function IOURequestStartPage({
const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {canBeMissing: false});
const [lastSelectedDistanceRates] = useOnyx(ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES, {canBeMissing: true});
const [draftTransactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_DRAFT, {canBeMissing: true});
- const [isMultiScanEnabled, setIsMultiScanEnabled] = useState(false);
const [currentDate] = useOnyx(ONYXKEYS.CURRENT_DATE, {canBeMissing: true});
const {isOffline} = useNetwork();
const [nvpDismissedProductTraining] = useOnyx(ONYXKEYS.NVP_DISMISSED_PRODUCT_TRAINING, {canBeMissing: true});
@@ -177,7 +176,6 @@ function IOURequestStartPage({
if (transaction?.iouRequestType === newIOUType) {
return;
}
- setIsMultiScanEnabled(false);
initMoneyRequest({
reportID,
policy,
@@ -334,13 +332,12 @@ function IOURequestStartPage({
{() => (
<TabScreenWithFocusTrapWrapper>
<IOURequestStepScan
+ key={transactionRequestType}
route={route}
navigation={navigation}
onLayout={(setTestReceiptAndNavigate) => {
setTestReceiptAndNavigateRef.current = setTestReceiptAndNavigate;
}}
- isMultiScanEnabled={isMultiScanEnabled}
- setIsMultiScanEnabled={setIsMultiScanEnabled}
isStartingScan
/>
</TabScreenWithFocusTrapWrapper>
diff --git a/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx b/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx
index 2e94666bb0f..9d7d4a615da 100644
--- a/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx
+++ b/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx
@@ -61,9 +61,7 @@ function IOURequestStepScan({
transaction: initialTransaction,
currentUserPersonalDetails,
onLayout,
- isMultiScanEnabled = false,
isStartingScan = false,
- setIsMultiScanEnabled,
}: IOURequestStepScanProps) {
const theme = useTheme();
const styles = useThemeStyles();
@@ -198,6 +196,7 @@ function IOURequestStepScan({
// Shared business logic from useReceiptScan hook
const {
isEditing,
+ isMultiScanEnabled,
canUseMultiScan,
shouldAcceptMultipleFiles,
startLocationPermissionFlow,
@@ -226,11 +225,9 @@ function IOURequestStepScan({
currentUserPersonalDetails,
backTo,
backToReport,
- isMultiScanEnabled,
isStartingScan,
updateScanAndNavigate,
getSource,
- setIsMultiScanEnabled,
});
const viewfinderLayout = useRef<LayoutRectangle>(null);
diff --git a/src/pages/iou/request/step/IOURequestStepScan/index.tsx b/src/pages/iou/request/step/IOURequestStepScan/index.tsx
index e7cdac3388c..a22744ae01a 100644
--- a/src/pages/iou/request/step/IOURequestStepScan/index.tsx
+++ b/src/pages/iou/request/step/IOURequestStepScan/index.tsx
@@ -56,9 +56,7 @@ function IOURequestStepScan({
transaction: initialTransaction,
currentUserPersonalDetails,
onLayout,
- isMultiScanEnabled = false,
isStartingScan = false,
- setIsMultiScanEnabled,
}: Omit<IOURequestStepScanProps, 'user'>) {
const theme = useTheme();
const styles = useThemeStyles();
@@ -96,6 +94,7 @@ function IOURequestStepScan({
const {
transactions,
isEditing,
+ isMultiScanEnabled,
canUseMultiScan,
isReplacingReceipt,
shouldAcceptMultipleFiles,
@@ -125,11 +124,9 @@ function IOURequestStepScan({
currentUserPersonalDetails,
backTo,
backToReport,
- isMultiScanEnabled,
isStartingScan,
updateScanAndNavigate,
getSource,
- setIsMultiScanEnabled,
});
const [videoConstraints, setVideoConstraints] = useState<MediaTrackConstraints>();
@@ -218,7 +215,6 @@ function IOURequestStepScan({
if (isAllScanFilesCanBeRead) {
return;
}
- setIsMultiScanEnabled?.(false);
removeTransactionReceipt(CONST.IOU.OPTIMISTIC_TRANSACTION_ID);
removeDraftTransactions(true);
});
diff --git a/src/pages/iou/request/step/IOURequestStepScan/types.ts b/src/pages/iou/request/step/IOURequestStepScan/types.ts
index 60ca9e955a1..09e1a448561 100644
--- a/src/pages/iou/request/step/IOURequestStepScan/types.ts
+++ b/src/pages/iou/request/step/IOURequestStepScan/types.ts
@@ -38,9 +38,6 @@ type UseReceiptScanParams = {
/** Report ID to navigate back to */
backToReport: string | undefined;
- /** Whether multi-scan is enabled */
- isMultiScanEnabled: boolean | undefined;
-
/** Whether the user is starting a scan request */
isStartingScan: boolean | undefined;
@@ -49,9 +46,6 @@ type UseReceiptScanParams = {
/** Returns a source URL for the file based on platform */
getSource: (file: FileObject) => string;
-
- /** Callback to update multi-scan enabled state in parent */
- setIsMultiScanEnabled: ((value: boolean) => void) | undefined;
};
type IOURequestStepScanProps = WithCurrentUserPersonalDetailsProps &
@@ -65,12 +59,6 @@ type IOURequestStepScanProps = WithCurrentUserPersonalDetailsProps &
*/
onLayout?: (setTestReceiptAndNavigate: () => void) => void;
- /** If the receipts preview should be shown */
- isMultiScanEnabled?: boolean;
-
- /** Updates isMultiScanEnabled flag */
- setIsMultiScanEnabled?: (value: boolean) => void;
-
/** Indicates whether users start to create scan request */
isStartingScan?: boolean;
};
diff --git a/src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts b/src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
index 748f328e263..01fa4d0d800 100644
--- a/src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
+++ b/src/pages/iou/request/step/IOURequestStepScan/useReceiptScan.ts
@@ -1,6 +1,6 @@
import TestReceipt from '@assets/images/fake-receipt.png';
import {hasSeenTourSelector} from '@selectors/Onboarding';
-import {useEffect, useState} from 'react';
+import {useState} from 'react';
import {InteractionManager} from 'react-native';
import {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
import useDefaultExpensePolicy from '@hooks/useDefaultExpensePolicy';
@@ -51,11 +51,9 @@ function useReceiptScan({
currentUserPersonalDetails,
backTo,
backToReport,
- isMultiScanEnabled = false,
isStartingScan = false,
updateScanAndNavigate,
getSource,
- setIsMultiScanEnabled,
}: UseReceiptScanParams) {
const {isBetaEnabled} = usePermissions();
const [shouldStartLocationPermissionFlow] = useOnyx(ONYXKEYS.NVP_LAST_LOCATION_PERMISSION_PROMPT, {
@@ -99,18 +97,11 @@ function useReceiptScan({
const shouldSkipConfirmation =
!!skipConfirmation && !!report?.reportID && !isArchived && !(isPolicyExpenseChat(report) && ((policy?.requiresCategory ?? false) || (policy?.requiresTag ?? false)));
+ const [isMultiScanEnabled, setIsMultiScanEnabled] = useState(false);
const [startLocationPermissionFlow, setStartLocationPermissionFlow] = useState(false);
const [receiptFiles, setReceiptFiles] = useState<ReceiptFile[]>([]);
const [shouldShowMultiScanEducationalPopup, setShouldShowMultiScanEducationalPopup] = useState(false);
- // Clear receipt files when multi-scan is disabled
- useEffect(() => {
- if (isMultiScanEnabled) {
- return;
- }
- setReceiptFiles([]);
- }, [isMultiScanEnabled]);
-
const blinkOpacity = useSharedValue(0);
const blinkStyle = useAnimatedStyle(() => ({
opacity: blinkOpacity.get(),
@@ -259,7 +250,11 @@ function useReceiptScan({
}
removeTransactionReceipt(CONST.IOU.OPTIMISTIC_TRANSACTION_ID);
removeDraftTransactions(true);
- setIsMultiScanEnabled?.(!isMultiScanEnabled);
+ const newValue = !isMultiScanEnabled;
+ setIsMultiScanEnabled(newValue);
+ if (!newValue) {
+ setReceiptFiles([]);
+ }
}
function dismissMultiScanEducationalPopup() {
@@ -273,6 +268,7 @@ function useReceiptScan({
return {
transactions,
isEditing,
+ isMultiScanEnabled,
canUseMultiScan,
isReplacingReceipt,
shouldAcceptMultipleFiles,
diff --git a/tests/ui/IOURequestStepScanTest.tsx b/tests/ui/IOURequestStepScanTest.tsx
index 67ff02410a0..127676c8604 100644
--- a/tests/ui/IOURequestStepScanTest.tsx
+++ b/tests/ui/IOURequestStepScanTest.tsx
@@ -201,9 +201,7 @@ describe('IOURequestStepScan', () => {
} as unknown as PlatformStackScreenProps<MoneyRequestNavigatorParamList, typeof SCREENS.MONEY_REQUEST.STEP_SCAN>['route']
}
navigation={{} as never}
- isMultiScanEnabled
isStartingScan
- setIsMultiScanEnabled={jest.fn()}
/>
</NavigationContainer>
</LocaleContextProvider>
diff --git a/tests/unit/hooks/useReceiptScan.test.ts b/tests/unit/hooks/useReceiptScan.test.ts
index 4722efe28eb..6e564052c09 100644
--- a/tests/unit/hooks/useReceiptScan.test.ts
+++ b/tests/unit/hooks/useReceiptScan.test.ts
@@ -76,9 +76,7 @@ function createDefaultParams(): Parameters<typeof useReceiptScan>[0] {
getSource: (file: {uri?: string}) => file?.uri ?? 'file://image.png',
backTo: undefined,
backToReport: undefined,
- isMultiScanEnabled: false,
isStartingScan: true,
- setIsMultiScanEnabled: undefined,
};
}There was a problem hiding this comment.
I think this belongs to Phase 6, but I'm fine to do it here since it's not a big change.
There was a problem hiding this comment.
Yes, I think we should follow the pattern of OP and do this in phase 6.
There was a problem hiding this comment.
ok, sounds good we can do it in Phase 6
|
Resolving merge conflicts. |
…pScan-clean-up-p2
|
There's no prettier diff after running |
Merge main, run |
roryabraham
left a comment
roryabraham
left a comment
There was a problem hiding this comment.
approving (anticipating further cleanup in upcoming phases of the linked issue)
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.25-0 🚀
|
5 participants
Explanation of Change
Fixed Issues
$ #79929
PROPOSAL: N/A
Tests
Test 01
Test 02
Offline tests
QA Steps
Same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android.Native.mp4
Android: mWeb Chrome
Android.mWeb.Chrome.mp4
iOS: Native
IOS.Native.mp4
iOS: mWeb Safari
IOS.mWeb.Safari.mp4
MacOS: Chrome / Safari
macOS.Chrome.mp4