Skip to content

[HOLD for payment 2023-05-22] [$1000] Avatars are not shown on bill split preview while offline #18472

Closed

Description

@Julesssss

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


Action Performed:

  • Locate a group chat with multiple users
  • Go offline
  • Create a bill split

Expected Result:

  • The avatars should be displayed

Actual Result:

  • The avatars are not displayed until you go online

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: Dev build
Reproducible in staging?: Yes (once this PR is deployed)
Reproducible in production?: No

Screenshot 2023-05-05 at 12 23 12

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0114b795aa8cbc7508
  • Upwork Job ID: 1655924184745324544
  • Last Price Increase: 2023-05-09

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on May 5, 2023
  2. self-assigned this
    on May 5, 2023
  3. melvin-bot commented on May 5, 2023

    @melvin-bot

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

  4. melvin-bot commented on May 5, 2023

    @melvin-bot

    Bug0 Triage Checklist (Main S/O)

    • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
    • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
      • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
    • This bug is reproducible using the reproduction steps in the OP. S/O
      • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
      • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
    • This issue is filled out as thoroughly and clearly as possible
      • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
    • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync
  5. Julesssss commented on May 5, 2023

    @Julesssss
    ContributorAuthor

    We shouldn't open this issue up for contributors until this PR is deployed.

  6. Prince-Mendiratta commented on May 7, 2023

    @Prince-Mendiratta
    Contributor

    Proposal

    Posting proposal to fix this issue once the linked PR has been merged.

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

    In this issue, we can notice that when the user is offline and sends a split bill request with more than 2 participants, the avatars in the IOU preview are not displayed until the user goes back online.

    What is the root cause of that problem?

    To render the list of participant avatars, we use the participants array in the originalMessage for each action. Optimistically, in a split bill request, we do not add the array of participants to the originalMessage. When the user is offline and sends the Split Bill request, the optimistic action is created without the participants array. This causes the avatars to not show up.

    const participantEmails = props.isBillSplit ? props.action.originalMessage.participants : [managerEmail, ownerEmail];

    When the user goes back online, the participants array is returned by the backend, causing the avatars to show up.

    This issue does not happen when there are just 2 users as in such a case, a different source is used to fetch user logins.

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

    We should add the participants array to the originalMessage optimistically as well. This can be done here:

    App/src/libs/ReportUtils.js

    Lines 1158 to 1160 in d120f6a

    if (type === CONST.IOU.REPORT_ACTION_TYPE.SPLIT) {
    delete originalMessage.IOUReportID;
    }

    We need to iterate the parameter participants and for each participant, push their login in an array. Once all users have been iterated, we assign this array to the originalMessage variable.

    Results
    2023-05-08.00-37-45.mp4
  7. stephanieelliott commented on May 8, 2023

    @stephanieelliott
    Contributor

    We shouldn't open this issue up for contributors until #18143 is deployed.

    Sounds good, PR is still under review so will hold on applying any labels.

  8. Julesssss commented on May 9, 2023

    @Julesssss
    ContributorAuthor

    THanks @stephanieelliott. We merged the PR, so please feel free to apply the label now

  9. changed the title [-][HOLD - pull/18143] Avatars are not shown on bill split preview while offline[/-] [+]Avatars are not shown on bill split preview while offline[/+] on May 9, 2023
  10. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on May 9, 2023
  11. changed the title [-]Avatars are not shown on bill split preview while offline[/-] [+][$1000] Avatars are not shown on bill split preview while offline[/+] on May 9, 2023
  12. 25 remaining items

  13. melvin-bot commented on May 15, 2023

    @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.
    • [@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
  14. s77rt commented on May 15, 2023

    @s77rt
    Member

    Regression Test Proposal

    1. Open App
    2. Turn off Internet Connection
    3. Split bill with any 2 users (Click on the FAB > Split bill and follow the process)
    4. Verify that the group chat has the IOU preview and it's greyed out
    5. Verify that user avatars are shown in the IOU preview

    Suggested test case C1971182.

  15. Prince-Mendiratta commented on May 15, 2023

    @Prince-Mendiratta
    Contributor

    Sharing the timeline of this issue to help with the eventual analysis, calculated with this tool.

    🐛 Issue created at: Fri, 05 May 2023 11:23:33 GMT
    🧯 Help Wanted at: Tue, 09 May 2023 13:14:38 GMT
    🕵🏻 Contributor assigned at: Tue, 09 May 2023 13:45:30 GMT
    🛸 PR created at: Tue, 09 May 2023 17:35:41 GMT
    🎯 PR merged at: Wed, 10 May 2023 08:51:18 GMT
    ⌛ Business Days Elapsed between assignment and PR merge: 1

  16. Julesssss commented on May 24, 2023

    @Julesssss
    ContributorAuthor

    Hey @stephanieelliott, I think this one is ready for payment.

  17. stephanieelliott commented on May 25, 2023

    @stephanieelliott
    Contributor

    Upwork job offered to @Prince-Mendiratta and @s77rt, please accept when you have a sec and I can issue payment!

  18. s77rt commented on May 25, 2023

    @s77rt
    Member

    Accepted!

  19. Prince-Mendiratta commented on May 25, 2023

    @Prince-Mendiratta
    Contributor

    Accepted ✅

  20. stephanieelliott commented on May 30, 2023

    @stephanieelliott
    Contributor

    All paid up, thank you!

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