diff --git a/config/eslint/eslint.seatbelt.tsv b/config/eslint/eslint.seatbelt.tsv index 7fc787551034..478e43c03a72 100644 --- a/config/eslint/eslint.seatbelt.tsv +++ b/config/eslint/eslint.seatbelt.tsv @@ -2106,7 +2106,7 @@ "../../tests/unit/ReportUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 19 "../../tests/unit/ReportUtilsTest.ts" "no-restricted-imports" 1 "../../tests/unit/ReportWelcomeTextTest.tsx" "@typescript-eslint/no-unsafe-type-assertion" 1 -"../../tests/unit/RequestConflictUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 11 +"../../tests/unit/RequestConflictUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 13 "../../tests/unit/RequestTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../tests/unit/RequireA11yDisableJustificationRuleTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../tests/unit/RequireLiveRegionForStatusUpdatesRuleTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 1 diff --git a/src/libs/actions/RequestConflictUtils.ts b/src/libs/actions/RequestConflictUtils.ts index 681b5d7c836b..69ba37179d64 100644 --- a/src/libs/actions/RequestConflictUtils.ts +++ b/src/libs/actions/RequestConflictUtils.ts @@ -78,7 +78,20 @@ function resolveDuplicationConflictAction(persistedRequests: AnyRequest[], reque function resolveOpenReportDuplicationConflictAction(persistedRequests: Array>, parameters: OpenReportParams): ConflictActionData { for (let index = 0; index < persistedRequests.length; index++) { const request = persistedRequests.at(index); - if (request?.command === WRITE_COMMANDS.OPEN_REPORT && request.data?.reportID === parameters.reportID && request.data?.emailList === parameters.emailList) { + + // Skip irrelevant requests immediately + if (request?.command !== WRITE_COMMANDS.OPEN_REPORT || request.data?.reportID !== parameters.reportID) { + continue; + } + + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + const queuedHasParticipants = !!(request.data?.emailList || request.data?.accountIDList); + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + const newHasParticipants = !!(parameters.emailList || parameters.accountIDList); + + const isExactParticipantMatch = (request.data?.emailList ?? '') === (parameters.emailList ?? '') && (request.data?.accountIDList ?? '') === (parameters.accountIDList ?? ''); + + if (isExactParticipantMatch || (!queuedHasParticipants && newHasParticipants) || (queuedHasParticipants && !newHasParticipants)) { // If the previous request had guided setup data, we can safely ignore the new request if (request.data.guidedSetupData) { return { @@ -93,10 +106,6 @@ function resolveOpenReportDuplicationConflictAction(persis // ReportFetchHandler when the screen mounts has no participants. Replacing would drop the // accountIDList, leaving the server with no way to resolve the optimistic reportID — Auth // returns NIL reportSummary and PHP throws "Report not found" (da7984df). - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const queuedHasParticipants = !!(request.data?.emailList || request.data?.accountIDList); - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const newHasParticipants = !!(parameters.emailList || parameters.accountIDList); if (queuedHasParticipants && !newHasParticipants) { return { conflictAction: { diff --git a/src/pages/DynamicReportParticipantsInvitePage.tsx b/src/pages/DynamicReportParticipantsInvitePage.tsx index c5b14f2db470..94b0ebf6f20f 100644 --- a/src/pages/DynamicReportParticipantsInvitePage.tsx +++ b/src/pages/DynamicReportParticipantsInvitePage.tsx @@ -1,3 +1,4 @@ +import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView'; import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; import ScreenWrapper from '@components/ScreenWrapper'; @@ -11,6 +12,7 @@ import useDynamicBackPath from '@hooks/useDynamicBackPath'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import usePersonalDetailSearchSelector from '@hooks/usePersonalDetailSearchSelector'; +import usePolicy from '@hooks/usePolicy'; import useThemeStyles from '@hooks/useThemeStyles'; import {inviteToGroupChat, searchUserInServer} from '@libs/actions/Report'; @@ -22,19 +24,21 @@ import {getHeaderMessage} from '@libs/PersonalDetailOptionsListUtils'; import type {OptionData} from '@libs/PersonalDetailOptionsListUtils'; import {getLoginsByAccountIDs} from '@libs/PersonalDetailsUtils'; import {addSMSDomainIfPhoneNumber, parsePhoneNumber} from '@libs/PhoneNumber'; -import {getGroupChatName} from '@libs/ReportNameUtils'; -import {getParticipantsAccountIDsForDisplay} from '@libs/ReportUtils'; +import {getReportName} from '@libs/ReportNameUtils'; +import {getParticipantsAccountIDsForDisplay, isCurrentUserSubmitter, isGroupChat, isGroupChatAdmin, isMoneyRequestReport, isOpenExpenseReport, isPolicyAdmin} from '@libs/ReportUtils'; +import StringUtils from '@libs/StringUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import {DYNAMIC_ROUTES} from '@src/ROUTES'; -import type {InvitedEmailsToAccountIDs} from '@src/types/onyx'; -import React, {useEffect, useState} from 'react'; +import reportByIDsSelector from '@selectors/ReportAttributes'; +import React, {useEffect, useMemo, useState} from 'react'; import type {WithReportOrNotFoundProps} from './inbox/report/withReportOrNotFound'; import withReportOrNotFound from './inbox/report/withReportOrNotFound'; +import getInvitedEmailsToAccountIDs from './InviteReportParticipantsPageUtils'; type DynamicReportParticipantsInvitePageProps = WithReportOrNotFoundProps & WithNavigationTransitionEndProps; @@ -46,7 +50,17 @@ function DynamicReportParticipantsInvitePage({report}: DynamicReportParticipants const [countryCode = CONST.DEFAULT_COUNTRY_CODE] = useOnyx(ONYXKEYS.COUNTRY_CODE); const [personalDetailsList] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); const [didScreenTransitionEnd, setDidScreenTransitionEnd] = useState(false); + const reportID = report.reportID; + const reportAttributesSelector = useMemo(() => reportByIDsSelector([reportID]), [reportID]); + const [reportAttributes] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, { + selector: reportAttributesSelector, + }); const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_PARTICIPANTS_INVITE.path); + const [session] = useOnyx(ONYXKEYS.SESSION); + const currentUserAccountID = Number(session?.accountID); + const policy = usePolicy(report.policyID); + const isReportSubmitterOrAdmin = isCurrentUserSubmitter(report) || isPolicyAdmin(policy) || (isGroupChat(report) && isGroupChatAdmin(report, currentUserAccountID)); + const shouldShowInvitePage = isReportSubmitterOrAdmin && (isGroupChat(report) || (isMoneyRequestReport(report) && isOpenExpenseReport(report))); // Any existing participants and Expensify emails should not be eligible for invitation const excludedUsers: Record = { @@ -114,7 +128,7 @@ function DynamicReportParticipantsInvitePage({report}: DynamicReportParticipants toggleSelection(option); }; - const reportName = getGroupChatName(formatPhoneNumber, undefined, true, report); + const reportName = StringUtils.lineBreaksToSpaces(getReportName(report, reportAttributes)); const goBack = () => { Navigation.goBack(backPath); @@ -124,19 +138,18 @@ function DynamicReportParticipantsInvitePage({report}: DynamicReportParticipants if (selectedOptions.length === 0) { return; } - const invitedEmailsToAccountIDs: InvitedEmailsToAccountIDs = {}; - for (const option of selectedOptions) { - const login = option.login ?? ''; - const accountID = option.accountID; - if (!login.toLowerCase().trim() || !accountID) { - continue; - } - invitedEmailsToAccountIDs[login] = accountID; + const invitedEmailsToAccountIDs = getInvitedEmailsToAccountIDs(selectedOptions); + if (Object.keys(invitedEmailsToAccountIDs).length === 0) { + return; } inviteToGroupChat(report, invitedEmailsToAccountIDs, personalDetailsList, formatPhoneNumber); goBack(); }; + if (!shouldShowInvitePage) { + return ; + } + const getHeaderMessageText = () => { if (sections.length > 0) { return ''; diff --git a/src/pages/DynamicReportParticipantsPage.tsx b/src/pages/DynamicReportParticipantsPage.tsx index 2c5f1ddc64d8..5a7e56130213 100755 --- a/src/pages/DynamicReportParticipantsPage.tsx +++ b/src/pages/DynamicReportParticipantsPage.tsx @@ -17,6 +17,7 @@ import useLocalize from '@hooks/useLocalize'; import useMobileSelectionMode from '@hooks/useMobileSelectionMode'; import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; +import usePolicy from '@hooks/usePolicy'; import useReportAttributes from '@hooks/useReportAttributes'; import useReportIsArchived from '@hooks/useReportIsArchived'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; @@ -30,6 +31,7 @@ import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {ParticipantsNavigatorParamList} from '@libs/Navigation/types'; import {temporaryGetDisplayNameOrDefault} from '@libs/PersonalDetailsUtils'; +import {isPolicyAdmin} from '@libs/PolicyUtils'; import {getReportName} from '@libs/ReportNameUtils'; import { getReportPersonalDetailsParticipants, @@ -37,9 +39,11 @@ import { isArchivedNonExpenseReport, isChatRoom, isChatThread, + isCurrentUserSubmitter, isGroupChatAdmin, isGroupChat as isGroupChatUtils, isMoneyRequestReport, + isOpenExpenseReport, isPolicyExpenseChat, isSelfDM, isTaskReport, @@ -86,6 +90,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr const currentUserAccountID = Number(session?.accountID); const isCurrentUserAdmin = isGroupChatAdmin(report, currentUserAccountID); const isGroupChat = isGroupChatUtils(report); + const policy = usePolicy(report.policyID); + const isReportSubmitterOrAdmin = isCurrentUserSubmitter(report) || isPolicyAdmin(policy) || (isGroupChat && isCurrentUserAdmin); + const shouldShowInviteButton = isReportSubmitterOrAdmin && (isGroupChat || (isMoneyRequestReport(report) && isOpenExpenseReport(report))); const isCurrentUserGroupChatAdmin = isGroupChat && isCurrentUserAdmin; const {isOffline} = useNetwork(); const canSelectMultiple = isGroupChat && isCurrentUserAdmin && (isSmallScreenWidth ? isMobileSelectionModeEnabled : true); @@ -104,8 +111,11 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr }; const [selectedMembers, setSelectedMembers] = useFilteredSelection(personalDetailsParticipants, filterParticipants); + const shouldShowBulkActionsDropdown = isGroupChat && (isSmallScreenWidth ? canSelectMultiple : selectedMembers.length > 0); const firstSelectedMember = selectedMembers?.at(0); - const [firstSelectedMemberDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: personalDetailsSelector(firstSelectedMember)}); + const [firstSelectedMemberDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, { + selector: personalDetailsSelector(firstSelectedMember), + }); // The Table stores selection as string keys, while this page tracks accountIDs as numbers. const onRowSelectionChange = (keys: string[]) => setSelectedMembers(keys.map(Number)); @@ -141,7 +151,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr const showRemoveMembersModal = async () => { const {action} = await showConfirmModal({ - title: translate('workspace.people.removeMembersTitle', {count: selectedMembers.length}), + title: translate('workspace.people.removeMembersTitle', { + count: selectedMembers.length, + }), prompt: translate('workspace.people.removeMembersPrompt', { count: selectedMembers.length, memberName: formatPhoneNumber(firstSelectedMemberDetails?.displayName ?? ''), @@ -177,7 +189,12 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr keyForList: `${accountID}`, accountID, login: details?.login ?? '', - name: formatPhoneNumber(temporaryGetDisplayNameOrDefault({passedPersonalDetails: details, translate})), + name: formatPhoneNumber( + temporaryGetDisplayNameOrDefault({ + passedPersonalDetails: details, + translate, + }), + ), email: formatPhoneNumber(details?.login ?? ''), isAdmin: role === CONST.REPORT.ROLE.ADMIN, isGroupChat, @@ -195,7 +212,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr const bulkActionsButtonOptions: Array> = [ { - text: translate('workspace.people.removeMembersTitle', {count: selectedMembers.length}), + text: translate('workspace.people.removeMembersTitle', { + count: selectedMembers.length, + }), value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.REMOVE, icon: icons.RemoveMembers, onSelected: showRemoveMembersModal, @@ -204,7 +223,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr if (isAtLeastOneAdminSelected) { bulkActionsButtonOptions.push({ - text: translate('workspace.people.makeMember', {count: selectedMembers.length}), + text: translate('workspace.people.makeMember', { + count: selectedMembers.length, + }), value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.MAKE_MEMBER, icon: icons.User, onSelected: () => changeUserRole(CONST.REPORT.ROLE.MEMBER), @@ -213,7 +234,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr if (isAtLeastOneMemberSelected) { bulkActionsButtonOptions.push({ - text: translate('workspace.people.makeGroupAdmin', {count: selectedMembers.length}), + text: translate('workspace.people.makeGroupAdmin', { + count: selectedMembers.length, + }), value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.MAKE_ADMIN, icon: icons.MakeAdmin, onSelected: () => changeUserRole(CONST.REPORT.ROLE.ADMIN), @@ -250,9 +273,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr subtitle={StringUtils.lineBreaksToSpaces(getReportName(report, reportAttributes))} /> - {isGroupChat && ( + {shouldShowInviteButton && ( - {(isSmallScreenWidth ? canSelectMultiple : selectedMembers.length > 0) ? ( + {shouldShowBulkActionsDropdown ? ( variant={CONST.BUTTON_VARIANT.SUCCESS} shouldAlwaysShowDropdownMenu @@ -278,7 +301,7 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr )} - + { + describe('getInvitedEmailsToAccountIDs', () => { + it('preserves optimistic account IDs for new invitees', () => { + // Given a typed invitee that does not have a real account yet + const login = 'new-user@example.com'; + const selectedOptions = [ + { + login, + accountID: 0, + }, + ]; + + // When building the invite map + const invitedEmailsToAccountIDs = getInvitedEmailsToAccountIDs(selectedOptions); + + // Then the optimistic accountID is preserved for the API layer + expect(Object.entries(invitedEmailsToAccountIDs)).toStrictEqual([[login, 0]]); + }); + + it('skips malformed selected options', () => { + // Given selected options that cannot be invited + const selectedOptions = [ + { + accountID: 123, + }, + { + login: 'missing-account-id@example.com', + }, + ]; + + // When building the invite map + const invitedEmailsToAccountIDs = getInvitedEmailsToAccountIDs(selectedOptions); + + // Then no invalid invitee is sent to the API layer + expect(invitedEmailsToAccountIDs).toStrictEqual({}); + }); + }); +}); diff --git a/tests/unit/RequestConflictUtilsTest.ts b/tests/unit/RequestConflictUtilsTest.ts index d3c809fa826f..6daf31a5b8e6 100644 --- a/tests/unit/RequestConflictUtilsTest.ts +++ b/tests/unit/RequestConflictUtilsTest.ts @@ -47,6 +47,34 @@ describe('RequestConflictUtils', () => { expect(result).toEqual({conflictAction: {type: 'replace', index: 2}}); }); + it.each([ + ['push when accountIDList differs', {emailList: '', accountIDList: '483'}, {emailList: '', accountIDList: ''}, {type: 'noAction'}], + ['replace when accountIDList matches', {emailList: '', accountIDList: '483'}, {emailList: '', accountIDList: '483'}, {type: 'replace', index: 0}], + ['replace when optional participant lists are empty', {}, {emailList: '', accountIDList: ''}, {type: 'replace', index: 0}], + ])('resolveOpenReportDuplicationConflictAction should %s', (_description, queuedData, nextData, expectedConflictAction) => { + const reportID = '8071514414018373'; + + // Given an existing OpenReport request + const persistedRequests = [ + { + command: WRITE_COMMANDS.OPEN_REPORT, + data: { + reportID, + ...queuedData, + }, + }, + ]; + + // When another OpenReport is queued for the same report + const result = resolveOpenReportDuplicationConflictAction(persistedRequests, { + reportID, + ...nextData, + }); + + // Then the request should only replace when participant identifiers match + expect(result).toEqual({conflictAction: expectedConflictAction}); + }); + it('resolveCommentDeletionConflicts should return push when no special comments are found', () => { const persistedRequests = [{command: 'OpenReport'}, {command: 'AddComment', data: {reportActionID: 2}}, {command: 'CloseAccount'}]; const reportActionID = '1'; @@ -204,6 +232,18 @@ describe('RequestConflictUtils', () => { const result = resolveOpenReportDuplicationConflictAction(persistedRequests, {reportID: '1', accountIDList: '10,20'} as never); expect(result).toEqual({conflictAction: {type: 'replace', index: 0}}); }); + + it('should return noAction when queued request has participants but new follow-up request has empty participant strings', () => { + const persistedRequests = [{command: WRITE_COMMANDS.OPEN_REPORT, data: {reportID: '1', accountIDList: '483'}}]; + const result = resolveOpenReportDuplicationConflictAction(persistedRequests, {reportID: '1', emailList: '', accountIDList: ''} as never); + expect(result).toEqual({conflictAction: {type: 'noAction'}}); + }); + + it('should push when accountIDList genuinely differs (distinct active lists)', () => { + const persistedRequests = [{command: WRITE_COMMANDS.OPEN_REPORT, data: {reportID: '1', accountIDList: '483'}}]; + const result = resolveOpenReportDuplicationConflictAction(persistedRequests, {reportID: '1', accountIDList: '484'} as never); + expect(result).toEqual({conflictAction: {type: 'push'}}); + }); }); describe('resolveDetachReceiptConflicts', () => {