[$500] Money Request grouped message jumping after adding one more Money Request

[m-natarajan]

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.44-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @VickyStash
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1708959155001449

Action Performed:

1. Open a chat with any user
2. Create a Money Request for that user
3. Send several text messages
4. Create one more Money Request
5. The grouped message will be shown at the end of the chat for a couple of seconds and then jump back to the old position

Expected Result:

Grouped Money Request message should be displayed in the order of last money request was created

Actual Result:

Grouped Money Request message jump back to the old position in the chat, though a newer money request was created

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

bug_recording.mp4

Recording.2788.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017edbff2437b3b0b7
• Upwork Job ID: 1762291292036972544
• Last Price Increase: 2024-03-05

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017edbff2437b3b0b7

[melvin-bot]

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[wildan-m]

Proposal

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

Money Request grouped messages jump after adding another Money Request and not displayed in the order of last money request was created

What is the root cause of that problem?

When money is requested, if there is an active request, it will use the existing request instead of creating a new one.

App/src/libs/actions/IOU.ts

Lines 898 to 901 in a0e444e

	if (reportPreviewAction) {
	reportPreviewAction = ReportUtils.updateReportPreview(iouReport, reportPreviewAction, false, comment, optimisticTransaction);
	} else {
	reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport, comment, optimisticTransaction);

We sort the report action by the created property which makes the preview position of the money request remains unchanged.

App/src/libs/ReportActionsUtils.ts

Lines 220 to 225 in a0e444e

	const sortedActions = reportActions?.filter(Boolean).sort((first, second) => {
	// First sort by timestamp
	if (first.created !== second.created) {
	return (first.created < second.created ? -1 : 1) * invertedMultiplier;
	}

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

This solution will require a backend fix to update the 'lastModified' property.

We can make modifications to the 'lastModified' here:

App/src/libs/ReportUtils.ts

Lines 3305 to 3307 in a0e444e

	return {
	...reportPreviewAction,
	message: [

To

    const message = getReportPreviewMessage(iouReport, reportPreviewAction);
    return {
        ...reportPreviewAction,
        lastModified: DateUtils.getDBTime(),
        message: [
            {

And modify the sorting algorithm to utilize the lastModified property value (if available) for REPORTPREVIEW

Modify the following code:

App/src/libs/ReportActionsUtils.ts

Lines 220 to 225 in a0e444e

	const sortedActions = reportActions?.filter(Boolean).sort((first, second) => {
	// First sort by timestamp
	if (first.created !== second.created) {
	return (first.created < second.created ? -1 : 1) * invertedMultiplier;
	}

To

    const getTimestamp = (reportAction: ReportAction) =>
        reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.REPORTPREVIEW && reportAction.lastModified
            ? reportAction.lastModified
            : reportAction.created;

    const sortedActions = reportActions?.filter(Boolean).sort((first, second) => {
        const firstTimestamp = getTimestamp(first);
        const secondTimestamp = getTimestamp(second);
        if (firstTimestamp !== secondTimestamp) {
            return (firstTimestamp < secondTimestamp ? -1 : 1) * invertedMultiplier;
        }

We can also filter by 'childType' if needed.
Branch to test

What alternative solutions did you explore? (Optional)

We can revert this PR and update the created property in the backend each time we perform updateReportPreview. Although it works, I don't recommend it since it will cause the data to lose its created date information.

Note: Reverting the PR is not necessarily indicative of a regression; it is just one way to restore the created property.

[wildan-m]

@mkhutornyi, @miljakljajic Proposal updated with alternative solution.

[brunovjk]

Is this the same #30147?

[mkhutornyi]

reviewing 1 proposal

[mkhutornyi]

Clarifying the expected behavior in slack

[melvin-bot]

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

[mkhutornyi]

🎀 👀 🎀

[melvin-bot]

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

[wildan-m]

@mkhutornyi @yuwenmemon

🎀 👀 🎀

What does that mean? Does it signify the acceptance of my proposal? If so, who will assist with the backend part?

[mkhutornyi]

@wildan-m I pulled up engineer in advance to confirm the expected result. Not approved any proposals yet.

[Tony-MK]

@mkhutornyi, I think this is solved based on this #36501 (comment).

[wildan-m]

@Tony-MK depending on which expected behavior we choose

This post expected behavior is:

Grouped Money Request message should be displayed in the order of last money request was created

I'd suggest updating the position using the lastModified prop for easier navigation. For instance, Having a pending money request buried among hundreds of subsequent chats is not only frustrating for the user but also counterproductive. It forces the user to tediously scroll up through a mountain of messages just to locate the grouped report preview. This is not an efficient use of time and can lead to important information being overlooked or forgotten. Implementing a more organized system for displaying pending requests would greatly improve the user experience and streamline communication processes.

The current behavior will also lead to this new problem.

[miljakljajic]

Apologies - I've reopened the issue, I misunderstood this #37245 (comment) as consensus that the issue should be closed!

[wildan-m]

@mkhutornyi @yuwenmemon Is there any possibility of re-evaluating the expected behavior? Or what are the design/technical constraints preventing the re-arrangement of the report preview order?

[melvin-bot]

@yuwenmemon, @miljakljajic, @mkhutornyi Whoops! This issue is 2 days overdue. Let's get this updated quick!

[yuwenmemon]

Heads up I'm going OOO through next week so @miljakljajic we might want to ping someone else to answer the question above.

[miljakljajic]

Raising the question in VIP split

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot]

@yuwenmemon, @miljakljajic, @mkhutornyi Whoops! This issue is 2 days overdue. Let's get this updated quick!

[miljakljajic]

@m-natarajan can you try to reproduce this and see if its still an issue?

[wildan-m]

Yes, it still has not met the expected outcome.

Grouped Money Request message should be displayed in the order of last money request was created

Kapture.2024-04-17.at.16.25.19.mp4

@miljakljajic any response from the VIP split?

[melvin-bot]

@yuwenmemon, @miljakljajic, @mkhutornyi Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@yuwenmemon, @miljakljajic, @mkhutornyi 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[miljakljajic]

No response: https://expensify.slack.com/archives/C05RECHFBEW/p1712673042136469

bumping

[melvin-bot]

@yuwenmemon, @miljakljajic, @mkhutornyi Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[wildan-m]

@miljakljajic still no response? Perhaps you should tag engineers who are familiar with the issue.

[miljakljajic]

I think we can close this out - it hasn't been reproducible for a while and it doesn't seem like its a high priority for split right now.