You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[$250] Scan - LHN preview for expense report shows 0.00 when there is only one scan expense #47843
@twisterdotcom 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
Please re-state the problem that we are trying to solve in this issue.
LHN last message shows 0 amount in one-expense report if the expense is a scan request.
What is the root cause of that problem?
The one-expense report "lastMessageText" itself is "$0.00 expense". Looks like the last message when we do a scan request is always like that. But if we open an empty transaction thread, the last message shown in LHN is "No activity yet". I believe we want the last message for one-expense report to be the same as transaction thread, so I'll explain why the last message shown in LHN is different.
When we get the last message of a report, we take its last visible action and we create it based on the action type.
In one-expense report, the last report action is null because the last report action is the CREATED action, but we filter it out, so, it fallback to the lastMessageText which is "$0.00 expense".
This is the same case as one-transaction thread too, but the lastMessageText is empty, so, it gets the last message from the welcome message, which is also empty for a chat thread (including transaction thread), so we finally fallback to "No activity yet".
What changes do you think we should make in order to solve the problem?
To show "No activity yet" for one-expense report too,
If it's a one-transaction report and the last report action is empty (null), then return empty string as the last message from OptionsListUtils.getLastMessageTextForReport.
if (!lastReportAction && transactionThreadReportIDs[reportID]) {
return "";
} else if (ReportUtils.isArchivedRoom(report, reportNameValuePairs)) {
transactionThreadReportIDs is a new variable that stores the one-transaction thread report ID for each one-expense report.
changed the title [-]Scan - LHN preview for expense report shows 0.00 when there is only one scan expense[/-][+][$250] Scan - LHN preview for expense report shows 0.00 when there is only one scan expense[/+]on Aug 22, 2024
What changes do you think we should make in order to solve the problem?
I think we should not filter out the IOU action in OptionsListUtils because we will use this action to get the correct last message text. For example, if the receipt is scanning, LHN should display Receipt scanning... that is the behavior when the expense report is not a combined report.
To do that we can create a new param in getCombinedReportActions like shouldFilterIOUAction with default value is true then if it's false we will return early here
if (!isMoneyRequestAction(action) || !shouldFilterIOUAction) {
return true;
}
What alternative solutions did you explore? (Optional)
One question about the approach though (and maybe this can be discussed in the PR) is why do we need a new param shouldFilterIOUAction? Or asked differently, when would we use true and when would we use false? If we only set it to false for one-transaction reports, could we count how many items are in filteredParentReportActions instead?
Basically, I'm thinking if we can avoid introducing a new param, it's one less chance that someone will use it wrong in the future and cause a bug.
📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻
Keep in mind: Code of Conduct | Contributing 📖
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!
Activity
melvin-bot commentedon Aug 22, 2024
Triggered auto assignment to @twisterdotcom (
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.IuliiaHerets commentedon Aug 22, 2024
We think that this bug might be related to #wave-collect - Release 1
IuliiaHerets commentedon Aug 22, 2024
@twisterdotcom 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
bernhardoj commentedon Aug 22, 2024
Proposal
Please re-state the problem that we are trying to solve in this issue.
LHN last message shows 0 amount in one-expense report if the expense is a scan request.
What is the root cause of that problem?
The one-expense report "lastMessageText" itself is "$0.00 expense". Looks like the last message when we do a scan request is always like that. But if we open an empty transaction thread, the last message shown in LHN is "No activity yet". I believe we want the last message for one-expense report to be the same as transaction thread, so I'll explain why the last message shown in LHN is different.
When we get the last message of a report, we take its last visible action and we create it based on the action type.
App/src/libs/OptionsListUtils.ts
Lines 593 to 595 in 50de692
If the last message is still empty, we fallback to the report
lastMessageTextproperty.App/src/libs/OptionsListUtils.ts
Lines 671 to 677 in 50de692
In one-expense report, the last report action is null because the last report action is the CREATED action, but we filter it out, so, it fallback to the
lastMessageTextwhich is "$0.00 expense".This is the same case as one-transaction thread too, but the
lastMessageTextis empty, so, it gets the last message from the welcome message, which is also empty for a chat thread (including transaction thread), so we finally fallback to "No activity yet".App/src/libs/SidebarUtils.ts
Lines 448 to 449 in 50de692
App/src/libs/SidebarUtils.ts
Lines 488 to 492 in 50de692
What changes do you think we should make in order to solve the problem?
To show "No activity yet" for one-expense report too,
OptionsListUtils.getLastMessageTextForReport.transactionThreadReportIDsis a new variable that stores the one-transaction thread report ID for each one-expense report.App/src/libs/OptionsListUtils.ts
Lines 309 to 313 in 50de692
getWelcomeMessageto return empty welcome message for one-expense report too.App/src/libs/SidebarUtils.ts
Lines 488 to 492 in 50de692
We will pass the reportActions as the param of
getWelcomeMessage.[-]Scan - LHN preview for expense report shows 0.00 when there is only one scan expense[/-][+][$250] Scan - LHN preview for expense report shows 0.00 when there is only one scan expense[/+]melvin-bot commentedon Aug 22, 2024
Job added to Upwork: https://www.upwork.com/jobs/~01676bf97c74d0768c
melvin-bot commentedon Aug 22, 2024
Triggered auto assignment to Contributor-plus team member for initial proposal review - @DylanDylann (
External)nkdengineer commentedon Aug 23, 2024
Proposal
Please re-state the problem that we are trying to solve in this issue.
LHN preview for the expense report shows 0.00.
This issue only happens when there is only one scan expense in the report.
What is the root cause of that problem?
When the expense report is a combined report, we filtered out the
IOUaction of expense report viagetCombinedReportActionsfunction hereApp/src/libs/OptionsListUtils.ts
Lines 310 to 313 in 156f0a2
That makes
lastReportActionherenullthen we fallback toreport.lastMessageTextwhich is0.00 expensein this caseApp/src/libs/OptionsListUtils.ts
Line 595 in 156f0a2
What changes do you think we should make in order to solve the problem?
I think we should not filter out the
IOUaction inOptionsListUtilsbecause we will use this action to get the correct last message text. For example, if the receipt is scanning, LHN should displayReceipt scanning...that is the behavior when the expense report is not a combined report.App/src/libs/OptionsListUtils.ts
Lines 310 to 313 in 156f0a2
To do that we can create a new param in
getCombinedReportActionslikeshouldFilterIOUActionwith default value istruethen if it'sfalsewe will return early hereWhat alternative solutions did you explore? (Optional)
NA
8 remaining items
francoisl commentedon Aug 28, 2024
Yeah I would tend to agree too.
One question about the approach though (and maybe this can be discussed in the PR) is why do we need a new param
shouldFilterIOUAction? Or asked differently, when would we usetrueand when would we usefalse? If we only set it tofalsefor one-transaction reports, could we count how many items are infilteredParentReportActionsinstead?Basically, I'm thinking if we can avoid introducing a new param, it's one less chance that someone will use it wrong in the future and cause a bug.
melvin-bot commentedon Aug 28, 2024
📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
Offer link
Upwork job
melvin-bot commentedon Aug 28, 2024
📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻
Keep in mind: Code of Conduct | Contributing 📖
melvin-bot commentedon Sep 20, 2024
This issue has not been updated in over 15 days. @francoisl, @twisterdotcom, @DylanDylann, @nkdengineer eroding to Monthly issue.
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!
DylanDylann commentedon Sep 23, 2024
@twisterdotcom @francoisl Please help to process payment here
twisterdotcom commentedon Sep 23, 2024
Payment Summary: