[$250] Workspace - Workspace chat shows default avatar when custom avatar is uploaded offline
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.68-0 Reproducible in staging?: Y Reproducible in production?: Y **Issue was found when executing this PR ** https://github.com/Expensify/App/pull/53100 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal
Action Performed:
- Go to staging.new.expensify.com
- Go offline.
- Create a workspace.
- Go to workspace settings > Profile.
- Click on the workspace avatar.
- Upload a photo.
- Go to workspace chat.
Expected Result:
The workspace avatar in workspace chat should show the custom avatar.
Actual Result:
The workspace avatar in workspace chat shows default avatar when custom avatar is uploaded offline.
Workspace switcher shows the custom avatar instead of default avatar for the workspace.
Workaround:
Unknown
Platforms:
- [x] Android: Standalone
- [x] Android: HybridApp
- [x] Android: mWeb Chrome
- [x] iOS: Standalone
- [x] iOS: HybridApp
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/2e4fce97-bf55-4db5-8d68-0cc1bfea79bf
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021864244048539221826
- Upwork Job ID: 1864244048539221826
- Last Price Increase: 2024-12-04
Issue Owner
Current Issue Owner: @paultsimura
Triggered auto assignment to @JmillsExpensify (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.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Workspace - Workspace chat shows default avatar when custom avatar is uploaded offline
What is the root cause of that problem?
When we set workspace avatar in offline personalDetails?.[report?.ownerAccountID ?? -1]?.avatar here will be undefined => icon here will be default avatar
https://github.com/Expensify/App/blob/64eaf2fdd357bd06008adfb9c0c049e09cd375b9/src/libs/ReportUtils.ts#L2411
What changes do you think we should make in order to solve the problem?
We should pass policy?.avatar as fallback in here
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? policy?.avatarURL ?? FallbackAvatar,
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
NA
What alternative solutions did you explore? (Optional)
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
Proposal
Please re-state the problem that we are trying to solve in this issue.
The workspace avatar in workspace chat shows default avatar when custom avatar is uploaded offline.
What is the root cause of that problem?
When setting the workspace avatar, we only update policy avatarUrl
https://github.com/Expensify/App/blob/60e9c7c69fa37a6fd320ea94d6b6047dc718d783/src/libs/actions/Policy/Policy.ts#L1032
So in getWorkspaceIcon function, we will return iconFromCache.icon
https://github.com/Expensify/App/blob/60e9c7c69fa37a6fd320ea94d6b6047dc718d783/src/libs/ReportUtils.ts#L2071-L2073
What changes do you think we should make in order to solve the problem?
When setting the workspace avatar, we need to update the expense chat report policy avatar URL too
https://github.com/Expensify/App/blob/60e9c7c69fa37a6fd320ea94d6b6047dc718d783/src/libs/actions/Policy/Policy.ts#L1047
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${getPolicyExpenseChat(accountID, policy?.id ?? '-1')?.reportID ?? '-1'}`,
value: {
policyAvatar: file.uri,
},
},
And will set to default in failureData
What alternative solutions did you explore? (Optional)
NA
Proposal
Please re-state the problem that we are trying to solve in this issue.
WS chat shows default WS avatar.
What is the root cause of that problem?
When we create a new workspace, the avatar is initially a default avatar. When we change the avatar, the avatar URL is changed in the onyx. However, the icon returns the cached avatar, that is the default avatar. https://github.com/Expensify/App/blob/60e9c7c69fa37a6fd320ea94d6b6047dc718d783/src/libs/ReportUtils.ts#L2059-L2073
isSameAvatarURL is false, but report?.policyAvatar === undefined is true, so we return the cache. The reason we added report?.policyAvatar === undefined is explained here.
The reason is, if we receive an invoice from a workspace that we don't have the data, the only way to get the avatar source is from the invoice report policyAvatar. This avatar is then cached. But just in case there is another report that is related to that invoice policy with missing policyAvatar, the avatar will be the default avatar and we don't want to replace the cached avatar with the default avatar.
But in this issue, report?.policyAvatar is undefined, but we have the policy data avatarURL.
What changes do you think we should make in order to solve the problem?
To fix this, we should use the cache if both report?.policyAvatar and policy avatarURL are missing (undefined).
if (iconFromCache && (isSameAvatarURL || !policyAvatarURL) && !hasWorkSpaceNameChanged) {
return iconFromCache.icon;
}
@JmillsExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@JmillsExpensify Huh... This is 4 days overdue. Who can take care of this?
Job added to Upwork: https://www.upwork.com/jobs/~021864244048539221826
Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura (External)
Good polish item for us to handle in the name of offline first, so I'm opening up to the community.
Reviewing soon π
Thanks for the proposals, y'all.
The one by @bernhardoj has the most to-the-root RCA analysis and a proper solution. The other 2, unfortunately, look quite hacky and situational.
πππ C+ reviewed
Triggered auto assignment to @MarioExpensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
Yep, @bernhardoj's solution sounds great. Thank you @paultsimura, let's move forward!
π£ @paultsimura π An offer has been automatically sent to your Upwork account for the Reviewer role π Thanks for contributing to the Expensify app!
PR is ready
cc: @paultsimura
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.74-8 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/53618
If no regressions arise, payment will be issued on 2024-12-19. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @paultsimura requires payment automatic offer (Reviewer)
- @bernhardoj requires payment through NewDot Manual Requests
@paultsimura @JmillsExpensify @paultsimura The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]
Payment summary:
Contributor: @bernhardoj $250 via New Expensify Reviewer: @paultsimura $250 via Upwork
@bernhardoj feel free to request now. @paultsimura please complete the BZ checklist and we can get the regression test and Upwork payment handled.
Requested in ND.
Payment summary:
Contributor: @bernhardoj $250 via New Expensify Reviewer: @paultsimura $250 via Upwork
$250 approved for @bernhardoj
BugZero Checklist:
- [x] [Contributor] Classify the bug:
Bug classification
Source of bug:
- [ ] 1a. Result of the original design (eg. a case wasn't considered)
- [x] 1b. Mistake during implementation
- [ ] 1c. Backend bug
- [ ] 1z. Other:
Where bug was reported:
- [x] 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
- [ ] 2b. Reported on staging (eg. found during regression or PR testing)
- [ ] 2d. Reported on a PR
- [ ] 2z. Other:
Who reported the bug:
- [ ] 3a. Expensify user
- [ ] 3b. Expensify employee
- [ ] 3c. Contributor
- [x] 3d. QA
- [ ] 3z. Other:
-
[x] [Contributor] 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: https://github.com/Expensify/App/pull/49145/files#r1891617459
-
[x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A
-
[x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
-
[ ] [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.
Link to issue:
Regression Test Proposal
Test:
- Go offline
- Create a workspace
- Go to workspace settings > Profile
- Change the workspace avatar
- Go to workspace chat
- Verify the workspace avatar on the LHN and workspace chat show the custom avatar
Do we agree π or π
Thanks @JmillsExpensify, the checklist is βοΈ