-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[$500] LHN - Task name not displayed in chat preview in LHN after re-logging #33165
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01dfcd1e43b6e247d3 |
Triggered auto assignment to @dylanexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Task name not displayed in chat preview in LHN after re-logging What is the root cause of that problem?The root cause lies in here Lines 46 to 47 in 8bef4bb
Until the report is opened by the user the reportAction has neither childReportID nor childReportName so getTaskTitle is returning an empty string.
What changes do you think we should make in order to solve the problem?In App/src/libs/OptionsListUtils.js Lines 418 to 419 in 8bef4bb
to
What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.The name of the task on the LHN is not correct when re-login What is the root cause of that problem?When we first log in, we get the empty title of the task here because we dont have a Lines 45 to 49 in ab138ee
What changes do you think we should make in order to solve the problem?Since we updated the method to get the title of the task here we want to translate it. const taskReportID = reportAction?.childReportID ?? (reportAction as OriginalMessageAddComment)?.originalMessage?.taskReportID ?? ''; What alternative solutions did you explore? (Optional)N/A |
Hi, Will review today. |
TY! @abdulrahuman5196 what's our findings? |
Re-checking again now |
@namhihi237 's proposal here #33165 (comment) looks good and works well. For some reason, the We can also fix it in backend by making sure childReportID is being populated. 🎀 👀 🎀 |
Triggered auto assignment to @marcaaron, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
Hmm, I think I disagree with the recommended proposal as it does not seem like it would fix the problem.
It does sound like a backend issue.
Ok but that's a problem because now the title is getting populated two different ways. Which way is the right way? Let's pick one and make sure it works vs. having various fallbacks and workarounds. cc @thienlnam |
@marcaaron currently in the originalMessage we always has a taskReportID if the reportAction is task. So i think we can only use it from here |
Ok, but which one is the correct one to use? @thienlnam maybe has the answer... let's wait and see. |
Hmm I think this was broken recently - historically we had it populate either the task report or the reportAction because if you never open the task report, you don't ever have the task report fetched. We've made a lot of BE changes but every task change should now also update the parent reportAction and if not we should make sure to update that flow. So because the scenario where you can have someone that never opens the task report and therefore never gets those task pusher updates, we should use the reportAction data |
@thienlnam We use reportAction data here but in this case, When the task has the last reportAction as |
We should probably update the BE so it always returns the childReportID |
@thienlnam Here we use the condition in originalMessage to check whether the created report action is created. App/src/libs/ReportActionsUtils.ts Lines 612 to 614 in 7b836cf
And we use it in a few places, like here: App/src/libs/OptionsListUtils.js Lines 419 to 420 in 7b836cf
That's why I think why we should consistently use it to get the taskReportID in |
@marcaaron, @abdulrahuman5196, @dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@thienlnam @abdulrahuman5196 @dylanexpensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks! |
@thienlnam do we think this fits in #vip-vsb? |
Yeah, this could be a good low polish for vip-vsb |
@thienlnam @abdulrahuman5196 @dylanexpensify this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:
Thanks! |
Current assignee @abdulrahuman5196 is eligible for the Internal assigner, not assigning anyone new. |
added appropriate designation |
no movement |
Same as last week. |
same |
Thoughts on closing this? Doesn't seem valuable to work on at this time |
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.13-4
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: Applause-Internal Team
Slack conversation: @
Action Performed:
Expected Result:
The task name is displayed in the chat preview in LHN as the last message
Actual Result:
Only "Task for" is displayed until the chat is opened
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Bug6314571_1702646639077.video_2023-12-14_15-06-02.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: