[HOLD for payment 2024-12-03] [$250] Threads - Reply in threads option shown for parent message in thread

[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!

---

Version Number: 9.0.44-6
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): Negasofonias@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to any chat and open a thread or create one
2. open the thread and right click on the parent message and observe that reply in a thread option is available
3. Click on it and observe it just focuses the composer box

Expected Result:

Reply in a thread option should not be shown for the parent message

Actual Result:

Reply in a thread option is available with out any purpose

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

Bug6617626_1727458359699.scrnli_9_27_2024_8-23-11_PM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021843831874684578762
• Upwork Job ID: 1843831874684578762
• Last Price Increase: 2024-10-23
• Automatic offers:
• allgandalf | Reviewer | 104652631

Issue Owner

Current Issue Owner: @anmurali

[melvin-bot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@anmurali FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-10-04 19:38:57 UTC.

Proposal

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

Reply in threads option shown for parent message in thread

What is the root cause of that problem?

We use this check to show Reply In thread.

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

Line 183 in 8f35228

return !ReportUtils.shouldDisableThread(reportAction, reportID);

It was working fine until we added ancestor.report.

App/src/pages/home/report/ReportActionItemParentAction.tsx

Line 144 in 8f35228

report={ancestor.report}

This function checks if it's the thread's first chat. However, the ancestor.report we're using now passes the parent report's reportID instead of the current report's. As a result, this check will return false.

App/src/libs/ReportUtils.ts

Lines 1559 to 1561 in 8f35228

	function isThreadFirstChat(reportAction: OnyxInputOrEntry<ReportAction>, reportID: string): boolean {
	return reportAction?.childReportID?.toString() === reportID;
	}

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

We should pass the correct reportID to isThreadFirstChat. In BaseReportActionContextMenu we can use useCurrentReportID() to get the reportID and then we can pass that reportID here

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx

Line 124 in 8f35228

}: BaseReportActionContextMenuProps) {

And then we can pass currentReportIDValue?.currentReportID to this function instead of reportID

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

Line 183 in 8f35228

return !ReportUtils.shouldDisableThread(reportAction, reportID);

Optional: We can pass currentReportIDValue?.currentReportID as third parameter in shouldDisableThread and only use it to pass it to isThreadFirstChat.

We can check for other places too where we need to pass the correct Id.

What alternative solutions did you explore? (Optional)

We can create a function in ContextMenuActions.tsx to get current report Id.

pseudoCode

function getCurrentReportID(){
    const currentReportIDValue = useCurrentReportID();
    return currentReportIDValue?.currentReportID ?? ''
}

and then in here we can get the currentReportId value

      const currentReportId = getCurrentReportID()

and then we can pass it to isThreadFirstChat

Optional: We can pass currentReportID as third parameter in shouldDisableThread and only use it to pass it to isThreadFirstChat.

[bernhardoj]

Proposal

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

Reply in thread option appears for thread first chat.

What is the root cause of that problem?

We check whether the user can reply in thread, but only if it's not a thread first chat.

App/src/libs/ReportUtils.ts

Line 7463 in 99f280b

isThreadFirstChat(reportAction, reportID)

isThreadFirstChat compares the action childReportID with the passed reportID.

App/src/libs/ReportUtils.ts

Lines 1559 to 1561 in 99f280b

	function isThreadFirstChat(reportAction: OnyxInputOrEntry<ReportAction>, reportID: string): boolean {
	return reportAction?.childReportID?.toString() === reportID;
	}

The reportID is coming from ReportActionItem. However, after this PR, the reportID contains the reportID of the report where the action belongs to.

For example, if we create a thread from report A, the reportID will be report A instead of B. So, isThreadFirstChat will always be false.

This also causes the Join Thread to shows up for thread first chat.

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

Instead of using isThreadFirstChat() function which requires a reportID that can be anything, we can have a new prop for ReportActionItem also called isThreadFirstChat. It will only be true if the ReportActionItem is coming from ReportActionItemParentAction.

App/src/pages/home/report/ReportActionItemParentAction.tsx

Lines 129 to 131 in 99f280b

	<ReportActionItem
	onPress={
	ReportUtils.canCurrentUserOpenReport(ancestorReports.current?.[ancestor?.report?.reportID ?? '-1'])

Then, pass the prop down to BaseReportActionContextMenu. BaseReportActionContextMenu will use it for the shouldShow function.

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx

Lines 188 to 203 in 99f280b

	contextAction.shouldShow(
	type,
	reportAction,
	isArchivedRoom,
	betas,
	anchor,
	isChronosReport,
	reportID,
	isPinnedChat,
	isUnreadChat,
	!!isOffline,
	isMini,
	isProduction,
	moneyRequestAction,
	areHoldRequirementsMet,
	),

Then, we pass it to shouldDisableThread.

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

Lines 179 to 184 in 99f280b

	shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport, reportID) => {
	if (type !== CONST.CONTEXT_MENU_TYPES.REPORT_ACTION) {
	return false;
	}
	return !ReportUtils.shouldDisableThread(reportAction, reportID);
	},

And replace the function here with the new isThreadFirstChat param

App/src/libs/ReportUtils.ts

Line 7463 in 99f280b

isThreadFirstChat(reportAction, reportID)

We will replace all isThreadFirstChat() usage with the new prop.

[melvin-bot]

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

[melvin-bot]

@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@anmurali, @allgandalf Eep! 4 days overdue now. Issues have feelings too...

[allgandalf]

Not overdue sir melvin! 🙇

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@anmurali @allgandalf this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

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

[Nodebrute]

@allgandalf bump for review

[allgandalf]

I didn't realise there were proposals here, I will review today 👍

[allgandalf]

@bernhardoj 's solution looks good to me. This way we make sure that we only show the reply in thread option if the ReportActionItem is coming from ReportActionItemParentAction.

🎀👀🎀 C+ reviewed

[melvin-bot]

📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[bernhardoj]

PR is ready

cc: @allgandalf

[allgandalf]

PR is on staging

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.66-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix reply in thread shows for thread parent #51721

If no regressions arise, payment will be issued on 2024-12-03. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment through NewDot Manual Requests
• @allgandalf requires payment automatic offer (Reviewer)

[melvin-bot]

@allgandalf @anmurali @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[allgandalf]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: https://github.com/Expensify/App/pull/43518/files#r1861560822

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

• None

Test:

1. Send a message to any chat
2. Right-click/long-press and choose reply in thread
3. Right-click/long-press the thread parent message

Verify there is no Reply in thread option

4. (Web/Desktop) Hover over the message and press the 3-dot button

Verify there is no Reply in thread option

Do we agree 👍 or 👎

[allgandalf]

@anmurali can you pay this one please

[bernhardoj]

Requested in ND.

[allgandalf]

not overdue, pending payment

[melvin-bot]

@anmurali, @neil-marcellini, @bernhardoj, @allgandalf Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[JmillsExpensify]

Waiting on payment summary

[melvin-bot]

@anmurali, @neil-marcellini, @bernhardoj, @allgandalf 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[anmurali]

Payment Summary

• @bernhardoj $250 approved to be paid via NewDot Manual Requests
• @allgandalf $250 to be paid via automatic offer

[JmillsExpensify]

$250 approved for @bernhardoj