Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[HOLD for payment][$250] - LHN preview is missing distance rate name in distance rate removal system message #46967

Closed lanitochka17 closed 3 weeks ago

lanitochka17 commented 3 months ago

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.17-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4827991 Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

Expected Result:

The LHN preview will display the complete distance rate removal system message

Actual Result:

The LHN preview is missing the name of the removed distance rate in the system message

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/02c7414b-77e1-4a1a-9b7e-bce0b636b452

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016424eee2de4a22ba
  • Upwork Job ID: 1821384517474279929
  • Last Price Increase: 2024-08-22
  • Automatic offers:
    • FitseTLT | Contributor | 103698148
Issue OwnerCurrent Issue Owner: @allroundexperts
melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @kadiealexander (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.

lanitochka17 commented 3 months ago

We think that this bug might be related to #wave-collect - Release 1

lanitochka17 commented 3 months ago

@kadiealexander 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

FitseTLT commented 3 months ago

Edited by proposal-police: This proposal was edited at 2024-08-07 19:39:54 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

LHN preview is missing distance rate name in distance rate removal system message

What is the root cause of that problem?

In getOptionData we are not dealing with POLICY_CHANGE_LOG.DELETE_CUSTOM_UNIT_RATE action types when getting alternateText for the LHN so it is falling back to setting it to the lastMessage of the report which misses distance rate name https://github.com/Expensify/App/blob/b806725c60547ac3e71470670cf34a0de107000b/src/libs/SidebarUtils.ts#L431-L432

What changes do you think we should make in order to solve the problem?

We need to set alternateText to the getReportActionMessageText of the lastAction if the last action type is POLICY_CHANGE_LOG.DELETE_CUSTOM_UNIT_RATE (FYI: That is how the report action is displayed in report action item) here

else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.DELETE_CUSTOM_UNIT_RATE) {
            result.alternateText = ReportActionsUtils.getReportActionMessageText(lastAction) ?? '';
        } 

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 3 months ago

Job added to Upwork: https://www.upwork.com/jobs/~016424eee2de4a22ba

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)

BhuvaneshPatil commented 3 months ago

@lanitochka17 @kadiealexander Can you try reproducing the bug. I was able to do that yesterday but not anymore

trjExpensify commented 3 months ago

CC: @neil-marcellini who's working on this project externally with context to help squash these? I think this one is conditional on the p2p beta for rate, so #vip-vsb related.

melvin-bot[bot] commented 3 months ago

@allroundexperts, @kadiealexander Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 3 months ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

melvin-bot[bot] commented 3 months ago

@allroundexperts, @kadiealexander 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

tsa321 commented 3 months ago

Maybe a BE issue. The report.lastMessageText doesn't include the rate name.

melvin-bot[bot] commented 2 months ago

@allroundexperts, @kadiealexander 10 days overdue. I'm getting more depressed than Marvin.

melvin-bot[bot] commented 2 months ago

@allroundexperts @kadiealexander this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 2 months ago

@allroundexperts, @kadiealexander 12 days overdue now... This issue's end is nigh!

melvin-bot[bot] commented 2 months ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

kadiealexander commented 2 months ago

@allroundexperts please take a look at the proposal above!

lanitochka17 commented 2 months ago

Issue is still reproducible

https://github.com/user-attachments/assets/59a7b738-23e3-444e-88d9-93b73e8eddba

allroundexperts commented 2 months ago

@FitseTLT's proposal works good. I think the solution proposed is making sense as well. Let's go with them.

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @Julesssss, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] commented 2 months ago

πŸ“£ @FitseTLT πŸŽ‰ 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 πŸ“–

neil-marcellini commented 2 months ago

I'm now realizing that this is related to distance rate configuration alone, not the P2P distance project.

melvin-bot[bot] commented 1 month ago

This issue has not been updated in over 15 days. @Julesssss, @allroundexperts, @FitseTLT, @kadiealexander 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!

allroundexperts commented 1 month ago

I think this is pending payment. Can someone please take a look?

Julesssss commented 1 month ago

HI @kadiealexander would you mind checking that payment hasn't occurred yet? Thanks

FitseTLT commented 1 month ago

@kadiealexander Payment overdue

FitseTLT commented 1 month ago

@kadiealexander Can you pls issue payment here?? Thx

kadiealexander commented 1 month ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

kadiealexander commented 1 month ago

Payouts due:

Upwork job is here.

JmillsExpensify commented 1 month ago

$250 approved for @allroundexperts

melvin-bot[bot] commented 3 weeks ago

@Julesssss, @allroundexperts, @FitseTLT, @kadiealexander Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Julesssss commented 3 weeks ago

Hey @allroundexperts has this been paid out?

allroundexperts commented 3 weeks ago

Yes, feel free to close.