Fix the IOU badge on newly created reports #2543

tgolen · 2021-04-22T21:57:03Z

Held on:

Fix mergeCollection bug react-native-onyx#66
https://github.com/Expensify/Auth/pull/5505. -- [Jules] This is no longer a blocker as this Auth PR introduced the same change. Instead you should ensure you pull the latest Auth masterchanges

Details

Fixes the IOU badge when an IOU report is created for the first time

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/161189

Tests

Go to http://localhost:8080/iou/request
Request money from someone that has an account, but you've never chatted with before
Verify that once the request is done, there is a new row in the left-hand-navigation and it has a badge with the correct dollar amount
Go to http://localhost:8080/iou/split
Create a split from someone that has an account, but you've never chatted with before
Verify that once the split is created, there is a new row in the left-hand-navigation and it has a badge with the correct dollar amount
Repeat both tests with people you HAVE chatted with before (just to check for regressions)

QA (on web only)

Go to http://staging.expensify.cash/iou/request
Request money from someone that has an account, but you've never chatted with before
Verify that once the request is done, there is a new row in the left-hand-navigation and it has a badge with the correct dollar amount
Go to http://staging.expensify.cash/iou/split
Create a split from someone that has an account, but you've never chatted with before
Verify that once the split is created, there is a new row in the left-hand-navigation and it has a badge with the correct dollar amount
Repeat both tests with people you HAVE chatted with before (just to check for regressions)

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

tgolen · 2021-04-22T22:11:58Z

Sorry, puller bear got a little happy. You can still review if you want @deetergp, but I've removed the review request for you.

tgolen · 2021-04-22T22:23:18Z

