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
2.99k stars 2.5k forks source link

Report field - LHN shows incorrect preview when editing report field & thread shows "You" #41074

Closed izarutskaya closed 1 week ago

izarutskaya commented 3 weeks 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: 1.4.66-2 Reproducible in staging?: Y Reproducible in production?: Y Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

Precondition:

  1. Go to staging.new.expensify.com
  2. Go to workspace chat.
  3. Submit an expense.
  4. Go to combined report.
  5. Click Submit to submit the report.
  6. Click on the report field (dropdown or date).
  7. Change any value in the report field.
  8. Note that LHN shows (email).
  9. Go to a different report and go back to the combined report.
  10. Note the LHN shows the correct system message preview.
  11. Right click on the report field system message > Reply in thread.

Expected Result:

In Step 8, when modifying report field, LHN preview will show the correct system message preview for the edit. In Step 11, the thread header for system message will show the complete system message.

Actual Result:

In Step 8, LHN preview initially shows (email) after editing report field. It only shows the correct system message preview after revisiting the report in Step 9. In Step 11, the thread header for system message shows "You".

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/61bdd369-0438-447b-bcb8-3c9f45795705

View all open jobs on GitHub

melvin-bot[bot] commented 3 weeks ago

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

izarutskaya commented 3 weeks ago

We think this issue might be related to the #collect project.

melvin-bot[bot] commented 2 weeks ago

@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

anmurali commented 2 weeks ago

Hmmm I cannot reproduce this but the site is also very unstable right now. I will try again after the deploy.

melvin-bot[bot] commented 2 weeks ago

@anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick!

bernhardoj commented 1 week ago

Proposal

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

There are 2 issues, first, the expense report LHN last message includes the current user display name and then disappears after reopening it. Second, the thread title of the edit field system message only shows "You".

What is the root cause of that problem?

For the first issue, I can't reproduce it because the action system message and the last message don't update immediately after after updating the report field. When the last message is updated, I don't see the current user email. If it happens again, then most likely it's because of the wrong lastActorAccountID.

For the second issue, the thread title, the logic to get the parent action message is below, https://github.com/Expensify/App/blob/0c2d5130727105535ee16ec0b5e932456ce3fea5/src/libs/ReportUtils.ts#L3048-L3059

It will take the action message array first item and it only contains "You",

image

while the chat message combines all message array into 1 text, that's why we can see the full text in the chat, but not in the header title.

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

We can add another condition in getReportActionMessage to check for CHANGE_FIELD action so it returns ReportActionsUtils.getReportActionMessageText because ReportActionsUtils.getReportActionMessageText combines all message into 1 text/string, https://github.com/Expensify/App/blob/0c2d5130727105535ee16ec0b5e932456ce3fea5/src/libs/ReportUtils.ts#L3052-L3054 https://github.com/Expensify/App/blob/0c2d5130727105535ee16ec0b5e932456ce3fea5/src/libs/ReportActionsUtils.ts#L1151-L1152

but I think it's better if we just return ReportActionsUtils.getReportActionMessageText here. https://github.com/Expensify/App/blob/0c2d5130727105535ee16ec0b5e932456ce3fea5/src/libs/ReportUtils.ts#L3058

return Str.removeSMSDomain(ReportActionsUtils.getReportActionMessageText(reportAction));
anmurali commented 1 week ago

Cannot reproduce.

bernhardoj commented 1 week ago

@anmurali hi, it's still reproducible, can you recheck, please?

https://github.com/Expensify/App/assets/50919443/45ec45b8-ed24-4717-bbeb-4e2d3d68d870

bernhardoj commented 4 days ago

@anmurali hi, can you recheck this one please?