Skip to content

[HOLD for payment 2024-02-20] [$500] LHN - Task name on secondary user LHN does not update until open the report #35002

Description

@lanitochka17

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.30-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:
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:

Precondition:
Login the same user to different devices

  1. From PC navigate to workspace chat> Assign a Task> Complete the flow
  2. Login the same user on mobile app/web
  3. Navigate to LHN

Expected Result:

Full task name should be present on workspace chat on LHN

Actual Result:

Instead of full name - "Task for" is present on LHN. When user open the chat and navigate to LHN - full task name can be seen

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

Bug6352511_1706041482878.Recording__1888.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01318631513a1f1561
  • Upwork Job ID: 1749892970798456832
  • Last Price Increase: 2024-01-23
  • Automatic offers:
    • s77rt | Reviewer | 28128418
    • rojiphil | Contributor | 28128419

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jan 23, 2024
  2. changed the title [-]LHN - Task name on secondary user LHN does not update until open the report[/-] [+][$500] LHN - Task name on secondary user LHN does not update until open the report[/+] on Jan 23, 2024
  3. melvin-bot commented on Jan 23, 2024

    @melvin-bot
  4. melvin-bot commented on Jan 23, 2024

    @melvin-bot

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

  5. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jan 23, 2024
  6. melvin-bot commented on Jan 23, 2024

    @melvin-bot

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

  7. cheng-kang commented on Jan 24, 2024

    @cheng-kang

    Proposal

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

    LHN - Task name on secondary user LHN does not update until open the report

    What is the root cause of that problem?

    The response of the initial OpenApp api call after user first logs in does not include childReportID which is needed for getting task name. Hence the initial experience is broken.

    Screenshot 2024-01-23 at 11 30 45 PM

    The data is, however, return in OpenReport api call, which would merge to Onyx and bring back the right last message in the sidebar.

    Screenshot 2024-01-23 at 11 31 10 PM

    The issue is actually reproduceable across all platforms.

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

    I would highly suggest a change in the OpenApp endpoint to include all the necessary data. It would be a lot cleaner.

    If we want to do it on frontend, the change would be:

    • Subscribe to ONYXKEYS.COLLECTION.REPORT in OptionsListUtils.ts
    • Add callback to fill childReportID into reportAction by finding the relevant report by parentReportActionID . The callback will need to execute both when ONYXKEYS.COLLECTION.REPORT and ONYXKEYS.COLLECTION.REPORT_ACTIONS have any update to avoid any race condition.

    What alternative solutions did you explore? (Optional)

    Another frontend alternative solution is to add a transform pipeline step here: https://github.com/Expensify/App/blob/main/src/components/LHNOptionsList/LHNOptionsList.tsx#L138, which does the same data filling step mentioned above (find relevant report by parentReportActionID and backfill the report's reportId as childReportID to reportAction).

    I would not recommend this alternative because it's not a pattern that I've seen in the code base. For consistency, we should follow the practice in https://github.com/Expensify/App/blob/main/src/libs/OptionsListUtils.js and keep all data massaging process encapsulated in these connected utils.

  8. rojiphil commented on Jan 25, 2024

    @rojiphil
    Contributor

    Proposal

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

    LHN - Task name on secondary user LHN does not update until open the report

    What is the root cause of that problem?

    As there is a purposeful reduction in data sent from BE in OpenApp, the report actions for a displayed report in LHN can be partially present. Dependence of LHN subtitle display for task on report action’s childReportID as mentioned here will not assure a correct task subtitle. This is the root cause of the problem.

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

    It does not make sense to form a task title when taskTitle is empty here
    So, we can solve this problem here by either

    A) Falling back on reportAction’s message text like this or

    return taskTitle ? Localize.translateLocal('task.messages.created', {title: taskTitle}) : reportAction?.message?.[0]?.text;

    B) Falling back on Report’s lastMessageText here by simply returning empty string like this

    return taskTitle ? Localize.translateLocal('task.messages.created', {title: taskTitle}) : '';

    Recently, we have also done a similar change for ReportPreview here

    What alternative solutions did you explore? (Optional)

  9. s77rt commented on Jan 25, 2024

    @s77rt
    Member

    @cheng-kang Thanks for the proposal. Your RCA makes sense. But I think OpenApp not returning full report action data is on purpose. As for the frontend suggested solutions those look an overkill, we should not fill childReportID manually.

  10. s77rt commented on Jan 25, 2024

    @s77rt
    Member

    @rojiphil Thanks for the proposal. Your RCA is correct. Solution B looks good to me.

    🎀 👀 🎀 C+ reviewed
    Link to proposal

  11. 16 remaining items

  12. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed on Feb 13, 2024
  13. changed the title [-][$500] LHN - Task name on secondary user LHN does not update until open the report[/-] [+][HOLD for payment 2024-02-20] [$500] LHN - Task name on secondary user LHN does not update until open the report[/+] on Feb 13, 2024
  14. melvin-bot commented on Feb 13, 2024

    @melvin-bot

    Reviewing label has been removed, please complete the "BugZero Checklist".

  15. melvin-bot commented on Feb 13, 2024

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.40-5 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-02-20. 🎊

    For reference, here are some details about the assignees on this issue:

  16. melvin-bot commented on Feb 13, 2024

    @melvin-bot

    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:

    • [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
    • [@s77rt] 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:
    • [@s77rt] A discussion in #expensify-bugs 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:
    • [@s77rt] Determine if we should create a regression test for this bug.
    • [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
    • [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
  17. s77rt commented on Feb 13, 2024

    @s77rt
    Member
  18. bfitzexpensify commented on Feb 15, 2024

    @bfitzexpensify
    Contributor

    Switching this to Daily so that it's on my list for Tuesday.

  19. bfitzexpensify commented on Feb 19, 2024

    @bfitzexpensify
    Contributor

    Payment due tomorrow.

  20. bfitzexpensify commented on Feb 20, 2024

    @bfitzexpensify
    Contributor

    Payments complete, we're all done here - thanks everyone!

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 contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions