App: [HOLD for payment 2024-04-25] [$500] DeleteComment not working as expected
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Issue reported by: @shubham1206agra @rayane-djouah Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1711678317008389?thread_ts=1711571474.268739&cid=C01GTK53T8Q
Action Performed:
- Add a comment
- Create a thread in the same comment
- Add another comment inside the thread
- Try to delete the comment in the thread
- Observe that the
DeleteCommentAPI never runs inside the network tab - Try to go to the same thread after navigating away
Expected Result:
The comment deleted in step 4 should be gone.
Actual Result:
The comment deleted in step 4 remains.
Workaround:
n/a
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
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~014a6349d3b575268d
- Upwork Job ID: 1775247417597710336
- Last Price Increase: 2024-04-02
- Automatic offers:
- shubham1206agra | Contributor | 0
- bernhardoj | Contributor | 0
About this issue
- Original URL
- State: open
- Created 3 months ago
- Comments: 34 (21 by maintainers)
No worries, I don’t think you or @bernhardoj did anything wrong - you’ve both been great. ❤️ I just decided to bring this internal because we have a fireroom. I agree you should both be compensated fully, and I’d appreciate your help reviewing the new PR to ensure that it ends up fixing the problem(s) without introducing new ones. :teamwork:
I should also be compensated fully as it was not blocking from my side but @bernhardoj’s side.
reviewed the PR
PR is ready
cc: @shubham1206agra
@ishpaul777 Just put it on hold. We can retest once this issue is fixed.
Thanks all! I’ve update the assignments 👍
Ok no problem
Proposal
Please re-state the problem that we are trying to solve in this issue.
Delete comment request is never processed.
What is the root cause of that problem?
We include delete comment as part of the conflicting request and
shouldIncludeCurrentRequestis also true (except thread parent) https://github.com/Expensify/App/blob/879378ff68dc026c1b4ba6af30b70051b99c6d35/src/libs/actions/Report.ts#L1258-L1267so when we do a delete comment, it conflicts with itself and is removed from the request list.
What changes do you think we should make in order to solve the problem?
From my understanding, the purpose of
getConflictingRequestsis:The 1st point works fine, but not with the 2nd. There are 2 issues with the 2nd point:
So, here is what I propose:
shouldIncludeCurrentRequestwithshouldExcludeCurrentRequestWhenConflict, the value stays the sameaction.isOptimisticActionis true https://github.com/Expensify/App/blob/879378ff68dc026c1b4ba6af30b70051b99c6d35/src/libs/actions/Report.ts#L1262PersistedRequest.save, don’t include the current request (requestToPersist) to the array yet, this means we don’t needpotentiallyConflictingRequestsanymore. We will passrequeststogetConflictingRequestshttps://github.com/Expensify/App/blob/879378ff68dc026c1b4ba6af30b70051b99c6d35/src/libs/actions/PersistedRequests.ts#L20-L21hasConflictwith an initial value of false.conflictingRequestsis not emptyshouldExcludeCurrentRequestWhenConflictis false or there is no conflict, add the current request to the request listdiff