-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 for payment 2024-07-22] [HOLD for payment 2024-07-17] [$250] Expensify chat - Incorrect LHN preview for Expensify chat #43830
Comments
Triggered auto assignment to @JmillsExpensify ( |
@JmillsExpensify 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 |
We think that this bug might be related to #vip-vsp |
Opening up to the community. This is a #vip-vsb initiative. |
Job added to Upwork: https://www.upwork.com/jobs/~016857ff3bf2660def |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ZhenjaHorbach ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.inconsistent chat system message with LNH when lastMessageText is empty What is the root cause of that problem?We have a new implement show default system chat message from lines App/src/components/ReportWelcomeText.tsx Lines 146 to 150 in f24f4c3
But when build side bar option we only have 2 condition show message when lastMessageText is empty from lines Lines 386 to 392 in 7213c4b
What changes do you think we should make in order to solve the problem?We have to and more condition to show LHN preview message by lastMessageText = ReportUtils.isSelfDM(report)
? Localize.translate(preferredLocale, 'reportActionsView.beginningOfChatHistorySelfDM')
+ : ReportUtils.isSystemChat(report) ? Localize.translate(preferredLocale, 'reportActionsView.beginningOfChatHistorySystemDM')
: Localize.translate(preferredLocale, 'reportActionsView.beginningOfChatHistory') What alternative solutions did you explore? (Optional) |
Triggered auto assignment to @grgia, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@ZhenjaHorbach ah this issue can check by eslint and update on PR |
@JmillsExpensify, @grgia, @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@JmillsExpensify before I assign, what's the expected behavior here? It seems like no chats have been sent in the example video chat. Do we want to use the Welcome Text as the LHN preview? Techically "This is the beginning of your chat .." is correct for these |
Yeah, I think we are actually going to standardize on the welcome text very soon. Isn't that what you are working on @danielrvidal? |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@JmillsExpensify, @grgia, @ZhenjaHorbach Huh... This is 4 days overdue. Who can take care of this? |
@danielrvidal bump on #43830 (comment) |
@JmillsExpensify, @grgia, @ZhenjaHorbach Still overdue 6 days?! Let's take care of this! |
Hi @ZhenjaHorbach PR is ready for review |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.5-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-07-17. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.6-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-07-22. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
https://github.com/Expensify/App/pull/41290/files#r1680686700
NA
I'm not sure we need a regression test for this bug since it's a very minor issue.
NA |
@JmillsExpensify, @suneox, @grgia, @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Not overdue @JmillsExpensify |
@JmillsExpensify, @suneox, @grgia, @ZhenjaHorbach Still overdue 6 days?! Let's take care of this! |
Payment summary: Contributor: @suneox $250 |
Both contributors paid via Upwork. Closing! |
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.84-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
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
Expensify chat preview in LHN should display "Welcome! Let's get you set up." or "Message Concierge for help with setup"
Actual Result:
Expensify chat preview in LHN displays "This is the beginning of your chat with Expensify"
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6514462_1718473042888.expensify.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @JmillsExpensifyThe text was updated successfully, but these errors were encountered: