[$500] Threads - 'n replies' appears under parent message in thread and it leads to not here page

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.8-0
Reproducible in staging?: Y
Reproducible in production?: Y
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go to any chat
3. Send a message
4. Right click on the message > Reply in thread
5. Send a reply in thread
6. Click on 'n replies'

Expected Result:

In Step 5, there should be no 'n replies' under the parent message in thread

Actual Result:

In Step 5, there is 'n replies' under the parent message in thread. The 'n replies' link leads to not here page

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6301740_1701776763006.bandicam_2023-12-05_15-22-50-354.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0127edd78e0e28e371
• Upwork Job ID: 1732057684569538560
• Last Price Increase: 2023-12-05

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0127edd78e0e28e371

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• 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
• 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.
• 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.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[paultsimura]

Proposal

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

Inside a thread, the initial report action has a replies counter increased, which leads to nowhere.

What is the root cause of that problem?

This is a BE issue: when executing OpenReport on clicking "Reply in thread" for the first time, the childReportID of the parent reportActionItem is set optimistically:

{
  "onyxMethod": "merge",
  "key": "reportActions_6248612838888226",
  "value": {
    "3456415368628704237": {
      "childReportID": "7502146048786021",
      "childType": "chat"
    }
  }
}

However, when the API response comes, for some reason it contains the following command that removes the childReportID:

{
  "key": "reportActions_6248612838888226",
  "onyxMethod": "merge",
  "value": {
    "3456415368628704237": {
      "childReportID": null
    }
  }
}

This results in ReportUtils.isThreadFirstChat(props.action, props.report.reportID) being false here, which leads to displaying the replies counter:

App/src/pages/home/report/ReportActionItem.js

Line 487 in dad6391

const shouldDisplayThreadReplies = hasReplies && props.action.childCommenterCount && !ReportUtils.isThreadFirstChat(props.action, props.report.reportID);

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

The BE should not remove the parent action item's childReportID. Or is there any specific logic that requires this?

What alternative solutions did you explore? (Optional)

[melvin-bot]

📣 @postonsundays! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[situchan]

Seems like recent regression from backend

[chiItepin]

Proposal

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

The "n replies" button redirects to a broken view when the user sends a reply inside a reply view.

What is the root cause of that problem?

props.action.childReportID is undefined, however, hasReplies in ReportActionItem is true which results on rendering the "n replies" button while already on the replies view.

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

Rather than setting childReportID through buildOptimisticAddCommentReportAction, the "n replies" button should not be displayed in this scenario since the user is already in the reply view. This can be asserted if we go back to the initial chat, refresh the app and then select the "n replies" button, which would redirect to the replies view but we can confirm there's no "n replies" button anymore, which means this is what the behavior should always be.

The hasReplies button needs to be updated to the following:
const hasReplies = numberOfThreadReplies > 0 && props.action.childReportID; @conorpendergrast FYI

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[yh-0218]

Proposal

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

The "n replies" button redirects to a broken view when the user sends a reply inside a reply view.

What is the root cause of that problem?

Because action doesn't have childReportID on ReportActionItem.
In current code, childReportID value is appeared and disappeared again soon when click reply.

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

