[PAY 8/13] Request Money / Split Money Greater than or equal to 100,000,000 InDefinite Loading

[Santhosh-Sellavel]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Create a new Request Money / Split
2. Enter Amount 100,000,000

Expected Result:

Case 1: If there is an upper Request limit, the user should be notified with an error message.
Case 2: No upper limit means the request should have been sent successfully.

Actual Result:

Indefinite loading

Request Money:

Screen.Recording.2021-07-17.at.8.22.30.PM.mov

Split:

Workaround:

Request amount less than 100,000,000. Even 99,999,999 works.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Proposal

Need inputs from the Expensify team, this might be a back-end issue if there is no upper limit.

But I propose proper error handling should be done for situations like this. If a request fails based on request type we need to show appropriate messages to the user.

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @bfitzexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Triggered auto assignment to @thienlnam (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[thienlnam]

In the video you put in the description, it looks like we're actually getting a response back of invalid amount so it seems like this is handled in the back-end but just needs some front-end handling for when we get that error message

[thienlnam]

Luckily, my friends only owe me 99 million instead of 100 million

[Santhosh-Sellavel]

In the video you put in the description, it looks like we're actually getting a response back of invalid amount so it seems like this is handled in the back-end but just needs some front-end handling for when we get that error message

If I request in Rupees ₹100,000,000, Request fails.

But If request in pound £10,000,000, Request success.

Now User Owes more than ₹1,000,000,000.

100 Million rupees is invalid if I ask in rupees.
But I can ask 1 billion rupees if asked in £ pound.

@thienlnam

[thienlnam]

Interesting, are the responses the same on all the failed transactions?

[Santhosh-Sellavel]

@thienlnam Following are responses for failed transactions
Request Money :
{"jsonCode":405,"message":"405 invalid amount","requestID":"-------SOMEID-------"}

Split Bill:
{"jsonCode":404,"message":"404 Splits amount does not equal total amount","requestID":"-------SOMEID-----"}

[thienlnam]

Thanks @Santhosh-Sellavel, I think the most straightforward solution here is to catch those errors and show some kind of error message / cancel the transaction so the user doesn't sit on an infinite spinner

[parasharrajat]

@Santhosh-Sellavel You are going to be rich soon. 🤣

[Santhosh-Sellavel]

Hey @thienlnam,

1. For reference, Is there any existing error message shown to the user?
   I found Growl used to show alert errors.
2. What message to show, the message from the response object or any template message with i18N?

In this method,

App/src/libs/actions/IOU.js

Line 66 in ad77386

function createIOUTransaction(params) {

We can catch errors usingjsonCode in response, as shown below

Show_notification_without_dismissal.mp4

Is this okay or we can dismiss the iou modal?

[thienlnam]

Hey @Santhosh-Sellavel - great questions

I'm going to loop in @shawnborton for design ideas regarding what an error would should look like in the IOU flow, and what the expected behavior would be. I would expect that if an amount is invalid we would just put them back at the amount screen.

What message to show, the message from the response object or any template message with i18N?

We should probably clean up the message a bit, maybe just put 'Invalid Amount' instead of having the error code

[shawnborton]

@thienlnam I'm a little out of the loop here, but I think if an invalid number is entered on the number screen, we should validate the field right there and not wait until the next step to throw an error.

[thienlnam]

There are some amounts that are actual numbers but cause out backend to throw an error and I'm guessing it has to do with limits on integer constants that we have back there. That's actually probably a better way to handle it and we can just hardcode a limit of 99 million? (Only consideration here is that different currencies have different conversion values but 99 million has got to be a ton of money for any currency)

[Santhosh-Sellavel]

@shawnborton
Ya we can do that too in client side, if request/split amount should be less than 100,000,000 million numbers and does not worry but currency selection.

Consider if request fails for any other reasons, we need do handling, because infinite loading occurs back again. So we need handling for that.

@thienlnam
Can we remove json code from message in backend, thats the right place to do. Its redunant we have jsonCode in response.

[thienlnam]

@Santhosh-Sellavel
We handle all of our responses this way so it's unlikely that we would remove it for just this message. I would add handling based on the specific error message you expect to see and just use a clean message for it like Invalid Amount

[Santhosh-Sellavel]

@thienlnam Okay will do it here.

For Request Money - jsonCode 405 message is Invalid Amount.

1. For Split money - jsonCode 404 message is Splits amount does not equal total amount?
2. For other error codes?
3. With i18N right?

[thienlnam]

Those codes and messages are good, for anything else lets give a generic error like 'Unexpected error, please try again later'

With i18N right?

Yup 👍

[Santhosh-Sellavel]

Added i18N file as discussed added following, (Let me know if any name correction)
under iou,

Added a method handleIOUError (will rename if needed)

In both createTransaction methods will just invoke the above method as follows,

@thienlnam @shawnborton
Can we just dismiss modals after showing an error?
If not, what do for each error case?

I'm waiting for inputs, thanks

[thienlnam]

Hey @Santhosh-Sellavel, can you add me as a reviewer on the PR that you push up? I can comment on implementation specific details there. If possible, I think we should try to have them try to re-enter an amount

[Santhosh-Sellavel]

Hey @Santhosh-Sellavel, can you add me as a reviewer on the PR that you push up? I can comment on implementation specific details there. If possible, I think we should try to have them try to re-enter an amount

Roger that

[mallenexpensify]

@thienlnam I'm guessing this should be an External issue, right? If so I'll post the job to Upwork. Also if so, should I hire @Santhosh-Sellavel ?

[thienlnam]

Nice catch - Yes, please do @mallenexpensify I've just merged his PR

[MelvinBot]

Triggered auto assignment to @adelekennedy (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[thienlnam]

Sorry for the ping @adelekennedy! Letting @mallenexpensify wrap up the upwork posting

[mallenexpensify]

https://www.upwork.com/jobs/~0186df8dda363f929c
@Santhosh-Sellavel , can you please 'submit a proposal'/apply in Upwork and I'll hire you there? Please confirm here once you do.

[Santhosh-Sellavel]

https://www.upwork.com/jobs/~0186df8dda363f929c
@Santhosh-Sellavel , can you please 'submit a proposal'/apply in Upwork and I'll hire you there? Please confirm here once you do.

Done @mallenexpensify

[mallenexpensify]

Hired @Santhosh-Sellavel in Upwork!

[thienlnam]

PR is staging checklist, changes have been merged

[mallenexpensify]

Per the PR, deployed to staging two days ago, I'll issue payment on Friday if there are no regressions

[Santhosh-Sellavel]

Are there any regressions, can we close this?
@mallenexpensify

[mallenexpensify]

Paid @Santhosh-Sellavel with bonus for reporting the issue. Code has been on production for 5 days without a regression, closing now (not sure why it didn't auto-close before)