[HOLD for payment 2024-06-11] CRITICAL: [Track Expense] [$250] Self DM doesn't scroll down when tracking new expense

[quinthar]

Problem:

When viewing the self DM on desktop Chrome, and adding a new tracked expense, it should scroll down to show the new tracked expense just like when you post a message it will scroll down to show the new message. Similarly, when the actionable whisper comes after it, the screen should scroll down to show it. However, it doesn't.

Slack discussion (and video) here: https://expensify.slack.com/archives/C049HHMV9SM/p1713729858187919

Solution:

Two things:

1. Auto-scroll the selfDM upon tracking a new expense to show the newly tracked expense
2. Auto-scroll again when an actionable whisper appears

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e9e386da4e15219d
• Upwork Job ID: 1782138796167315456
• Last Price Increase: 2024-04-21
• Automatic offers:
• rojiphil | Reviewer | 0
• tienifr | Contributor | 0

Issue Owner

Current Issue Owner: @CortneyOfstad

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

⚠️ This issue has had its price increased by 4x or more. Please review the issue and ensure the price is correct.

[melvin-bot]

Upwork job price has been updated to $250

[Pycomet]

Contributor details
Your Expensify account email: alfredemmanuelinyang@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/codefred

This is my first proposal to work for Expensify. I've been able to test with the app, but not yet able to preview the video as I am waiting on my slack invite.

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[charles-liang]

Contributor details
Your Expensify account email: charles.xy.liang@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01620db3c30d4b7945

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[tienifr]

Proposal

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

Self DM doesn't scroll down when tracking new expense

What is the root cause of that problem?

Here, we're early returning (thus not scrolling user to the bottom) if hasNewestReportActionRef.current is false. This is because in case of comment linking, where hasNewestReportAction is false, we don't want to scroll users to the bottom and distracting them from the comment they're viewing.

But when we check hasNewestReportAction here, we're checking equality of sortedReportActions?.[0].created and report.lastVisibleActionCreated.

This doesn't work some cases:

• When the latest report action is a ACTIONABLETRACKEXPENSEWHISPER, the created of that report action is slightly more than lastVisibleActionCreated of the report (probably because in the back-end the date was generated at different times)
• If offline mode, when we create a new track expense, we're not setting report.lastVisibleActionCreated of the chat report to the created of the latest iou action, so sortedReportActions?.[0].created and report.lastVisibleActionCreated are different, and when we send a comment after that, it will never scroll to the bottom

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

We should update this to use >= check instead.

The only case where it does not hasNewestReportAction is when the sortedReportActions?.[0].created is smaller than report.lastVisibleActionCreated. That means we're only loading the relevant report actions to the linked comment, not the latest. But as long as the created of the latest action is equal or bigger than report.lastVisibleActionCreated, that means we already fetched the latest report actions up until report.lastVisibleActionCreated date, so we have the latest

What alternative solutions did you explore? (Optional)

We can keep the strict equal condition, but will need to:

• Update the back-end to always have report.lastVisibleActionCreated strictly equal to the created of it's latest report action (like in the case of ACTIONABLETRACKEXPENSEWHISPER)
• Update the optimistic data of the chatReport/iouReport when tracking expenses, to set lastVisibleActionCreated equal to the created of the newly added iouAction. And we need to double check other types of report actions too and make sure lastVisibleActionCreated is set properly (track expense for workspace, task, ...)

Currently the ACTIONABLETRACKEXPENSEWHISPER is not created optimistically when tracking expenses, if we want to create it optimistically, the approach will be the same but there'll be a few additional changes as suggested here.

[sword1202]

Contributor details
Expensify account email: payoneermap1202@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0194d962e1f9098218

[melvin-bot]

📣 @sword1202! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[sword1202]

Contributor details
Your Expensify account email: payoneermap1202@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0194d962e1f9098218

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[saifelance]

Proposal

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

Self DM doesn't scroll down when tracking new expense

What is the root cause of that problem?

Here, we're early returning (thus not scrolling user to the bottom) if hasNewestReportActionRef.current is false. This is because in case of comment linking, where hasNewestReportAction is false, we don't want to scroll users to the bottom and distracting them from the comment they're viewing.

But when we check hasNewestReportAction here, we're checking equality of sortedReportActions?.[0].created and report.lastVisibleActionCreated.

This doesn't work some cases:

• When the latest report action is a ACTIONABLETRACKEXPENSEWHISPER, the created of that report action is slightly more than lastVisibleActionCreated of the report (probably because in the back-end the date was generated at different times)
• If offline mode, when we create a new track expense, we're not setting report.lastVisibleActionCreated of the chat report to the created of the latest iou action, so sortedReportActions?.[0].created and report.lastVisibleActionCreated are different, and when we send a comment after that, it will never scroll to the bottom

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

We should update this to use >= check instead.

