search

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.34k stars 2.77k forks source link

[$250] Expense - Preview in main chat briefly disappears after renaming report and deleting expense #44607

Open izarutskaya opened 2 months ago

izarutskaya commented 2 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-3.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/4678766 Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com/
  2. Go to workspace chat.
  3. Submit two expenses.
  4. Go to expense report.
  5. Click Title field.
  6. Change the report title and save it.
  7. Submit another expense.
  8. Go to main chat and back to expense report for the rename message to show up.
  9. Go to transaction thread of the latest expense (submitted in Step 7).
  10. Click on the report header > Delete expense.
  11. Delete the expense.

Expected Result:

App will return to expense report because there are still two expenses in the report.

Actual Result:

App returns to main chat, and the expense preview in the main chat disappears briefly.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/0fb0c015-df4a-4724-9ebb-f961c368aad8

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d610c7726bd86731
  • Upwork Job ID: 1807883887257636893
  • Last Price Increase: 2024-07-08
melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @johncschuster (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 2 months ago

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

bernhardoj commented 2 months ago

Proposal

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

Deleting a transaction removes the expense report if the last action in the expense report is renamed action.

What is the root cause of that problem?

The expense report will be deleted if the last message text of the expense report is empty. https://github.com/Expensify/App/blob/cb8fce822d0719989a335011f7715d3e6f670efe/src/libs/actions/IOU.ts#L5313-L5315

We get the last message from the HTML of the message here (getTextFromHtml). https://github.com/Expensify/App/blob/cb8fce822d0719989a335011f7715d3e6f670efe/src/libs/ReportActionsUtils.ts#L765-L771

However, the renamed action doesn't have HTML.

image

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

Instead of using getTextFromHtml, we can use getReportActionMessageText.

let messageText = getReportActionMessageText(lastVisibleAction) ?? '';

getReportActionMessageText will get from the HTML but fallback to text if it doesn't exist. https://github.com/Expensify/App/blob/53d08c16f98ee7a7dcd201d40bd62c7be662aff9/src/libs/ReportActionsUtils.ts#L1353-L1360

So the iou report last message won't be empty and the expense report won't be deleted.

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01d610c7726bd86731

melvin-bot[bot] commented 2 months ago

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

trjExpensify commented 2 months ago

report titles/fields is a project in wave-control.

melvin-bot[bot] commented 2 months ago

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

sobitneupane commented 2 months ago

Thanks for the proposal @bernhardoj

Can you please add more details in the Solution section of your proposal. How will it solve the issue?

bernhardoj commented 2 months ago

Updated

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? 💸

sobitneupane commented 2 months ago

Thanks @bernhardoj

Proposal from @bernhardoj looks good to me.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

@tylerkaraszewski @johncschuster @sobitneupane 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!

sobitneupane commented 1 month ago

PR deployed to production 1 week ago. https://github.com/Expensify/App/pull/45365#issuecomment-2244528145 This is ready for payment.

melvin-bot[bot] commented 4 weeks ago

This issue has not been updated in over 15 days. @tylerkaraszewski, @johncschuster, @sobitneupane, @bernhardoj 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!

sobitneupane commented 4 weeks ago

@johncschuster This is ready for payment. PR was developed a month back. https://github.com/Expensify/App/pull/45365#issuecomment-2244528145

sobitneupane commented 2 weeks ago

Bump @johncschuster

bernhardoj commented 2 weeks ago

Payment pending. Requested in ND.

johncschuster commented 1 week ago

Sorry for the delay! Working on this now.

johncschuster commented 1 week ago

Payment Summary:

Contributor: @bernhardoj owed $250 via NewDot Contributor+: @sobitneupane owed $250 via NewDot

JmillsExpensify commented 1 week ago

$250 approved for @bernhardoj

johncschuster commented 1 week ago

@sobitneupane, do we need any regression test steps? If so, can you provide them?

sobitneupane commented 1 week ago

Regression Test Proposal

  1. Open a workspace chat
  2. Submit 2 money requests
  3. Rename the expense report title
  4. Reopen/refresh the expense report to see the renamed system message
  5. Add another money request
  6. Delete the recently added money request
  7. Verify the expense report isn't deleted and you stay at the expense report page

Do we agree 👍 or 👎