Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD] [Help Wanted for completing] [$250] LHN - System message remains on LHN when create new room via announce room #43737

Open
2 of 6 tasks
lanitochka17 opened this issue Jun 13, 2024 · 47 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Help Wanted Apply this label when an issue is open to proposals by contributors Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff Monthly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 13, 2024

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: 1.4.83-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: N/A
Email or phone of affected tester (no customers): gocemate+a256@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Create Workspace
  2. Go to #announce room
  3. Send a message "#room-1"
  4. Verify that system message "Heads up, room-1 doesn't exist yet. Do you want to create it ?" appears
  5. Click "Yes"
  6. Verify that system message disappears from chat history
  7. Take a look at #announce room in LHN

Expected Result:

System message should be dismissed

Actual Result:

System message "Heads up, room-1 doesn't exist yet. Do you want to create it ?" remains on LHN when create new room via announce room

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

Bug6512441_1718312003111.Recording__3211.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0150a19511c968b1de
  • Upwork Job ID: 1803219974130567450
  • Last Price Increase: 2024-06-26
  • Automatic offers:
    • ishpaul777 | Contributor | 102911885
    • dominictb | Contributor | 102941077
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 13, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

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

@lanitochka17
Copy link
Author

@strepanier03 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@Tony-MK
Copy link
Contributor

Tony-MK commented Jun 14, 2024

Proposal

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

LHN - System message remains on LHN when create new room via announce room

What is the root cause of that problem?

The root cause of the problem is that the report lastMessageText is not cleared once the actionable whisper is acted upon.

const lastMessageTextFromReport = OptionsListUtils.getLastMessageTextForReport(report, lastActorDetails, policy);

Therefore, the result.alternateText will be the lastMessageText based on the following last else statement line in the SidebarUtils file.

} else {
result.alternateText = lastMessageTextFromReport.length > 0 ? lastMessageText : ReportActionsUtils.getLastVisibleMessage(report.reportID, {}, lastAction)?.lastMessageText;
if (!result.alternateText) {
result.alternateText = Localize.translate(preferredLocale, 'report.noActivityYet');
}
}

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

We need the clear the lastMessageText for the report from the onyx's optimisticData in the resolveActionableReportMentionWhisper function.

....
{
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.REPORT}${reportId}`,
    value: {lastMessageText: ''},
},
...

@melvin-bot melvin-bot bot added the Overdue label Jun 17, 2024
Copy link

melvin-bot bot commented Jun 17, 2024

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

@strepanier03
Copy link
Contributor

Confirmed reproducible. Doesn't update on a refresh either.

image

@melvin-bot melvin-bot bot removed the Overdue label Jun 19, 2024
@strepanier03 strepanier03 added the External Added to denote the issue can be worked on by a contributor label Jun 19, 2024
@melvin-bot melvin-bot bot changed the title LHN - System message remains on LHN when create new room via announce room [$250] LHN - System message remains on LHN when create new room via announce room Jun 19, 2024
Copy link

melvin-bot bot commented Jun 19, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 19, 2024
Copy link

melvin-bot bot commented Jun 19, 2024

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

@dominictb
Copy link
Contributor

dominictb commented Jun 19, 2024

Proposal

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

System message "Heads up, room-1 doesn't exist yet. Do you want to create it ?" remains on LHN when create new room via announce room

What is the root cause of that problem?

When resolving the report mention whisper in

function resolveActionableReportMentionWhisper(
, the report mention whisper will disappear.

But we didn't restore the last message text of the report back to what it was before the whisper, like we did for when we delete the report comment (whispering disappearing so it's same as being deleted) https://github.com/Expensify/App/blob/main/src/libs/actions/Report.ts#L1339-L1357

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

When resolving the report mention whisper, optimistically restore the last message text of the report back to the last message text to that of the latest visible message, like we did when deleting report comment

For testing, in

, add

let optimisticReport: Partial<Report> = {
      lastMessageTranslationKey: '',
      lastMessageText: '',
      lastVisibleActionCreated: '',
  };
  const optimisticReportActions: OnyxCollection<OptimisticAddCommentReportAction> = {};
  optimisticReportActions[reportAction.reportActionID] = {
      originalMessage: {
          resolution,
      },
  };
  const {lastMessageText = '', lastMessageTranslationKey = ''} = ReportUtils.getLastVisibleMessage(reportId, optimisticReportActions as ReportActions);
  
  if (lastMessageText || lastMessageTranslationKey) {
      const lastVisibleAction = ReportActionsUtils.getLastVisibleAction(reportId, optimisticReportActions as ReportActions);
      const lastVisibleActionCreated = lastVisibleAction?.created;
      const lastActorAccountID = lastVisibleAction?.actorAccountID;
      optimisticReport = {
          lastMessageTranslationKey,
          lastMessageText,
          lastVisibleActionCreated,
          lastActorAccountID,
      };
  }

And in here, add

{
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.REPORT}${reportId}`,
    value: optimisticReport,
},

