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.

Sign up for GitHub

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

Jump to bottom

Fix WS name and avatar are not updated dynamically in the chat #49403

Merged
merged 4 commits into from
Sep 27, 2024
+16 −53
Merged

Fix WS name and avatar are not updated dynamically in the chat #49403

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
36cf24f
fix ws name and avatar are not updated dynamically
bernhardoj Sep 18, 2024
550caca
Merge branch 'main' into fix/48710-ws-avatar-and-name-not-updated-dyn…
bernhardoj Sep 24, 2024
6e108b1
add null safety
bernhardoj Sep 24, 2024
83e3911
Merge branch 'main' into fix/48710-ws-avatar-and-name-not-updated-dyn…
bernhardoj Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1939,7 +1939,7 @@ function getWorkspaceIcon(report: OnyxInputOrEntry<Report>, policy?: OnyxInputOr
const iconFromCache = workSpaceIconsCache.get(cacheKey);
// 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;
const policyAvatarURL = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatarURL || report?.policyAvatar;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

As explained in my proposal, the LHN ws avatar is updated because the announce room of the workspace doesn't have policyAvatar, so it fallbacks to the policy.avatarURL, but I just recheck and now the announce room has the policyAvatar. So, I updated the fallback here

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const policyExpenseChatAvatarSource = policyAvatarURL || getDefaultWorkspaceAvatar(workspaceName);

Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/HeaderView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function HeaderView({report, parentReportAction, reportID, onNavigationMenuButto
const isTaskReport = ReportUtils.isTaskReport(report);
const reportHeaderData = !isTaskReport && !isChatThread && report?.parentReportID ? parentReport : report;
// Use sorted display names for the title for group chats on native small screen widths
const title = ReportUtils.getReportName(reportHeaderData, undefined, parentReportAction, personalDetails, invoiceReceiverPolicy);
const title = ReportUtils.getReportName(reportHeaderData, policy, parentReportAction, personalDetails, invoiceReceiverPolicy);
const subtitle = ReportUtils.getChatRoomSubtitle(reportHeaderData);
const parentNavigationSubtitleData = ReportUtils.getParentNavigationSubtitle(reportHeaderData);
const reportDescription = ReportUtils.getReportDescriptionText(report);
Expand Down Expand Up @@ -124,7 +124,7 @@ function HeaderView({report, parentReportAction, reportID, onNavigationMenuButto

const shouldShowSubscript = ReportUtils.shouldReportShowSubscript(report);
const defaultSubscriptSize = ReportUtils.isExpenseRequest(report) ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT;
const icons = ReportUtils.getIcons(reportHeaderData, personalDetails, null, '', -1, undefined, invoiceReceiverPolicy);
const icons = ReportUtils.getIcons(reportHeaderData, personalDetails, null, '', -1, policy, invoiceReceiverPolicy);
const brickRoadIndicator = ReportUtils.hasReportNameError(report) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';
const shouldShowBorderBottom = !isTaskReport || !shouldUseNarrowLayout;
const shouldDisableDetailPage = ReportUtils.shouldDisableDetailPage(report);
Expand Down
57 changes: 9 additions & 48 deletions src/pages/home/report/ReportActionItemCreated.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,32 @@
import lodashIsEqual from 'lodash/isEqual';
import React, {memo} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx, withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import MultipleAvatars from '@components/MultipleAvatars';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import ReportWelcomeText from '@components/ReportWelcomeText';
import useLocalize from '@hooks/useLocalize';
import usePolicy from '@hooks/usePolicy';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ReportUtils from '@libs/ReportUtils';
import {navigateToConciergeChatAndDeleteReport} from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetailsList, Policy, Report} from '@src/types/onyx';
import AnimatedEmptyStateBackground from './AnimatedEmptyStateBackground';

type ReportActionItemCreatedOnyxProps = {
/** The report currently being looked at */
report: OnyxEntry<Report>;

/** The policy object for the current route */
policy: OnyxEntry<Policy>;

/** Personal details of all the users */
personalDetails: OnyxEntry<PersonalDetailsList>;
};

type ReportActionItemCreatedProps = ReportActionItemCreatedOnyxProps & {
type ReportActionItemCreatedProps = {
/** The id of the report */
reportID: string;

/** The id of the policy */
// eslint-disable-next-line react/no-unused-prop-types
policyID: string | undefined;
};
function ReportActionItemCreated({report, personalDetails, policy, reportID}: ReportActionItemCreatedProps) {
function ReportActionItemCreated({policyID, reportID}: ReportActionItemCreatedProps) {
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
const policy = usePolicy(policyID);
const styles = useThemeStyles();

const {translate} = useLocalize();
Expand All @@ -47,7 +37,7 @@ function ReportActionItemCreated({report, personalDetails, policy, reportID}: Re
return null;
}

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

if (ReportUtils.isInvoiceRoom(report) && ReportUtils.isCurrentUserInvoiceReceiver(report)) {
Expand Down Expand Up @@ -98,33 +88,4 @@ function ReportActionItemCreated({report, personalDetails, policy, reportID}: Re

ReportActionItemCreated.displayName = 'ReportActionItemCreated';

export default withOnyx<ReportActionItemCreatedProps, ReportActionItemCreatedOnyxProps>({
report: {
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
},

policy: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
},

personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
})(
memo(
ReportActionItemCreated,
(prevProps, nextProps) =>
prevProps.policy?.name === nextProps.policy?.name &&
prevProps.policy?.avatarURL === nextProps.policy?.avatarURL &&
prevProps.report?.stateNum === nextProps.report?.stateNum &&
prevProps.report?.statusNum === nextProps.report?.statusNum &&
prevProps.report?.lastReadTime === nextProps.report?.lastReadTime &&
prevProps.report?.description === nextProps.report?.description &&
prevProps.personalDetails === nextProps.personalDetails &&
prevProps.policy?.description === nextProps.policy?.description &&
prevProps.report?.reportName === nextProps.report?.reportName &&
prevProps.report?.avatarUrl === nextProps.report?.avatarUrl &&
lodashIsEqual(prevProps.report?.invoiceReceiver, nextProps.report?.invoiceReceiver) &&
prevProps.report?.errorFields === nextProps.report?.errorFields,
),
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
);
export default memo(ReportActionItemCreated);
6 changes: 4 additions & 2 deletions src/pages/home/report/ReportActionItemSingle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Text from '@components/Text';
import Tooltip from '@components/Tooltip';
import UserDetailsTooltip from '@components/UserDetailsTooltip';
import useLocalize from '@hooks/useLocalize';
import usePolicy from '@hooks/usePolicy';
import useStyleUtils from '@hooks/useStyleUtils';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -82,6 +83,7 @@ function ReportActionItemSingle({
const {translate} = useLocalize();
const personalDetails = usePersonalDetails() ?? CONST.EMPTY_OBJECT;
const actorAccountID = ReportUtils.getReportActionActorAccountID(action, iouReport);
const policy = usePolicy(report?.policyID);
const [invoiceReceiverPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : -1}`);

let displayName = ReportUtils.getDisplayNameForParticipant(actorAccountID);
Expand All @@ -98,9 +100,9 @@ function ReportActionItemSingle({
let avatarId: number | string | undefined = actorAccountID;

if (isWorkspaceActor) {
displayName = ReportUtils.getPolicyName(report);
displayName = ReportUtils.getPolicyName(report, undefined, policy);
actorHint = displayName;
avatarSource = ReportUtils.getWorkspaceIcon(report).source;
avatarSource = ReportUtils.getWorkspaceIcon(report, policy).source;
avatarId = report?.policyID;
} else if (action?.delegateAccountID && personalDetails[action?.delegateAccountID]) {
// We replace the actor's email, name, and avatar with the Copilot manually for now. And only if we have their
Expand Down
Loading