Refactor iou page by separating each step as individual screen #17964

bernhardoj · 2023-04-25T12:22:49Z

Details

Participants selection is lost when the component unmount. This PR refactor money request page to have their own screen for each step and save the currency, amount, description, and participants in a context provider.

Fixed Issues

$ #17662
PROPOSAL: #17662 (comment)

Tests

Same as QA Steps

Verify that no errors appear in the JS console

Offline tests

Same as QA Steps

QA Steps

Open a group chat (chat with >=3 members)
Press the "+" button in composer
Press Split Bill
Enter any amount and press Next
Unselect any amount of participants
Press Amount to edit the amount
Go back
Verify the unselected participants stay unselected
Go back again
Press Next
Verify the same thing on step 8

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2023-05-19.at.19.27.50.mov

Mobile Web - Chrome

WhatsApp.Video.2023-05-19.at.20.22.35.mp4

Mobile Web - Safari

Screen.Recording.2023-05-19.at.19.52.03.mov

Desktop

Screen.Recording.2023-05-19.at.19.36.20.mov

iOS

Screen.Recording.2023-05-19.at.19.50.47.mov

Android

WhatsApp.Video.2023-05-19.at.20.22.34.mov

MelvinBot · 2023-04-25T12:22:55Z

@PauloGasparSv @thesahindia One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

bernhardoj · 2023-04-25T12:27:38Z

Additional video to make sure iou request is not broken:

Send money

Screen.Recording.2023-05-19.at.18.20.13.mov

Send money (from FAB)

Screen.Recording.2023-05-19.at.18.32.03.mov

Request money

Screen.Recording.2023-05-19.at.18.23.45.mov

Request money (from FAB)

Screen.Recording.2023-05-19.at.18.32.38.mov

Split money (from FAB)

Screen.Recording.2023-05-19.at.20.38.09.mov

Split money

Screen.Recording.2023-05-19.at.20.37.35.mov

Request Money in Policy Expense Chat

Screen.Recording.2023-06-02.at.22.26.13.mov

Request Money in Policy Expense Chat (FAB)

Screen.Recording.2023-06-02.at.22.36.11.mov

bernhardoj · 2023-04-25T12:30:36Z

Details of the refactor:

MoneyRequestModal removed
Each step (amount, participants, and confirmation) now has their own screen wrapped with ScreenWrapper
Amount, currency, participants, and description is stored in Onyx.
Logic from MoneyRequestModal moved to the appropriate step page, for example createTransaction moved to MoneyRequestConfirmPage
Because the IOU type is now taken from the route params, it's possible for user to include invalid IOU type, thus I added a FullPageNotFoundView to show not found interface when the IOU type is not one of send, request, or split.

bernhardoj · 2023-04-25T12:39:56Z

