[TS migration] Migrate StepScreenWrapper, StepScreenDragAndDropWrapper, IOURequestStepConfirmation and IOURequestStepCategory pages to TypeScript by BrtqKr · Pull Request #39175 · Expensify/App

BrtqKr · 2024-03-28T12:58:29Z

Details

Fixed Issues

$ #38918
$ #38913
PROPOSAL:

Tests

Select a chat
Press + on the left side of the input
Press "request money"
Go through every possible flow selecting different currency, value, description, merchant, category, tag
Press request and validate if the money request resulted in a proper chat entry being displayed.

Verify that no errors appear in the JS console

Offline tests

Select a chat
Turn off the network connection, for example toggle the flight mode
Press + on the left side of the input
Press "request money"
Go through every possible flow selecting different currency, value, description, merchant, category, tag
Press request and validate if the money request resulted in a proper placeholder for the entry being displayed.
After turning on the network, the request should be sent, as in the standard flow

QA Steps

Same as in Tests/Offline tests section

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.2024-04-03.at.17.59.24.mov

Android: mWeb Chrome

Screen.Recording.2024-04-04.at.10.32.24.mov

iOS: Native

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-04-03.at.17.38.41.mp4

iOS: mWeb Safari

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-04-03.at.18.46.31.mp4

MacOS: Chrome / Safari

Screen.Recording.2024-04-04.at.10.56.39.mov

MacOS: Desktop

Screen.Recording.2024-04-04.at.10.40.11.mov

kubabutkiewicz · 2024-03-29T09:29:27Z

@BrtqKr any updates here ?

BrtqKr · 2024-03-29T09:39:22Z

@BrtqKr any updates here ?

only some parts of IOURequestStepCategory are left, but the transactions for the IOU utils calls haven't been validated at any point, so it's going a little bit slower than I expected

BrtqKr · 2024-04-02T09:05:56Z

src/libs/SidebarUtils.ts

-        alternateText: null,
+        alternateText: undefined,
        allReportErrors: OptionsListUtils.getAllReportErrors(report, reportActions),
        brickRoadIndicator: null,
        tooltipText: null,
        subtitle: null,
-        login: null,
-        accountID: null,
+        login: undefined,
+        accountID: undefined,
        reportID: '',
-        phoneNumber: null,
+        phoneNumber: undefined,
        isUnread: null,
        isUnreadWithMention: null,
        hasDraftComment: false,
-        keyForList: null,
-        searchText: null,
+        keyForList: undefined,
+        searchText: undefined,


@kubabutkiewicz from what I've seen you've been working on that part, was it necessary for everything in here to be marked as null? It's been causing conflicts with Participant type and since it had to be used as Participant | OptionData because of this:

const participants = useMemo( () => transaction?.participants?.map((participant) => { const participantAccountID = participant.accountID ?? 0; return participantAccountID ? OptionsListUtils.getParticipantsOption(participant, personalDetails) : OptionsListUtils.getReportOption(participant); }), [transaction?.participants, personalDetails], );

null and undefined discrepancy wouldn't allow this to go through. Additionally I don't see any places where this is checked for null explicitly

I am not sure 100%, but I think the null's was there when I started to work on that and I just left it

ok, I'll check it during tests then, thank you

…8913

blazejkustra · 2024-04-02T09:16:23Z

src/components/withCurrentUserPersonalDetails.tsx

    WithCurrentUserPersonalDetails.displayName = `WithCurrentUserPersonalDetails(${getComponentDisplayName(WrappedComponent)})`;

-    return React.forwardRef(WithCurrentUserPersonalDetails);
+    return WithCurrentUserPersonalDetails;


This might break a lot of components, I think forwardRef should stay here

leftover, in the end I used a hook, so there wouldn't be any type issues anyway

blazejkustra · 2024-04-02T09:17:59Z

src/libs/actions/IOU.ts

    Onyx.merge(`${ONYXKEYS.NVP_LAST_PAYMENT_METHOD}`, {[policyID]: paymentMethod});
 }

+export type {GPSPoint as GpsPoint};


