[$500] Deleting the money request causes the welcome message to disappear

[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-3
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: @hungvu193
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1709178638097409

Action Performed:

1. Request money with an account that you don't have any message before.
2. Click on that money request and delete it.

Expected Result:

Welcome message shouldn't disappear

Actual Result:

Welcome message disappear

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

Recording.2807.mp4

bug.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01af5e498ce9f387d6
• Upwork Job ID: 1763340495313043456
• Last Price Increase: 2024-03-07

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

Proposal

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

Deleting money request report action from a new report with an account that you didn't have any contact before, causes the report's welcome message to disappear (blank ReportScreen).

The report itself also disappears from LHN when clicking on another report if the only report action left is the CREATED one (welcome message).

What is the root cause of that problem?

We have 2 issues here:

1. Main issue: as mentioned by #37576 (comment), when the money request report action is deleted, BE sets all actions of that report to null, including the CREATED action (welcome message).

I tested with a comment action instead and this doesn't happen, the CREATED is not set to null by BE, only the corresponding ADDCOMMENT action is removed by BE - as it should.

Upon calling OpenReport again by refresh page or resize to small then back to large -> the CREATED action will be repopulated so this means BE sets CREATED to null only visually as a result of DeleteMoneyRequest call specifically, without actually deleting the action from db like it does for the actual action we're deleting.

2. Secondary issue (expected behaviour?): the report will disappear from LHN if all report actions / messages are deleted and only the CREATED action (welcome message) is left - the report usually disappears when clicking on another LHN report.

Code reference for issue 2:

App/src/libs/ReportUtils.ts

Lines 3990 to 3993 in 48657b4

	// Hide chats between two users that haven't been commented on from the LNH
	if (excludeEmptyChats && isEmptyChat && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report) && canHideReport) {
	return false;
	}

The code mentioned above is the root cause for issue 2 since the shouldReportBeInOptionList function's logic returns true only for the report ID's which should be shown within the LHN report list.

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

1. For main issue (1) we have 2 ways of going about it:

Back-end: simply fix the DeleteMoneyRequest call's pusher and API response to ensure that when the money request action is deleted, we're not setting the CREATED action to null.

OR

Front-end

.

Even though as explained in the issue 1 RCA, BE sets the CREATED action to null visually when DeleteMoneyRequest is called, I noticed we have functionality which calls OpenReport on screen resize which repopulates the CREATED action:

Video

Screen.Recording.2024-03-01.at.07.17.02.mov

The logic which calls the OpenReport in this case is here:

App/src/pages/home/report/ReportActionsView.js

Lines 154 to 166 in 48657b4

	useEffect(() => {
	const prevIsSmallScreenWidth = prevIsSmallScreenWidthRef.current;
	// If the view is expanded from mobile to desktop layout
	// we update the new marker position, mark the report as read, and fetch new report actions
	const didScreenSizeIncrease = prevIsSmallScreenWidth && !props.isSmallScreenWidth;
	const didReportBecomeVisible = isReportFullyVisible && didScreenSizeIncrease;
	if (didReportBecomeVisible) {
	openReportIfNecessary();
	}
	// update ref with current state
	prevIsSmallScreenWidthRef.current = props.isSmallScreenWidth;
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [props.isSmallScreenWidth, props.reportActions, isReportFullyVisible]);

So as a FE solution we can have similar logic to call openReportIfNecessary() if the following conditions are true:

• all report's reportActions are empty and report is missing CREATED action (for good measure)

if the conditions are both true -> we trigger OpenReport call which will repopulate the CREATED action -> solving our issue.

Code would look like this:

    useEffect(() => {
        if (!_.isEmpty(props.reportActions)) {
            return;
        }
        // if there are no reportActions including CREATED (might've been set to null by BE)
        // call OpenReport to re-populate CREATED action
        if (!_.some(props.reportActions, ReportActionsUtils.isCreatedAction) && isReportFullyVisible) {
            openReportIfNecessary();
        }
    }, [props.reportActions, isReportFullyVisible]);

.

2. For the secondary issue (2):

• within this code block -> return true instead of false so that we always show empty reports in LHN whenever a conversation was started even if all actions were deleted - as long as only the CREATED report action exists.

Videos (main issue)

MacOS: Chrome / Safari

Before	After
Screen.Recording.2024-03-01.at.08.02.14.mov	Screen.Recording.2024-03-01.at.08.24.46.mov

Videos (secondary issue)

MacOS: Chrome / Safari

Before	After
Screen.Recording.2024-03-01.at.08.35.27.mov	Screen.Recording.2024-03-01.at.08.36.52.mov

[brandonhenry]

Proposal

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

The issue involves a user deleting a money request in a chat where they have not previously interacted with the other party, which unexpectedly results in the disappearance of the welcome message. Furthermore, this action causes the chat to vanish from the Left Hand Navigation (LHN) when the user navigates to another chat.

What is the root cause of that problem?

The root cause lies in the logic implemented in the application that determines whether a chat should be displayed in the LHN. The current implementation is designed to hide chats that have no interactions (messages or actions) besides the welcome message. This is based on the assumption that an empty chat or a chat with only a welcome message is not valuable to the user and clutters the LHN. Specifically, the conditional logic within the shouldReportBeInOptionList function is designed to exclude these chats from the LHN, resulting in the reported behavior when all messages in a new chat are deleted.

App/src/libs/ReportUtils.ts

Lines 3990 to 3993 in 48657b4

	// Hide chats between two users that haven't been commented on from the LNH
	if (excludeEmptyChats && isEmptyChat && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report) && canHideReport) {
	return false;
	}

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