src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

 };

 const MoneyRequestParticipantsPage = (props) => {
-    if (props.iou.loading) {


Removed because there will be never a loading happen when we load the participants.

bernhardoj · 2023-04-25T12:43:17Z

src/pages/iou/steps/MoneyRequestAmountPage.js

+        if (this.isEditing) {
+            this.props.redirectIfEmpty([this.props.participants, this.props.amount], this.iouType, this.reportID);
+        } else {
+            // Set the money request participants based on the report participants


Taken from here

App/src/pages/iou/MoneyRequestModal.js

Lines 119 to 123 in d19326e

const [selectedOptions, setSelectedOptions] = useState(

ReportUtils.isPolicyExpenseChat(props.report)

? OptionsListUtils.getPolicyExpenseReportOptions(props.report)

: OptionsListUtils.getParticipantsOptions(props.report, props.personalDetails),

);

bernhardoj · 2023-04-25T12:43:57Z

src/pages/iou/steps/MoneyRequestAmountPage.js

+     *
+     * @returns {String}
+     */
+    getTitleForStep() {


Taken from here

App/src/pages/iou/MoneyRequestModal.js

Lines 197 to 210 in d19326e

const titleForStep = useMemo(() => {

if (currentStepIndex === 0) {

const confirmIndex = _.indexOf(steps, Steps.MoneyRequestConfirm);

if (previousStepIndex === confirmIndex) {

return props.translate('iou.amount');

}

if (props.iouType === CONST.IOU.MONEY_REQUEST_TYPE.SEND) {

return props.translate('iou.sendMoney');

}

return props.translate(props.hasMultipleParticipants ? 'iou.splitBill' : 'iou.requestMoney');

}

return props.translate('iou.cash');

// eslint-disable-next-line react-hooks/exhaustive-deps -- props does not need to be a dependency as it will always exist

}, [currentStepIndex, props.translate, steps]);

src/pages/iou/steps/MoneyRequestConfirmPage.js

bernhardoj · 2023-04-25T12:45:53Z

src/pages/iou/steps/MoneyRequestConfirmPage.js

+        prevCreatingIOUTransactionStatusRef.current = lodashGet(props.iou, 'creatingIOUTransaction');
+    });
+
+    const createTransaction = useCallback((selectedParticipants) => {


Taken from here

App/src/pages/iou/MoneyRequestModal.js

Lines 312 to 356 in d19326e

const createTransaction = useCallback((selectedParticipants) => {

const reportID = lodashGet(props.route, 'params.reportID', '');

const trimmedComment = props.iou.comment.trim();

// IOUs created from a group report will have a reportID param in the route.

// Since the user is already viewing the report, we don't need to navigate them to the report

if (props.hasMultipleParticipants && CONST.REGEX.NUMBER.test(reportID)) {

IOU.splitBill(

selectedParticipants,

props.currentUserPersonalDetails.login,

amount,

trimmedComment,

props.iou.selectedCurrencyCode,

props.preferredLocale,

reportID,

);

return;

}

// If the request is created from the global create menu, we also navigate the user to the group report

if (props.hasMultipleParticipants) {

IOU.splitBillAndOpenReport(

selectedParticipants,

props.currentUserPersonalDetails.login,

amount,

trimmedComment,

props.iou.selectedCurrencyCode,

props.preferredLocale,

);

return;

}

if (!selectedParticipants[0].login) {

// TODO - request to the policy expense chat. Not implemented yet!

// Will be implemented here: https://github.com/Expensify/Expensify/issues/270581

return;

}

IOU.requestMoney(

props.report,

Math.round(amount * 100),

props.iou.selectedCurrencyCode,

props.currentUserPersonalDetails.login,

selectedParticipants[0],

trimmedComment,

);

}, [amount, props.iou.comment, props.currentUserPersonalDetails.login, props.hasMultipleParticipants, props.iou.selectedCurrencyCode, props.preferredLocale, props.report, props.route]);

bernhardoj · 2023-04-25T12:46:27Z

src/pages/iou/steps/MoneyRequestConfirmPage.js

+     *
+     * @param {String} paymentMethodType
+     */
+    const sendMoney = useCallback((paymentMethodType) => {


Taken from here

App/src/pages/iou/MoneyRequestModal.js

Lines 267 to 307 in d19326e

const sendMoney = useCallback((paymentMethodType) => {

const amountInDollars = Math.round(amount * 100);

const currency = props.iou.selectedCurrencyCode;

const trimmedComment = props.iou.comment.trim();

const participant = selectedOptions[0];

if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.ELSEWHERE) {

IOU.sendMoneyElsewhere(

props.report,

amountInDollars,

currency,

trimmedComment,

props.currentUserPersonalDetails.login,

participant,

);

return;

}

if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.PAYPAL_ME) {

IOU.sendMoneyViaPaypal(

props.report,

amountInDollars,

currency,

trimmedComment,

props.currentUserPersonalDetails.login,

participant,

);

return;

}

if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.EXPENSIFY) {

IOU.sendMoneyWithWallet(

props.report,

amountInDollars,

currency,

trimmedComment,

props.currentUserPersonalDetails.login,

participant,

);

}

}, [amount, props.iou.comment, selectedOptions, props.currentUserPersonalDetails.login, props.iou.selectedCurrencyCode, props.report]);

src/pages/iou/withMoneyRequest.js

bernhardoj · 2023-05-19T12:41:39Z

@PauloGasparSv @s77rt Ready for review!

mountiny · 2023-06-20T19:46:58Z

More merge conflicts

thienlnam

Lots of changes! Just a few comments but it's looking good

thienlnam · 2023-06-20T20:52:54Z

src/libs/actions/IOU.js

+ * @param {String} reportID
+ */
+function startMoneyRequest(iouType, reportID = '') {
+    resetMoneyRequestInfo(`${iouType}${reportID}`);


Do we need to call this when we start a money request? It looks like we always call resetMoneyRequestInfo whenever we complete a command so is this redundant?

I think we still need this if you abandon the money request (close money request modal without completing the process)

But then at that point don't you want to continue the money request? I guess this is a NAB though

I think we always want to clear it when we open it from FAB or Composer. We're following the same pattern with Assign Task.

thienlnam · 2023-06-20T20:57:55Z

src/pages/iou/steps/MoneyRequestAmountPage.js

@@ -74,24 +106,63 @@ class MoneyRequestAmountPage extends React.Component {
    }

    componentDidMount() {
-        this.focusTextInput();
+        if (this.isEditing) {


Hmm I wonder if we should handle these shouldReset / isEditing in a different way that is more DRY

Are these to handle new money request drafts?

I was thinking to create a custom hook as the reset logic is being used in the amount, participant, confirmation, and description page, but both the amount and description page is still class component.

Are these to handle new money request drafts?

The reset logic is to prevent the user from doing a money request of type A with money request data of type B. For example, doing a send request with saved split request data. This is possible by directly accessing the URL (change from /send/new/confirmation to /split/new/confirmation). More context here #17964 (comment)

thienlnam · 2023-06-20T20:58:35Z

src/pages/iou/steps/MoneyRequestAmountPage.js

+            const selectedAmountAsString = this.props.iou.amount ? CurrencyUtils.convertToWholeUnit(this.props.iou.currency, this.props.iou.amount).toString() : '';
+            this.setState({
+                amount: selectedAmountAsString,
+                selection: {


What is this for?

Hmm, can you specify what you are referring to?

To put the cursor at the end of the number. We're following the same logic from constructor

App/src/pages/iou/steps/MoneyRequestAmountPage.js

Lines 97 to 106 in 7c04c2f

this.state = {

amount: selectedAmountAsString,

selectedCurrencyCode: props.iou.currency,

shouldUpdateSelection: true,

selection: {

start: selectedAmountAsString.length,

end: selectedAmountAsString.length,

},

};

}

bernhardoj · 2023-06-21T05:16:27Z

Conflicts solved! Both issues here #17964 (comment) are solved too.

Added some default values in getParticipantsOptions in case the user does not exist in personal details. Previously, login and displayName is always returned from getPersonalDetailsByLogins.
bernhardoj@ef3d955#diff-733528d957ae6a80eb863637fa5d54278c89c6121998f3574fa8c0aba935daebL181-L187

mountiny

Great work on this one. @s77rt I hope this was tested thorough as this is a big refactoring :D

One change requested

mountiny · 2023-06-21T10:13:13Z

src/libs/Navigation/AppNavigator/ModalStackNavigators.js

+    },
+    {
+        getComponent: () => {
+            const MoneyRequestParticipantsPage = require('../../../pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage').default;


That is a mistake no?

Suggested change

const MoneyRequestParticipantsPage = require('../../../pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage').default;

const MoneyRequestParticipantsPage = require('../../../pages/iou/steps/MoneyRequstParticipantsPage').default;

Nope. That is how it's structured

But I realized the folder name has a typo Requst 🤣. Should we fix it now? I'm afraid changing it will make the review hard.

Ah that is confusing.

Lets fix it in a follow up

mountiny · 2023-06-21T10:20:19Z

src/libs/OptionsListUtils.js

+function getParticipantsOptions(participants, personalDetails) {
+    const details = getPersonalDetailsForAccountIDs(_.pluck(participants, 'accountID'), personalDetails);
+    return _.map(participants, (participant) => {


Why did we have to change how this loop works?

Because participants is now an array of objects, it looks like this: [{accountID: 1, selected: true, ...}]. Previously, it was a report object and we take the participantAccountIDs which is just an array.

Also, we need selected attribute from each participant.

src/libs/actions/IOU.js

All good

s77rt · 2023-06-21T12:59:49Z

src/libs/OptionsListUtils.js

@@ -213,20 +213,22 @@ function getParticipantsOptions(participants, personalDetails) {
    const details = getPersonalDetailsForAccountIDs(_.pluck(participants, 'accountID'), personalDetails);
    return _.map(participants, (participant) => {
        const detail = details[participant.accountID];
+        const login = detail.login || participant.login;


Why we are falling back to participant.login? i.e. if login is undefined, what would be wrong with that?

The login is needed for displayName, alternateText, and icons.name

s77rt · 2023-06-21T13:04:00Z

src/libs/OptionsListUtils.js

@@ -213,20 +213,22 @@ function getParticipantsOptions(participants, personalDetails) {
    const details = getPersonalDetailsForAccountIDs(_.pluck(participants, 'accountID'), personalDetails);
    return _.map(participants, (participant) => {
        const detail = details[participant.accountID];
+        const login = detail.login || participant.login;
+        const displayName = detail.displayName || LocalePhoneNumber.formatPhoneNumber(login);


I don't think we need a fallback here either. getPersonalDetailsForAccountIDs will return the data from onyx and displayName seems always set.

The fallback is for a case where the user doesn't exist, for example when you search a random email. More explanation in my previous comment here #17964 (comment)

s77rt · 2023-06-21T13:05:08Z

src/libs/OptionsListUtils.js

-            alternateText: Str.isSMSLogin(detail.login || '') ? LocalePhoneNumber.formatPhoneNumber(detail.login) : detail.login || detail.displayName,
+            alternateText: LocalePhoneNumber.formatPhoneNumber(login) || displayName,


What is this change for? login is not guaranteed to be a phone number

Just simplifying it actually 😅. LocalePhoneNumber.formatPhoneNumber(login) will return it untouched if it's not a phone number.

App/src/libs/LocalePhoneNumber.js

Lines 19 to 33 in 0b7455f

function formatPhoneNumber(number) {

if (!number) {

return '';

}

const numberWithoutSMSDomain = Str.removeSMSDomain(number);

const parsedPhoneNumber = parsePhoneNumber(numberWithoutSMSDomain);

// return the string untouched if it's not a phone number

if (!parsedPhoneNumber.valid) {

if (parsedPhoneNumber.number && parsedPhoneNumber.number.international) {

return parsedPhoneNumber.number.international;

}

return numberWithoutSMSDomain;

}

We also did (use LocalePhoneNumber.formatPhoneNumber(login) without extra checking) that previously here
bernhardoj@ef3d955#diff-733528d957ae6a80eb863637fa5d54278c89c6121998f3574fa8c0aba935daebL181-L187

s77rt · 2023-06-21T13:05:53Z

src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsSelector.js

@@ -140,7 +140,7 @@ class MoneyRequestParticipantsSelector extends Component {
     * @param {Object} option
     */
    addSingleParticipant(option) {
-        this.props.onAddParticipants([{accountID: option.accountID, isPolicyExpenseChat: option.isPolicyExpenseChat, reportID: option.reportID, selected: true}]);
+        this.props.onAddParticipants([{accountID: option.accountID, login: option.login, isPolicyExpenseChat: option.isPolicyExpenseChat, reportID: option.reportID, selected: true}]);


Do we still need the login here? Can't we drop login now and just relay on accountID?

It's needed for all things above.

thienlnam

Alright, going to send it since Vit's comments were addressed and so we can get this tested heavily through QA

Nice work on this!

OSBotify · 2023-06-21T17:00:56Z

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

marcaaron · 2023-06-21T18:25:45Z

src/pages/iou/MoneyRequestDescriptionPage.js

+                Navigation.goBack(ROUTES.getMoneyRequestRoute(this.iouType, this.reportID), true);
+            }
+        }
+    }


This code need to be documented better. It is not obvious at all what is happening here. Please explain in the code what "reset" means in this context and what an iou.id even is? When did we start adding "id" to the IOU and why...

We start to add an ID to avoid a problem explained here #17964 (comment). The ID is iouType + reportID

App/src/pages/iou/MoneyRequestDescriptionPage.js

Lines 53 to 54 in e6c4b4e

const moneyRequestId = `${this.iouType}${this.reportID}`;

const shouldReset = this.props.iou.id !== moneyRequestId;

Every time we start a money request, it will set an ID to it

App/src/libs/actions/IOU.js

Lines 1298 to 1301 in e6c4b4e

function startMoneyRequest(iouType, reportID = '') {

resetMoneyRequestInfo(`${iouType}${reportID}`);

Navigation.navigate(ROUTES.getMoneyRequestRoute(iouType, reportID));

}

what "reset" means

I think this can be found in the function itself.

App/src/libs/actions/IOU.js

Lines 65 to 69 in e6c4b4e

/**

* Reset money request info from the store with its initial value

* @param {String} id

*/

function resetMoneyRequestInfo(id = '') {

Inspired from

App/src/libs/actions/Task.js

Lines 16 to 21 in e6c4b4e

/**

* Clears out the task info from the store

*/

function clearOutTaskInfo() {

Onyx.set(ONYXKEYS.TASK, null);

}

I would appreciate it if the code could be annotated more thoroughly. Enhancing code documentation is essential for our team's effectiveness. Can we kindly address this?

Regarding the current solution, I remain uncertain about its efficiency. It appears more like a workaround to me, rather than a refined resolution.

Sure, I can create a follow-up PR to add the comment and fix this typo #17964 (comment)

But, should we wait until the regression period ends before opening the PR?

Regarding the current solution, I remain uncertain about its efficiency. It appears more like a workaround to me, rather than a refined resolution.

I can't say for sure whether it's a workaround or not, but at least that's what we can come up with considering so many cases using Onyx.

@bernhardoj It might be good to document the code now if not done already. I am also wondering about the logic done here.

Yes, but to do that, I'm thinking of a plan.

Currently, this reset logic exists on the Money request amount, participant, description, and confirm page, all basically have the same logic. I don't want to repeatedly write the comment on each page, so what about we create a custom hook called useMoneyRequestEffect that handles this logic? If you agree,

We need to migrate MoneyRequestDescriptionPage to a functional component, it's a fairly simple page, I can handle that.

Currently, MoneyRequestAmountPage is being used for the amount and edit amount page.

App/src/libs/Navigation/AppNavigator/ModalStackNavigators.js

Lines 38 to 49 in cc5526b

getComponent: () => {

const MoneyRequestAmountPage = require('../../../pages/iou/steps/MoneyRequestAmountPage').default;

return MoneyRequestAmountPage;

},

name: 'Money_Request',

},

{

getComponent: () => {

const MoneyRequestEditAmountPage = require('../../../pages/iou/steps/MoneyRequestAmountPage').default;

return MoneyRequestEditAmountPage;

},

name: 'Money_Request_Amount',

We actually want this logic for the edit amount page only and I don't think conditionally using hook is correct.

if (isEditing.current) useMoneyRequestEffect();

I see we have an issue #23149 to make MoneyRequestAmountPage more reusable, we can wait for this to complete and then separate the page for the edit amount page.

OR

Replace the Onyx with Context approach 😄. This will completely remove this reset logic thing.

@parasharrajat @marcaaron

I agree with that issue. IMO, the logic is fairly complex and confusing. Refactor is a good thing to do. We can wait on that refactor or you can help write proper comments during the refactor.

So, migrating description page to functional and create useMoneyRequestEffect after MoneyRequestAmountPage refactor is done. Is that correct?

Kind of. let's wait and see the end result of refactoring and then we can re-evaluate.

@parasharrajat the refactor is done. What is the process here to do the follow-up? Should I just open the PR?

As a reminder, here is the plan I have:

Separate edit amount from NewRequestAmountPage to its own page

Create a new hook called useMoneyRequestEffect that will contain this logic

App/src/pages/iou/MoneyRequestDescriptionPage.js

Lines 60 to 70 in fe02d63

useEffect(() => {

const moneyRequestId = `${iouType}${reportID}`;

const shouldReset = iou.id !== moneyRequestId;

if (shouldReset) {

IOU.resetMoneyRequestInfo(moneyRequestId);

}

if (_.isEmpty(iou.participants) || (iou.amount === 0 && !iou.receiptPath) || shouldReset) {

Navigation.goBack(ROUTES.getMoneyRequestRoute(iouType, reportID), true);

}

}, [iou.id, iou.participants, iou.amount, iou.receiptPath, iouType, reportID]);

Replace the above logic (and on other pages) with the new hook.

Update the comment in the new hook

OSBotify · 2023-06-22T17:21:32Z

🚀 Deployed to staging by https://github.com/thienlnam in version: 1.3.31-0 🚀

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

OSBotify · 2023-06-23T21:06:28Z

🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.31-3 🚀

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

OSBotify · 2023-06-23T21:26:13Z

🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.31-3 🚀

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

eVoloshchak · 2023-08-02T15:49:27Z

src/pages/iou/steps/MoneyRequestAmountPage.js

        }

-        this.setState({selectedCurrencyCode: this.props.iou.selectedCurrencyCode});
+        if (prevProps.iou.amount !== this.props.iou.amount) {


This has caused a small regression in #22361
The dot wasn't removed from the end of amount when navigating back to the screen (in an edge case where amount is the same, more details here)
We resolved this by moving CurrencyUtils.convertToWholeUnit logic into a separate method and calling it in useFocusEffect (when page is focused)

tgolen · 2023-08-24T20:09:44Z

src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

+        amount: PropTypes.number,
+        participants: PropTypes.arrayOf(
+            PropTypes.shape({
+                accountID: PropTypes.number,


Could you please come back and update this code to include comments on all these properties?

Add descriptions to all propTypes using a single line comment above the definition. No need to document the types, but add some context for each property so that other developers understand the intended use.

https://github.com/Expensify/Style-Guides/blob/main/react.md#things-to-do

Sure, here is the PR

refactor iou page by seperating each step as individual screen

c586193

bernhardoj requested a review from a team as a code owner April 25, 2023 12:22

melvin-bot bot requested review from PauloGasparSv and thesahindia and removed request for a team April 25, 2023 12:22

This comment was marked as outdated.

Sign in to view

bernhardoj commented Apr 25, 2023

View reviewed changes

aimane-chnaif mentioned this pull request Apr 25, 2023

[Pay 6/30] [$1000] App gets stuck in infinite loop between cash and amount page when sending money #17637

Closed

6 tasks

trim money request description

368e8b8

bernhardoj commented Apr 25, 2023

View reviewed changes

src/pages/iou/steps/MoneyRequestConfirmPage.js Outdated Show resolved Hide resolved

bernhardoj commented Apr 25, 2023

View reviewed changes

src/pages/iou/withMoneyRequest.js Outdated Show resolved Hide resolved

aimane-chnaif mentioned this pull request Apr 25, 2023

[$1000] Payment method dropdown option resets everytime we click on amount and go back #17805

Closed

6 tasks

update param type and comment

a58e71e

aimane-chnaif mentioned this pull request Apr 26, 2023

[$1000] Refreshing the page when setting the descriptive text of a IOU action and then clicking on back arrow leads back to the chat page #17829

Closed

6 tasks

cead22 changed the title ~~refactor iou page by separating each step as individual screen~~ Refactor iou page by separating each step as individual screen Apr 26, 2023

aimane-chnaif mentioned this pull request Apr 28, 2023

[HOLD #17662] [$1000] Double sliding animation on Send/Request money #17710

Closed

6 tasks

PauloGasparSv assigned bernhardoj May 1, 2023

bernhardoj mentioned this pull request May 16, 2023

Fix currency selection is saved even though we don't press Save yet #18995

Merged

57 tasks

bernhardoj added 4 commits May 19, 2023 18:56

Merge branch 'main' into fix/17662-refactor-iou-page

bab5196

fix lint

97d2be9

simplify code

d378732

move context value to moneyRequest object

7b7acdb

thienlnam reviewed Jun 20, 2023

View reviewed changes

bernhardoj added 2 commits June 21, 2023 12:44

Merge branch 'main' into fix/17662-refactor-iou-page

6ff8594

fix empty user detail when select non exist user

2c5797c

bernhardoj mentioned this pull request Jun 21, 2023

The description when clicked again shows blank #21139

Closed

6 tasks

mountiny previously requested changes Jun 21, 2023

View reviewed changes

update comment

a2bef09

s77rt suggested changes Jun 21, 2023

View reviewed changes

s77rt approved these changes Jun 21, 2023

View reviewed changes

thienlnam approved these changes Jun 21, 2023

View reviewed changes

thienlnam merged commit 7c04c2f into Expensify:main Jun 21, 2023

marcaaron reviewed Jun 21, 2023

View reviewed changes

OSBotify mentioned this pull request Jun 22, 2023

Deploy Checklist: New Expensify 2023-06-22 #21330

Closed

39 tasks

kowczarz mentioned this pull request Jun 23, 2023

Fix users list not showing on split confirm #20277

Closed

57 tasks

fedirjh mentioned this pull request Jun 24, 2023

Fix crash when edit a message after coming back from a child report #21227

Merged

57 tasks

dhairyasenjaliya mentioned this pull request Jul 5, 2023

[Awaiting Payment August 2nd] [$1000] input is not auto-focused on debit card details page #21587

Closed

6 tasks

parasharrajat referenced this pull request in bernhardoj/Expensify Jul 19, 2023

dismiss modal on split bill

74dac7e

kavimuru mentioned this pull request Jul 23, 2023

[$1000] Money request page crash after sign in #23420

Closed

6 tasks

situchan mentioned this pull request Jul 31, 2023

[HOLD for payment 2023-08-10] App crashes when clicking 'Split bill' button for newly created account #23951

Closed

6 tasks

eVoloshchak reviewed Aug 2, 2023

View reviewed changes

eVoloshchak mentioned this pull request Aug 2, 2023

[HOLD for payment 2023-08-01] [$1000] Web - App does not remove dot from amount in split bill after first try #22361

Closed

6 tasks

tgolen reviewed Aug 24, 2023

View reviewed changes

bernhardoj mentioned this pull request Aug 25, 2023

Update money request related pages PropTypes #25914

Merged

59 tasks

jeet-dhandha mentioned this pull request Sep 19, 2023

[HOLD for payment 2023-09-29] [HOLD for payment 2023-09-27] IOU - RHP does not disappear after sending a request money #27724

Closed

6 tasks

	const [selectedOptions, setSelectedOptions] = useState(
	ReportUtils.isPolicyExpenseChat(props.report)
	? OptionsListUtils.getPolicyExpenseReportOptions(props.report)
	: OptionsListUtils.getParticipantsOptions(props.report, props.personalDetails),
	);

	const titleForStep = useMemo(() => {
	if (currentStepIndex === 0) {
	const confirmIndex = _.indexOf(steps, Steps.MoneyRequestConfirm);
	if (previousStepIndex === confirmIndex) {
	return props.translate('iou.amount');
	}
	if (props.iouType === CONST.IOU.MONEY_REQUEST_TYPE.SEND) {
	return props.translate('iou.sendMoney');
	}
	return props.translate(props.hasMultipleParticipants ? 'iou.splitBill' : 'iou.requestMoney');
	}
	return props.translate('iou.cash');
	// eslint-disable-next-line react-hooks/exhaustive-deps -- props does not need to be a dependency as it will always exist
	}, [currentStepIndex, props.translate, steps]);

	const createTransaction = useCallback((selectedParticipants) => {
	const reportID = lodashGet(props.route, 'params.reportID', '');
	const trimmedComment = props.iou.comment.trim();

	// IOUs created from a group report will have a reportID param in the route.
	// Since the user is already viewing the report, we don't need to navigate them to the report
	if (props.hasMultipleParticipants && CONST.REGEX.NUMBER.test(reportID)) {
	IOU.splitBill(
	selectedParticipants,
	props.currentUserPersonalDetails.login,
	amount,
	trimmedComment,
	props.iou.selectedCurrencyCode,
	props.preferredLocale,
	reportID,
	);
	return;
	}

	// If the request is created from the global create menu, we also navigate the user to the group report
	if (props.hasMultipleParticipants) {
	IOU.splitBillAndOpenReport(
	selectedParticipants,
	props.currentUserPersonalDetails.login,
	amount,
	trimmedComment,
	props.iou.selectedCurrencyCode,
	props.preferredLocale,
	);
	return;
	}
	if (!selectedParticipants[0].login) {
	// TODO - request to the policy expense chat. Not implemented yet!
	// Will be implemented here: https://github.com/Expensify/Expensify/issues/270581
	return;
	}
	IOU.requestMoney(
	props.report,
	Math.round(amount * 100),
	props.iou.selectedCurrencyCode,
	props.currentUserPersonalDetails.login,
	selectedParticipants[0],
	trimmedComment,
	);
	}, [amount, props.iou.comment, props.currentUserPersonalDetails.login, props.hasMultipleParticipants, props.iou.selectedCurrencyCode, props.preferredLocale, props.report, props.route]);

	const sendMoney = useCallback((paymentMethodType) => {
	const amountInDollars = Math.round(amount * 100);
	const currency = props.iou.selectedCurrencyCode;
	const trimmedComment = props.iou.comment.trim();
	const participant = selectedOptions[0];

	if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.ELSEWHERE) {
	IOU.sendMoneyElsewhere(
	props.report,
	amountInDollars,
	currency,
	trimmedComment,
	props.currentUserPersonalDetails.login,
	participant,
	);
	return;
	}

	if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.PAYPAL_ME) {
	IOU.sendMoneyViaPaypal(
	props.report,
	amountInDollars,
	currency,
	trimmedComment,
	props.currentUserPersonalDetails.login,
	participant,
	);
	return;
	}

	if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.EXPENSIFY) {
	IOU.sendMoneyWithWallet(
	props.report,
	amountInDollars,
	currency,
	trimmedComment,
	props.currentUserPersonalDetails.login,
	participant,
	);
	}
	}, [amount, props.iou.comment, selectedOptions, props.currentUserPersonalDetails.login, props.iou.selectedCurrencyCode, props.report]);

	this.state = {
	amount: selectedAmountAsString,
	selectedCurrencyCode: props.iou.currency,
	shouldUpdateSelection: true,
	selection: {
	start: selectedAmountAsString.length,
	end: selectedAmountAsString.length,
	},
	};
	}

	const MoneyRequestParticipantsPage = require('../../../pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage').default;
	const MoneyRequestParticipantsPage = require('../../../pages/iou/steps/MoneyRequstParticipantsPage').default;

		alternateText: Str.isSMSLogin(detail.login \|\| '') ? LocalePhoneNumber.formatPhoneNumber(detail.login) : detail.login \|\| detail.displayName,
		alternateText: LocalePhoneNumber.formatPhoneNumber(login) \|\| displayName,

	function formatPhoneNumber(number) {
	if (!number) {
	return '';
	}

	const numberWithoutSMSDomain = Str.removeSMSDomain(number);
	const parsedPhoneNumber = parsePhoneNumber(numberWithoutSMSDomain);

	// return the string untouched if it's not a phone number
	if (!parsedPhoneNumber.valid) {
	if (parsedPhoneNumber.number && parsedPhoneNumber.number.international) {
	return parsedPhoneNumber.number.international;
	}
	return numberWithoutSMSDomain;
	}

	const moneyRequestId = `${this.iouType}${this.reportID}`;
	const shouldReset = this.props.iou.id !== moneyRequestId;

	function startMoneyRequest(iouType, reportID = '') {
	resetMoneyRequestInfo(`${iouType}${reportID}`);
	Navigation.navigate(ROUTES.getMoneyRequestRoute(iouType, reportID));
	}

	/**
	* Reset money request info from the store with its initial value
	* @param {String} id
	*/
	function resetMoneyRequestInfo(id = '') {

	/**
	* Clears out the task info from the store
	*/
	function clearOutTaskInfo() {
	Onyx.set(ONYXKEYS.TASK, null);
	}

	getComponent: () => {
	const MoneyRequestAmountPage = require('../../../pages/iou/steps/MoneyRequestAmountPage').default;
	return MoneyRequestAmountPage;
	},
	name: 'Money_Request',
	},
	{
	getComponent: () => {
	const MoneyRequestEditAmountPage = require('../../../pages/iou/steps/MoneyRequestAmountPage').default;
	return MoneyRequestEditAmountPage;
	},
	name: 'Money_Request_Amount',

	useEffect(() => {
	const moneyRequestId = `${iouType}${reportID}`;
	const shouldReset = iou.id !== moneyRequestId;
	if (shouldReset) {
	IOU.resetMoneyRequestInfo(moneyRequestId);
	}

	if (_.isEmpty(iou.participants) \|\| (iou.amount === 0 && !iou.receiptPath) \|\| shouldReset) {
	Navigation.goBack(ROUTES.getMoneyRequestRoute(iouType, reportID), true);
	}
	}, [iou.id, iou.participants, iou.amount, iou.receiptPath, iouType, reportID]);

Refactor iou page by separating each step as individual screen #17964

Refactor iou page by separating each step as individual screen #17964

Conversation

bernhardoj commented Apr 25, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

MelvinBot commented Apr 25, 2023

bernhardoj commented Apr 25, 2023 • edited Loading

bernhardoj commented Apr 25, 2023 • edited Loading

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernhardoj Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernhardoj commented May 19, 2023

mountiny commented Jun 20, 2023

thienlnam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernhardoj commented Jun 21, 2023 • edited Loading

mountiny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thienlnam left a comment

Choose a reason for hiding this comment

OSBotify commented Jun 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OSBotify commented Jun 22, 2023

OSBotify commented Jun 23, 2023

OSBotify commented Jun 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernhardoj commented Apr 25, 2023 •

edited

Loading

bernhardoj commented Apr 25, 2023 •

edited

Loading

bernhardoj commented Apr 25, 2023 •

edited

Loading

bernhardoj Apr 25, 2023 •

edited

Loading

bernhardoj commented Jun 21, 2023 •

edited

Loading