[HOLD for Payment 2024-09-24][$250] Invoice - Default workspace avatar is displayed for invoice receiver when it has custom avatar

[izarutskaya]

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: v9.0.26-5
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/4864174
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

• Workspace has custom avatar.

1. Go to staging.new.expensify.com
2. Go to FAB > Send invoice.
3. Enter amount. select user and send invoice.
4. As invoice receiver, go to invoice chat.

Expected Result:

Invoice receiver should be able to see the custom workspace avatar.

Actual Result:

Invoice receiver is unable to see the custom workspace avatar. It shows the default avatar instead.

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

Bug6573880_1723822556534.20240816_233027.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014c6b9826c08600f4
• Upwork Job ID: 1830746639785190043
• Last Price Increase: 2024-09-09

Issue Owner

Current Issue Owner: @mallenexpensify

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #vip-bills

[Nodebrute]

Proposal

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

Default workspace avatar is displayed for invoice receiver when it has custom avatar

What is the root cause of that problem?

Here we get the avatar from getWorkspaceAvatar

App/src/pages/home/report/ReportActionItemSingle.tsx

Line 103 in 04ffa6c

avatarSource = ReportUtils.getWorkspaceAvatar(report);

and in here the avatar URL will be empty and it will fallback to workspace default avatar

App/src/libs/ReportUtils.ts

Line 1856 in 04ffa6c

const avatar = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL ?? '';

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

We need to change this line

App/src/libs/ReportUtils.ts

Line 1856 in 04ffa6c

const avatar = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL ?? '';

We have avatar stored in report?.policyAvatar we can do something like this. This is the same way we are getting avatar here. This is why we don't see this problem in header.

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

What alternative solutions did you explore? (Optional)

We can use this function to get workspace icons in ReportActionItemSingle.

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[mallenexpensify]

Unable to reproduce, throwing a retest on here

[Nodebrute]

@mallenexpensify Hey, it's reproducible on the latest main branch.

[mallenexpensify]

I tested on v9.0.26-6 and this issue was reported with Version Number: v9.0.26-5
@Nodebrute , can you outline your reproduction steps? Reckon one of us is doing something different than the other.

[Nodebrute]

@mallenexpensify Here are the steps I followed (v9.0.27-0)

1. Have a workspace with a picture.
2. Go to FAB -> Send Invoice.
3. Send the invoice to a 1:1 DM.
4. For the invoice receiver, it shows the default avatar in the message.

Screen.Recording.2024-08-31.at.5.25.52.AM.1.mov

[bernhardoj]

Proposal

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

Default WS avatar is displayed in invoice room if the user isn't part of the workspace.

What is the root cause of that problem?

When user A sends an invoice to user B from a workspace that user B is not a member of, the policy data will be undefined. When we are trying to get the workspace icon,

App/src/libs/ReportUtils.ts

