[HOLD #40724] [$250] IOU –Expense chat doesn't scroll to bottom when send message and create second money request

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.63-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #39685

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Log in
3. Create a new WS
4. Navigate to WS chat and create a request
5. Open IOU details
6. Send a message
7. Create a request

Expected Result:

Expense chat scroll to bottom when send a message and create second money request

Actual Result:

Expense chat doesn't scroll to bottom when send a message and create second money request

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

Add any screenshot/video evidence

Bug6454206_1713469798697.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cba2ec575ed8acd2
• Upwork Job ID: 1784978907081064448
• Last Price Increase: 2024-05-07
• Automatic offers:
• hoangzinh | Reviewer | 0
• samilabud | Contributor | 0

[melvin-bot]

Triggered auto assignment to @puneetlath (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]

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

[puneetlath]

@lanitochka17 is this happening on specific platforms? Or all?

[lanitochka17]

@puneetlath only Android

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[samilabud]

Proposal

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

IOU –Expense chat doesn't scroll to bottom when send message or money request

What is the root cause of that problem?

Currently we are using DateUtils library to create the optimistic values, for example when we send a new message the report created and lastVisibleActionCreated are not using the same instance/values of the DateUtils. we are using it two times which cause the difference between them causing hasNewestReportAction to be false and this makes the auto scroll not works.

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

We should modify every method related to build optimistic when sending a new comment, split expense, submit expense, etc.

To fix the new comment sending, the created date is assigned using same function used for lastVisibleActionCreated DateUtils.getDBTimeWithSkew (in our scenarios without a offset), this is the part of the code:

App/src/libs/ReportUtils.ts

Lines 3442 to 3444 in 9f51274

	avatar: allPersonalDetails?.[accountID ?? -1]?.avatar ?? UserUtils.getDefaultAvatarURL(accountID),
	created: DateUtils.getDBTimeWithSkew(Date.now() + createdOffset),
	message: [

To add a comment we use the addAction method:

App/src/libs/actions/Report.ts

Lines 439 to 448 in 9f51274

	/**
	* Add up to two report actions to a report. This method can be called for the following situations:
	*
	* - Adding one comment
	* - Adding one attachment
	* - Add both a comment and attachment simultaneously
	*/
	function addActions(reportID: string, text = '', file?: FileObject) {
	let reportCommentText = '';
	let reportCommentAction: OptimisticAddCommentReportAction | undefined;

But this never going to match because the lastVisibleActionCreated is created in different function in different time, so we should modify the buildOptimisticAddCommentReportAction to accept the currentTime and use it to specify the created prop like:

created: currentTime ?? DateUtils.getDBTimeWithSkew(Date.now() + createdOffset),

To send a split expense we should modify createSplitsAndOnyxData to use the same currentTime like:

    const currentTime = DateUtils.getDBTime();
    splitChatReport.lastReadTime = currentTime;
    splitChatReport.lastMessageText = splitIOUReportAction.message?.[0]?.text;
    splitChatReport.lastMessageHtml = splitIOUReportAction.message?.[0]?.html;
    splitChatReport.lastActorAccountID = currentUserAccountID;
    splitChatReport.lastVisibleActionCreated = currentTime;
    splitIOUReportAction.created = currentTime;

(For the last one it might be better to modify ReportUtils.buildOptimisticIOUReportAction and getOrCreateOptimisticSplitChatReport to accept currentTime as a new parameter, but currently there are other mutations for splitChatReport.)

To submit an expense we should modify buildOptimisticReportPreview to accept a new parameter containing the created time:

function buildOptimisticReportPreview(
    .
    .
    .
    created = DateUtils.getDBTime(),

This way we can send the create date from getMoneyRequestInformation passing the same currentTime like:

const currentTime = DateUtils.getDBTime();
.
.
.
let chatReport = !isEmptyObject(parentChatReport) && parentChatReport?.reportID ? parentChatReport : null;
**if (chatReport) {
      chatReport.lastVisibleActionCreated = currentTime;
  }**
.
.
.
if (reportPreviewAction) {
        reportPreviewAction = ReportUtils.updateReportPreview(iouReport, reportPreviewAction, false, comment, optimisticTransaction);
    } else {
        reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport, comment, optimisticTransaction, '', **currentTime**);

Tests:

Sending a new message and a new split expense

Fixed.optimistic.time.-.autoscroll.test.mp4

Sending a new "submit expense":

Submit.expense.test.mov

[puneetlath]

@hoangzinh thoughts on the proposals so far?

[hoangzinh]

but we got the issue when we set the FlatList without this prop, the Flatlist component do not detects the change of this if we add this after a render.

@samilabud I don't think your RC is correct. Can you check why user just sent a new message but shouldEnableAutoScrollToTopThreshold is false, lead to "we set the FlatList without this prop" as you said above?

[samilabud]

but we got the issue when we set the FlatList without this prop, the Flatlist component do not detects the change of this if we add this after a render.

@samilabud I don't think your RC is correct. Can you check why user just sent a new message but shouldEnableAutoScrollToTopThreshold is false, lead to "we set the FlatList without this prop" as you said above?

Hi, when the reportActionsView load, set a variable to determinate if there are a new message, when the user sent a new message that value is false and later we send that false into the next statement:

const shouldEnableAutoScroll = hasNewestReportAction && (!reportActionID || !isNavigatingToLinkedMessage);

Because of this false value, the FlatList going to take that behavior not matter what we are sending as a message.

[hoangzinh]

Actually, your solution doesn't work in offline mode

Screen.Recording.2024-05-03.at.18.19.58.mp4

when the user sent a new message that value is false

@samilabud If we put a log there, we can see that value would be true after seconds. Can you find a reason for it?

[samilabud]

Actually, your solution doesn't work in offline mode

Screen.Recording.2024-05-03.at.18.19.58.mp4

when the user sent a new message that value is false

@samilabud If we put a log there, we can see that value would be true after seconds. Can you find a reason for it?

Hey @hoangzinh, as far I can see to determinate if we has a new message we are comparing the last time we synchronized with the server (lastVisibleActionCreated) vs the current time of the last message we sent (reportActions[0]?.created). This cause the next behavior:

If we are online

1. When the chat render the reportActions[0]?.created match with lastVisibleActionCreated so we got true (for shouldEnableAutoScroll).
2. When we send a message the lastVisibleActionCreated remains unchanged and reportActions[0]?.created change to the current time, this makes that shouldEnableAutoScroll switch to false**. (it seems the logic behind is to wait for the server response to do the automatic scroll down)**
3. After we synchronized with the server (when we got the response that the message was sent) we got a true ( in shouldEnableAutoScroll) because reportActions[0]?.created match with lastVisibleActionCreated match again.

If we are offline

1. Same as first step when online
2. Same as second step when online
3. We are not synchronizing with the server (we don't make any request) the shouldEnableAutoScroll remains false because reportActions[0]?.created is changing to the current time but lastVisibleActionCreated keeps as the same until we get online again.

As I said in before it seems the logic behind is to wait for the server response to do the automatic scroll down, so maybe this is the expected behavior, except for the error which was reported here.

Please let me know if I should verify deeper or if you have any other suggestion.

[melvin-bot]

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

[hoangzinh]

2. When we send a message the lastVisibleActionCreated remains unchanged and reportActions[0]?.created change to the current time

@samilabud I don't think it's correct. Can you check again? I think the auto scroll behavior in chat should work for both online and offline mode.

[samilabud]

Hi everyone, I have created the PR, but still waiting for the offer.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot]

📣 @hoangzinh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @samilabud 🎉 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 📖

[puneetlath]

FYI @samilabud the C+ (in this case @hoangzinh) provides the recommendation on who to hire, but you aren't actually hired until an Expensify internal engineer (in this case me) agrees.

[samilabud]

FYI @samilabud the C+ (in this case @hoangzinh) provides the recommendation on who to hire, but you aren't actually hired until an Expensify internal engineer (in this case me) agrees.

oh ok, good to know, thanks!

[melvin-bot]

This issue has not been updated in over 15 days. @puneetlath, @samilabud, @hoangzinh 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!

[puneetlath]

What's the status of the PR @samilabud @hoangzinh?

[samilabud]

What's the status of the PR @samilabud @hoangzinh?

Ready to review since: #40594 (comment)

[samilabud]

Oh no my bad, was reviewed and waiting for: #42030 (comment)

[puneetlath]

From what I can see you've been hired in Upwork. I'm more asking about the status of the PR.

[samilabud]

about the status of the PR.

The PR is ready, and the review was done but my reviewer was asking about if we can merge this in #42030 (comment).

I don't pretty sure about the next steps, sorry.

[puneetlath]

@allroundexperts where do you think we go from here?

[hoangzinh]

I think we can put this issue on hold for #40724

[hoangzinh]

@puneetlath the holding PR has been merged. It appears that our issue has been resolved. Can we ask QA to test this issue again?

Screen.Recording.2024-06-25.at.17.44.48.mov

[kavimuru]

Bug is fixed.

Screenrecorder-2024-06-25-18-31-58-959.mp4

[puneetlath]

Ok @hoangzinh so is any payment needed here? Or was this ultimately solved by a different PR?

[hoangzinh]

@puneetlath IMO, to be fair, I think we should make payment for @samilabud and me here. We made efforts to find the root cause/solution and the PR is almost ready. But it was put back to on hold because there was another similar issue that provided a better solution (comment here and here)

[puneetlath]

Ok seems fair. I've paid @samilabud and sent you an offer @hoangzinh: https://www.upwork.com/nx/wm/offer/102916271

Ping me here when you accept.

[hoangzinh]

Accepted. Thanks @puneetlath

[puneetlath]

Ok all paid. Thanks y'all.