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 for payment 2024-10-07][$250] Workspace – Workspace name/avatar not updated on another device if workspace chat is open #48710

Closed
1 of 6 tasks
lanitochka17 opened this issue Sep 6, 2024 · 27 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 6, 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: 9.0.30-7
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4936874
Email or phone of affected tester (no customers): ponikarchuks+3@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Log in
  3. Go to Account settings > Workspaces
  4. Create a new Workspace
  5. Log in as the same user on another device
  6. On another device open Workspace chat created on step 4
  7. On main device change Workspace avatar and name
  8. Observe the Workspace chat header on another device

Expected Result:

Workspace name/avatar updated on another device workspace chat header

Actual Result:

Workspace name/avatar not updated on another device workspace chat header

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

Bug6595144_1725613214496.WS_Avatar_name.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021833620003356682080
  • Upwork Job ID: 1833620003356682080
  • Last Price Increase: 2024-09-17
  • Automatic offers:
    • rojiphil | Reviewer | 104007817
Issue OwnerCurrent Issue Owner: @neil-marcellini
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

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

@bernhardoj
Copy link
Contributor

Proposal

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

WS name & avatar isn't updated dynamically.

What is the root cause of that problem?

The name of the workspace comes from the policy object, but we pass undefined to the policy param of getReportName.

const title = ReportUtils.getReportName(reportHeaderData, undefined, parentReportAction, personalDetails, invoiceReceiverPolicy);

react-complier automatically memoized the result, so it won't be re-calculated when the policy data is changed.

For the workspace avatar, we get the avatar from the report policyAvatar and fallback to the policy avatarURL.

const policyAvatarURL = report?.policyAvatar || allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL;

However, the Pusher only updates the policy.avatarURL, but doesn't send the updated policyAvatar. But if we look at the LHN, the avatar is updated, that's because we use caching for the avatar. The announce room of the workspace doesn't have policyAvatar, so it will use the updated policy.avatarURL and save it to the cache. When the LHN tries to get the avatar, the cache is available, so it shows the updated avatar.

But this doesn't happen with the header or the report welcome text avatar because the same reason as above, that is we don't pass the policy to the function.

const icons = ReportUtils.getIcons(reportHeaderData, personalDetails, null, '', -1, undefined, invoiceReceiverPolicy);

let icons = ReportUtils.getIcons(report, personalDetails, null, '', -1, undefined, invoiceReceiverPolicy);

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

We need to pass policy to both getIcons and getReportName.

const title = ReportUtils.getReportName(reportHeaderData, undefined, parentReportAction, personalDetails, invoiceReceiverPolicy);

const icons = ReportUtils.getIcons(reportHeaderData, personalDetails, null, '', -1, undefined, invoiceReceiverPolicy);

let icons = ReportUtils.getIcons(report, personalDetails, null, '', -1, undefined, invoiceReceiverPolicy);

optionally, I think we should prioritize the policy avatarURL instead of the report.policyAvatar.

const policyAvatarURL = report?.policyAvatar || allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL;

@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
@OfstadC
Copy link
Contributor

OfstadC commented Sep 9, 2024

Looking at this today

@melvin-bot melvin-bot bot removed the Overdue label Sep 9, 2024
@OfstadC
Copy link
Contributor

OfstadC commented Sep 10, 2024

I'm not able to reproduce. Mine took a few seconds but updated on it's own - I tried it on both sides of the chat. Once updating the name and image. Once with just the image. Both updated.

@kavimuru
Copy link

Still reproducible by the tester.

bandicam.2024-09-10.20-34-51-842.2.mp4

@OfstadC
Copy link
Contributor

OfstadC commented Sep 10, 2024

Thanks @kavimuru !

@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label Sep 10, 2024
@melvin-bot melvin-bot bot changed the title Workspace – Workspace name/avatar not updated on another device if workspace chat is open [$250] Workspace – Workspace name/avatar not updated on another device if workspace chat is open Sep 10, 2024
Copy link

melvin-bot bot commented Sep 10, 2024

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

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

melvin-bot bot commented Sep 10, 2024

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

@rojiphil
Copy link
Contributor