Lines 2246 to 2250 in 117b961

	const icons = [getWorkspaceIcon(report, policy)];
	if (isInvoiceRoom(report)) {
	if (report?.invoiceReceiver?.type === CONST.REPORT.INVOICE_RECEIVER_TYPE.INDIVIDUAL) {
	icons.push(...getIconsForParticipants([report?.invoiceReceiver.accountID], personalDetails));

we already prioritize the avatar from report.policyAvatar. However, the icon is still wrong and it happens after we introduce the caching in #46886.

App/src/libs/ReportUtils.ts

Lines 1937 to 1948 in 117b961

	const policyAvatarURL = report?.policyAvatar || allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL;
	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	const policyExpenseChatAvatarSource = policyAvatarURL || getDefaultWorkspaceAvatar(workspaceName);
	const workspaceIcon: Icon = {
	source: policyExpenseChatAvatarSource ?? '',
	type: CONST.ICON_TYPE_WORKSPACE,
	name: workspaceName,
	id: report?.policyID,
	};
	workSpaceIconsCache.set(cacheKey, {name: workspaceName, icon: workspaceIcon});
	return workspaceIcon;

The caching somehow saves the default avatar for the workspace. Here is the logic to decide whether we can get the avatar from the cache or need to save it to the cache or not.

App/src/libs/ReportUtils.ts

Lines 1925 to 1934 in 117b961

	const cacheKey = report?.policyID ?? workspaceName;
	const iconFromCache = workSpaceIconsCache.get(cacheKey);
	const avatarURL = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL;
	const isSameAvatarURL = iconFromCache?.icon?.source === avatarURL;
	const isDefaultWorkspaceAvatar = !avatarURL && typeof iconFromCache?.icon?.source !== 'string';
	const hasWorkSpaceNameChanged = iconFromCache?.name !== workspaceName;
	if (iconFromCache && (isSameAvatarURL || isDefaultWorkspaceAvatar) && !hasWorkSpaceNameChanged) {
	return iconFromCache.icon;
	}

The first condition is of course to make sure the cache exists. Second, it checks whether the cached image source and the policy avatar source are the same or not. We will focus on the second condition.

As mentioned above, if the user isn't part of the WS, then the policy data will be undefined, so, the avatarURL is undefined (because we don't use report.policyAvatar here) and the isSameAvatarURL condition is always false. This means, we never get the source from the cache and keep setting a new object to the cache, even though the final value is the same.

It still works fine until the report passed to the function (getWorkspaceIcon) has the same policyID (which means the same cache key) but missing policyAvatar.

If the policyAvatar is missing, a default WS avatar will be saved to the cache.

App/src/libs/ReportUtils.ts

Line 1939 in 117b961

const policyExpenseChatAvatarSource = policyAvatarURL || getDefaultWorkspaceAvatar(workspaceName);

And because it's a default avatar, we will return it from the cache.

App/src/libs/ReportUtils.ts

Lines 1930 to 1934 in 117b961

	const isDefaultWorkspaceAvatar = !avatarURL && typeof iconFromCache?.icon?.source !== 'string';
	const hasWorkSpaceNameChanged = iconFromCache?.name !== workspaceName;
	if (iconFromCache && (isSameAvatarURL || isDefaultWorkspaceAvatar) && !hasWorkSpaceNameChanged) {
	return iconFromCache.icon;
	}

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

First, we need to solve the issue where we never use the cache if the policy is undefined by using report?.policyAvatar when comparing the cache source. To do that, we can replace avatarURL with policyAvatarURL (we can even use policyExpenseChatAvatarSource so we can remove isDefaultWorkspaceAvatar condition too).

App/src/libs/ReportUtils.ts

Lines 1927 to 1937 in 117b961

	const avatarURL = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL;
	const isSameAvatarURL = iconFromCache?.icon?.source === avatarURL;
	const isDefaultWorkspaceAvatar = !avatarURL && typeof iconFromCache?.icon?.source !== 'string';
	const hasWorkSpaceNameChanged = iconFromCache?.name !== workspaceName;
	if (iconFromCache && (isSameAvatarURL || isDefaultWorkspaceAvatar) && !hasWorkSpaceNameChanged) {
	return iconFromCache.icon;
	}
	// disabling to protect against empty strings
	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	const policyAvatarURL = report?.policyAvatar || allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL;

Next, to avoid saving default avatar when policyAvatar is missing, we can return the cache if policyAvatar is undefined.

if (iconFromCache && (isSameAvatarURL || isDefaultWorkspaceAvatar || report?.policyAvatar === undefined) && !hasWorkSpaceNameChanged) {
    return iconFromCache.icon;
}

Next, we need to replace getWorkspaceAvatar usage with getWokspaceIcon().source because getWorkspaceAvatar doesn't have the caching and doesn't prioritize report.policyAvatar.

App/src/pages/home/report/ReportActionItemSingle.tsx

Line 103 in 117b961

avatarSource = ReportUtils.getWorkspaceAvatar(report);

App/src/pages/ReportAvatar.tsx

Line 27 in 117b961

const avatarURL = ReportUtils.getWorkspaceAvatar(report);

Last, I notice that the avatar doesn't update immediately, so we need to update report action item memo to compare for report.policyAvatar too.

App/src/pages/home/report/ReportActionItem.tsx

Lines 1010 to 1014 in 117b961

	memo(ReportActionItem, (prevProps, nextProps) => {
	const prevParentReportAction = prevProps.parentReportAction;
	const nextParentReportAction = nextProps.parentReportAction;
	return (
	prevProps.displayAsGroup === nextProps.displayAsGroup &&

[bernhardoj]

@mallenexpensify I can reproduce it by sending an invoice from user A to B using a workspace that user B is not a member of.

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

@Pujan92 plz attempt reproduction, if you're able to, plz review the proposals above. thx

[melvin-bot]

@Pujan92, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[rlinoz]

Just sent one in a collect policy using https://expensify.com

Would you mind trying that?

[bernhardoj]

Still failed.

[bernhardoj]

Nevermind, it works now after I Clear cache and restart

[bernhardoj]

It's very late here, I'll continue tomorrow!

[bernhardoj]

PR is ready

cc: @Pujan92

[rlinoz]

Automation didn't work, this is in prod as of 3 days ago #49145 (comment)

[melvin-bot]

@rlinoz, @Pujan92, @mallenexpensify, @bernhardoj Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

Contributor: @bernhardoj due $250 via NewDot
Contributor+: @Pujan92 due $250 via NewDot

@Pujan92 plz complete the BZ checklist.

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:

• [@] The PR that introduced the bug has been identified. Link to the PR:
• [@] 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:
• [@] 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:
• [@] Determine if we should create a regression test for this bug.
• [@] 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.
• [@bz] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[Pujan92]

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:

• [@Pujan92] The PR that introduced the bug has been identified. Link to the PR: No, we have had this util function getWorkspaceAvatar for a long and the invoice feature was added later. So there isn't any specific PR of causing this bug.
• [@Pujan92] 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: N/A
• [@Pujan92] 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: N/A
• [@Pujan92] Determine if we should create a regression test for this bug. - Yes
• [@Pujan92] 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.

Regression Test Steps

1. Upload an avatar for workspace W if it is a default avatar currently
2. As user A, send an invoice to user B from workspace W(user B isn't a member of this workspace)
3. As user B, verify the custom avatar has been shown in the report details

[rlinoz]

I agree we should add the step to check the workspace avatar, that can be added to the testrail that caught this bug: https://expensify.testrail.io/index.php?/tests/view/4864174

[mallenexpensify]

Thanks @PauloGasparSv and @rlinoz
Test case created
https://github.com/Expensify/Expensify/issues/431500

[garrettmknight]

$250 approved for @Pujan92