Open IuliiaHerets opened 1 week ago
Triggered auto assignment to @stephanieelliott (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.
Edited by proposal-police: This proposal was edited at 2024-11-13 13:06:19 UTC.
Web - Thread - Noting happen when clicking "Join thread" button unless returning to inbox again
We are unnecessarily displaying join option for report actions that are not parent of a chat thread https://github.com/Expensify/App/blob/4d9be4eb2537b49fa6cf35135770cf3c6d62a1cd/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L313 and when we press the join button openReport is called with new child report id here https://github.com/Expensify/App/blob/4d9be4eb2537b49fa6cf35135770cf3c6d62a1cd/src/libs/actions/Report.ts#L1871 so when we navigate back to the inbox it will be opened as the last accessed report
We should only show join option on a report action if there is a chat thread associated with that report action that the user can join into (otherwise the join button will be a duplicate of reply in thread, there only difference being reply in thread opens the child report ). So we should check if the report action has a childReportID
to show join menu
return (
!subscribed &&
!isWhisperAction &&
!isTaskAction &&
!isExpenseReportAction &&
!isThreadFirstChat &&
!!reportAction?.childReportID &&
(shouldDisplayThreadReplies || (!isDeletedAction && !isArchivedRoom))
);
We won't need the else condition here https://github.com/Expensify/App/blob/4d9be4eb2537b49fa6cf35135770cf3c6d62a1cd/src/libs/actions/Report.ts#L1852
Edited by proposal-police: This proposal was edited at 2024-11-19 08:56:27 UTC.
"Join thread" option navigating to "reply in thread" page after navigating to other bottom navigations like Search or Setting and returning back to inbox
The difference is here, if reportAction?.childReportNotificationPreference
is undefined
, getChildReportNotificationPreference
function returns always
if the user is the creator of the action, otherwise it returns hidden
Join thread
, a new report is created and when we go back from search it is displayed as the last accessed reportIn this function, we should navigate to the thread if we create a new report
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(newChat.reportID));
Join thread
menu item for actions from another user if reportAction?.childReportNotificationPreference
is undefined
which means doesn't exist . To do that we can get childReportNotificationPreference
here from reportAction?.childReportNotificationPreference
const childReportNotificationPreference = reportAction?.childReportNotificationPreference;
https://github.com/Expensify/App/blob/b1c1a4b53746f6e9ba0a7853faaf42695ca4c067/src/pages/home/report/ContextMen u/ContextMenuActions.tsx#L305
And update the subscribed
const subscribed = !(childReportNotificationPreference === 'hidden');
Agree this seems buggy, adding labels.
Job added to Upwork: https://www.upwork.com/jobs/~021858763130278701100
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External
)
Both of the proposals lack clear explanations.
Apart from this, the expected behavior here should be that the user does not navigate to the thread in any way. Please update your proposals for it. Ref: https://github.com/Expensify/App/pull/27994
@parasharrajat I have tried to make it more readable based on your suggestion. Can you check it again? Thx
Thanks, both of you. But I think you missed the following part of the comment.
Apart from this, the expected behavior here should be that the user does not navigate to the thread in any way. Please update your proposals for it. Ref: https://github.com/Expensify/App/pull/27994
Apart from this, the expected behavior here should be that the user does not navigate to the thread in any way.
@parasharrajat What does that mean? My alternative solution will only hide the Join thread
, not add any navigate to thread logic.
Thanks, both of you. But I think you missed the following part of the comment.
Apart from this, the expected behavior here should be that the user does not navigate to the thread in any way. Please update your proposals for it. Ref: #27994
@parasharrajat I have not proposed to navigate to the thread on join thread but only make the join thread option to be available only if the thread linked to the report action exists. Is there anything you haven't understood?
I am telling the expected behaviour for this issue. It's not a feedback for any proposal.
I am telling the expected behaviour for this issue. It's not a feedback for any proposal.
@parasharrajat There is no navigation going on join thread even on current main, the current issue is making the impression of it happening. When you navigate back to Inbox from search we open the recently accessed report but on join thread we called openReport
, so that the previously unsubscribed chat thread will be available in FE, but that openReport
request will make the chat thread as the lastAccessedReport, hence, that's why it navigates to the joined new chat thread on going back to inbox as shown in the OP.
But the purpose of the join thread button is, as clearly indicated in the PR you linked, to allow a user join back a thread they have unsubscribed. Thus, we shouldn't show the join thread menu for report actions that are not yet have a linked child report/ chat thread.
I am telling the expected behaviour for this issue. It's not a feedback for any proposal.
When we click on Join thread, a new report is created and when we go back from search it is displayed as the last accessed report
@parasharrajat As I explained in the RCA, the thread is opened after going back from the search page because it's a newly created report. With my alternative solution, no new report is created then this bug doesn't happen.
It is briefly mentioned here https://github.com/Expensify/App/pull/27994#discussion_r1364689708. IT should work like get Notified about replied
feature of Slack.
I agree that navigation is happening due to lastaccessed report and openReport call. But that is what I am saying that it should not happen here.
But the purpose of the join thread button is, as clearly indicated in the PR you linked, to allow a user join back a thread they have unsubscribed. Thus, we shouldn't show the join thread menu for report actions that are not yet have a linked child report/ chat thread.
This is absolutely correct but this feature also works for chats where no thread is created so far.
Subscribing to a thread without comments (Need 2 users, User A and User B):
- With User B, comment in a chat
- With User A, subscribe to the comment that was just created
- With User B, reply to the comment
- Ensure that the thread shows up in User As LHN and they receive a notification
- Verify that no errors appear in the JS console
Here are the test steps for this one from that PR.
@srikarparsi Can you please help us clarify the purpose of this feature?
Sorry, the purpose of Join thread
? If so, yes it should mimic get Notified about replied
in replies. It shouldn't open the thread report.
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: v9.0.61-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): testpayment935+613@gmail.com Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
The "Join Thread" option should navigate to thread immediately or not navigating at all unless clicking "reply in thread"
Actual Result:
"Join thread" option navigating to "reply in thread" page after navigating to other bottom navigations like Search or Setting and returning back to inbox
Workaround:
Unknown
Platforms:
Screenshots/Videos
https://github.com/user-attachments/assets/55c0e212-9e0c-459e-8fe4-c83ee1eb9cec
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @parasharrajat