App: [HOLD for payment 2023-10-09] HIGH: LHN logic is broken for paid reports
Correct logic as follows:
policyExpenseChat (member view)
- First line:
%workspaceName%(note: name depends on who’s looking at it, this is the member case) - Second line: latest message in the workspace chat. We a report has just paid paid, that’s
%workspaceName% paid %amount%
Report (member & admin)
- First line: report name (e.g.
%workspaceName% paid %amount%) - Second line: latest message, (e.g.
%userDisplayName% paid %amount%
At the moment the policyExpenseChat is incorrectly showing the second line from the report (e.g. Niki paid…) instead of what should should actually be the second line for the policyExpenseChat.
Example
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~0159fb834042f1a338
- Upwork Job ID: 1698698179177246720
- Last Price Increase: 2023-09-04
About this issue
- Original URL
- State: closed
- Created 10 months ago
- Comments: 71 (45 by maintainers)
@koko57 can you write a message on the new issue so I can assign you? Thanks! https://github.com/Expensify/App/issues/30042
Yes @trjExpensify would you be able to create a new one for this? You will be the best in defining exactly what needs to be done
$500 payment approved for @mananjadhav based on this summary.
I agree with Manan that this PR did not touch the logic of who the action is coming from.
And it feels like the subscript in of the paid expense report, that could be mixture of the backend changes to return both participants in the expense report and this change.
Either way we made this logic exteremely brittle depending of various things and these never ending regressions are consequence of it. I am not sure how to tackle this.
@koko57 we could try to abstract away some of the logic to determine to LHN preview messages and then create unit tests for that method based on various scenarios of the reports, would that be feasible?
I’ll be on top of this one.
Ah, actually I see your response a couple of hours ago in the PR. Thanks, let’s stay focused on this as we’d like to close the loop on this issue this week.
It can be a follow up, the tests should make sure the message is correctly composed for the LHN, but yeah it might be better for different test file
This looks good to me, I think I would name the
lastActoraslastActorDisplayNameto be more specific. Can you please raise a PR? thanks!🎉 lets do this!
Let us know if things are not clear from the issue body?
I think we will have to split the
getReportPreviewMessageinto two methods to make this cleaner even though they share lots of similar logic