Is this necessary?

It's used in two separate places and the other type originating from coords has many more fields, which aren't optional, so we probably want to avoid applying it. I can still define this type inline though, up to you 🤷

blazejkustra · 2024-04-02T09:18:35Z

src/pages/iou/request/step/IOURequestStepCategory.tsx

+    /** The draft transaction that holds data to be persisted on the current transaction */
+    splitDraftTransaction: OnyxEntry<Transaction>;
+    /** The policy of the report */
+    policy: OnyxEntry<Policy>;
+    /** Collection of categories attached to a policy */
+    policyCategories: OnyxEntry<PolicyCategories>;
+    /** Collection of tags attached to a policy */
+    policyTags: OnyxEntry<PolicyTagList>;
+    /** The actions from the parent report */
+    reportActions: OnyxEntry<ReportActions>;
+    /** Session info for the currently logged in user. */
+    session: OnyxEntry<Session>;


Nit: Add empty lines between props here

blazejkustra · 2024-04-02T09:22:49Z

src/pages/iou/request/step/IOURequestStepCategory.tsx

+
+function IOURequestStepCategory({
+    report,
+    stackProps: {


We don't use stackProps anywhere in the project, let's use route as before

Also you might face the problem described here, please have a look

blazejkustra · 2024-04-02T09:27:49Z

src/pages/iou/request/step/IOURequestStepCategory.tsx

+const IOURequestStepCategoryWithWritableReportOrNotFound = withWritableReportOrNotFound(IOURequestStepCategory);
+/* eslint-disable rulesdir/no-negated-variables */
+const IOURequestStepCategoryWithFullTransactionOrNotFound = withFullTransactionOrNotFound(IOURequestStepCategoryWithWritableReportOrNotFound);
+export default withOnyx<IOURequestStepCategoryProps, IOURequestStepCategoryOnyxProps>({


The order of hocs have changed a(b(c())) -> c(b(a())). This might break the component in very unexpected ways 😅

Here is how Marcin did it in other PR

blazejkustra · 2024-04-02T09:28:39Z

src/pages/iou/steps/MoneyRequestConfirmPage.js

Was it added by mistake? Or extracted from another file?

I changed some of the IOU utils to accept named params, but in the meantime, someone removed this whole file. I already got rid of it

blazejkustra · 2024-04-02T09:29:34Z

src/pages/iou/request/step/IOURequestStepConfirmation.tsx

+const IOURequestStepConfirmationWithWritableReportOrNotFound = withWritableReportOrNotFound(IOURequestStepConfirmation);
+/* eslint-disable rulesdir/no-negated-variables */
+const IOURequestStepConfirmationWithFullTransactionOrNotFound = withFullTransactionOrNotFound(IOURequestStepConfirmationWithWritableReportOrNotFound);
+export default withOnyx<IOURequestStepConfirmationProps, IOURequestStepConfirmationOnyxProps>({


…8913

blazejkustra · 2024-04-02T13:52:16Z

src/pages/iou/request/step/IOURequestStepCategory.tsx

+    const isEditingSplitBill = isEditing && iouType === CONST.IOU.TYPE.SPLIT;
+    const transactionCategory = ReportUtils.getTransactionDetails(isEditingSplitBill && !lodashIsEmpty(splitDraftTransaction) ? splitDraftTransaction : transaction)?.category;
+
+    const reportAction = reportActions?.[report?.parentReportActionID ?? reportActionID] ?? null;


Just saying: ?? instead of || might change the logic a bit, you might have to use eslint disable comment.

An in other places too, just be careful and let's test the component thoroughly

it didn't seem to break this, but I don't want to risk breaking anything, I'll leave it as it was

blazejkustra · 2024-04-02T13:55:35Z

src/pages/iou/request/step/IOURequestStepConfirmation.tsx

+    policy: OnyxEntry<Policy>;
+    policyCategories: OnyxEntry<PolicyCategories>;
+    policyTags: OnyxEntry<PolicyTagList>;
+    personalDetails: OnyxEntry<PersonalDetailsList>;


Let's not remove comments for props

blazejkustra · 2024-04-02T13:55:41Z

src/pages/iou/request/step/IOURequestStepConfirmation.tsx

-    report: reportPropTypes,
+type IOURequestStepConfirmationProps = IOURequestStepConfirmationOnyxProps &
+    WithWritableReportOrNotFoundProps<typeof SCREENS.MONEY_REQUEST.STEP_CONFIRMATION> & {
+        transaction: OnyxEntry<Transaction>;


blazejkustra · 2024-04-02T13:57:34Z

src/pages/iou/request/step/IOURequestStepConfirmation.tsx

+    personalDetails: {
+        key: ONYXKEYS.PERSONAL_DETAILS_LIST,
+    },


Go ahead and remove personalDetails from here, instead use useCurrentUserPersonalDetails hook, this should do the trick

it has already been removed on the main, so it got resolved during the branch update

blazejkustra · 2024-04-02T13:58:37Z

Also typecheck is failing @BrtqKr

fabioh8010 · 2024-04-04T05:37:02Z

Any updates here @BrtqKr ?

…8913

fedirjh · 2024-04-07T23:25:14Z

src/pages/iou/request/step/StepScreenWrapper.tsx

-StepScreenWrapper.defaultProps = defaultProps;
-
-export default StepScreenWrapper;
+export default forwardRef(StepScreenWrapper);


Suggested change

export default forwardRef(StepScreenWrapper);

StepScreenWrapper.displayName = 'StepScreenWrapper';

export default StepScreenWrapper;

Why do we use forwardRef in this component?

For some reason, when I was working on that, the steps where throwing errors during render attempt, but it might have been fixed when I changed the callOrReturn content. I already removed it

…8913

fedirjh · 2024-04-08T16:20:49Z

Bug: Cannot request money from new users (whom you have not chatted with before)

Select request money from the global creation menu
Select any amount and press next
Select an email address that you haven't chatted with before.
Press "request money"
Verify you are redirected to a new report and the money request is successful

Screen.Recording.2024-04-08.at.5.10.10.PM.mov

This is how it works on staging

Screen.Recording.2024-04-08.at.5.14.13.PM.mov

…8913

BrtqKr · 2024-04-09T09:53:31Z

@fedirjh I ran this reproduction and it seems to be working as expected. If there are any steps I need to include, that are not present in the recording, please let me know

Screen.Recording.2024-04-09.at.11.49.58.mov

fedirjh · 2024-04-09T14:18:10Z

If there are any steps I need to include, that are not present in the recording, please let me know

@BrtqKr It seems you have tested the split flow. However, the bug occurs with the request flow.

CleanShot.2024-04-09.at.15.16.52.mp4

fedirjh · 2024-04-09T14:44:10Z

src/pages/iou/request/step/IOURequestStepConfirmation.tsx

+            if (!report || !transaction) {
+                return;
+            }


Suggested change

if (!report || !transaction) {

return;

}

if (!transaction) {

return;

}

@BrtqKr This line is causing the bug in #39175 (comment) . We should allow creation of money requests when the report is not yet created.

fedirjh · 2024-04-09T16:16:28Z

It looks like we have some lint errors:

…8913

fedirjh

Looks good to me and tests well 🚀

melvin-bot · 2024-04-09T22:58:05Z

We did not find an internal engineer to review this PR, trying to assign a random engineer to #38918 as well as to this PR... Please reach out for help on Slack if no one gets assigned!

OSBotify · 2024-04-10T01:15:55Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-04-10T17:45:48Z

🚀 Deployed to staging by https://github.com/aldo-expensify in version: 1.4.62-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2024-04-10T17:54:41Z

🚀 Deployed to staging by https://github.com/aldo-expensify in version: 1.4.62-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2024-04-10T19:33:54Z

🚀 Deployed to staging by https://github.com/aldo-expensify in version: 1.4.62-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2024-04-18T04:31:39Z

🚀 Deployed to production by https://github.com/thienlnam in version: 1.4.62-17 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

ikevin127 · 2025-05-21T20:43:05Z

src/pages/iou/request/step/IOURequestStepCategory.tsx

+        <StepScreenWrapper
+            headerTitle={translate('common.category')}
+            onBackButtonPress={navigateBack}
+            shouldShowWrapper
+            shouldShowNotFoundPage={shouldShowNotFoundPage}
+            testID={IOURequestStepCategory.displayName}
+            includeSafeAreaPaddingBottom={false}
+        >


Because StepScreenWrapper has shouldEnableKeyboardAvoidingView set to true by default (when not passed), this led to the following issue:

[Due for payment 2025-05-28] [$250] Android - Category - Category lists below is blank when deselecting it. #61645

which we addressed by passing shouldEnableKeyboardAvoidingView={false} in order to disable the keyboard avoiding view.

BrtqKr added 8 commits March 25, 2024 16:24

migrate step screen wrapper

c495dc6

migrate step drag and drop wrapper

7833b99

migrate iouRequest wip

a738e50

apply prop types, general types changes wip

860c889

receipts types wip

cb63fd6

callback params cleanups

eb5bfcf

type compose, type function params

6e3d1d6

cleanup optional params, participants wip

cafcdab

BrtqKr added 4 commits March 29, 2024 12:25

make function params named, modify types, add transaction checks

c04e0a4

cleanup

7658a25

migrate iouRequestStepCategory, adjust route types

0e457d8

cleanup

6a947cc

BrtqKr commented Apr 2, 2024

View reviewed changes

BrtqKr added 2 commits April 2, 2024 11:10

cleanup

6845b0d

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

dfe2baa

…8913

blazejkustra suggested changes Apr 2, 2024

View reviewed changes

BrtqKr added 3 commits April 2, 2024 12:55

review fixes

9c8afd3

lint fix

236eec3

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

9f58b7b

…8913

blazejkustra suggested changes Apr 2, 2024

View reviewed changes

BrtqKr added 4 commits April 2, 2024 16:33

fix stepscreenwrapper

058dafa

fix step category

091da01

fix request money flow

5550a81

fix forwardref

7d23a77

BrtqKr added 2 commits April 4, 2024 11:30

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

23e9788

…8913

review fixes

431470b

fedirjh reviewed Apr 8, 2024

View reviewed changes

remove forwardRef

3fe9419

BrtqKr requested a review from fedirjh April 8, 2024 08:49

BrtqKr added 3 commits April 8, 2024 11:51

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

072cd8a

…8913

fix tests

9c4c43a

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

07967fb

…8913

BrtqKr added 2 commits April 9, 2024 09:51

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

e5ec150

…8913

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

9e17db0

…8913

fedirjh suggested changes Apr 9, 2024

View reviewed changes

BrtqKr added 2 commits April 9, 2024 22:31

fix request money flow

1314c4d

Merge remote-tracking branch 'origin/main' into brtqkr/ts-migration/3…

3d7f0b7

…8913

fedirjh approved these changes Apr 9, 2024

View reviewed changes

melvin-bot bot requested a review from aldo-expensify April 9, 2024 23:00

aldo-expensify approved these changes Apr 10, 2024

View reviewed changes

aldo-expensify merged commit aed0bf0 into Expensify:main Apr 10, 2024

github-actions bot mentioned this pull request Apr 10, 2024

Deploy Checklist: New Expensify 2024-04-10 #40026

Closed

ikevin127 reviewed May 21, 2025

View reviewed changes

Conversation

BrtqKr commented Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

kubabutkiewicz commented Mar 29, 2024

Uh oh!

BrtqKr commented Mar 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blazejkustra commented Apr 2, 2024

Uh oh!

fabioh8010 commented Apr 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fedirjh commented Apr 8, 2024

Uh oh!

BrtqKr commented Apr 9, 2024

Uh oh!

fedirjh commented Apr 9, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fedirjh commented Apr 9, 2024

Uh oh!

fedirjh left a comment

Choose a reason for hiding this comment

Uh oh!

melvin-bot bot commented Apr 9, 2024

Uh oh!

BrtqKr commented Mar 28, 2024 •

edited

Loading