The proposed solution involves modifying the conditional logic that determines which chats are displayed in the LHN. Rather than removing the lines of code that exclude empty chats from the LHN, a more nuanced approach should be taken to address the issue while maintaining the original intent of the logic.

A proposed change could involve adding a condition that checks for the existence of a welcome message as a valid report action that qualifies the chat to remain in the LHN. This would ensure that new chats, even if their only content (a money request) is deleted, would still retain their place in the LHN due to the presence of the welcome message.

Here is a pseudocode example of the proposed modification:

// In App/src/libs/ReportUtils.ts, within the shouldReportBeInOptionList function

// Existing condition that checks various factors to decide if a report should be hidden
if (excludeEmptyChats && isEmptyChat && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report) && canHideReport) {
    // Add an additional check to see if the report contains a welcome message
    if (reportContainsWelcomeMessage(report)) {
        return true; // Keep the report in the LHN if it contains a welcome message
    }
    return false; // Hide the chat if it doesn't meet the new condition
}

// Implement the reportContainsWelcomeMessage function to check for a welcome message in the report
function reportContainsWelcomeMessage(report) {
    // Logic to determine if a welcome message exists in the report
    // This would involve checking the report's actions for a specific type or marker of the welcome message
}

This seems the ideal solution since we don't want to alter the original logic here as that will mess up the chat between two users who did not have the welcome message but also do not have any messages together (thus it should not show in LHN)

What alternative solutions did you explore? (Optional)

An alternative solution considered was the direct removal of the code lines that cause empty chats to be hidden from the LHN (or essentially have the current line return true). However, this approach could lead to unintended consequences, such as cluttering the LHN with truly empty chats that have no significance to the user. Therefore, the proposed solution aims to strike a balance by modifying the logic to consider chats with welcome messages as significant. This maintains the clean and user-friendly interface of the LHN while addressing the issue at hand.

[bernhardoj]

This is a BE issue. The DeleteMoneyRequest response sends a SET request that clear the report actions.

It should be a merge.

[ikevin127]

Updated proposal

• split RCA / solution in 2 issues to offer broader coverage depending on the issue's targeted expected result

@bernhardoj That's true, I updated my proposal mentioning that. I also tend towards a BE fix for there since it only happens with the DeleteMoneyRequest -> something went wrong with the endpoint because it didn't use to do that to the CREATED action.

[DylanDylann]

@adelekennedy This is a BE bug, please help to add an internal label to get an eye from the internal engineer

[omarnagy91]

Proposal :

Problem

Deleting a money request causes the welcome message to disappear in chat reports where the money request was the only message.

Root Cause

The getLastVisibleMessage function in ReportUtils.ts returns a new object with only the lastMessageText property set when a money request is deleted. This overwrites the existing lastMessageText property on the report object, causing the welcome message to disappear.

Proposed Changes

Instead of returning a new object with only lastMessageText when a money request is deleted, we should check if the report already has a lastMessageText property set. If it does, we should return the existing lastMessageText. This will ensure that the welcome message is not overwritten.

Here's the updated code for the getLastVisibleMessage function:

function getLastVisibleMessage(reportID: string, actionsToMerge: ReportActions = {}): LastVisibleMessage {
    const report = getReport(reportID);
    const lastVisibleAction = ReportActionsUtils.getLastVisibleAction(reportID, actionsToMerge);

    // For Chat Report with deleted parent actions, let us fetch the correct message
    if (ReportActionsUtils.isDeletedParentAction(lastVisibleAction) && !isEmptyObject(report) && isChatReport(report)) {
        // Instead of returning a new object with only lastMessageText,
        // we will return the report's lastMessageText if it exists.
        // This ensures that the welcome message is not overwritten.
        return {
            lastMessageText: report.lastMessageText ?? '',
        };
    }

    // Fetch the last visible message for report represented by reportID and based on actions to merge.
    return ReportActionsUtils.getLastVisibleMessage(reportID, actionsToMerge);
}

Alternative Solutions

We could consider adding additional logic to the getDeletedParentActionMessageForChatReport function to check if the report has a welcome message and return that instead of the deleted message text. However, this would add additional complexity and may not be as reliable as the proposed solution.

We believe that the proposed solution is the simplest and most effective way to address this issue.

[melvin-bot]

Current assignee @DylanDylann is eligible for the Internal assigner, not assigning anyone new.

[adelekennedy]

added internal but we will need to add this to a wave/vip to get internal help - tomorrow there will be an update

[adelekennedy]

I'm a bit stuck here on the best process - I would consider this a polish since it shouldn't block a release but am not sure how to classify it in the new project board

[melvin-bot]

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