When we create reply report and success, we didn't save childReportID

 onyxData.successData.push({
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newReportObject.parentReportID}`,
                value: {[parentReportActionID]: {childReportID: reportID, childType: CONST.REPORT.TYPE.CHAT}},
});

Need to add this before failed controller

App/src/libs/actions/Report.js

Line 623 in 52a6383

onyxData.failureData.push({

What alternative solutions did you explore? (Optional)

Screen.Recording.2023-12-06.at.10.24.58.AM.mov

[paultsimura]

Seems like it's the same root cause as #32485 (comment) (BE regression).

[dukenv0307]

Proposal

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

Thread first chat misses many context menu options when we don't send any message in the thread.

What is the root cause of that problem?

When we create a thread and don't send any messages, reportAction doesn't contain childReportID field

App/src/libs/ReportUtils.ts

Line 916 in 5a0b7fd

return reportAction?.childReportID?.toString() === reportID;

So isThreadFirstChat function returns false and then getOriginalReportID returns the wrong report which makes the context menu option wrong

App/src/libs/ReportUtils.ts

Lines 3919 to 3923 in 5a0b7fd

	function getOriginalReportID(reportID: string, reportAction: OnyxEntry<ReportAction>): string | undefined {
	const currentReportAction = ReportActionsUtils.getReportAction(reportID, reportAction?.reportActionID ?? '');
	return isThreadFirstChat(reportAction, reportID) && Object.keys(currentReportAction ?? {}).length === 0
	? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.parentReportID
	: reportID;

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

1. Maybe it's new expected from BE.
   So if we don't want to fix this from BE we can update isThreadFirstChat function to compare report.parentReportActionID and reportAction.reportActionID instead of comparing reportAction.childReportID and reportID

2. To find the child report when clicking on reply in thread we can check if childReportID is undefined, we will find the report which has parentReportActionID is equal to reportAction.reportActionID from allReport.

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Line 140 in 4029eed

Report.navigateToAndOpenChildReport(lodashGet(reportAction, 'childReportID', '0'), reportAction, reportID);

What alternative solutions did you explore? (Optional)

BE should return childReportID for parent report action when we create a thread

[yh-0218]

Hi, @conorpendergrast @allroundexperts
Now we have many issue because of this issue.
Please check my proposal. Thanks.

[mallenexpensify]

@allroundexperts please prioritize this issue, the bug is pretty bad. Thx
Also, confirming the bug is happening on both staging and production.

[allroundexperts]

I agree with @paultsimura and @dukenv0307's analysis that something got changed on the backend. We should ideally fix this from the backend but if not, then @dukenv0307's proposal would work just fine!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[yh-0218]

hi, @allroundexperts how about my proposal? I think that is main problem.

[yh-0218]

I think this is not backend issue.

And my proposal fix several issues. @allroundexperts
#32659 and etc...

Screen.Recording.2023-12-08.at.8.06.15.AM.mov

[allroundexperts]

Hi @yh-0218!

Reason why I did not choose your proposal was that I wasn't able to find the pattern of setting success data with values other than null in our code. If you think this is not an issue with the backend, do you mind pointing to the piece of code we recently added which caused this? Thanks

[aldo-expensify]

https://github.com/Expensify/Web-Expensify/pull/40080 is in staging

[mallenexpensify]

Tested on staging and wasn't able to reproduce (was only a few tests though)

[paultsimura]

Wasn't able to reproduce

Yes, now the API response returns a valid childReportID for the parent reportAction.
It already fixes a couple of thread-related issues: this one is not reproducible on Staging, and neither is #32519

[melvin-bot]

@Beamanator @conorpendergrast @allroundexperts @aldo-expensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[aldo-expensify]

PR https://github.com/Expensify/Web-Expensify/pull/40080 hit production yesterday

[aldo-expensify]

Tested in production, the bug is not there anymore.

[mallenexpensify]

It's back!!!!
I'm on Desktop Version 1.4.16-2

In the past, when I've encountered the bug, it was when I sent a message, this time it's when someone else is.

[mallenexpensify]

Updated app this morn then noticed.
Logged out/in and it's still showing.

[aldo-expensify]

Since it is present since you log in, it is not related to onyx updates sent through the pusher... it probably has a different cause

[melvin-bot]

@Beamanator @conorpendergrast @allroundexperts @aldo-expensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

@Beamanator, @conorpendergrast, @allroundexperts, @aldo-expensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[aldo-expensify]

We don't know if the issue at hand is Internal anymore. Should we create a new issue with reproduction steps to start a clean investigation?

[melvin-bot]

@conorpendergrast @allroundexperts @aldo-expensify this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

[mallenexpensify]

Pretty sure this is fixed, I'm unable to reproduce so I'm closing. Comment/reopen if you disagree