Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
16 changes: 9 additions & 7 deletions src/libs/ReportNameUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {
ReportAction,
ReportActions,
ReportAttributesDerivedValue,
ReportMetadata,
ReportNameValuePairs,
Transaction,
} from '@src/types/onyx';
Expand Down Expand Up @@ -141,6 +140,7 @@ import {
getPolicyName,
getReimbursementDeQueuedOrCanceledActionMessage,
getReimbursementQueuedActionMessage,
getPendingDeleteMemberAccountIDs,
getReportMetadata,
getReportOrDraftReport,
getTransactionReportName,
Expand Down Expand Up @@ -191,6 +191,7 @@ type ComputeReportName = {
conciergeReportID?: string;
reportAttributes?: ReportAttributesDerivedValue['reports'];
isTrackIntentUser: boolean | undefined;
pendingDeleteMemberAccountIDs?: string[];
};

let allPersonalDetails: OnyxEntry<PersonalDetailsList>;
Expand Down Expand Up @@ -263,18 +264,17 @@ function getGroupChatName(
participants?: SelectedParticipant[],
shouldApplyLimit = false,
report?: OnyxEntry<Report>,
reportMetadataParam?: OnyxEntry<ReportMetadata>,
pendingDeleteMemberAccountIDs?: string[],
): string | undefined {
// If we have a report always try to get the name from the report.
if (report?.reportName) {
return report.reportName;
}

const reportMetadata = reportMetadataParam ?? getReportMetadata(report?.reportID);
// TODO: Remove the getReportMetadata fallback once https://github.com/Expensify/App/issues/66421 is done
Comment thread
daledah marked this conversation as resolved.
const resolvedPendingDeleteMemberAccountIDs = pendingDeleteMemberAccountIDs ?? getPendingDeleteMemberAccountIDs(getReportMetadata(report?.reportID)?.pendingChatMembers);

const pendingMemberAccountIDs = new Set(
reportMetadata?.pendingChatMembers?.filter((member) => member.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).map((member) => member.accountID),
);
const pendingMemberAccountIDs = new Set(resolvedPendingDeleteMemberAccountIDs);
let participantAccountIDs =
participants?.map((participant) => participant.accountID) ??
Object.keys(report?.participants ?? {})
Expand Down Expand Up @@ -953,6 +953,7 @@ function computeReportName({
conciergeReportID,
reportAttributes,
isTrackIntentUser,
pendingDeleteMemberAccountIDs,
}: ComputeReportName): string {
if (!report?.reportID) {
return '';
Expand Down Expand Up @@ -999,6 +1000,7 @@ function computeReportName({
conciergeReportID,
reportAttributes,
isTrackIntentUser,
pendingDeleteMemberAccountIDs: undefined,
});
return getCreatedReportForUnapprovedTransactionsMessage(originalID, reportName, isOriginalReportDeleted(parentReportAction, originalReport), translate);
}
Expand Down Expand Up @@ -1033,7 +1035,7 @@ function computeReportName({
}

if (isGroupChat(report)) {
return getGroupChatName(formatPhoneNumberPhoneUtils, undefined, true, report) ?? '';
return getGroupChatName(formatPhoneNumberPhoneUtils, undefined, true, report, pendingDeleteMemberAccountIDs) ?? '';
}

let formattedName: string | undefined;
Expand Down
26 changes: 26 additions & 0 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6326,6 +6326,31 @@ function getPendingChatMembers(accountIDs: number[], previousPendingChatMembers:
return [...previousPendingChatMembers, ...pendingChatMembers];
}

/**
* Returns account IDs whose latest pending action is DELETE.
* Uses `findLast` so that a subsequent ADD (e.g. re-invite while offline) takes precedence over an earlier DELETE.
*/
function getPendingDeleteMemberAccountIDs(pendingChatMembers: PendingChatMember[] | undefined): string[] {
if (!pendingChatMembers?.length) {
return [];
}

const seen = new Set<string>();
const result: string[] = [];
for (const member of pendingChatMembers) {
seen.add(member.accountID);
}

for (const accountID of seen) {
const latestAction = pendingChatMembers.findLast((member) => member.accountID === accountID);
if (latestAction?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) {
result.push(accountID);
}
}

return result;
}
Comment on lines +6329 to +6352

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@daledah Could you explain why do we need to handle like this? It looks more complicated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann pendingChatMembers is an append-only array - when a user is removed and then re-invited while offline, both a DELETE and a subsequent ADD entry exist for the same accountID.

Example: [{accountID: '1', pendingAction: 'delete'}, {accountID: '1', pendingAction: 'add'}] - member 1 should not be excluded from the group name because the latest action is add.


/**
* Gets the parent navigation subtitle for the report
*/
Expand Down Expand Up @@ -13917,6 +13942,7 @@ export {
getParticipantsAccountIDsForDisplay,
getParticipantsList,
getPendingChatMembers,
getPendingDeleteMemberAccountIDs,
getPersonalDetailsForAccountID,
getPolicyDescriptionText,
getPolicyExpenseChat,
Expand Down
6 changes: 6 additions & 0 deletions src/libs/actions/OnyxDerived/configs/reportAttributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
isPolicyAdmin,
isPolicyExpenseChat,
isProcessingReport,
getPendingDeleteMemberAccountIDs,
isValidReport,
} from '@libs/ReportUtils';
import SidebarUtils from '@libs/SidebarUtils';
Expand Down Expand Up @@ -231,6 +232,7 @@ export default createOnyxDerivedValueConfig({
policyTags,
conciergeReportID,
introSelected,
reportMetadata,
],
{currentValue, sourceValues},
) => {
Expand Down Expand Up @@ -521,6 +523,9 @@ export default createOnyxDerivedValueConfig({
actionTargetReportActionID = actionGreenTargetReportActionID;
}

const reportReportMetadata = reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`];
const pendingDeleteMemberAccountIDs = getPendingDeleteMemberAccountIDs(reportReportMetadata?.pendingChatMembers);

acc[report.reportID] = {
reportName: report
? computeReportName({
Expand All @@ -538,6 +543,7 @@ export default createOnyxDerivedValueConfig({
conciergeReportID: conciergeReportID ?? undefined,
reportAttributes: currentValue?.reports,
isTrackIntentUser: isTrackIntentUserSelector(introSelected),
pendingDeleteMemberAccountIDs,
})
: '',
isEmpty: generateIsEmptyReport(report, isReportArchived),
Expand Down
4 changes: 3 additions & 1 deletion src/pages/DynamicReportParticipantsInvitePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {newAccountIDsAndLoginsSelector, personalDetailsLoginsSelector} from '@sr
import type {InvitedEmailsToAccountIDs} from '@src/types/onyx';
import getEmptyArray from '@src/types/utils/getEmptyArray';

import {pendingDeleteMemberAccountIDsSelector} from '@selectors/ReportMetaData';
import React, {useEffect, useState} from 'react';

import type {WithReportOrNotFoundProps} from './inbox/report/withReportOrNotFound';
Expand All @@ -48,6 +49,7 @@ function DynamicReportParticipantsInvitePage({report}: DynamicReportParticipants
const [participantLogins = getEmptyArray<string>()] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {
selector: personalDetailsLoginsSelector(getParticipantsAccountIDsForDisplay(report, false, true)),
});
const [pendingDeleteMemberAccountIDs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report?.reportID}`, {selector: pendingDeleteMemberAccountIDsSelector});
const [didScreenTransitionEnd, setDidScreenTransitionEnd] = useState(false);
const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_PARTICIPANTS_INVITE.path);

Expand Down Expand Up @@ -115,7 +117,7 @@ function DynamicReportParticipantsInvitePage({report}: DynamicReportParticipants
toggleSelection(option);
};

const reportName = getGroupChatName(formatPhoneNumber, undefined, true, report);
const reportName = getGroupChatName(formatPhoneNumber, undefined, true, report, pendingDeleteMemberAccountIDs);

const goBack = () => {
Navigation.goBack(backPath);
Expand Down
6 changes: 5 additions & 1 deletion src/pages/GroupChatNameEditPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import {pendingDeleteMemberAccountIDsSelector} from '@src/selectors/ReportMetaData';
import INPUT_IDS from '@src/types/form/NewChatNameForm';
import type {Report as ReportOnyxType} from '@src/types/onyx';
import type {Errors} from '@src/types/onyx/OnyxCommon';
Expand All @@ -40,12 +41,15 @@ function GroupChatNameEditPage({report}: GroupChatNameEditPageProps) {
const reportID = report?.reportID;
const isUpdatingExistingReport = !!reportID;
const [groupChatDraft, groupChatDraftMetadata] = useOnyx(ONYXKEYS.NEW_GROUP_CHAT_DRAFT);
const [pendingDeleteMemberAccountIDs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report?.reportID}`, {selector: pendingDeleteMemberAccountIDsSelector});

const styles = useThemeStyles();
const {translate, formatPhoneNumber} = useLocalize();
const {inputCallbackRef} = useAutoFocusInput();

const existingReportName = report ? getGroupChatName(formatPhoneNumber, undefined, false, report) : getGroupChatName(formatPhoneNumber, groupChatDraft?.participants);
const existingReportName = report
? getGroupChatName(formatPhoneNumber, undefined, false, report, pendingDeleteMemberAccountIDs)
: getGroupChatName(formatPhoneNumber, groupChatDraft?.participants);
Comment thread
daledah marked this conversation as resolved.
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const currentChatName = reportID ? existingReportName : groupChatDraft?.reportName || existingReportName;

Expand Down
5 changes: 5 additions & 0 deletions src/selectors/ReportMetaData.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {getPendingDeleteMemberAccountIDs} from '@libs/ReportUtils';

import type {ReportLoadingState, ReportMetadata} from '@src/types/onyx';

import type {OnyxEntry} from 'react-native-onyx';
Expand All @@ -16,6 +18,8 @@ const isLoadingInitialReportActionsSelector = (loadingState: OnyxEntry<ReportLoa
const pendingChatMembersSelector = (reportMetadata: OnyxEntry<ReportMetadata>): OnyxEntry<ReportMetadata> =>
reportMetadata ? {pendingChatMembers: reportMetadata.pendingChatMembers} : undefined;

const pendingDeleteMemberAccountIDsSelector = (reportMetadata: OnyxEntry<ReportMetadata>) => getPendingDeleteMemberAccountIDs(reportMetadata?.pendingChatMembers);

const pendingNewTransactionIDsSelector = (reportMetadata: OnyxEntry<ReportMetadata>) => reportMetadata?.pendingNewTransactionIDs;

const isOptimisticReportSelector = (reportMetadata: OnyxEntry<ReportMetadata>) => reportMetadata?.isOptimisticReport;
Expand All @@ -28,4 +32,5 @@ export {
isOptimisticReportSelector,
pendingNewTransactionIDsSelector,
pendingChatMembersSelector,
pendingDeleteMemberAccountIDsSelector,
};
39 changes: 39 additions & 0 deletions tests/unit/ReportNameUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,45 @@ describe('ReportNameUtils', () => {
await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, fakePersonalDetails);
expect(getGroupChatName(formatPhoneNumber, undefined, false, report)).toEqual('Eight, Five, Four, One, Seven, Six, Three, Two');
});

it('excludes participants whose accountIDs are in pendingDeleteMemberAccountIDs', async () => {
const report: Report = {
...createRegularChat(1, [1, 2, 3, 4]),
chatType: CONST.REPORT.CHAT_TYPE.GROUP,
reportName: '',
};
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, fakePersonalDetails);

expect(getGroupChatName(formatPhoneNumber, undefined, false, report, ['2', '4'])).toEqual('One, Three');
});

it('includes all participants when pendingDeleteMemberAccountIDs is empty', async () => {
const report: Report = {
...createRegularChat(1, [1, 2, 3, 4]),
chatType: CONST.REPORT.CHAT_TYPE.GROUP,
reportName: '',
};
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, fakePersonalDetails);

expect(getGroupChatName(formatPhoneNumber, undefined, false, report, [])).toEqual('Four, One, Three, Two');
});

it('uses passed pendingDeleteMemberAccountIDs instead of falling back to report metadata', async () => {
const report: Report = {
...createRegularChat(1, [1, 2, 3, 4]),
chatType: CONST.REPORT.CHAT_TYPE.GROUP,
reportName: '',
};
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, fakePersonalDetails);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`, {
pendingChatMembers: [{accountID: '1', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}],
});

expect(getGroupChatName(formatPhoneNumber, undefined, false, report, ['3'])).toEqual('Four, One, Two');
});
});
});

