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
2 changes: 1 addition & 1 deletion config/eslint/eslint.seatbelt.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 14 additions & 5 deletions src/libs/actions/RequestConflictUtils.ts

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.

this file change is not related to the issue ... is it intended?

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.

yes it is, it solves messaging the invited member error

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.

sorry i am not following. What messaging error? It would be nice if you could share can video

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.

here's video @abzokhattab

Video.mp4

Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,20 @@ function resolveDuplicationConflictAction(persistedRequests: AnyRequest[], reque
function resolveOpenReportDuplicationConflictAction<TKey extends OnyxKey>(persistedRequests: Array<OnyxRequest<TKey>>, 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 {
Expand All @@ -93,10 +106,6 @@ function resolveOpenReportDuplicationConflictAction<TKey extends OnyxKey>(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: {
Expand Down
39 changes: 26 additions & 13 deletions src/pages/DynamicReportParticipantsInvitePage.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand All @@ -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;

Expand All @@ -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;

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.

With the Invite button now visible for more report types, this page is also reachable via deep link (e.g. /r/{reportID}/participants/invite) for any report — including ones where the button wouldn't be shown on the parent page. Should we add a guard here mirroring shouldShowInviteButton so deep links to disallowed report types don't land on a working invite UI?

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.

I think we should add a guard here mirroring shouldShowInviteButton cc @M00rish

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.

can you please elaborate more on this @abzokhattab, did not really get the point, thanks.

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.

the issue is that this page only checks if the report exists (via withReportOrNotFound), it doesn't check the report type. So if someone hits /r/{reportID}/participants/invite directly (deep link, bookmark, refresh, whatever) for a report type where we never wanted to allow inviting — like a chat thread or task report — they'll still get a fully working invite UI.

The button on the parent page is gated by shouldShowInviteButton, but the page itself isn't, so that's the only thing keeping users out today. We should mirror the same check here so both entry points stay consistent.

Something like:

function InviteReportParticipantsPage({report}: InviteReportParticipantsPageProps) {
    const shouldShowInviteButton = isGroupChat(report) || isMoneyRequestReport(report);
    if (!shouldShowInviteButton) {
        return <FullPageNotFoundView shouldShow />;
    }
    // ...rest of the component
}

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<string, boolean> = {
Expand Down Expand Up @@ -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);
Expand All @@ -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 <FullPageNotFoundView shouldShow />;
}

const getHeaderMessageText = () => {
if (sections.length > 0) {
return '';
Expand Down
41 changes: 32 additions & 9 deletions src/pages/DynamicReportParticipantsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -30,16 +31,19 @@ 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,
isAnnounceRoom,
isArchivedNonExpenseReport,
isChatRoom,
isChatThread,
isCurrentUserSubmitter,
isGroupChatAdmin,
isGroupChat as isGroupChatUtils,
isMoneyRequestReport,
isOpenExpenseReport,
isPolicyExpenseChat,
isSelfDM,
isTaskReport,
Expand Down Expand Up @@ -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)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep group chat controls visible to group admins

When the report is a group chat, this new isReportSubmitterOrAdmin requirement hides the entire action container for group-chat admins who are not also the report owner/workspace admin. The table still enables selection from isCurrentUserGroupChatAdmin, so those admins can select members but no longer see Remove / Make admin actions, and non-owner group chat members also lose the invite button that existed before this change. Gate the group-chat case on the group-chat permissions, or apply the submitter/policy-admin check only to open expense reports.

Useful? React with 👍 / 👎.

const isCurrentUserGroupChatAdmin = isGroupChat && isCurrentUserAdmin;
const {isOffline} = useNetwork();
const canSelectMultiple = isGroupChat && isCurrentUserAdmin && (isSmallScreenWidth ? isMobileSelectionModeEnabled : true);
Expand All @@ -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));
Expand Down Expand Up @@ -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 ?? ''),
Expand Down Expand Up @@ -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,
Expand All @@ -195,7 +212,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr

const bulkActionsButtonOptions: Array<DropdownOption<WorkspaceMemberBulkActionType>> = [
{
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,
Expand All @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -250,9 +273,9 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr
subtitle={StringUtils.lineBreaksToSpaces(getReportName(report, reportAttributes))}
/>
<View style={[styles.pl5, styles.pr5]}>

@abzokhattab abzokhattab May 12, 2026

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.

Small consistency note (relates to lines 316-318, not changed in this PR): memberNotFoundMessage still keys on isGroupChat, so the empty-search hint "use the invite button above" won't show for money request reports even though the Invite button is now visible right above. In practice this is unreachable today (the search input only appears at 12+ participants per CONST.STANDARD_LIST_ITEM_LIMIT, which money request reports rarely hit), but i think it should still be consistent with the rest of the file

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.

@M00rish can you handle it please?

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.

done

{isGroupChat && (
{shouldShowInviteButton && (
<View style={styles.w100}>
{(isSmallScreenWidth ? canSelectMultiple : selectedMembers.length > 0) ? (
{shouldShowBulkActionsDropdown ? (
<ButtonWithDropdownMenu<WorkspaceMemberBulkActionType>
variant={CONST.BUTTON_VARIANT.SUCCESS}
shouldAlwaysShowDropdownMenu
Expand All @@ -278,7 +301,7 @@ function DynamicReportParticipantsPage({report}: DynamicReportParticipantsPagePr
</View>
)}
</View>
<View style={[styles.w100, isGroupChat ? styles.mt3 : styles.mt0, styles.flex1]}>
<View style={[styles.w100, shouldShowInviteButton ? styles.mt3 : styles.mt0, styles.flex1]}>
<ReportParticipantsTable
ref={tableRef}
members={participants}
Expand Down
23 changes: 23 additions & 0 deletions src/pages/InviteReportParticipantsPageUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type {InvitedEmailsToAccountIDs} from '@src/types/onyx';

type InviteOption = {
accountID?: number;
login?: string;
};

function getInvitedEmailsToAccountIDs(selectedOptions: InviteOption[]): InvitedEmailsToAccountIDs {
const invitedEmailsToAccountIDs: InvitedEmailsToAccountIDs = {};

for (const option of selectedOptions) {
const login = option.login ?? '';
const accountID = option.accountID;
if (!login.trim() || accountID === undefined) {
continue;
}
invitedEmailsToAccountIDs[login] = accountID;
}

return invitedEmailsToAccountIDs;
}

export default getInvitedEmailsToAccountIDs;
40 changes: 40 additions & 0 deletions tests/unit/InviteReportParticipantsPageTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import getInvitedEmailsToAccountIDs from '@pages/InviteReportParticipantsPageUtils';

describe('InviteReportParticipantsPage', () => {
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({});
});
});
});
Loading
Loading