I ran out of time today to finish the cross-platform testing, so I've only verified it on web for now. I'll try to get to the others tomorrow. Come to think of it though... is this testable on the other platforms @Julesssss (since you can't modify the URL in any other platform other than web).

src/components/IOUConfirmationList.js

Julesssss · 2021-04-23T14:13:27Z

Come to think of it though... is this testable on the other platforms @Julesssss (since you can't modify the URL in any other platform other than web).

Good point. All platforms are (internally) testable with the following diff. This adds 'Request Money' and 'Split Bill' options to the global CreateMenu:

diff --git a/src/pages/home/sidebar/SidebarScreen.js b/src/pages/home/sidebar/SidebarScreen.js
index b1af36bba..f94eac8ec 100644
--- a/src/pages/home/sidebar/SidebarScreen.js
+++ b/src/pages/home/sidebar/SidebarScreen.js
@@ -103,6 +103,16 @@ class SidebarScreen extends Component {
                                     text: 'New Group',
                                     onSelected: () => Navigation.navigate(ROUTES.NEW_GROUP),
                                 },
+                                {
+                                    icon: ChatBubble,
+                                    text: 'Request Money',
+                                    onSelected: () => Navigation.navigate(ROUTES.IOU_REQUEST),
+                                },
+                                {
+                                    icon: Users,
+                                    text: 'Split Bill',
+                                    onSelected: () => Navigation.navigate(ROUTES.IOU_BILL),
+                                },
                             ]}
 
                         /**

src/components/IOUConfirmationList.js

Julesssss · 2021-04-23T14:26:44Z

src/libs/actions/IOU.js

-                    getSimplifiedIOUReport(iouReportObjects[0]));
-            }
+            _.each(reportsData, (reportData) => {
+                // First, the existing chat report needs updated with the details about the new IOU


NAB:

// First, the existing chat report needs updated with the details about the new IOU // First, the existing chat report needs to be updated with the details about the new IOU

Julesssss · 2021-04-23T14:29:31Z

src/libs/actions/IOU.js

+            // This data needs to go from this:
+            // {reportIDList: [1, 2], chatReportIDList: [3, 4]}
+            // to this:
+            // [{reportID: 1, chatReportID: 3}, {reportID: 2, chatReportID: 4}]
+            // in order for getIOUReportsForNewTransaction to know which IOU reports are associated with which
+            // chat reports


I still like the idea of providing a better data structure in Auth, but let's not block the issue. We can create this as a non-blocking IOU task here.

Julesssss

The fix tested well for me on Web/Desktop/Android, but I think there's a regression: Opening a new chat/group chat report triggers the following error:

Julesssss · 2021-04-23T15:00:07Z

Ignore the last comment, I've seen the same issue on other PRs. I think it is fixed on the latest main commit.

tgolen · 2021-04-26T16:46:07Z

Looks like this is working well now after a few things in Onyx have been changed. I've tested the changes locally and I'm just waiting for the Onyx PR to get merged before releasing the hold on this.

tgolen · 2021-04-26T19:51:54Z

Updated and I have removed the HOLD

Julesssss · 2021-04-27T10:42:56Z

Would you mind merging main, as this issue is still occurring when I try to verify the issue with a regular chat report (IOU Reports work as expected).

While I'm sure that these changes will work with chat reports, I would like to confirm just to be sure.

Julesssss

Tests well with IOU Reports. Nice improvements, too 👍

tgolen · 2021-04-27T16:07:56Z

Yep, looks like merging in main makes that error go away. Updated!

Julesssss

Thanks for merging main, I can confirm that the issue is also resolved for the chat report flow.

@sketchydroide leaving the merge to you 👍

Julesssss · 2021-04-29T12:01:21Z

Bumping @sketchydroide :)

sketchydroide

LGTM

OSBotify · 2021-04-29T15:19:31Z

🚀 Deployed to staging in version: 1.0.33-2🚀

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

isagoico · 2021-04-29T17:58:18Z

@tgolen @Julesssss I don't think we're able to test this PR, can you confirm?

tgolen · 2021-04-29T19:37:53Z

This should be able to be tested on web only by using those links. (eg. https://staging.expensify.cash/iou/request and https://staging.expensify.cash/iou/split). Sorry I wasn't more clear in the description! I'll update it.

isagoico · 2021-04-29T22:16:41Z

@tgolen We were unable to access the iOU page, when navigating to the links provided on the steps, we're just being redirected to staging.expensify.cash/.
We tried with Expensifail accounts and we are being redirected to a blank page.

trjExpensify · 2021-04-29T22:23:18Z

Hey @isagoico! You should be able to do it like this:

go to an existing chat report on staging e.g https://staging.expensify.cash/r/123456
switch out the r/ for either /iou/request/ or /iou/split/ e.g https://staging.expensify.cash/iou/request/123456

isagoico · 2021-04-29T22:28:08Z

~~Same issue as before! Here's the screenshot~~ scratch that, seems like I typed something wrong.

OSBotify · 2021-05-08T01:43:19Z

🚀 Deployed to production in version: 1.0.39-5🚀

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

tgolen added 3 commits April 22, 2021 10:13

Fix a propType warning

62c0f3b

DRY code, improve and clean up params

8bd18fa

Make sure that splits still work

4def951

tgolen mentioned this pull request Apr 22, 2021

Fix mergeCollection bug Expensify/react-native-onyx#66

Merged

tgolen changed the title ~~Tgolen iou badge~~ [HOLD Auth#5505] [HOLD react-native-onyx#66]Tgolen iou badge Apr 22, 2021

Fix the PR template

62adbe4

tgolen requested review from Julesssss and a team April 22, 2021 22:03

tgolen self-assigned this Apr 22, 2021

MelvinBot requested review from sketchydroide and removed request for a team April 22, 2021 22:03

tgolen changed the title ~~[HOLD Auth#5505] [HOLD react-native-onyx#66]Tgolen iou badge~~ [HOLD Auth#5505] [HOLD react-native-onyx#66] Fix the IOU badge on newly created reports Apr 22, 2021

tgolen marked this pull request as ready for review April 22, 2021 22:09

tgolen requested a review from a team as a code owner April 22, 2021 22:09

MelvinBot requested review from deetergp and removed request for a team April 22, 2021 22:09

tgolen removed the request for review from deetergp April 22, 2021 22:11

tgolen commented Apr 22, 2021

View reviewed changes

src/components/IOUConfirmationList.js Show resolved Hide resolved

tgolen commented Apr 22, 2021

View reviewed changes

src/components/IOUConfirmationList.js Outdated Show resolved Hide resolved

Julesssss reviewed Apr 23, 2021

View reviewed changes

src/components/IOUConfirmationList.js Show resolved Hide resolved

Julesssss reviewed Apr 23, 2021

View reviewed changes

Julesssss requested changes Apr 23, 2021

View reviewed changes

Julesssss self-requested a review April 23, 2021 15:00

tgolen changed the title ~~[HOLD Auth#5505] [HOLD react-native-onyx#66] Fix the IOU badge on newly created reports~~ [HOLD Auth#5504] [HOLD react-native-onyx#66] Fix the IOU badge on newly created reports Apr 23, 2021

tgolen changed the title ~~[HOLD Auth#5504] [HOLD react-native-onyx#66] Fix the IOU badge on newly created reports~~ [HOLD react-native-onyx#66] Fix the IOU badge on newly created reports Apr 23, 2021

tgolen added 3 commits April 23, 2021 14:18

Fix a react prop warning

6e024b0

Removing debug log

0f278f3

Only update reports if it can locate the proper ID

827c7b7

Update Onyx to latest version

7fef891

tgolen changed the title ~~[HOLD react-native-onyx#66] Fix the IOU badge on newly created reports~~ Fix the IOU badge on newly created reports Apr 26, 2021

Remove debug statements

c67ca96

Julesssss approved these changes Apr 27, 2021

View reviewed changes

Merge branch 'main' into tgolen-iou-badge

99f8c2d

Julesssss approved these changes Apr 27, 2021

View reviewed changes

sketchydroide approved these changes Apr 29, 2021

View reviewed changes

Julesssss merged commit 7d0987a into main Apr 29, 2021

Julesssss deleted the tgolen-iou-badge branch April 29, 2021 14:46

OSBotify mentioned this pull request Apr 29, 2021

Deploy Checklist: Expensify.cash 2021-04-13 #2380

Closed

66 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the IOU badge on newly created reports #2543

Fix the IOU badge on newly created reports #2543

tgolen commented Apr 22, 2021 •

edited

Loading

tgolen commented Apr 22, 2021

tgolen commented Apr 22, 2021

Julesssss commented Apr 23, 2021 •

edited

Loading

Julesssss Apr 23, 2021

Julesssss Apr 23, 2021

Julesssss left a comment

Julesssss commented Apr 23, 2021

tgolen commented Apr 26, 2021

tgolen commented Apr 26, 2021

Julesssss commented Apr 27, 2021

Julesssss left a comment

tgolen commented Apr 27, 2021

Julesssss left a comment

Julesssss commented Apr 29, 2021

sketchydroide left a comment

OSBotify commented Apr 29, 2021

isagoico commented Apr 29, 2021

tgolen commented Apr 29, 2021

isagoico commented Apr 29, 2021

trjExpensify commented Apr 29, 2021

isagoico commented Apr 29, 2021 •

edited

Loading

OSBotify commented May 8, 2021

Fix the IOU badge on newly created reports #2543

Fix the IOU badge on newly created reports #2543

Conversation

tgolen commented Apr 22, 2021 • edited Loading

Details

Fixed Issues

Tests

QA (on web only)

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

tgolen commented Apr 22, 2021

tgolen commented Apr 22, 2021

Julesssss commented Apr 23, 2021 • edited Loading

Julesssss Apr 23, 2021

Choose a reason for hiding this comment

Julesssss Apr 23, 2021

Choose a reason for hiding this comment

Julesssss left a comment

Choose a reason for hiding this comment

Julesssss commented Apr 23, 2021

tgolen commented Apr 26, 2021

tgolen commented Apr 26, 2021

Julesssss commented Apr 27, 2021

Julesssss left a comment

Choose a reason for hiding this comment

tgolen commented Apr 27, 2021

Julesssss left a comment

Choose a reason for hiding this comment

Julesssss commented Apr 29, 2021

sketchydroide left a comment

Choose a reason for hiding this comment

OSBotify commented Apr 29, 2021

isagoico commented Apr 29, 2021

tgolen commented Apr 29, 2021

isagoico commented Apr 29, 2021

trjExpensify commented Apr 29, 2021

isagoico commented Apr 29, 2021 • edited Loading

OSBotify commented May 8, 2021

tgolen commented Apr 22, 2021 •

edited

Loading

Julesssss commented Apr 23, 2021 •

edited

Loading

isagoico commented Apr 29, 2021 •

edited

Loading