Expand Down
43 changes: 43 additions & 0 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ import {
getParentNavigationSubtitle,
getParsedComment,
getParticipantsList,
getPendingDeleteMemberAccountIDs,
getPolicyChangeLogCopyMessage,
getPolicyExpenseChat,
getPolicyIDsWithEmptyReportsForAccount,
Expand Down Expand Up @@ -21276,3 +21277,45 @@ describe('areAllRequestsBeingSmartScanned', () => {
expect(areAllRequestsBeingSmartScanned(undefined, reportPreviewAction, transactions)).toBe(false);
});
});

describe('getPendingDeleteMemberAccountIDs', () => {
it('returns empty array for undefined input', () => {
expect(getPendingDeleteMemberAccountIDs(undefined)).toEqual([]);
});

it('returns empty array for empty array input', () => {
expect(getPendingDeleteMemberAccountIDs([])).toEqual([]);
});

it('returns account IDs with DELETE as the latest pending action', () => {
const pendingChatMembers = [
{accountID: '1', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE},
{accountID: '2', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD},
];

const result = getPendingDeleteMemberAccountIDs(pendingChatMembers);
expect(result).toEqual(['1']);
});

it('uses findLast so a subsequent ADD overrides an earlier DELETE for the same member', () => {
const pendingChatMembers = [
{accountID: '1', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE},
{accountID: '1', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD},
];

const result = getPendingDeleteMemberAccountIDs(pendingChatMembers);
expect(result).toEqual([]);
});

it('correctly handles mixed scenarios with multiple members', () => {
const pendingChatMembers = [
{accountID: '1', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE},
{accountID: '2', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE},
{accountID: '1', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD},
{accountID: '3', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE},
];

const result = getPendingDeleteMemberAccountIDs(pendingChatMembers);
expect(result).toEqual(['2', '3']);
});
});
1 change: 1 addition & 0 deletions tests/unit/reportAttributesTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ jest.mock('@libs/ReportUtils', () => ({
actionTargetReportActionID: undefined,
})),
generateIsEmptyReport: jest.fn(() => false),
getPendingDeleteMemberAccountIDs: jest.fn(() => []),
hasVisibleReportFieldViolations: jest.fn(() => false),
isArchivedReport: jest.fn(() => false),
isValidReport: jest.fn(() => true),
Expand Down
Loading