Expensify
[$250] Track expense - App returns to main chat when sending reply to the deleted expense thread
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: 9.0.71-0 Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes If this was caught during regression testing, add the test name, ID and link from TestRail: Exp Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team
Action Performed:
- Go to staging.new.expensify.com
- Go to self DM.
- Click + > Track expense > Manual.
- Track a manual expense.
- Go to transaction thread.
- Add a reply.
- Click on the report header > Delete expense.
- Delete the expense.
- Refresh the page to see the deleted thread with reply.
- Click on the reply.
- Send another reply to the thread.
Expected Result:
App will not return to main chat after sending a reply in the thread of deleted track expense.
Actual Result:
App returns to main chat every time when sending a reply in the thread of deleted track expense.
Workaround:
Unknown
Platforms:
- [x] Android: Standalone
- [x] Android: HybridApp
- [x] Android: mWeb Chrome
- [x] iOS: Standalone
- [x] iOS: HybridApp
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/403fd7e0-c789-4425-b721-25deea530344
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021866466017302505976
- Upwork Job ID: 1866466017302505976
- Last Price Increase: 2024-12-10
Issue Owner
Current Issue Owner: @ikevin127
Triggered auto assignment to @zanyrenney (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.
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
@zanyrenney Huh... This is 4 days overdue. Who can take care of this?
Job added to Upwork: https://www.upwork.com/jobs/~021866466017302505976
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ikevin127 (External)
I am Michał from Callstack - expert contributor group. I’d like to work on this job.
Proposal
Please re-state the problem that we are trying to solve in this issue.
App returns to main chat every time when sending a reply in the thread of deleted track expense.
What is the root cause of that problem?
The issue originates from the logic introduced in this PR, where the app navigates to the main chat if the last comment is deleted. The isDeletedParentAction function is used to determine if the parent action is deleted.
https://github.com/Expensify/App/blob/50c1db81004285968c6388bc941860f554d411e6/src/libs/ReportActionsUtils.ts#L142-L144
When adding a new comment via the AddComment API, the returned Onyx data is missing the isDeletedParentAction field. As a result:
isDeletedParentAction incorrectly returns false.
This triggers a condition in a useEffect, which causes the app to navigate to the main chat upon adding a comment.
https://github.com/Expensify/App/blob/50c1db81004285968c6388bc941860f554d411e6/src/pages/home/ReportScreen.tsx#L601
API Detail
From:
https://github.com/Expensify/App/blob/50c1db81004285968c6388bc941860f554d411e6/src/libs/ReportActionsUtils.ts#L142-L144
To:
((reportActionMessage?.isDeletedParentAction ?? false) || !!reportActionMessage?.deleted) && (reportAction?.childVisibleActionCount ?? 0) > 0.
And a similar logic can be applied in isMessageDeleted
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
N/A
What alternative solutions did you explore? (Optional)
N/A 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.
@MrMuzyk from Callstack showed interested in picking this one up based on https://github.com/Expensify/App/issues/53539#issuecomment-2538500144.
🎀👀🎀 C+ reviewed
Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
The source of the problem presented by @linhvovan29546 is correct. However the solution - even though it works - it seems to me more like an optional fix than a proper one.
When adding a new comment via the AddComment API, the returned Onyx data is missing the isDeletedParentAction field
Somewhere along the way isDeletedParentAction flag disappears between onyx updates and I'm trying to find the exact spot where it happens. This is the actual root cause and it might be causing more problems that we are not aware of. Tracking and fixing it would be a proper fix.
I haven't found the exact spot and cause for this yet. Doing my best
Proposal
Please re-state the problem that we are trying to solve in this issue.
App returns to main chat every time when sending a reply in the thread of deleted track expense.
What is the root cause of that problem?
isDeletedParentAction is missing from the report's message inside Onyx. I've tried tracking down where exactly is disappears and I believe it happens after AddComment command. Optimistic data doesn't seem to change it and it's gonne after API call finishes. Response returned from API after posting a comment via AddComment command doesn't return isDeletedParentAction field inside message. This is gives a false positive in one of the condition leading to unnecessary redirect
Report inside Onyx before AddComment
Response from AddComment
What changes do you think we should make in order to solve the problem?
Investigate AddComment command on the BE side. I believe that API should return isDeletedParentAction field inside a message in a response to adding a comment but for some reason it doesn't do it now.
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
What alternative solutions did you explore? (Optional)
ReportActionsUtils.isDeletedParentAction is returning incorrect value because it expects isDeletedParentAction flag. If that flag is missing we can check if field called deleted is present. I believe usually both of the fields should be present at the same time so checking for either of them might be a good temporary solution
Investigate
AddCommentcommand on the BE side. I believe that API should returnisDeletedParentActionfield inside a message in a response to adding a comment but for some reason it doesn't do it now.
I think @neil-marcellini should be able to help with this part to figure out whether this requires a BE fix or we should do something FE wise.
@neil-marcellini, @ikevin127, @zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!
Awaiting Neil's feedback on https://github.com/Expensify/App/issues/53539#issuecomment-2539718510 whether we should fix this on BE side or go the route of a temporary FE fix as suggested by alternative from https://github.com/Expensify/App/issues/53539#issuecomment-2539270244.
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@neil-marcellini @ikevin127 @zanyrenney 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!
@neil-marcellini, @ikevin127, @zanyrenney Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Still awaiting Neil's feedback on https://github.com/Expensify/App/issues/53539#issuecomment-2539718510 whether we should fix this on BE side or go the route of a temporary FE fix as suggested by alternative from https://github.com/Expensify/App/issues/53539#issuecomment-2539270244.
@neil-marcellini, @ikevin127, @zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@neil-marcellini, @ikevin127, @zanyrenney Eep! 4 days overdue now. Issues have feelings too...
Still awaiting Neil's feedback on https://github.com/Expensify/App/issues/53539#issuecomment-2539718510 whether we should fix this on BE side or go the route of a temporary FE fix as suggested by alternative from https://github.com/Expensify/App/issues/53539#issuecomment-2539270244.
Wow sorry for the delay guys. I've been OOO, but also please don't hesitate to DM me on Slack. I'm much more likely to see that than GH tags.
What I don't understand is why the app is allowing us to comment on a deleted thread in the first place. That seems like where we should focus our attention. Maybe that has to do with isDeletedParentAction as others have suggested, but I think the problem would occur before adding a comment on the deleted thread. The problem has occurred before that point; after deleting the expense I don't think we should be able to comment on the thread.
I will investigate myself.
Oh right, so I seem to remember that we allow commenting on deleted expenses because sometimes people like to do that and there's really no need to limit the conversation.
So yes the question is why does AddComment not return the message with isDeletedParentAction. It's also important to note that since the report action message is an array, the merge operation on the report action fully replaces the message array value vs merging the single object, which is why isDeletedParentAction is removed when it was set previously.
I will look into why this isn't sent from the backend when adding a comment
The fix is up and should be working. I want to make it work for tasks too, manual test, and then I can get the PR ready for review.
@neil-marcellini, @ikevin127, @zanyrenney Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@neil-marcellini, @ikevin127, @zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!
@neil-marcellini, @ikevin127, @zanyrenney Eep! 4 days overdue now. Issues have feelings too...