In PR phase we can polish this.

We need to do the same fix in the back-end because when the user logs in and logs out, it will still show the Whisper as the last message. The data is returned from the back-end (lastMessageText, lastMessageTranslationKey, ...).

What alternative solutions did you explore? (Optional)

Seems that when clicking Yes in the Whisper in offline mode, we don't create the room optimistically. If optimistic creation of rooms is needed, a new ticket can be created to handle it

@getusha getusha removed their assignment Jun 23, 2024
@getusha
Copy link
Contributor

getusha commented Jun 23, 2024

@ishpaul777 will be taking over

@melvin-bot melvin-bot bot added the Overdue label Jun 23, 2024
Copy link

melvin-bot bot commented Jun 24, 2024

@strepanier03 Huh... This is 4 days overdue. Who can take care of this?

@ishpaul777
Copy link
Contributor

ishpaul777 commented Jun 26, 2024

@strepanier03 Can you please assign me here, i missed it since this doesn't show up on my K2, i'll review proposals today

Copy link

melvin-bot bot commented Jun 26, 2024

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

Copy link

melvin-bot bot commented Jun 26, 2024

@strepanier03 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@ishpaul777
Copy link
Contributor

@dominictb your proposal sounds good just one question, if we logout and login again Does the last message still shows whisper message?

@Tony-MK
Copy link
Contributor

Tony-MK commented Jun 27, 2024

@ishpaul777, Sorry to disturb you.

I mentioned this slack thread some proposals are not much different from the rest.

According, to the contributing guide, when posting a proposal, the difference has to meaningful to avoid changing the words of the previous proposal which most of the points has been stated.

Therefore, can you consider this?

Nonetheless, I respect your decision.

Thanks.

@dominictb
Copy link
Contributor

@dominictb your proposal sounds good just one question, if we logout and login again Does the last message still shows whisper message?

@ishpaul777 It will still show the whisper message, so we need the same fix in the back-end, I've updated my proposal to mention the back-end fix.

@Tony-MK I don't think our solutions have any similarity, the suggested fixes are different, the outcomes are different.

But @ishpaul777 will decide.

Copy link

melvin-bot bot commented Jun 27, 2024

@strepanier03 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@ishpaul777
Copy link
Contributor

Hello @Tony-MK, In this case @dominictb solution was most complete and correct, we can not simply set lastMessageText to be empty string we'd have to change it to lastMessageText that is visible to user. The difference was a major one not minor.

@ishpaul777
Copy link
Contributor

Not overdue on me, Unassigning for now, feel free to assign if this needs C+ service at any point of time

@melvin-bot melvin-bot bot removed the Overdue label Aug 26, 2024
@ishpaul777 ishpaul777 removed their assignment Aug 26, 2024
@strepanier03 strepanier03 added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 3, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 3, 2024
@strepanier03 strepanier03 removed Overdue Awaiting Payment Auto-added when associated PR is deployed to production labels Sep 3, 2024
@strepanier03 strepanier03 added Monthly KSv2 and removed Weekly KSv2 labels Sep 3, 2024
@melvin-bot melvin-bot bot removed the Overdue label Sep 3, 2024
@dominictb
Copy link
Contributor

@strepanier03 Can you add HOLD to the title to indicate we're holding BE work?

@Julesssss Julesssss added the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Sep 30, 2024
@mvtglobally
Copy link

issue is still repro

1728834244550.system_message_in_lhn.mp4

@mvtglobally mvtglobally removed the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Oct 14, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 31, 2024
@strepanier03 strepanier03 changed the title [Help Wanted for completing] [$250] LHN - System message remains on LHN when create new room via announce room [HOLD] [Help Wanted for completing] [$250] LHN - System message remains on LHN when create new room via announce room Nov 5, 2024
@strepanier03
Copy link
Contributor

This is set internal and I tied it to quality.

@melvin-bot melvin-bot bot removed the Overdue label Nov 5, 2024
@muttmuure muttmuure added the Hot Pick Ready for an engineer to pick up and run with label Nov 19, 2024
@muttmuure muttmuure moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 19, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label Dec 7, 2024
@Julesssss
Copy link
Contributor

Not overdue, awaiting further retesting

@melvin-bot melvin-bot bot removed the Overdue label Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Help Wanted Apply this label when an issue is open to proposals by contributors Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff Monthly KSv2
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

9 participants