Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.58k stars 2.92k forks source link

Workspace - Workspace chat shows default avatar when custom avatar is uploaded offline #53257

Open IuliiaHerets opened 9 hours ago

IuliiaHerets commented 9 hours ago

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.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): applausetester+232803@applause.expensifail.com Issue reported by: Applause Internal

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go offline.
  3. Create a workspace.
  4. Go to workspace settings > Profile.
  5. Click on the workspace avatar.
  6. Upload a photo.
  7. 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:

Screenshots/Videos

https://github.com/user-attachments/assets/2e4fce97-bf55-4db5-8d68-0cc1bfea79bf

View all open jobs on GitHub

melvin-bot[bot] commented 9 hours ago

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.

daledah commented 8 hours ago

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.

Themoonalsofall commented 8 hours ago

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

bernhardoj commented 8 hours ago

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;
}