@bernhardoj proposal LGTM as updation of workspace name and avatar requires policy in getIcons and getReportName. Regarding the optional suggestion, let us not change the existing order of report policyAvatar and policy avatarURL unless there is a concrete need to do this.
🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Sep 11, 2024

Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Copy link

melvin-bot bot commented Sep 16, 2024

@rojiphil, @OfstadC, @neil-marcellini Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Sep 16, 2024
@OfstadC
Copy link
Contributor

OfstadC commented Sep 16, 2024

Friendly bump @neil-marcellini 😃 . Thank you!

Copy link

melvin-bot bot commented Sep 17, 2024

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

@neil-marcellini
Copy link
Contributor

@bernhardoj proposal LGTM as updation of workspace name and avatar requires policy in getIcons and getReportName. Regarding the optional suggestion, let us not change the existing order of report policyAvatar and policy avatarURL unless there is a concrete need to do this. 🎀👀🎀 C+ reviewed

I agree, hiring 🚀

@melvin-bot melvin-bot bot removed the Overdue label Sep 17, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 17, 2024
Copy link

melvin-bot bot commented Sep 17, 2024

📣 @rojiphil 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

@OfstadC
Copy link
Contributor

OfstadC commented Sep 18, 2024

Any update here @bernhardoj @rojiphil? 😃

@bernhardoj
Copy link
Contributor

PR is ready

cc: @rojiphil

Copy link

melvin-bot bot commented Sep 30, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@OfstadC OfstadC changed the title [$250] Workspace – Workspace name/avatar not updated on another device if workspace chat is open [Hold for payment 2024-10-07][$250] Workspace – Workspace name/avatar not updated on another device if workspace chat is open Oct 1, 2024
@OfstadC
Copy link
Contributor

OfstadC commented Oct 1, 2024

Deployed to production yesterday. Will issue payment on 2024-10-07.

@rojiphil can you propose regression testing if needed? Thank you!

@rojiphil
Copy link
Contributor

rojiphil commented Oct 2, 2024

  • [@rojiphil] The PR that introduced the bug has been identified. Link to the PR: Offending PR
  • [@rojiphil] 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: Added comment
  • [@rojiphil] 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: Not Required. Existing checklist is good enough to capture such issues.
  • [@rojiphil] Determine if we should create a regression test for this bug. : Yes. We can
  • [@rojiphil] 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.

Steps:
Prerequisite: Login using the same account in two devices: Device 1 and Device 2. Also, ensure that Device 1 and Device 2 receives pusher updates so that the UI get’s updated automatically.

  1. Device 1: Navigate to a Workspace Chat
  2. Device 2 : Navigate to Account Settings -> Workspace -> Profile
  3. Device 2 : Change the workspace avatar and name.
  4. Device 1: Verify that the Workspace Chat header and Item Created action reflects the updated workspace avatar. Also verify that the updated workspace name is reflected in header.

@rojiphil
Copy link
Contributor

rojiphil commented Oct 2, 2024

can you propose regression testing if needed? Thank you!

@OfstadC I have completed the BZ checklist which includes the regression test. Thanks.

@OfstadC
Copy link
Contributor

OfstadC commented Oct 4, 2024

Just a note for myself for Monday's payment - @bernhardoj is paid via New Expensify & @rojiphil is issued payment in Upwork

@OfstadC
Copy link
Contributor

OfstadC commented Oct 7, 2024

@rojiphil It looks like you haven't accepted the offer yet. Could you please accept this offer so I can issue payment in Upwork? 😃

@OfstadC
Copy link
Contributor

OfstadC commented Oct 7, 2024

Payment Summary

@rojiphil
Copy link
Contributor

rojiphil commented Oct 8, 2024

It looks like you haven't accepted the offer yet. Could you please accept this offer so I can issue payment in Upwork? 😃

@OfstadC Accepted Offer. Thanks.

@bernhardoj
Copy link
Contributor

Requested in ND.

@OfstadC OfstadC closed this as completed Oct 8, 2024
@github-project-automation github-project-automation bot moved this from Polish to Done in [#whatsnext] #wave-collect Oct 8, 2024
@JmillsExpensify
Copy link

$250 approved for @bernhardoj

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. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Status: Done
Development

No branches or pull requests

8 participants