-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix object mutation causes the WS item in search shows last message instead of WS name #54351
Merged
Beamanator
merged 5 commits into
Expensify:main
from
bernhardoj:fix/53361-ws-last-msg-shows-in-search
Jan 8, 2025
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
fa4c4b3
don't mutate the report object
bernhardoj 07752f2
lint
bernhardoj 3786b92
Merge branch 'main' into fix/53361-ws-last-msg-shows-in-search
bernhardoj 3cf8c71
Merge branch 'main' into fix/53361-ws-last-msg-shows-in-search
bernhardoj b9905de
don't force policy name preview
bernhardoj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -371,15 +371,15 @@ function isPersonalDetailsReady(personalDetails: OnyxEntry<PersonalDetailsList>) | |
* Get the participant option for a report. | ||
*/ | ||
function getParticipantsOption(participant: ReportUtils.OptionData | Participant, personalDetails: OnyxEntry<PersonalDetailsList>): Participant { | ||
const detail = getPersonalDetailsForAccountIDs([participant.accountID ?? -1], personalDetails)[participant.accountID ?? -1]; | ||
const detail = participant.accountID ? getPersonalDetailsForAccountIDs([participant.accountID], personalDetails)[participant.accountID] : undefined; | ||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
const login = detail?.login || participant.login || ''; | ||
const displayName = LocalePhoneNumber.formatPhoneNumber(PersonalDetailsUtils.getDisplayNameOrDefault(detail, login || participant.text)); | ||
|
||
return { | ||
keyForList: String(detail?.accountID), | ||
login, | ||
accountID: detail?.accountID ?? -1, | ||
accountID: detail?.accountID, | ||
text: displayName, | ||
firstName: detail?.firstName ?? '', | ||
lastName: detail?.lastName ?? '', | ||
|
@@ -506,11 +506,11 @@ function getIOUReportIDOfLastAction(report: OnyxEntry<Report>): string | undefin | |
* Get the last message text from the report directly or from other sources for special cases. | ||
*/ | ||
function getLastMessageTextForReport(report: OnyxEntry<Report>, lastActorDetails: Partial<PersonalDetails> | null, policy?: OnyxEntry<Policy>): string { | ||
const reportID = report?.reportID ?? '-1'; | ||
const lastReportAction = lastVisibleReportActions[reportID] ?? null; | ||
const reportID = report?.reportID; | ||
const lastReportAction = reportID ? lastVisibleReportActions[reportID] : undefined; | ||
|
||
// some types of actions are filtered out for lastReportAction, in some cases we need to check the actual last action | ||
const lastOriginalReportAction = lastReportActions[reportID] ?? null; | ||
const lastOriginalReportAction = reportID ? lastReportActions[reportID] : undefined; | ||
let lastMessageTextFromReport = ''; | ||
|
||
if (report?.private_isArchived) { | ||
|
@@ -540,12 +540,14 @@ function getLastMessageTextForReport(report: OnyxEntry<Report>, lastActorDetails | |
lastMessageTextFromReport = ReportUtils.formatReportLastMessageText(properSchemaForMoneyRequestMessage); | ||
} else if (ReportActionUtils.isReportPreviewAction(lastReportAction)) { | ||
const iouReport = ReportUtils.getReportOrDraftReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastReportAction)); | ||
const lastIOUMoneyReportAction = allSortedReportActions[iouReport?.reportID ?? '-1']?.find( | ||
(reportAction, key): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> => | ||
ReportActionUtils.shouldReportActionBeVisible(reportAction, key, ReportUtils.canUserPerformWriteAction(report)) && | ||
reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && | ||
ReportActionUtils.isMoneyRequestAction(reportAction), | ||
); | ||
const lastIOUMoneyReportAction = iouReport?.reportID | ||
? allSortedReportActions[iouReport.reportID]?.find( | ||
(reportAction, key): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> => | ||
ReportActionUtils.shouldReportActionBeVisible(reportAction, key, ReportUtils.canUserPerformWriteAction(report)) && | ||
reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && | ||
ReportActionUtils.isMoneyRequestAction(reportAction), | ||
) | ||
: undefined; | ||
const reportPreviewMessage = ReportUtils.getReportPreviewMessage( | ||
!isEmptyObject(iouReport) ? iouReport : null, | ||
lastIOUMoneyReportAction, | ||
|
@@ -562,9 +564,9 @@ function getLastMessageTextForReport(report: OnyxEntry<Report>, lastActorDetails | |
lastMessageTextFromReport = ReportUtils.getReimbursementDeQueuedActionMessage(lastReportAction, report, true); | ||
} else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) { | ||
lastMessageTextFromReport = ReportUtils.getDeletedParentActionMessageForChatReport(lastReportAction); | ||
} else if (ReportActionUtils.isPendingRemove(lastReportAction) && ReportActionUtils.isThreadParentMessage(lastReportAction, report?.reportID ?? '-1')) { | ||
} else if (ReportActionUtils.isPendingRemove(lastReportAction) && report?.reportID && ReportActionUtils.isThreadParentMessage(lastReportAction, report.reportID)) { | ||
lastMessageTextFromReport = Localize.translateLocal('parentReportAction.hiddenMessage'); | ||
} else if (ReportUtils.isReportMessageAttachment({text: report?.lastMessageText ?? '-1', html: report?.lastMessageHtml, type: ''})) { | ||
} else if (ReportUtils.isReportMessageAttachment({text: report?.lastMessageText ?? '', html: report?.lastMessageHtml, type: ''})) { | ||
lastMessageTextFromReport = `[${Localize.translateLocal('common.attachment')}]`; | ||
} else if (ReportActionUtils.isModifiedExpenseAction(lastReportAction)) { | ||
const properSchemaForModifiedExpenseMessage = ModifiedExpenseMessage.getForReportAction(report?.reportID, lastReportAction); | ||
|
@@ -704,7 +706,7 @@ function createOption( | |
hasMultipleParticipants = personalDetailList.length > 1 || result.isChatRoom || result.isPolicyExpenseChat || ReportUtils.isGroupChat(report); | ||
subtitle = ReportUtils.getChatRoomSubtitle(report); | ||
|
||
const lastActorDetails = personalDetailMap[report.lastActorAccountID ?? -1] ?? null; | ||
const lastActorDetails = report.lastActorAccountID ? personalDetailMap[report.lastActorAccountID] : null; | ||
const lastActorDisplayName = getLastActorDisplayName(lastActorDetails, hasMultipleParticipants); | ||
const lastMessageTextFromReport = getLastMessageTextForReport(report, lastActorDetails); | ||
let lastMessageText = lastMessageTextFromReport; | ||
|
@@ -919,7 +921,13 @@ function createOptionList(personalDetails: OnyxEntry<PersonalDetailsList>, repor | |
|
||
const allPersonalDetailsOptions = Object.values(personalDetails ?? {}).map((personalDetail) => ({ | ||
item: personalDetail, | ||
...createOption([personalDetail?.accountID ?? -1], personalDetails, reportMapForAccountIDs[personalDetail?.accountID ?? -1], {}, {showPersonalDetails: true}), | ||
...createOption( | ||
[personalDetail?.accountID ?? CONST.DEFAULT_NUMBER_ID], | ||
personalDetails, | ||
reportMapForAccountIDs[personalDetail?.accountID ?? CONST.DEFAULT_NUMBER_ID], | ||
{}, | ||
{showPersonalDetails: true}, | ||
), | ||
})); | ||
|
||
return { | ||
|
@@ -1167,7 +1175,7 @@ function getValidOptions( | |
shouldSeparateWorkspaceChat = false, | ||
}: GetOptionsConfig = {}, | ||
): Options { | ||
const topmostReportId = Navigation.getTopmostReportId() ?? '-1'; | ||
const topmostReportId = Navigation.getTopmostReportId(); | ||
|
||
// Filter out all the reports that shouldn't be displayed | ||
const filteredReportOptions = options.reports.filter((option) => { | ||
|
@@ -1270,12 +1278,6 @@ function getValidOptions( | |
|
||
if (includeRecentReports) { | ||
for (const reportOption of allReportOptions) { | ||
/** | ||
* By default, generated options does not have the chat preview line enabled. | ||
* If showChatPreviewLine or forcePolicyNamePreview are true, let's generate and overwrite the alternate text. | ||
*/ | ||
reportOption.alternateText = getAlternateText(reportOption, {showChatPreviewLine, forcePolicyNamePreview}); | ||
|
||
// Skip [email protected] | ||
if (reportOption.login === CONST.EMAIL.NOTIFICATIONS) { | ||
continue; | ||
|
@@ -1287,7 +1289,7 @@ function getValidOptions( | |
const shouldShowInvoiceRoom = | ||
includeInvoiceRooms && | ||
ReportUtils.isInvoiceRoom(reportOption.item) && | ||
ReportUtils.isPolicyAdmin(reportOption.policyID ?? '', policies) && | ||
ReportUtils.isPolicyAdmin(reportOption.policyID, policies) && | ||
!reportOption.private_isArchived && | ||
PolicyUtils.canSendInvoiceFromWorkspace(reportOption.policyID); | ||
|
||
|
@@ -1311,17 +1313,14 @@ function getValidOptions( | |
continue; | ||
} | ||
|
||
reportOption.isSelected = isReportSelected(reportOption, selectedOptions); | ||
reportOption.isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption); | ||
|
||
if (action === CONST.IOU.ACTION.CATEGORIZE) { | ||
const reportPolicy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${reportOption.policyID}`]; | ||
if (reportPolicy?.areCategoriesEnabled) { | ||
recentReportOptions.push(reportOption); | ||
} | ||
} else { | ||
recentReportOptions.push(reportOption); | ||
} | ||
/** | ||
* By default, generated options does not have the chat preview line enabled. | ||
* If showChatPreviewLine or forcePolicyNamePreview are true, let's generate and overwrite the alternate text. | ||
*/ | ||
const alternateText = getAlternateText(reportOption, {showChatPreviewLine, forcePolicyNamePreview}); | ||
const isSelected = isReportSelected(reportOption, selectedOptions); | ||
const isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption); | ||
let lastIOUCreationDate; | ||
|
||
// Add a field to sort the recent reports by the time of last IOU request for create actions | ||
if (preferRecentExpenseReports) { | ||
|
@@ -1334,10 +1333,27 @@ function getValidOptions( | |
const iouReportActions = iouReportID ? allSortedReportActions[iouReportID] ?? [] : []; | ||
const lastIOUAction = iouReportActions.find((iouAction) => iouAction.actionName === CONST.REPORT.ACTIONS.TYPE.IOU); | ||
if (lastIOUAction) { | ||
reportOption.lastIOUCreationDate = lastIOUAction.lastModified; | ||
lastIOUCreationDate = lastIOUAction.lastModified; | ||
} | ||
} | ||
} | ||
|
||
const newReportOption = { | ||
...reportOption, | ||
alternateText, | ||
isSelected, | ||
isBold, | ||
lastIOUCreationDate, | ||
}; | ||
|
||
if (action === CONST.IOU.ACTION.CATEGORIZE) { | ||
const reportPolicy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${newReportOption.policyID}`]; | ||
if (reportPolicy?.areCategoriesEnabled) { | ||
recentReportOptions.push(newReportOption); | ||
} | ||
} else { | ||
recentReportOptions.push(newReportOption); | ||
} | ||
} | ||
} else if (recentAttendees && recentAttendees?.length > 0) { | ||
recentAttendees.filter((attendee) => attendee.login ?? attendee.displayName).forEach((a) => optionsToExclude.push({login: a.login ?? a.displayName})); | ||
|
@@ -1390,7 +1406,6 @@ function getSearchOptions(options: OptionList, betas: Beta[] = [], isUsedInChatF | |
includeMultipleParticipantReports: true, | ||
showChatPreviewLine: isUsedInChatFinder, | ||
includeP2P: true, | ||
forcePolicyNamePreview: true, | ||
includeOwnedWorkspaceChats: true, | ||
includeThreads: true, | ||
includeMoneyRequests: true, | ||
|
@@ -1438,8 +1453,8 @@ function getIOUConfirmationOptionsFromPayeePersonalDetail(personalDetail: OnyxEn | |
], | ||
descriptiveText: amountText ?? '', | ||
login: personalDetail?.login ?? '', | ||
accountID: personalDetail?.accountID ?? -1, | ||
keyForList: String(personalDetail?.accountID ?? -1), | ||
accountID: personalDetail?.accountID ?? CONST.DEFAULT_NUMBER_ID, | ||
keyForList: String(personalDetail?.accountID ?? CONST.DEFAULT_NUMBER_ID), | ||
}; | ||
} | ||
|
||
|
@@ -1517,14 +1532,14 @@ function formatMemberForList(member: ReportUtils.OptionData): MemberForList { | |
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
alternateText: member.alternateText || member.login || '', | ||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
keyForList: member.keyForList || String(accountID ?? -1) || '', | ||
keyForList: member.keyForList || String(accountID ?? CONST.DEFAULT_NUMBER_ID) || '', | ||
isSelected: member.isSelected ?? false, | ||
isDisabled: member.isDisabled ?? false, | ||
accountID, | ||
login: member.login ?? '', | ||
icons: member.icons, | ||
pendingAction: member.pendingAction, | ||
reportID: member.reportID ?? '-1', | ||
reportID: member.reportID, | ||
}; | ||
} | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol
lastMessageText ?? -1
🤦