-
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
[HOLD for payment 2024-12-03] [Search v1.2] - Paid expenses do not have "Paid" label when the expenses are grouped expenses #52369
Comments
Triggered auto assignment to @abekkala ( |
Triggered auto assignment to @Julesssss ( |
💬 A slack conversation has been started in #expensify-open-source |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
|
Demoting |
Job added to Upwork: https://www.upwork.com/jobs/~021856301612409763010 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha ( |
Hi @IuliiaHerets, the reproduction steps aren't added yet. Could you please tag me when they are listed, thanks. |
ProposalPlease re-state the problem that we are trying to solve in this issue.The status of the transaction does not show as paid when the transaction is part of a thread with multiple transaction and all transactions are paid. What is the root cause of that problem?To build the rows of the search results we call In this function we try to resolve the action of the transaction in this line: What changes do you think we should make in order to solve the problem?We should remove that part of the condition that checks for single transaction and make the conditon into this:
And rely on the action returned by the backend for this transaction instead of using the isSettled method which relies on the report that the transaction belongs to because the report does not have the right data to understand individual transaction status. To do that we can at this line after when we resolved the transaction:
What alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2024-11-12 14:35:16 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Paid expenses do not have "Paid" label when the expenses are grouped expenses What is the root cause of that problem?When we get the actions, we check if the transaction is From One Transaction Report, for the bug description, it is false so we return the action to be Lines 253 to 255 in cd3f30f
What changes do you think we should make in order to solve the problem?Here, to fix this, we should also check whether the report is a Lines 260 to 263 in cd3f30f
So we should update the code as such: function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTransactionAction {
const isTransaction = isTransactionEntry(key);
const transaction = isTransaction ? data[key] : undefined;
const report = isTransaction ? data[`${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`] : data[key];
if ((!isTransaction && !isReportEntry(key)) || (isTransaction && !data[key].isFromOneTransactionReport && !ReportUtils.isMoneyRequestReport(report))) {
return CONST.SEARCH.ACTION_TYPES.VIEW;
} This will make sure that we only return the type as view if the report is a money request report. What alternative solutions did you explore? (Optional) |
In the We have a forced
If we want to show the
to remove the part that checks for the parent action:
|
Edited by proposal-police: This proposal was edited at 2024-11-12 14:54:39 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Paid expenses do not have "Paid" label when the expenses are grouped expenses What is the root cause of that problem?For combined transactions, Lines 253 to 255 in cd3f30f
but What changes do you think we should make in order to solve the problem?
This solution will not break anything Screen.Recording.2024-11-12.at.7.53.20.PM.movWhat alternative solutions did you explore? (Optional)Alternatively, we can remove this Line 253 in cd3f30f
and after this line we can add
Solution 3 Lines 252 to 255 in cd3f30f
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Search - Paid expenses do not have "Paid" label when the expenses are grouped expenses What is the root cause of that problem?Whenever we pay several transactions once, What changes do you think we should make in order to solve the problem?If we want to see 'paid' at the moment, we should remove this below condition.
What alternative solutions did you explore? (Optional)N/A Contributor detailsYour Expensify account email: anasup1995@gmail.com |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
Not ready to be paid, we just reverted the PR and I'll be working on a fix for this. |
@abekkala I'll unassign you since we reverted the PR and fixed this issue. I'm keeping it open to make sure I address the issue in the v2 of my PR. |
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.61-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-11-21. 🎊 |
@luacmartins @luacmartins The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.66-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-12-03. 🎊 |
@luacmartins @luacmartins The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
This is done |
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.60-0
Reproducible in staging?: Y
Reproducible in production?: N/A - new feature, doesn't exist in prod
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation (hyperlinked to channel name):
Action Performed:
Expected Result:
Describe what you think should've happened
Actual Result:
Describe what actually happened
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Bug6662249_1731401348204.20241112_164526.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: