[$1000] Request money - '1 Reply' is not shown when money request report is created in offline mode

[lanitochka17]

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. Go to staging.new.expensify.com
2. Click + button > Request moneybags
3. Complete the request money process
4. Repeat Step 2 and 3 in offline modeling

Expected Result:

After money request is created in offline mode, the IOU preview will show '1 Reply'

Actual Result:

After money request is created in offline mode, the IOU preview does not show '1 Reply'

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [x] Android / native
• [x] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [x] MacOS / Chrome
• [ ] MacOS / Desktop

Version Number: 1.3.50.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/78819774/a665776e-db75-4dab-9f64-208927fe009b

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017562e6ccd02ef924
• Upwork Job ID: 1691103716394532864
• Last Price Increase: 2023-08-14
• Automatic offers:
• samh-nl | Contributor | 26259731

[melvin-bot[bot]]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot[bot]]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• [x] This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• [x] This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[samh-nl]

Proposal

Please re-state the problem that we are trying to solve in this issue.

'1 Reply' is not shown when money request report is created in offline mode.

What is the root cause of that problem?

The optimistically set data of the REPORTPREVIEW report action is incomplete, lacking several child-related fields including childVisibleActionCount, childCommenterCount.

This results in shouldDisplayThreadReplies being set to false, therefore not rendering the thread reply.

https://github.com/Expensify/App/blob/245b81d8901530853a01c5da9ea8939b620c2050/src/pages/home/report/ReportActionItem.js#L369-L374

https://github.com/Expensify/App/blob/245b81d8901530853a01c5da9ea8939b620c2050/src/pages/home/report/ReportActionItem.js#L404

What changes do you think we should make in order to solve the problem?

In IOU.requestMoney, the report preview action data is built in ReportUtils.buildOptimisticReportPreview.

We should set the missing fields here, including childVisibleActionCount and childCommenterCount, which should both have a value of 1. However, also other child fields are missing and may need to be added:

childLastVisibleActionCreated
childMoneyRequestCount
childOldestFourAccountIDs
childReportID
childStateNum
childStatusNum
childType

What alternative solutions did you explore? (Optional)

N/A

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Request money - '1 Reply' is not shown when money request report is created in offline mode

What is the root cause of that problem?

When we build optimisticData to request money or split bill, we don't update children's fileds for reportPreview action. That makes the thread item doesn't exist or the thread count doesn't update in offline mode

https://github.com/Expensify/App/blob/245b81d8901530853a01c5da9ea8939b620c2050/src/pages/home/report/ReportActionItem.js#L369-L374

What changes do you think we should make in order to solve the problem?

In buildOnyxDataForMoneyRequest, we should update the children's fields for reportPreviewAction by using ReportUtils.updateOptimisticParentReportAction function

const optimisticParentReportAction = ReportUtils.updateOptimisticParentReportAction(reportPreviewAction, iouAction.created, CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
...
onyxMethod: isNewChatReport ? Onyx.METHOD.SET : Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
    ...(isNewChatReport ? {[chatCreatedAction.reportActionID]: chatCreatedAction} : {}),
    [reportPreviewAction.reportActionID]: {
        ...reportPreviewAction,
        ...optimisticParentReportAction,
    }
},

https://github.com/Expensify/App/blob/245b81d8901530853a01c5da9ea8939b620c2050/src/libs/actions/IOU.js#L88

That will also fix for the case we create split bill in offline mode

What alternative solutions did you explore? (Optional)

NA

Result

Screencast from 04-08-2023 18:59:45.webm

[dylanexpensify]

@lanitochka17 mind confirming, we currently see the requests incomplete, which is correct given we're in offline mode. What's the error here? Apologies if I'm missing it!

[melvin-bot[bot]]

@dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[dylanexpensify]

Reviewing this today to post update!

[dukenv0307]

I think it should be updated in offline mode to be consistent with other actions. Actually now, when we add a comment in a thread in offline mode, thread count of parent report action also updated.

[dylanexpensify]

Got it, so @dukenv0307 we want to not have the parent report action updated?

[dukenv0307]

I means we should update the thread count of parent report action when we create a request money in offline mode to consistent when we add a comment. That is expected of this issue.

[dylanexpensify]

Didn't get an update today, will on Monday!

[dylanexpensify]

Reviewing today!

[dylanexpensify]

Reviewing today!

[dylanexpensify]

@luacmartins confirmed correct behavior, is a bug! Going to repro now then mark external if all good

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~017562e6ccd02ef924

[melvin-bot[bot]]

Current assignee @dylanexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

[dylanexpensify]

@mananjadhav let's get a review today!

[mananjadhav]

I'll review this today.

[dylanexpensify]

TY!

[dylanexpensify]

@mananjadhav mind posting your findings on review today?

[mananjadhav]

I did review the proposals. Both of them have the correct RCA, and both the solutions work. I will shortlist one of them by EOD/tomorrow morning. Need to implement and test.

[mananjadhav]

