Skip to content

[HOLD for payment 2025-02-05] [$250] Actionable whisper: the "New message" floating button appears when it shouldn't #49824

Closed

Description

@m-natarajan

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.40-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):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @paultsimura
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727286146850839

Action Performed:

  1. Create a room;
  2. Mention a user that is not a member of this room using the @{user} syntax;
  3. Notice the actionable whisper "Heads up, {user} isn't a member of this room" message appears;
  4. Click "Do nothing"

Expected Result:

  1. Create a room;
  2. Mention a user that is not a member of this room using the @{user} syntax;
  3. Notice the actionable whisper "Heads up, {user} isn't a member of this room" message appears;
  4. Click "Do nothing"

Actual Result:

The "New messages" floating button appears.

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

bug_2.mov
Recording.587.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021839703879939232277
  • Upwork Job ID: 1839703879939232277
  • Last Price Increase: 2024-09-27
  • Automatic offers:
    • ishpaul777 | Reviewer | 104253570
Issue OwnerCurrent Issue Owner: @sakluger

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Sep 26, 2024
  2. melvin-bot commented on Sep 26, 2024

    @melvin-bot

    Triggered auto assignment to @sakluger (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.

  3. bernhardoj commented on Sep 27, 2024

    @bernhardoj
    Contributor

    Proposal

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

    The New message shows after selecting Do nothing of an actionable mention whisper

    What is the root cause of that problem?

    In this issue, the New message shows because hasNewestReportAction is false.

    const canScrollToNewerComments = !isLoadingInitialReportActions && !hasNewestReportAction && sortedReportActions.length > 25 && !isLastPendingActionIsDelete;
    return (
    <>
    <FloatingMessageCounter
    isActive={(isFloatingMessageCounterVisible && !!unreadMarkerReportActionID) || canScrollToNewerComments}

    hasNewestReportAction compares if the report lastVisibileActionCreated equals the newest action created.

    const hasNewestReportAction = sortedVisibleReportActions[0]?.created === report.lastVisibleActionCreated;

    When we resolve the actionable mention whisper, we optimistically update the report lastVisibleActionCreated to the previous action created.

    const reportUpdateDataWithPreviousLastMessage = ReportUtils.getReportLastMessage(reportId, optimisticReportActions as ReportActions);

    However, the sortedVisibleReportActions here still contains the resolved actionable mention whisper.

    const hasNewestReportAction = sortedVisibleReportActions[0]?.created === report.lastVisibleActionCreated;

    We already filter here to filter out any action that shouldn't be visible, but the resolved whisper is not included.

    sortedReportActions.filter(
    (reportAction) =>
    (isOffline ||
    ReportActionsUtils.isDeletedParentAction(reportAction) ||
    reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ||
    reportAction.errors) &&
    ReportActionsUtils.shouldReportActionBeVisible(reportAction, reportAction.reportActionID),
    ),

    The reason the resolved whisper is not shown in the chat is because we manually return null if it's resolved.

    // If action is actionable whisper and resolved by user, then we don't want to render anything
    if (isActionableWhisper && (hasResolutionInOriginalMessage ? originalMessage.resolution : null)) {
    return null;
    }

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

    Instead of manually return null when the whisper action is resolved, we should filter it out early here by including it in shouldReportActionBeVisible.

    sortedReportActions.filter(
    (reportAction) =>
    (isOffline ||
    ReportActionsUtils.isDeletedParentAction(reportAction) ||
    reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ||
    reportAction.errors) &&
    ReportActionsUtils.shouldReportActionBeVisible(reportAction, reportAction.reportActionID),
    ),

    const originalMessage = getOriginalMessage(reportAction);
    const isOriginalMessageAnObject = originalMessage && typeof originalMessage === 'object';
    const hasResolutionInOriginalMessage = isOriginalMessageAnObject && 'resolution' in originalMessage;
    const isActionableWhisper = isActionableMentionWhisper(reportAction) || isActionableTrackExpense(reportAction) || isActionableReportMentionWhisper(reportAction);
    if (isActionableWhisper && (hasResolutionInOriginalMessage ? originalMessage.resolution : null)) {
        return false;
    }
    
  4. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Sep 27, 2024
  5. changed the title [-] Actionable whisper: the "New message" floating button appears when it shouldn't[/-] [+][$250] Actionable whisper: the "New message" floating button appears when it shouldn't[/+] on Sep 27, 2024
  6. melvin-bot commented on Sep 27, 2024

    @melvin-bot
  7. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Sep 27, 2024
  8. melvin-bot commented on Sep 27, 2024

    @melvin-bot

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

  9. ishpaul777 commented on Sep 27, 2024

    @ishpaul777
    Contributor

    Reviewing 👀

  10. ishpaul777 commented on Sep 27, 2024

    @ishpaul777
    Contributor

    @bernhardoj's Proposal makes sense 👍 and works well

    🎀 👀 🎀 C+ reviewed

    Screen.Recording.2024-09-27.at.10.32.32.PM.mov
  11. ishpaul777 commented on Sep 27, 2024

    @ishpaul777
    Contributor

    🎀 👀 🎀 C+ reviewed ^

  12. 44 remaining items

  13. sakluger commented on Feb 4, 2025

    @sakluger
    Contributor

    @paultsimura is not actually due any payment here, we have a bug where we put the person who reported the bug in the list of people who need to be paid.

  14. sakluger commented on Feb 4, 2025

    @sakluger
    Contributor

    @bernhardoj please complete the BZ checklist before the payment due date tommorrow. Thanks!

  15. bernhardoj commented on Feb 5, 2025

    @bernhardoj
    Contributor

    @ishpaul777 is the C+ here

  16. sakluger commented on Feb 5, 2025

    @sakluger
    Contributor

    Ah, sorry @bernhardoj and thanks for the clarification! @ishpaul777 - please finish the BZ checklist so I can complete the payment and close the issue.

  17. sakluger commented on Feb 5, 2025

    @sakluger
    Contributor

    Summarizing payment on this issue:

    Contributor: @bernhardoj $250, please request via NewDot
    Contributor+: @ishpaul777 $250, we will complete the Upwork contract payment once the BZ checklist is complete. Paid via Upwork

  18. ishpaul777 commented on Feb 5, 2025

    @ishpaul777
    Contributor

    Ah, sorry @bernhardoj and thanks for the clarification! @ishpaul777 - please finish the BZ checklist so I can complete the payment and close the issue.

    will do In few mins!

  19. ishpaul777 commented on Feb 5, 2025

    @ishpaul777
    Contributor

    BugZero Checklist:

    • [Contributor] Classify the bug:
    Bug classification

    Source of bug:

    • 1a. Result of the original design (eg. a case wasn't considered)
    • 1b. Mistake during implementation
    • 1c. Backend bug
    • 1z. Other:

    Where bug was reported:

    • 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
    • 2b. Reported on staging (eg. found during regression or PR testing)
    • 2d. Reported on a PR
    • 2z. Other:

    Who reported the bug:

    • 3a. Expensify user
    • 3b. Expensify employee
    • 3c. Contributor
    • 3d. QA
    • 3z. Other:
    • [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

      Link to comment: https://github.com/Expensify/App/pull/34560/files#r1943636054

    • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

      Link to discussion: Not criitcal

    • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again. - Yes

    • [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

      Link to issue:

    Regression Test Proposal

    Test:

    1. Create a room;
    2. Mention a user that is not a member of this room using the @{user} syntax;
    3. Notice the actionable whisper "Heads up, {user} isn't a member of this room" message appears;
    4. Click "Do nothing"
    5. verify The "New messages" floating button does not appears.

    Do we agree 👍 or 👎

  20. ishpaul777 commented on Feb 5, 2025

    @ishpaul777
    Contributor

    I have been paid already, see - #49824 (comment)

  21. sakluger commented on Feb 5, 2025

    @sakluger
    Contributor

    Thanks @ishpaul777, I missed that comment! Appreciate you calling it out.

  22. sakluger commented on Feb 5, 2025

    @sakluger
    Contributor

    Thanks everyone!

  23. bernhardoj commented on Feb 6, 2025

    @bernhardoj
    Contributor

    Requested in ND.

  24. JmillsExpensify commented on Feb 10, 2025

    @JmillsExpensify
    Contributor

    $250 approved for @bernhardoj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributorNot a priority

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions