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.

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

Allow system account chat to be listed in LHN, fix chat icons #41290

Merged
merged 15 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
14 changes: 12 additions & 2 deletions src/components/ReportWelcomeText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ function ReportWelcomeText({report, policy, personalDetails}: ReportWelcomeTextP
const isSelfDM = ReportUtils.isSelfDM(report);
const isInvoiceRoom = ReportUtils.isInvoiceRoom(report);
const isOneOnOneChat = ReportUtils.isOneOnOneChat(report);
const isSystemChat = ReportUtils.isSystemChat(report);
const isDefault = !(isChatRoom || isPolicyExpenseChat || isSelfDM || isInvoiceRoom);
const participantAccountIDs = Object.keys(report?.participants ?? {})
.map(Number)
.filter((accountID) => accountID !== session?.accountID || !isOneOnOneChat);
.filter((accountID) => accountID !== session?.accountID || (!isOneOnOneChat && !isSystemChat));
const isMultipleParticipant = participantAccountIDs.length > 1;
const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips(OptionsListUtils.getPersonalDetailsForAccountIDs(participantAccountIDs, personalDetails), isMultipleParticipant);
const isUserPolicyAdmin = PolicyUtils.isPolicyAdmin(policy);
Expand Down Expand Up @@ -77,8 +78,12 @@ function ReportWelcomeText({report, policy, personalDetails}: ReportWelcomeTextP
return translate('reportActionsView.yourSpace');
}

if (isSystemChat) {
return reportName;
}

return translate('reportActionsView.sayHello');
}, [isChatRoom, isInvoiceRoom, isSelfDM, translate, reportName]);
}, [isChatRoom, isInvoiceRoom, isSelfDM, isSystemChat, translate, reportName]);

return (
<>
Expand Down Expand Up @@ -144,6 +149,11 @@ function ReportWelcomeText({report, policy, personalDetails}: ReportWelcomeTextP
<Text>{translate('reportActionsView.beginningOfChatHistorySelfDM')}</Text>
</Text>
)}
{isSystemChat && (
<Text>
<Text>{translate('reportActionsView.beginningOfChatHistorySystemDM')}</Text>
</Text>
)}
Comment on lines +152 to +156
Copy link
Contributor

Choose a reason for hiding this comment

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

This change caused this issue

We have implemented show default system chat message
But we din't add condition for system chat for sidebar

{isDefault && (
<Text>
<Text>{translate('reportActionsView.beginningOfChatHistory')}</Text>
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ export default {
beginningOfChatHistoryPolicyExpenseChatPartTwo: ' and ',
beginningOfChatHistoryPolicyExpenseChatPartThree: ' starts here! 🎉 This is the place to chat, submit expenses and settle up.',
beginningOfChatHistorySelfDM: 'This is your personal space. Use it for notes, tasks, drafts, and reminders.',
beginningOfChatHistorySystemDM: "Welcome! Let's get you set up.",
chatWithAccountManager: 'Chat with your account manager here',
sayHello: 'Say hello!',
yourSpace: 'Your space',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ export default {
beginningOfChatHistoryPolicyExpenseChatPartTwo: ' y ',
beginningOfChatHistoryPolicyExpenseChatPartThree: ' empieza aquí! 🎉 Este es el lugar donde chatear y presentar o pagar gastos.',
beginningOfChatHistorySelfDM: 'Este es tu espacio personal. Úsalo para notas, tareas, borradores y recordatorios.',
beginningOfChatHistorySystemDM: '¡Bienvenido! Vamos a configurar tu cuenta.',
chatWithAccountManager: 'Chatea con tu gestor de cuenta aquí',
sayHello: '¡Saluda!',
yourSpace: 'Tu espacio',
Expand Down
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1867,7 +1867,7 @@ function getOptions(
allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.text?.toLowerCase()], 'asc');
}

const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}];
const optionsToExclude: Option[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from here #49673, we decide to exclude CONST.EMAIL.NOTIFICATIONS again and it handled here #50937


// If we're including selected options from the search results, we only want to exclude them if the search input is empty
// This is because on certain pages, we show the selected options at the top when the search input is empty
Expand Down
25 changes: 22 additions & 3 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1200,13 +1200,22 @@ function findLastAccessedReport(
}

if (isFirstTimeNewExpensifyUser) {
// Filter out the systemChat report from the reports list, as we don't want to drop the user into that report over Concierge when they first log in
sortedReports = sortedReports.filter((report) => !isSystemChat(report)) ?? [];
if (sortedReports.length === 1) {
return sortedReports[0];
}

return adminReport ?? sortedReports.find((report) => !isConciergeChatReport(report)) ?? null;
}

// If we only have two reports and one of them is the system chat, filter it out so we don't
// overwrite showing the concierge chat
const hasSystemChat = sortedReports.find((report) => isSystemChat(report)) ?? false;
if (sortedReports.length === 2 && hasSystemChat) {
sortedReports = sortedReports.filter((report) => !isSystemChat(report)) ?? [];
}

return adminReport ?? sortedReports.at(-1) ?? null;
}

Expand Down Expand Up @@ -2052,6 +2061,10 @@ function getIcons(
return getIconsForParticipants([currentUserAccountID ?? 0], personalDetails);
}

if (isSystemChat(report)) {
return getIconsForParticipants([CONST.ACCOUNT_ID.NOTIFICATIONS ?? 0], personalDetails);
}

if (isGroupChat(report)) {
const groupChatIcon = {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Expand Down Expand Up @@ -3229,6 +3242,10 @@ function getReportName(report: OnyxEntry<Report>, policy: OnyxEntry<Policy> = nu
formattedName = getDisplayNameForParticipant(currentUserAccountID, undefined, undefined, true);
}

if (isInvoiceRoom(report)) {
formattedName = getInvoicesChatName(report);
}

if (formattedName) {
return formattedName;
}
Expand Down Expand Up @@ -5200,11 +5217,13 @@ function shouldReportBeInOptionList({
!isMoneyRequestReport(report) &&
!isTaskReport(report) &&
!isSelfDM(report) &&
!isSystemChat(report) &&
!isGroupChat(report) &&
!isInvoiceRoom(report))
) {
return false;
}

if (!canAccessReport(report, policies, betas)) {
return false;
}
Expand Down Expand Up @@ -5267,7 +5286,7 @@ function shouldReportBeInOptionList({
}

// Hide chats between two users that haven't been commented on from the LNH
if (excludeEmptyChats && isEmptyChat && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report) && !isGroupChat(report) && canHideReport) {
if (excludeEmptyChats && isEmptyChat && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report) && !isSystemChat(report) && !isGroupChat(report) && canHideReport) {
return false;
}

Expand Down Expand Up @@ -5599,7 +5618,7 @@ function isGroupChatAdmin(report: OnyxEntry<Report>, accountID: number) {
*/
function getMoneyRequestOptions(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>, reportParticipants: number[], filterDeprecatedTypes = false): IOUType[] {
// In any thread or task report, we do not allow any new expenses yet
if (isChatThread(report) || isTaskReport(report) || isInvoiceReport(report)) {
if (isChatThread(report) || isTaskReport(report) || isInvoiceReport(report) || isSystemChat(report)) {
return [];
}

Expand Down Expand Up @@ -6614,7 +6633,7 @@ function canJoinChat(report: OnyxEntry<Report>, parentReportAction: OnyxEntry<Re
}

// Anyone viewing these chat types is already a participant and therefore cannot join
if (isRootGroupChat(report) || isSelfDM(report) || isInvoiceRoom(report)) {
if (isRootGroupChat(report) || isSelfDM(report) || isInvoiceRoom(report) || isSystemChat(report)) {
return false;
}

Expand Down
4 changes: 3 additions & 1 deletion src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ function getOrderedReportIDs(
const isFocused = report.reportID === currentReportId;
const allReportErrors = OptionsListUtils.getAllReportErrors(report, reportActions) ?? {};
const hasErrorsOtherThanFailedReceipt = doesReportHaveViolations || Object.values(allReportErrors).some((error) => error?.[0] !== 'report.genericSmartscanFailureMessage');
const shouldOverrideHidden = hasErrorsOtherThanFailedReceipt || isFocused || report.isPinned;

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const shouldOverrideHidden = hasErrorsOtherThanFailedReceipt || isFocused || report.isPinned || ReportUtils.isSystemChat(report);
NikkiWines marked this conversation as resolved.
Show resolved Hide resolved
if (isHidden && !shouldOverrideHidden) {
return false;
}
Expand Down
3 changes: 2 additions & 1 deletion src/pages/home/HeaderView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,12 @@ function HeaderView({
const isSelfDM = ReportUtils.isSelfDM(report);
const isGroupChat = ReportUtils.isGroupChat(report) || ReportUtils.isDeprecatedGroupDM(report);
const isOneOnOneChat = ReportUtils.isOneOnOneChat(report);
const isSystemChat = ReportUtils.isSystemChat(report);

// For 1:1 chat, we don't want to include currentUser as participants in order to not mark 1:1 chats as having multiple participants
const participants = Object.keys(report?.participants ?? {})
.map(Number)
.filter((accountID) => accountID !== session?.accountID || !isOneOnOneChat)
.filter((accountID) => accountID !== session?.accountID || (!isOneOnOneChat && !isSystemChat))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be (!isOneOnOneChat || !isSystemChat)?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, because !isOneOnOneChat will always be true for the systemChat, so we'd always end up including the current user in the participants list.

.slice(0, 5);
const isMultipleParticipant = participants.length > 1;

Expand Down
Loading