[VSB] [$500] Room - Inviting a user to the room does not remove other whisper messages

[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.42-1
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): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #34560

Action Performed:

1. Create a room
2. Mention a user in the chat
3. Again mention same user in chat
4. Click on "Invite them"
5. Notice other whisper message does not removed

Expected Result:

Inviting a user to the room other same whisper messages should be removed

Actual Result:

Inviting a user to the room does not remove other whisper messages

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

Bug6381090_1708024231225.2024-02-16_00-08-28.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013c530387770aef42
• Upwork Job ID: 1758212482614226944
• Last Price Increase: 2024-02-29

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[Krishna2323]

Proposal

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

Room - Inviting a user to the room does not remove other whisper messages

What is the root cause of that problem?

We don't have any implementation to handle duplicate ACTIONABLEMENTIONWHISPER action when user is invited or room member click on do nothing.

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

Since we have the participants list in the report and we get inviteeAccountIDs in ACTIONABLEMENTIONWHISPER action, we can call Report.resolveActionableMentionWhisper with resolveActionableMentionWhisper as a resolution when user already exist in the report participants list.

Example Code:

        function areAllValuesPresent(arr1, arr2) {
            // Convert arr2 to a Set for efficient membership testing
            const set2 = new Set(arr2);

            // Check if all elements of arr1 are present in arr2
            for (let i = 0; i < arr1.length; i++) {
                if (!set2.has(arr1[i])) {
                    return false;
                }
            }
            return true;
        }

        const membersAlreadyInvited = areAllValuesPresent(props.action.originalMessage.inviteeAccountIDs, props.report.participantAccountIDs);

        if (membersAlreadyInvited) {
            Report.resolveActionableMentionWhisper(props.report.reportID, props.action, CONST.REPORT.ACTIONABLE_MENTION_WHISPER_RESOLUTION.INVITE);
        }

We will use better approach for writing this code, this is only for example. We also need to add _.isEqual(prevProps.report.participantAccountIDs, nextProps.report.participantAccountIDs) in memo second parameter (propsAreEqual).

We will update participantAccountIDs of policy when we invite through resolveActionableMentionWhisper so we can remove duplicate whisper message in offline mode also.

Result

remove_whisper.mp4

[melvin-bot]

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

[brunovjk]

I believe this issure would be better solved on the backend side.

[strepanier03]

@parasharrajat - What do you think? Keep it external or move it internal for a BE fix?

[melvin-bot]

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

[parasharrajat]

Reviewing...

[brandonhenry]

Proposal

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

In the chat functionality, when multiple whispers (system messages prompting an action, such as inviting a user to the room) targeting the same user are present, they clutter the chat interface. This happens even after the user has been invited, leading to a confusing user experience. The expected behavior is to manage these whispers more efficiently to enhance chat readability and user experience.

What is the root cause of that problem?

The root cause is the lack of a mechanism to manage duplicate or redundant whispers in the chat. The current implementation does not account for the scenario where multiple whispers inviting the same user can accumulate, nor does it provide a method for resolving these whispers once the user has been invited, resulting in unnecessary clutter.

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

To address this issue, we propose implementing a new mechanism within the chat functionality that adheres to the following rules:

1. Maintain at least one whisper from each participant: Ensure that for each participant who has initiated an invite (whispered), at least one of their whispers remains visible in the chat to preserve the intent and context of the conversation.

2. Resolve all whispers upon invitation acceptance: When the targeted user accepts an invitation from any participant, all whispers related to that invitation should be resolved. This means either removing them from the chat view or marking them in a way that clearly indicates the invitation has been addressed.

Implementing these changes involves updating the getSortedReportActionsForDisplay function to filter and manage whispers based on the new criteria. Specifically, we need to adjust the logic to identify whispers that invite the same user and then apply the rules outlined above.

Here's a pseudo-code outline of the approach:

