Sync newly returned report objects from commands #3544

jasperhuangg · 2021-06-11T11:06:26Z

Details

We were re-fetching the chat and IOU report objects once the payIOU, payWithWallet, and rejectTransaction commands completed, which was completely unnecessary. Now, since we're returning the updated IOU and Chat report objects from https://github.com/Expensify/Web-Expensify/pull/31190, we can directly update Onyx with them.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/165863

Tests

Make sure you have https://github.com/Expensify/Web-Expensify/pull/31318 either pulled or deployed.

Create accounts A and B (or use any two testing accounts with no IOU report history between one another)
Use A to request money from B.
In account B, use "I'll settle up elsewhere" to pay the IOU report.
Verify that the IOU report is settled, and the UI is updated to reflect it:

4. Use A to request money from B. 5. In account B, decline the request. 6. Verify that the transaction was rejected correctly, and that the UI is updated to reflect it:

QA Steps

Same as the above tests

Tested On

Screenshots

android.mp4

desktop.mp4

ios.mp4

web.mp4

…nsaction

Julesssss · 2021-06-16T11:23:03Z

I'm happy to review this as an additional reviewer, but we should always assign Pullerbear :)

mountiny · 2021-06-18T09:27:08Z

I was testing it with the Web-Expensify brach and I was getting this stacktrace error and the create report button was just spinning for very long time. Not sure what to do about it, seems like Jules pointed at some problem with returning information about the Report, which could be the reason why it keeps spinning.

Julesssss · 2021-06-18T09:30:47Z

Not sure what to do about it, seems like Jules pointed at some problem with returning information about the Report, which could be the reason why it keeps spinning.

In my case, I didn't see the spinner. The request returned quickly, but it was missing the extra data 😕

mountiny · 2021-06-18T09:55:30Z

To be fair, there is something wrong with my dev environment, it is a second day it does not work as it should. I will escalate this to #engineering-chat

mountiny

Tested the branch with the relevant Web-E branch and it works well. I will approve since my comments are NAB, but consider implementing them and re-requesting the review. Good job with both of those PRs Jasper!

src/libs/actions/IOU.js

src/libs/actions/Report.js

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

jasperhuangg · 2021-06-21T03:08:23Z

Thanks for the suggestions @mountiny, requested another review!

mountiny

Great @jasperhuangg!

Julesssss · 2021-06-30T12:20:20Z

This is unblocked now, testing.

src/libs/actions/IOU.js

src/libs/actions/Report.js

Julesssss

It tests well, but I made a few suggestions.

mountiny · 2021-06-30T14:43:53Z

Tested it too, works as expected 👏

mountiny

NAB: In addition to Jules' comments, just a small grammar details to keep the comments consistent.

src/libs/actions/IOU.js

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

Julesssss · 2021-07-01T09:23:18Z

src/libs/actions/IOU.js

-
-            // Once we have successfully paid the IOU we will transfer the user to their platform of choice if they have
-            // selected something other than a manual settlement or Expensify Wallet e.g. Venmo or PayPal.me
-            if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.PAYPAL_ME) {
-                Linking.openURL(buildPayPalPaymentUrl(amount, submitterPayPalMeAddress, currency));
-            } else if (paymentMethodType === CONST.IOU.PAYMENT_TYPE.VENMO) {
-                Linking.openURL(buildVenmoPaymentURL(amount, submitterPhoneNumber));
-            }


Was this supposed to be removed? Am I missing something, or was this a mistake? Wouldn't this break Venmo/Paypal?

Julesssss

Changes look good, but I have one outstanding comment

Julesssss

My comment was referring to a commit that isn't included. Good to merge once Web-Expensify/pull/31318 is deployed to prod 👍

mountiny

Great! Let's wait for the deploy before merging.

jasperhuangg · 2021-07-08T01:53:49Z

Looks like https://github.com/Expensify/Web-Expensify/pull/31318 has been deployed, so I'm gonna merge this!

roryabraham · 2021-07-09T00:28:45Z

Unfortunately the deploy comments are not working right now. This was deployed to staging yesterday.

handle returned iou reports from payIOU, payWithWallet, and rejectTra…

c83af9f

…nsaction

jasperhuangg requested a review from Julesssss June 11, 2021 11:06

jasperhuangg self-assigned this Jun 11, 2021

remove debug statement

353cc70

jasperhuangg changed the title ~~Sync newly returned report objects from commands~~ [HOLD Web-Expensify 31190] Sync newly returned report objects from commands Jun 11, 2021

jasperhuangg added 2 commits June 11, 2021 19:11

remove unused

596fd4b

style

837ec48

jasperhuangg marked this pull request as ready for review June 15, 2021 11:00

jasperhuangg requested a review from a team as a code owner June 15, 2021 11:00

MelvinBot requested review from sketchydroide and removed request for a team June 15, 2021 11:00

jasperhuangg removed the request for review from sketchydroide June 15, 2021 11:01

Julesssss requested review from a team and removed request for Julesssss June 16, 2021 11:22

MelvinBot requested review from mountiny and removed request for a team June 16, 2021 11:22

mountiny previously approved these changes Jun 18, 2021

View reviewed changes

src/libs/actions/IOU.js Outdated Show resolved Hide resolved

src/libs/actions/IOU.js Outdated Show resolved Hide resolved

src/libs/actions/Report.js Outdated Show resolved Hide resolved

src/libs/actions/Report.js Outdated Show resolved Hide resolved

Update src/libs/actions/IOU.js

c440da2

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

jasperhuangg dismissed mountiny’s stale review via c440da2 June 21, 2021 03:07

jasperhuangg and others added 3 commits June 21, 2021 11:07

Update src/libs/actions/IOU.js

7b78037

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

Update src/libs/actions/Report.js

47ac6f0

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

Update src/libs/actions/Report.js

dca4bde

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

jasperhuangg requested a review from mountiny June 21, 2021 03:08

style

0620571

mountiny previously approved these changes Jun 21, 2021

View reviewed changes

jasperhuangg requested a review from Julesssss June 30, 2021 11:46