[HOLD for payment 2025-01-18] [$250] Search - Workspace chat preview, flickers between last message and information message

[IuliiaHerets]

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: 9.0.69-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
Issue reported by: Applause Internal Team

Action Performed:

1. Open the staging.new.expensify.com website.
2. Tap on "Settings" and select "Workspaces"
3. Open any workspace and tap on "Members"
4. Tap on "Invite Member"
5. Invite any member to workspace.
6. Return to LHN.
7. Open the workspace individual chat created for this invited member.
8. Send any message on it.
9. Tap on the search icon.
10. Start writing a search related to this last chat.
11. Verify that with every letter added on the search input, this chat preview keeps displaying the last sent message.

Expected Result:

The preview of the individual chat created for an invited workspace member, should keep displaying the last sent message when writing on the search input.

Actual Result:

Chat preview of individual chat created for an invited workspace member, flickers between the last sent message and the workspace information message with every letter added on the search input.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6681273_1733068550003.WS_Member_chat.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021863580183549143544
• Upwork Job ID: 1863580183549143544
• Last Price Increase: 2024-12-16
• Automatic offers:
• rojiphil | Reviewer | 105395423

Issue Owner

Current Issue Owner: @isabelastisser

[melvin-bot]

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

Triggered auto assignment to @Beamanator (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[mountiny]

This is a minor ux issue, feels like the state is changing between optimistic offline and the response from the server

Demoted

[mountiny]

cc @luacmartins @lakchote you might know the best about changes related to this area of code

[melvin-bot]

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

[melvin-bot]

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

[Beamanator]

Demoting makes sense to me 👍 making this external since the flicker sounds like a UI-only bug

[bernhardoj]

Proposal

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

Workspace chat subtitle text flickers when typing on search modal.

What is the root cause of that problem?

The flicker changes between the last message and the workspace name. When we get the option list subtitle, we mutate the original report object.

App/src/libs/OptionsListUtils.ts

Line 1196 in 892cef1

reportOption.alternateText = getAlternateText(reportOption, {showChatPreviewLine, forcePolicyNamePreview});

This report object comes from OptionsListContextProvider which shares the options list between multiple components. In our case, when we open the search page, there are 3 usages that calls getOptions, two of the usages pass forcePolicyNamePreview as true and the 3rd one passes forcePolicyNamePreview as false, and showChatPreviewLine as true.

App/src/libs/OptionsListUtils.ts

Lines 435 to 437 in 892cef1

	if ((option.isPolicyExpenseChat ?? false) || isAdminRoom || isAnnounceRoom) {
	return showChatPreviewLine && !forcePolicyNamePreview && formattedLastMessageText ? formattedLastMessageText : option.subtitle;
	}

This happens after #53293 which adds the showChatPreviewLine to fix #53291. But from my testing, even if I remove the changes from that PR, the issue doesn't happen anymore. I'm not sure what causes that issue (53291), but the changes from PR (53293) add the showChatPreviewLine to the participantsAutocompleteList section, while the issue happens on the recent reports/chats section.

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

We can probably revert #53293, but I think we should avoid mutating the original object in the first place. To do that, we need to convert this into a const first.

App/src/libs/OptionsListUtils.ts

Line 1196 in 892cef1

reportOption.alternateText = getAlternateText(reportOption, {showChatPreviewLine, forcePolicyNamePreview});

const alternateText = getAlternateText(reportOption, {showChatPreviewLine, forcePolicyNamePreview});

And instead of using the reportOption, we need to clone the object first.

App/src/libs/OptionsListUtils.ts

Lines 1244 to 1248 in 892cef1

	recentReportOptions.push(reportOption);
	}
	} else {
	recentReportOptions.push(reportOption);
	}

const newReportOption = {
    ...reportOption,
    alternateText,
}

We can do the same for this:

App/src/libs/OptionsListUtils.ts

Lines 1238 to 1239 in 892cef1

	reportOption.isSelected = isReportSelected(reportOption, selectedOptions);
	reportOption.isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption);

App/src/libs/OptionsListUtils.ts

Line 1261 in 892cef1

reportOption.lastIOUCreationDate = lastIOUAction.lastModified;

But I think the reason we mutate it so we don't affect the performance from the cloning operation. If we don't want to do the cloning, we can do the hard way, that is by changing the item structure to be like this:

{
option: reportOption,
alternateText,
isSelected,
...
}

There will be a lot of reactors and it actually looks similar to this.

App/src/libs/OptionsListUtils.ts

Lines 49 to 51 in 892cef1

	type SearchOption<T> = ReportUtils.OptionData & {
	item: T;
	};

[aldo-expensify]

but I think we should avoid mutating the original object in the first place. To do that, we need to convert this into a const first.

I think these mutations are very prone to introducing hard to find bugs so they should be removed.

But I think the reason we mutate it so we don't affect the performance from the cloning operation.

I'm pretty sure we can avoid performance issues without introducing mutations. How many options are we processing here?

We can do the same for this:
reportOption.isSelected = isReportSelected(reportOption, selectedOptions);
reportOption.isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption);

Yes, lets just avoid cloning the same object each time a key is changed :P

[luacmartins]

I agree that we should remove the mutations.

I'm pretty sure we can avoid performance issues without introducing mutations. How many options are we processing here?

This function is used by the LHN as well, so we are processing the full LHN results for users. Not sure how large that data set is though.

[lakchote]

If the current behavior is acceptable, then we can close this and wait until another unexpected issue caused by the mutation side-effect comes up.

This is not an acceptable behavior to have a flickering UI (bug starts happening at 0:43 in the video for those finding the issue).

We need to address this.

[bernhardoj]

Oh, I don't mean the flickering behavior. I and @rojiphil can't see the flickering anymore, but the mutation still causes a different expected behavior shown from the code which I explained here.

[melvin-bot]

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

[rojiphil]

I just mentioned that you can revert and see the behavior before that PR to prove the issue that is caused by mutating an object.

Ok. I will check this today and update.

[rojiphil]

@bernhardoj Ok. So, we do not need to revert #53293 as the changes are needed to function properly. Also, we are not able to reproduce the flicker anymore. But I do agree that there is a possibility of mutation causing an issue at some point when there are multiple re-renders. And I also agree that we can avoid the mutation issue by creating a copy of reportOption and using it. LGTM.

@bernhardoj proposal LGTM.
🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @Beamanator is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @rojiphil 🎉 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

[Beamanator]

Love where we got with this issue 👍

Though I def believe we need to update the issue title & OP reproduction steps...

So it sounds like the new problem is here: #53361 (comment)

Can y'all confirm & let me know what reproduction steps y'all are following so I can update the OP?

[bernhardoj]

Here are the steps:
Prerequisite:

• have a workspace
• have at least 1 other member
• send a message at least once to the member workspace chat

1. Open the search modal
2. Search for the member workspace chat
3. Observe that the subtitle shows the last message instead of the workspace name

NOTE: For user's own workspace chat, the workspace name is shown, so this issue only happens with member workspace chat

[bernhardoj]

PR is ready

cc: @rojiphil

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[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 9.0.83-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix object mutation causes the WS item in search shows last message instead of WS name #54351

If no regressions arise, payment will be issued on 2025-01-18. 🎊

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

• @rojiphil requires payment automatic offer (Reviewer)
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@rojiphil @isabelastisser @rojiphil The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]