function manageWhispersForDisplay(reportActions: ReportAction[]) {
    const whispers = extractWhispers(reportActions);
    const uniqueUserWhispers = keepAtLeastOneWhisperPerParticipant(whispers);
    return filterAndResolveWhispers(reportActions, uniqueUserWhispers);
}

function extractWhispers(reportActions) {
    // Logic to extract whisper actions
}

function keepAtLeastOneWhisperPerParticipant(whispers) {
    // Logic to ensure at least one whisper per participant remains
}

function filterAndResolveWhispers(reportActions, criteria) {
    // Logic to remove or resolve whispers based on the invitation acceptance and criteria
}

1. Identify and Group Whispers by Target User and Participant:

   • Modify the extractWhispers function to not only extract whisper messages but also to categorize them based on the target user each whisper is inviting and the participant who initiated the whisper. This step is crucial for understanding the context and dynamics of the invitations.

2. Maintain Context by Preserving At Least One Whisper per Participant:

   • Implement the keepAtLeastOneWhisperPerParticipant function to iterate through the categorized whispers. For each target user, ensure that at least one whisper from each participant remains visible in the chat.

3. Resolve or Remove Redundant Whispers Upon Invitation Acceptance:

   • The filterAndResolveWhispers function will be responsible for the final filtration and resolution of whispers based on the outlined criteria. Once a target user accepts an invitation (implied or explicitly detected through a chat action), this function will:
     • Remove all but one whisper per participant for that target user, if not already done by the previous step.
     • Optionally, mark the remaining whispers in a manner that indicates the invitation has been accepted or resolved, thus reducing the need for manual removal while maintaining the chat's context.

[parasharrajat]

I like @brandonhenry's suggestion, let me discuss this over slack to confirm. https://expensify.slack.com/archives/C01GTK53T8Q/p1708776157403919

[parasharrajat]

@brandonhenry Can we optimize the suggested logic? it seems $O(n^2)$? Also, code snippets are not a best practice in proposals. Please explain your changes.

Do you have any other ideas on hiding/removing duplicate messages? IMO, we should remove the duplicate system messages instead of filtering them. Because, there is no use of these messages when one of such message is resolved.

[brandonhenry]

@parasharrajat will have this to you today hopefully. My idea for the duplicate messages involves removing the previous duplicate system message before showing the new one (instead of simply showing the most recent one that came in only, we delete the old ones and then show the most recent one)

[strepanier03]

All good Melvin, not overdue.

[brandonhenry]

@parasharrajat updated proposal to address complexity concerns as well as duplicate system messages sticking

[parasharrajat]

Thanks for waiting. I have been discussing this over Slack. https://expensify.slack.com/archives/C01GTK53T8Q/p1708776157403919.

[jasperhuangg]

I'm assigned and will look into this once I've cleared off my plate a bit

[melvin-bot]

@strepanier03 @parasharrajat @jasperhuangg this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

[strepanier03]

Thanks @jasperhuangg!

[jasperhuangg]

Looking into this this week

[melvin-bot]

@strepanier03, @parasharrajat, @jasperhuangg Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jasperhuangg]

Not overdue!

[jasperhuangg]

Made some good progress on a PR with this today, looking to get it ready for review before EOW

[jasperhuangg]

More progress on tests, I think I should be able to get it out for review today

[jasperhuangg]

Ran into some hiccups with the tests, looking into them, but might have to continue investigation tomorrow since I've already worked on this a good amount today.

[jasperhuangg]

Still looking into the tests for this–they're a bit complicated since there's a good amount of code to be added for this.

[jasperhuangg]

PR is ready for review!

[melvin-bot]

@strepanier03, @parasharrajat, @jasperhuangg Eep! 4 days overdue now. Issues have feelings too...

[jasperhuangg]

Still under review, not overdue

[melvin-bot]

@strepanier03, @parasharrajat, @jasperhuangg Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jasperhuangg]

The Auth PR was deployed, we can close this out!