Open lanitochka17 opened 1 week ago
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.
@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
Edited by proposal-police: This proposal was edited at 2024-10-04 19:38:57 UTC.
Reply in threads option shown for parent message in thread
We use this check to show Reply In thread
.
https://github.com/Expensify/App/blob/8f3522803d7b9d070d519a7aff1c12cdeff10ed5/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L183
It was working fine until we added ancestor.report
.
https://github.com/Expensify/App/blob/8f3522803d7b9d070d519a7aff1c12cdeff10ed5/src/pages/home/report/ReportActionItemParentAction.tsx#L144
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.
https://github.com/Expensify/App/blob/8f3522803d7b9d070d519a7aff1c12cdeff10ed5/src/libs/ReportUtils.ts#L1559-L1561
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
https://github.com/Expensify/App/blob/8f3522803d7b9d070d519a7aff1c12cdeff10ed5/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx#L124
And then we can pass currentReportIDValue?.currentReportID
to this function instead of reportID
https://github.com/Expensify/App/blob/8f3522803d7b9d070d519a7aff1c12cdeff10ed5/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L183
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.
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
.
Reply in thread option appears for thread first chat.
We check whether the user can reply in thread, but only if it's not a thread first chat. https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/libs/ReportUtils.ts#L7463
isThreadFirstChat compares the action childReportID with the passed reportID. https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/libs/ReportUtils.ts#L1559-L1561
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.
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.
https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/pages/home/report/ReportActionItemParentAction.tsx#L129-L131
Then, pass the prop down to BaseReportActionContextMenu
. BaseReportActionContextMenu
will use it for the shouldShow
function.
https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx#L188-L203
Then, we pass it to shouldDisableThread. https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L179-L184
And replace the function here with the new isThreadFirstChat param https://github.com/Expensify/App/blob/99f280b0edae75ff59614a1c5e98c47b39800aa5/src/libs/ReportUtils.ts#L7463
We will replace all isThreadFirstChat()
usage with the new prop.
@anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick!
@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Job added to Upwork: https://www.upwork.com/jobs/~021843831874684578762
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allgandalf (External
)
@anmurali, @allgandalf Eep! 4 days overdue now. Issues have feelings too...
Not overdue sir melvin! 🙇
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
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:
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?
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/47692e6d-b6ab-439f-8064-dc6adf1942c3
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @allgandalf