I tested both the proposals, but the issue I found in both of them is, when I click on the IOU, it loads a blank report, with undefined in the URL. Can you folks confirm if this is the behavior you also see?

[dukenv0307]

@mananjadhav I think we have another bug when we click on thread item

[mananjadhav]

But isn't it because we don't have the optimistic data?

[dukenv0307]

I think when building optimistic report preview we should update childReportID or use getIOUReportIDFromReportActionPreview to get childReportID if the action is reportPreview to pass it to ReportActionItemThread

[samh-nl]

There's a number of missing child-related properties I listed, best to ensure all are populated optimistically?

[mananjadhav]

Thanks considering the RCA and that we've identified multiple properties in this proposal. I think we are good to use @samh-nl's proposal.

🎀 👀 🎀 C+ reviewed.

[melvin-bot[bot]]

Triggered auto assignment to @deetergp, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[dukenv0307]

@mananjadhav The proposal from @samh-nl doesn't work when we create another request money in the same reportPreivew.

[dukenv0307]

This proposal only set the value to 1 when building the optimistic and doesn't update when we create an other request money in the same reportPreivew and it also doesn't work when we create a split bill because it only call in requestMoney functionn.

[samh-nl]

I think @dukenv0307's use of the updateOptimisticParentReportAction function is correct to return some child-related fields, but approach would need to be expanded to ensure the other field(s) I listed are also populated.

Whether usage of this function is an implementation detail that can be done in the PR, not sure, I'll leave it up to you.

[dukenv0307]

Whether usage of this function is an implementation detail that can be done in the PR, not sure, I'll leave it up to you.

for the case childReportID, this is the lack of a field should be in buildOptimisticReportPreview function, @samh-nl's proposal doesn't work for the case the user creates the split bill with the already reportPreivew.

[dukenv0307]

In IOU.requestMoney, the report preview action data is built in ReportUtils.buildOptimisticReportPreview. We should set the missing fields here, including childVisibleActionCount and childCommenterCount , which should both have a value of 1.

This proposal clearly states to fix it here and just set that value to 1 and this doesn't work for all cases.

[mananjadhav]

@dukenv0307 This was a tough one for me. As I've mentioned both the proposals had the correct RCA, but both of them weren't completely correct. Considering, that both needed code changes in the PR I decided to use @samh-nl's proposal.

My focus was less on the split bill request. It's good to fix it, but not the deciding factor. Hope it clarifies my thought process. Rest I'll let @deetergp decide.

[dukenv0307]

@mananjadhav For childReportID, it's just a minor lack in buildOptimisticReportPreview because you can see if you don't click to thread you will open child report as well because ReportPreview doesn't use it to open childReport.

But Did you test this case in @samh-nl 's proposal:

1. Offline, create a new request money
2. Verify that report preview appears with thread count is 1.
3. Create a new request money again.
4. Verify that thread count now is 2.

I belive that this proposal doesn't work for this case because it's just update report preview when we create a new report preview. While my proposal can cover both cases.

[dylanexpensify]

reviewing

[melvin-bot[bot]]

📣 @mananjadhav Please request via NewDot manual requests for the Reviewer role ($1000)

[melvin-bot[bot]]

📣 @samh-nl 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[dukenv0307]

@mananjadhav @dylanexpensify Did you test this case in my comment here https://github.com/Expensify/App/issues/24129#issuecomment-1685077175?

[mananjadhav]

I did @dukenv0307. Thanks for the bump. @dylanexpensify I was waiting for @deetergp to go over the conversation before hiring @samh-nl here.

@deetergp Could you please look at the proposals/comments?

[dukenv0307]

My focus was less on the split bill request. It's good to fix it, but not the deciding factor.

@mananjadhav When split bill is created, the money report is also created. So I don't think it's out of scope in this issue,

[dylanexpensify]

Apologies - pending Scott review

[melvin-bot[bot]]

@deetergp @mananjadhav @dylanexpensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[dylanexpensify]

@deetergp can you dig into this today 🙇

[deetergp]

Thanks for your patience folks, sorry took me this long to get around to looking at this. As I understand it: @samh-nl's proposal was first and correct about needing to update all the fields and @dukenv0307 wouldn't have worked without increasing the number of updated fields, similar to the prior proposal but will handle the (plausible) edge case where an additional money request is sent and the thread count will increase. @samh-nl's proposal would not. Do I have that correct @mananjadhav?

[dukenv0307]

@deetergp My proposal would have worked for both case increase thread count and new thread count. Additional, my proposal will also work when we create split bill. And I think we should be The thread count of report preview isn't update when creating a request money in offline because reply 1 is only one case when we create a request money.

[dukenv0307]

@mountiny In the latest build, I saw that when we create a request money now the thread count doesn't increase. Is this the expected now? And can you please confirm the bug here is valid or not now?

[mountiny]

I will ask https://expensify.slack.com/archives/C01GTK53T8Q/p1692965572236189

[mountiny]

I think there is a new design and we are only showing the comments