The only case where it does not hasNewestReportAction is when the sortedReportActions?.[0].created is smaller than report.lastVisibleActionCreated. That means we're only loading the relevant report actions to the linked comment, not the latest. But as long as the created of the latest action is equal or bigger than report.lastVisibleActionCreated, that means we already fetched the latest report actions up until report.lastVisibleActionCreated date, so we have the latest

What alternative solutions did you explore? (Optional)

We can keep the strict equal condition, but will need to:

• Update the back-end to always have report.lastVisibleActionCreated strictly equal to the created of it's latest report action (like in the case of ACTIONABLETRACKEXPENSEWHISPER)
• Update the optimistic data of the chatReport/iouReport when tracking expenses, to set lastVisibleActionCreated equal to the created of the newly added iouAction. And we need to double check other types of report actions too and make sure lastVisibleActionCreated is set properly (track expense for workspace, task, ...)

@tienifr

The issue at hand is that the self direct message (DM) chat window fails to scroll down when tracking a new expense.

Root Cause:
The scrolling behavior is affected by the conditional logic checking the value of hasNewestReportActionRef.current. This logic prevents scrolling to the bottom if hasNewestReportActionRef.current is false, which occurs in scenarios like comment linking where scrolling down might be distracting. However, the comparison used (sortedReportActions?.[0].created andreport.lastVisibleActionCreated) leads to inconsistencies:

When the latest report action is of type ACTIONABLETRACKEXPENSEWHISPER, their created timestamps might not match exactly, likely due to differences in backend processing times.
In offline mode, the report.lastVisibleActionCreated isn't updated properly when a new expense is tracked, causing a discrepancy between sortedReportActions?.[0].created and report.lastVisibleActionCreated

To address this issue, we should modify the comparison logic to use a greater than or equal to (>=) check instead of strict equality. This change ensures that scrolling occurs if the created timestamp of the latest action is equal to or greater than report.lastVisibleActionCreated, indicating that the latest actions have been fetched.

Alternative Solutions Considered:
While retaining the strict equality condition is an option, it would necessitate updates in both the backend and the front end:

Backend updates to ensure report.lastVisibleActionCreated is strictly equal to the created timestamp of the latest report action, especially for ACTIONABLETRACKEXPENSEWHISPER.
Front end updates to optimistically set lastVisibleActionCreated to match the created timestamp of newly added report actions (e.g., track expenses for workspace, task, etc.). This approach requires thorough verification for different types of report actions to ensure lastVisibleActionCreated is set accurately.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[saifelance]

@MelvinBot I haven't received yet any upwork job offer? So will wait If we mutually agree to a work contract.

[tienifr]

@saifelance you're new here. Please carefully read the contributing guidelines https://github.com/Expensify/App/blob/main/contributingGuides%2FCONTRIBUTING.md and every markdown file in the project to understand how contributors can work.

[CortneyOfstad]

@rojiphil any feedback on the proposal listed?

[CortneyOfstad]

Was deployed to production 2 days ago by Yuwen. Going to adjust the title to reflect payment 👍

[CortneyOfstad]

Payment is set for May 15. In the meantime @rojiphil can you confirm if this will need a regression test or not? That way we can have that completed in advance and avoid any delay in payment. If that could be completed by EOD Monday, that would be great — thanks!

[tienifr]

@CortneyOfstad No. The PR is still in review #41634. cc @rojiphil Please help review the PR once you have a chance.

[CortneyOfstad]

@tienifr thank you! I adjusted the title. @rojiphil — any update on the PR review?

[rojiphil]

@CortneyOfstad Current status is that @tienifr has to address the review comments here

[CortneyOfstad]

We're in a merge freeze until Wednesday

[CortneyOfstad]

Merge freeze is planned to end today 🤞

[melvin-bot]

@rojiphil, @marcochavezf, @CortneyOfstad, @tienifr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[rojiphil]

@marcochavezf Please let us know your inputs on the comment here. Thanks.

[melvin-bot]

@rojiphil, @marcochavezf, @CortneyOfstad, @tienifr Still overdue 6 days?! Let's take care of this!

[CortneyOfstad]

Bump @marcochavezf here. Thanks!

[rojiphil]

@CortneyOfstad The comments as mentioned here are clarified already.
The current status is that I have just now approved the PR. Now, waiting for the final review from @marcochavezf

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.78-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix Self DM doesn't scroll down #41634

If no regressions arise, payment will be issued on 2024-06-11. 🎊

For reference, here are some details about the assignees on this issue:

• @rojiphil requires payment automatic offer (Reviewer)
• @tienifr requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@rojiphil] The PR that introduced the bug has been identified. Link to the PR:
• [@rojiphil] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@rojiphil] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@rojiphil] Determine if we should create a regression test for this bug.
• [@rojiphil] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@CortneyOfstad] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[CortneyOfstad]

@rojiphil can you have the checklist as soon as you have a chance? Just want to make sure there is no delays in issuing payments. Thank you!

[tienifr]

Critical / High-priority issues will remain at $500.

@CortneyOfstad This is a CRITICAL issue so according to the announcement, this should start at $500. Could you help to update the issue to reflect the same?

TIA!