fix: resolve comments

Author: thelullabyy

src/components/MoneyRequestConfirmationList/hooks/useFormErrorManagement.ts

@@ -3,7 +3,7 @@ import useLocalize from '@hooks/useLocalize';
 
 import {isAttendeeTrackingEnabled} from '@libs/PolicyUtils';
 import {areRequiredFieldsEmpty, getTag, hasMissingSmartscanFields, isMerchantMissing} from '@libs/TransactionUtils';
-import {isInvalidMerchantValue, isValidInputLength} from '@libs/ValidationUtils';
+import {isInvalidMerchantValue, isUntypedPlaceholderMerchant, isValidInputLength} from '@libs/ValidationUtils';
 import {getIsViolationFixed} from '@libs/Violations/ViolationsUtils';
 
 import CONST from '@src/CONST';
@@ -172,8 +172,7 @@ function useFormErrorManagement({
             return false;
         }
 
-        const isUntypedPlaceholder = !transaction?.isMerchantSet && isInvalidMerchantValue(trimmedMerchant);
-        if (!trimmedMerchant || isUntypedPlaceholder) {
+        if (!trimmedMerchant || isUntypedPlaceholderMerchant(transaction?.isMerchantSet, trimmedMerchant)) {
             return !isMerchantRequired;
         }
 

src/components/MoneyRequestConfirmationList/sections/MerchantField.tsx

@@ -10,7 +10,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
 import {clearMoneyRequestMerchant, setMoneyRequestMerchant} from '@libs/actions/IOU/MoneyRequest';
 import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute';
 import Navigation from '@libs/Navigation/Navigation';
-import {isInvalidMerchantValue, isValidInputLength} from '@libs/ValidationUtils';
+import {isUntypedPlaceholderMerchant, isValidInputLength} from '@libs/ValidationUtils';
 
 import {setDraftSplitTransaction} from '@userActions/IOU/Split';
 
@@ -62,7 +62,7 @@ function MerchantField({
     const merchantState = useTransactionSelector(transactionID, merchantStateSelector);
 
     const merchantValue = merchantState?.merchant ?? '';
-    const displayMerchantValue = !merchantState?.isMerchantSet && isInvalidMerchantValue(merchantValue) ? '' : merchantValue;
+    const displayMerchantValue = isUntypedPlaceholderMerchant(merchantState?.isMerchantSet, merchantValue) ? '' : merchantValue;
     const transactionHasReceipt = merchantState?.hasReceipt ?? false;
 
     // Determine if the merchant error should be displayed

src/libs/ValidationUtils.ts

@@ -818,6 +818,14 @@ function isInvalidMerchantValue(merchant?: string): boolean {
     return merchant === '' || merchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT || merchant === CONST.TRANSACTION.DEFAULT_MERCHANT;
 }
 
+/**
+ * Checks if a merchant is a placeholder the user never typed: the flow seeded the "Expense" / "(none)" value,
+ * so it should be treated as empty rather than as an invalid entry the user is responsible for.
+ */
+function isUntypedPlaceholderMerchant(isMerchantSet: boolean | undefined, merchant?: string): boolean {
+    return !isMerchantSet && isInvalidMerchantValue(merchant);
+}
+
 /**
  * Validates a 4-digit PIN for UK/EU Expensify Card.
  * PIN must be exactly 4 digits and not in the list of invalid/weak PINs.
@@ -926,6 +934,7 @@ export {
     isValidInputLength,
     isValidTaxIDEINNumber,
     isInvalidMerchantValue,
+    isUntypedPlaceholderMerchant,
     isValidPIN,
     containsHtmlTag,
 };

src/pages/iou/request/IOURequestStartPage.tsx

@@ -224,6 +224,11 @@ function IOURequestStartPage({
 
     const shouldShowWorkspaceSelectForPerDiem = moreThanOnePerDiemExist && !hasCurrentPolicyPerDiemEnabled;
 
+    // Every flow that reaches this page embeds the confirmation as its landing step except INVOICE, which stays on the
+    // amount-first flow. (`shouldUseTab` also excludes SEND, but the money-request route params type it away, so PAY is
+    // the only type this has to add back.)
+    // The pay quick action still writes SKIP_CONFIRMATION, but IOURequestStepAmount is its only reader and no longer
+    // mounts for PAY - the embedded confirmation carries the amount inline, so there is no separate step left to skip.
     const shouldEmbedConfirmation = isNewManualExpenseFlowEnabled && (shouldUseTab || iouType === CONST.IOU.TYPE.PAY);
 
     let manualContent: React.ReactNode;
@@ -237,7 +242,10 @@ function IOURequestStartPage({
                 reportDraft={reportDraft}
             />
         );
-    } else if (isScanRequest(transaction) || isPerDiemRequest(transaction)) {
+    } else if (shouldUseTab && (isScanRequest(transaction) || isPerDiemRequest(transaction))) {
+        // Only the tabbed flows can land here with a stale draft, and only they run the reset that clears it
+        // (`resetIOUTypeIfChanged` is wired to `onTabSelected` below). PAY renders no tabs, so it must skip this
+        // branch or a leftover scan/per-diem draft would strand it on a loader with no way out but the back button.
         // When switching from the Scan or Per diem tab, the shared draft is briefly still a scan/per-diem request
         // until the tab-switch reset rebuilds it as manual. Mounting the embedded confirmation against that stale
         // draft does throwaway work that is immediately discarded once the reset lands - for scan a heavy first

tests/ui/IOURequestStartPageManualTabTest.tsx

@@ -21,6 +21,7 @@ const TRANSACTION_ID = 'transaction1';
 const CONFIRMATION_TEST_ID = 'EmbeddedConfirmation';
 const LOADER_TEST_ID = 'manualTabPendingReset';
 const AMOUNT_TEST_ID = 'EmbeddedAmount';
+const CURRENT_USER_EMAIL = 'invoice.sender@example.com';
 
 jest.mock('@userActions/Tab');
 jest.mock('@rnmapbox/maps', () => ({
@@ -87,10 +88,21 @@ describe('IOURequestStartPage manual tab content', () => {
         });
     });
 
+    type RenderStartPageOptions = {
+        /** The request type the shared draft still carries when the page mounts. */
+        iouRequestType: IOURequestType;
+
+        /** The flow the page is started for - this is what decides whether tabs are rendered. */
+        iouType?: IOUType;
+
+        /** Whether the new manual expense flow beta is on. */
+        isNewManualExpenseFlowEnabled?: boolean;
+    };
+
     /**
      * Seeds the beta, the manual tab selection and a draft transaction of the given request type, then renders the page.
      */
-    async function renderStartPageWithDraftType(iouRequestType: IOURequestType, iouType: IOUType = CONST.IOU.TYPE.SUBMIT, isNewManualExpenseFlowEnabled = true) {
+    async function renderStartPage({iouRequestType, iouType = CONST.IOU.TYPE.SUBMIT, isNewManualExpenseFlowEnabled = true}: RenderStartPageOptions) {
         await act(async () => {
             await Onyx.set(ONYXKEYS.BETAS, isNewManualExpenseFlowEnabled ? [CONST.BETAS.NEW_MANUAL_EXPENSE_FLOW] : []);
             await Onyx.set(`${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.IOU_REQUEST_TYPE}`, CONST.TAB_REQUEST.MANUAL);
@@ -130,7 +142,7 @@ describe('IOURequestStartPage manual tab content', () => {
 
     it('shows a loader instead of the embedded confirmation while a per diem draft is still pending its reset to manual', async () => {
         // Given the new manual expense flow beta and a draft that is still a per diem request
-        await renderStartPageWithDraftType(CONST.IOU.REQUEST_TYPE.PER_DIEM);
+        await renderStartPage({iouRequestType: CONST.IOU.REQUEST_TYPE.PER_DIEM});
 
         // Then the manual tab waits for the reset instead of mounting the confirmation against the per diem draft
         expect(screen.getByTestId(LOADER_TEST_ID)).toBeOnTheScreen();
@@ -139,7 +151,7 @@ describe('IOURequestStartPage manual tab content', () => {
 
     it('shows a loader instead of the embedded confirmation while a scan draft is still pending its reset to manual', async () => {
         // Given the new manual expense flow beta and a draft that is still a scan request
-        await renderStartPageWithDraftType(CONST.IOU.REQUEST_TYPE.SCAN);
+        await renderStartPage({iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN});
 
         // Then the manual tab waits for the reset instead of mounting the confirmation against the scan draft
         expect(screen.getByTestId(LOADER_TEST_ID)).toBeOnTheScreen();
@@ -148,7 +160,7 @@ describe('IOURequestStartPage manual tab content', () => {
 
     it('shows the embedded confirmation once the draft is a manual request', async () => {
         // Given the new manual expense flow beta and a draft that has been reset to a manual request
-        await renderStartPageWithDraftType(CONST.IOU.REQUEST_TYPE.MANUAL);
+        await renderStartPage({iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL});
 
         // Then the confirmation is mounted and the pending-reset loader is gone
         expect(screen.getByTestId(CONFIRMATION_TEST_ID)).toBeOnTheScreen();
@@ -157,16 +169,45 @@ describe('IOURequestStartPage manual tab content', () => {
 
     it('lands the tab-less pay flow directly on the embedded confirmation instead of the amount page', async () => {
         // Given the new manual expense flow beta and a pay flow, which renders no tabs
-        await renderStartPageWithDraftType(CONST.IOU.REQUEST_TYPE.MANUAL, CONST.IOU.TYPE.PAY);
+        await renderStartPage({iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL, iouType: CONST.IOU.TYPE.PAY});
 
         // Then the details page is the landing page, so the amount page is never shown first
         expect(screen.getByTestId(CONFIRMATION_TEST_ID)).toBeOnTheScreen();
         expect(screen.queryByTestId(AMOUNT_TEST_ID)).not.toBeOnTheScreen();
     });
 
+    it('does not strand the tab-less pay flow on the pending-reset loader when the draft is still a scan request', async () => {
+        // Given a pay flow that mounts against a leftover scan draft from an earlier expense in the same chat
+        await renderStartPage({iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, iouType: CONST.IOU.TYPE.PAY});
+
+        // Then the confirmation mounts anyway - the pay flow renders no tabs, so the reset the loader waits on never runs
+        expect(screen.getByTestId(CONFIRMATION_TEST_ID)).toBeOnTheScreen();
+        expect(screen.queryByTestId(LOADER_TEST_ID)).not.toBeOnTheScreen();
+    });
+
+    it('keeps the amount page as the landing page for the invoice flow', async () => {
+        // Given an invoice flow, which is the one type left off the embedded confirmation
+        await act(async () => {
+            // AccessOrNotFoundWrapper gates the invoice flow behind an admin workspace that can send invoices.
+            await Onyx.set(ONYXKEYS.SESSION, {email: CURRENT_USER_EMAIL, accountID: 1});
+            await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}policy1`, {
+                id: 'policy1',
+                name: 'Invoice workspace',
+                type: CONST.POLICY.TYPE.TEAM,
+                role: CONST.POLICY.ROLE.ADMIN,
+                areInvoicesEnabled: true,
+            });
+        });
+        await renderStartPage({iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL, iouType: CONST.IOU.TYPE.INVOICE});
+
+        // Then it still lands on the amount page first
+        expect(screen.getByTestId(AMOUNT_TEST_ID)).toBeOnTheScreen();
+        expect(screen.queryByTestId(CONFIRMATION_TEST_ID)).not.toBeOnTheScreen();
+    });
+
     it('keeps the amount page as the landing page for the pay flow when the beta is off', async () => {
         // Given a pay flow started without the new manual expense flow beta
-        await renderStartPageWithDraftType(CONST.IOU.REQUEST_TYPE.MANUAL, CONST.IOU.TYPE.PAY, false);
+        await renderStartPage({iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL, iouType: CONST.IOU.TYPE.PAY, isNewManualExpenseFlowEnabled: false});
 
         // Then the legacy amount-first flow is preserved
         expect(screen.getByTestId(AMOUNT_TEST_ID)).toBeOnTheScreen();

tests/unit/hooks/useFormErrorManagement.test.tsx

@@ -50,8 +50,16 @@ const baseParams: Params = {
 };
 
 // A manual draft the user never typed a merchant into: `initMoneyRequest` seeds it with the "Expense" placeholder.
+const placeholderMerchantTransaction = createMock<OnyxTypes.Transaction>({
+    transactionID: 'txn1',
+    amount: 100,
+    merchant: CONST.TRANSACTION.DEFAULT_MERCHANT,
+    isMerchantSet: false,
+    comment: {},
+});
+
 const placeholderMerchantParams: Partial<Params> = {
-    transaction: createMock<OnyxTypes.Transaction>({transactionID: 'txn1', amount: 100, merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, isMerchantSet: false, comment: {}}),
+    transaction: placeholderMerchantTransaction,
     iouMerchant: CONST.TRANSACTION.DEFAULT_MERCHANT,
 };
 
@@ -169,13 +177,7 @@ describe('useFormErrorManagement', () => {
                 useFormErrorManagement({
                     ...baseParams,
                     ...placeholderMerchantParams,
-                    transaction: createMock<OnyxTypes.Transaction>({
-                        transactionID: 'txn1',
-                        amount: 100,
-                        merchant: CONST.TRANSACTION.DEFAULT_MERCHANT,
-                        isMerchantSet: true,
-                        comment: {},
-                    }),
+                    transaction: {...placeholderMerchantTransaction, isMerchantSet: true},
                     isPolicyExpenseChat: false,
                 }),
             {wrapper: Wrapper},
@@ -184,7 +186,7 @@ describe('useFormErrorManagement', () => {
         expect(result.current.isMerchantFieldValid).toBe(false);
     });
 
-    it('clears the invalid merchant error once the recipient changes from a workspace chat to a user (#96593)', async () => {
+    it('clears the invalid merchant error once the recipient changes from a workspace chat to a user (#96593)', () => {
         // Given an untouched manual draft (still carrying the placeholder merchant) headed for a workspace chat
         const {result, rerender} = renderHook(
             ({isPolicyExpenseChat}: {isPolicyExpenseChat: boolean}) =>