App icon indicating copy to clipboard operation
App copied to clipboard

Workspace – Workspace name/avatar not updated on another device if workspace chat is open

Open lanitochka17 opened this issue 1 year ago • 2 comments

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] 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): [email protected] 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
  • [x] MacOS: Chrome / Safari
  • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/50337814-f8af-4da3-a32b-d126f1a10ac4

View all open jobs on GitHub

lanitochka17 avatar Sep 06 '24 13:09 lanitochka17

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.

melvin-bot[bot] avatar Sep 06 '24 13:09 melvin-bot[bot]

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. https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/pages/home/HeaderView.tsx#L81

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. https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/libs/ReportUtils.ts#L1952

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. https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/pages/home/HeaderView.tsx#L128 https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/pages/home/report/ReportActionItemCreated.tsx#L50

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

We need to pass policy to both getIcons and getReportName. https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/pages/home/HeaderView.tsx#L81 https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/pages/home/HeaderView.tsx#L128 https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/pages/home/report/ReportActionItemCreated.tsx#L50

optionally, I think we should prioritize the policy avatarURL instead of the report.policyAvatar. https://github.com/Expensify/App/blob/7ebfaa0daaa43a20083250e0f263964ed5b647b2/src/libs/ReportUtils.ts#L1952

bernhardoj avatar Sep 07 '24 16:09 bernhardoj

Looking at this today

OfstadC avatar Sep 09 '24 13:09 OfstadC

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.

OfstadC avatar Sep 10 '24 15:09 OfstadC

Still reproducible by the tester.

https://github.com/user-attachments/assets/9b18770e-86cf-4fbf-ad47-763055eb64bb

kavimuru avatar Sep 10 '24 17:09 kavimuru

Thanks @kavimuru !

OfstadC avatar Sep 10 '24 18:09 OfstadC

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

melvin-bot[bot] avatar Sep 10 '24 21:09 melvin-bot[bot]

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

melvin-bot[bot] avatar Sep 10 '24 21:09 melvin-bot[bot]

@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

rojiphil avatar Sep 11 '24 18:09 rojiphil

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

melvin-bot[bot] avatar Sep 11 '24 18:09 melvin-bot[bot]

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

melvin-bot[bot] avatar Sep 16 '24 18:09 melvin-bot[bot]

Friendly bump @neil-marcellini 😃 . Thank you!

OfstadC avatar Sep 16 '24 18:09 OfstadC

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

melvin-bot[bot] avatar Sep 17 '24 16:09 melvin-bot[bot]

@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 🚀

neil-marcellini avatar Sep 17 '24 19:09 neil-marcellini

📣 @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

melvin-bot[bot] avatar Sep 17 '24 19:09 melvin-bot[bot]

Any update here @bernhardoj @rojiphil? 😃

OfstadC avatar Sep 18 '24 14:09 OfstadC

PR is ready

cc: @rojiphil

bernhardoj avatar Sep 18 '24 16:09 bernhardoj

⚠️ 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.

melvin-bot[bot] avatar Sep 30 '24 13:09 melvin-bot[bot]

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

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

OfstadC avatar Oct 01 '24 12:10 OfstadC

  • [x] [@rojiphil] The PR that introduced the bug has been identified. Link to the PR: Offending PR
  • [x] [@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
  • [x] [@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.
  • [x] [@rojiphil] Determine if we should create a regression test for this bug. : Yes. We can
  • [x] [@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 avatar Oct 02 '24 05:10 rojiphil

can you propose regression testing if needed? Thank you!

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

rojiphil avatar Oct 02 '24 05:10 rojiphil

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

OfstadC avatar Oct 04 '24 13:10 OfstadC

@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 avatar Oct 07 '24 20:10 OfstadC

Payment Summary

  • @rojiphil Paid $250 via Upwork
  • @bernhardoj due $250 via manual New Expensify request

OfstadC avatar Oct 07 '24 20:10 OfstadC

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.

rojiphil avatar Oct 08 '24 02:10 rojiphil

Requested in ND.

bernhardoj avatar Oct 08 '24 04:10 bernhardoj

$250 approved for @bernhardoj

JmillsExpensify avatar Oct 15 '24 11:10 JmillsExpensify