Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
6 tasks done
izarutskaya opened this issue Jun 28, 2024 · 25 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Monthly KSv2 Reviewing Has a PR in review

Comments

@izarutskaya
Copy link

izarutskaya commented Jun 28, 2024

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?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6527060_1719557112849.20240628_143744.mp4

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
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 28, 2024
Copy link

melvin-bot bot commented Jun 28, 2024

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
Copy link
Author

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

@bernhardoj
Copy link
Contributor

bernhardoj commented Jun 28, 2024

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.

App/src/libs/actions/IOU.ts

Lines 5313 to 5315 in cb8fce8

const iouReportLastMessageText = ReportActionsUtils.getLastVisibleMessage(iouReport?.reportID ?? '-1', updatedReportAction).lastMessageText;
const shouldDeleteIOUReport =
iouReportLastMessageText.length === 0 && !ReportActionsUtils.isDeletedParentAction(lastVisibleAction) && (!transactionThreadID || shouldDeleteTransactionThread);

We get the last message from the HTML of the message here (getTextFromHtml).

let messageText = getTextFromHtml(message?.html) ?? '';
if (messageText) {
messageText = StringUtils.lineBreaksToSpaces(String(messageText)).substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim();
}
return {
lastMessageText: messageText,
};

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.

function getReportActionMessageText(reportAction: OnyxEntry<ReportAction>): string {
if (!Array.isArray(reportAction?.message)) {
return getReportActionText(reportAction);
}
// Sometime html can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
return reportAction?.message?.reduce((acc, curr) => `${acc}${getTextFromHtml(curr?.html || curr?.text)}`, '') ?? '';
}

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

@melvin-bot melvin-bot bot added the Overdue label Jul 1, 2024
Copy link

melvin-bot bot commented Jul 1, 2024

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

@johncschuster johncschuster added the External Added to denote the issue can be worked on by a contributor label Jul 1, 2024
@melvin-bot melvin-bot bot changed the title Expense - Preview in main chat briefly disappears after renaming report and deleting expense [$250] Expense - Preview in main chat briefly disappears after renaming report and deleting expense Jul 1, 2024
Copy link

melvin-bot bot commented Jul 1, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 1, 2024
Copy link

melvin-bot bot commented Jul 1, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Jul 1, 2024
@trjExpensify
Copy link
Contributor

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

Copy link

melvin-bot bot commented Jul 5, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jul 5, 2024
@sobitneupane
Copy link
Contributor

Thanks for the proposal @bernhardoj

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

@melvin-bot melvin-bot bot removed the Overdue label Jul 8, 2024
@bernhardoj
Copy link
Contributor

Updated

Copy link

melvin-bot bot commented Jul 8, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@sobitneupane
Copy link
Contributor

Thanks @bernhardoj

Proposal from @bernhardoj looks good to me.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Jul 10, 2024

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

Copy link

melvin-bot bot commented Jul 12, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jul 12, 2024
@melvin-bot melvin-bot bot removed Help Wanted Apply this label when an issue is open to proposals by contributors Overdue labels Jul 12, 2024
@sobitneupane
Copy link
Contributor

PR deployed to production 1 week ago. #45365 (comment) This is ready for payment.

@melvin-bot melvin-bot bot removed the Weekly KSv2 label Aug 21, 2024
Copy link

melvin-bot bot commented Aug 21, 2024

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!

@melvin-bot melvin-bot bot added the Monthly KSv2 label Aug 21, 2024
@sobitneupane
Copy link
Contributor

@johncschuster This is ready for payment. PR was developed a month back. #45365 (comment)

@sobitneupane
Copy link
Contributor

Bump @johncschuster

@bernhardoj
Copy link
Contributor

Payment pending. Requested in ND.

@johncschuster
Copy link
Contributor

Sorry for the delay! Working on this now.

@johncschuster
Copy link
Contributor

Payment Summary:

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

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

@johncschuster
Copy link
Contributor

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

@sobitneupane
Copy link
Contributor

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 👎

@johncschuster
Copy link
Contributor

Thanks, @sobitneupane! Please request payment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Monthly KSv2 Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

7 participants