-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[$250] [Search v1.2] Expense preview count & expense report do not update after deleting expense from Search #47617
Comments
Triggered auto assignment to @sakluger ( |
We think that this bug might be related to #wave-collect - Release 1 |
@sakluger 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 |
Job added to Upwork: https://www.upwork.com/jobs/~0164693244e7052703 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura ( |
ProposalNeed to have an updated transactions list while creating supportText in ReportPreview.tsx. For this the deleted transactions should be set to null. Please re-state the problem that we are trying to solve in this issue.The transactions list consists deleted transactions. What is the root cause of that problem?During the deletion process - transactions are not deleting, and still exists in the transactions list. That's why the number of shown transactions is wrong. What changes do you think we should make in order to solve the problem?Set deleted transactions to null What alternative solutions did you explore? (Optional)N/A |
📣 @MarynaKip! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@MarynaKip |
Edited by proposal-police: This proposal was edited at 2023-10-07T14:30:00Z. ProposalPlease re-state the problem that we are trying to solve in this issue.expense report do not update after deleting expense from Search What is the root cause of that problem?After deleting a transaction on the Search page, the transaction still remains in ONYX. What changes do you think we should make in order to solve the problem?We can change the For convenience, the entire code is included here: Thus, after executing the API request, we will remove the transaction data from Onyx. What alternative solutions did you explore? (Optional)I’m leaving the previous suggestion in the alternative solutions section. transactionIDList.forEach((transactionID) => {
optimisticData.push({
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {},
});
finallyData.push({
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {},
});
}); |
ProposalPlease re-state the problem that we are trying to solve in this issue.The expense preview count does not get updated when the report is deleted from the Search page. What is the root cause of that problem?The corresponding transactions are not set to null and the report actions are not filtered out if deleted. What changes do you think we should make in order to solve the problem?Only setting the transactions to null will fix the count as also suggested by @Amoralchik, but it causes this bug. We also need to filter out the deleted report action from What alternative solutions did you explore? (Optional)Alternatively, we can follow a similar approach as done in Lines 5754 to 5984 in 78c20f0
|
@ikevin127 another weird timecode by the proposal-police: #47617 (comment) Is it being worked on? |
@paultsimura We just pushed some changes related to this last week but seems like this is still happening. Digging into it to find out why ⛏️ |
Tagging @luacmartins since I believe you've worked with the bulk actions on the BE side. This might be a BE issue: the Could you please verify? |
Yea, I agree this is a BE issue and we're missing sending those updates. |
Moving to wave-control project. |
This seems to have the same root cause as #47385. |
I asked again for a retest in the Slack QA channel: https://expensify.slack.com/archives/C9YU7BX5M/p1726853413386789 |
Able to reproduce this 20240921_183532.mp4 |
@luacmartins it seems that the hold issue didn't fix this one. Any ideas why it didn't fix this one? |
Not sure, gonna remove the hold to investigate. |
@sakluger, @paultsimura, @luacmartins Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Not overdue, waiting for @luacmartins. |
Haven't been able to prioritize this one yet, but it's on my list for this week. |
@luacmartins any updates? |
@sakluger, @paultsimura, @luacmartins Huh... This is 4 days overdue. Who can take care of this? |
I'm working on a draft PR |
PR in review |
@luacmartins I see that the PR was deployed. Does anything else need to be done here? Otherwise, I think we can just close the issue since there was no C+ review. |
I think we're good to close. Thanks for the ping! |
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: v9.0.21-3
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
In Step 9, the expense preview count should update and no longer shows "2 expenses".
In Step 10, the expense report will be a combined report as there is only one expense.
Actual Result:
In Step 9, the expense preview count does not update and still shows "2 expenses".
In Step 10, the expense report does not show combined report when there is only one expense.
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6575243_1723987158308.20240818_211250.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @luacmartinsThe text was updated successfully, but these errors were encountered: