From 80032675eeb691117e4c685df427273e8fd49dcb Mon Sep 17 00:00:00 2001 From: Rutika Pawar <183392827+twilight2294@users.noreply.github.com> Date: Tue, 16 Jun 2026 07:20:50 +0000 Subject: [PATCH 1/4] fix invite workflow bugs --- src/components/ApprovalWorkflowSection.tsx | 70 ++++++++++--------- .../WorkspaceInviteMessageComponent.tsx | 4 +- .../approvals/ApprovalWorkflowEditor.tsx | 8 ++- ...paceWorkflowsApprovalsExpensesFromPage.tsx | 61 ++++++++++------ 4 files changed, 89 insertions(+), 54 deletions(-) diff --git a/src/components/ApprovalWorkflowSection.tsx b/src/components/ApprovalWorkflowSection.tsx index 964711ae285f..0b85b18d3208 100644 --- a/src/components/ApprovalWorkflowSection.tsx +++ b/src/components/ApprovalWorkflowSection.tsx @@ -80,6 +80,10 @@ function ApprovalWorkflowSection({ const sortedMembers = approvalWorkflow.isDefault ? [] : sortAlphabetically(approvalWorkflow.members, 'displayName', localeCompare); + // Mirror the approver row's pending opacity on the "Expenses from" row so both fade together while + // the workflow (e.g. a member invited offline) is still pending server confirmation. + const membersPendingAction = sortedMembers.find((member) => !!member.pendingFields?.submitsTo)?.pendingFields?.submitsTo; + const members = approvalWorkflow.isDefault ? translate('workspace.common.everyone') : sortedMembers.map((m) => Str.removeSMSDomain(m.displayName)).join(', '); const memberPills = sortedMembers.map((m) => ({ @@ -115,38 +119,40 @@ function ApprovalWorkflowSection({ )} - - {!isDisabled && onShowAllMembersPress ? ( - - ) : ( - - )} - - ) : undefined - } - sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.WORKFLOWS.APPROVAL_SECTION_EXPENSES_FROM} - /> + + + {!isDisabled && onShowAllMembersPress ? ( + + ) : ( + + )} + + ) : undefined + } + sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.WORKFLOWS.APPROVAL_SECTION_EXPENSES_FROM} + /> + {approvalWorkflow.approvers.map((approver, index) => ( { const firstApproverEmail = approvalWorkflow.approvers?.at(0)?.email ?? ''; - const backTo = approvalWorkflow.action === CONST.APPROVAL_WORKFLOW.ACTION.EDIT ? ROUTES.WORKSPACE_WORKFLOWS_APPROVALS_EDIT.getRoute(policyID, firstApproverEmail) : undefined; + // Always pass a backTo so that after editing expenses-from (including the invite-a-member detour), + // we return to the page we came from. For EDIT that's the edit page; for CREATE we're on the + // confirm (new) page, so return there instead of falling through to the Approver step. + const backTo = + approvalWorkflow.action === CONST.APPROVAL_WORKFLOW.ACTION.EDIT + ? ROUTES.WORKSPACE_WORKFLOWS_APPROVALS_EDIT.getRoute(policyID, firstApproverEmail) + : ROUTES.WORKSPACE_WORKFLOWS_APPROVALS_NEW.getRoute(policyID); Navigation.navigate(ROUTES.WORKSPACE_WORKFLOWS_APPROVALS_EXPENSES_FROM.getRoute(policyID, backTo)); }, [approvalWorkflow.action, approvalWorkflow.approvers, policyID]); diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx index b3c4afb8e754..5fcdaae1b22b 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx @@ -20,7 +20,9 @@ import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {WorkspaceSplitNavigatorParamList} from '@libs/Navigation/types'; import {getPersonalDetailByEmail} from '@libs/PersonalDetailsUtils'; +import {addSMSDomainIfPhoneNumber} from '@libs/PhoneNumber'; import {canEditWorkspaceSettings, getDefaultApprover, getExcludedUsers, getMemberAccountIDsForWorkspace, isPendingDeletePolicy} from '@libs/PolicyUtils'; +import type {AvatarSource} from '@libs/UserAvatarUtils'; import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper'; import MemberRightIcon from '@pages/workspace/MemberRightIcon'; import withPolicyAndFullscreenLoading from '@pages/workspace/withPolicyAndFullscreenLoading'; @@ -37,6 +39,14 @@ import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue'; type WorkspaceWorkflowsApprovalsExpensesFromPageProps = WithPolicyAndFullscreenLoadingProps & PlatformStackScreenProps; +// A user invited by phone is stored in the workflow with the raw login the admin typed, but the workspace +// keys its members by the canonical SMS login (e164@expensify.sms). Normalize before any membership or +// dedup comparison so a freshly invited phone number isn't treated as both a pending invite and a separate +// member (which made it show twice and lose its selection on back). This is a no-op for email logins. +function normalizeLogin(login: string | null | undefined): string { + return addSMSDomainIfPhoneNumber(login ?? ''); +} + function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportData = true, route}: WorkspaceWorkflowsApprovalsExpensesFromPageProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); @@ -114,15 +124,15 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat setSelectedMembers( approvalWorkflow.members .filter((member) => { - const isPolicyMember = !!policy?.employeeList?.[member.email]; + const isPolicyMember = !!policy?.employeeList?.[normalizeLogin(member.email)]; // Keep policy members. For non-policy members, only keep if they're // in the invite draft (meaning an invite flow is actively in progress). // This mirrors the card flow's handleBackButtonPress cleanup pattern. return isPolicyMember || invitedEmailsToAccountIDsDraft?.[member.email] != null; }) .map((member) => { - let accountID = Number(policyMemberEmailsToAccountIDs[member.email] ?? ''); - const isPolicyMember = !!policy?.employeeList?.[member.email]; + let accountID = Number(policyMemberEmailsToAccountIDs[normalizeLogin(member.email)] ?? ''); + const isPolicyMember = !!policy?.employeeList?.[normalizeLogin(member.email)]; const personalDetail = getPersonalDetailByEmail(member.email); @@ -154,7 +164,7 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat // Only show right element for policy members rightElement: isPolicyMember ? ( @@ -208,7 +218,9 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat }; }) .filter( - (member) => (!policy?.preventSelfApproval || !approversEmail?.includes(member.login)) && !selectedMembers.some((selectedOption) => selectedOption.login === member.login), + (member) => + (!policy?.preventSelfApproval || !approversEmail?.includes(member.login)) && + !selectedMembers.some((selectedOption) => normalizeLogin(selectedOption.login) === normalizeLogin(member.login)), ); members.push(...availableMembers); @@ -233,8 +245,8 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat // Add search results that are not already workspace members const searchResults = [...availableOptions.recentReports, ...availableOptions.personalDetails].filter((option) => { - const isMember = policy?.employeeList?.[option.login ?? '']; - const isAlreadyInList = members.some((m) => m.login === option.login); + const isMember = policy?.employeeList?.[normalizeLogin(option.login)]; + const isAlreadyInList = members.some((m) => normalizeLogin(m.login) === normalizeLogin(option.login)); return !isMember && !isAlreadyInList; }); @@ -267,21 +279,24 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat policyMemberEmailsToAccountIDs, ]); - const goBack = useCallback(() => { + // Drop any selected members who never made it into the workspace. They were staged for invite but + // never confirmed, so leaving them in approvalWorkflow.members would carry an un-invited user into + // the form and fail backend validation with "Approvals can only be set for members of the policy". + // This must run on every back path, including when an explicit backTo is honored below. + const dropUnconfirmedStagedMembers = useCallback(() => { // Going back means we're done with this expenses-from session, so any // hand-off to the invite-message page is no longer in flight. isHandingOffToInviteRef.current = false; - // Drop any selected members who never made it into the workspace. They - // were staged for invite but never confirmed, so leaving them in - // approvalWorkflow.members would carry an un-invited user into the form - // and fail backend validation with "Approvals can only be set for - // members of the policy". const stagedMembers = approvalWorkflow?.members ?? []; - const confirmedMembers = stagedMembers.filter((m) => !!policy?.employeeList?.[m.email]); + const confirmedMembers = stagedMembers.filter((m) => !!policy?.employeeList?.[normalizeLogin(m.email)]); if (confirmedMembers.length !== stagedMembers.length) { setApprovalWorkflowMembers(confirmedMembers); } + }, [approvalWorkflow?.members, policy?.employeeList]); + + const goBack = useCallback(() => { + dropUnconfirmedStagedMembers(); let backTo; if (approvalWorkflow?.action === CONST.APPROVAL_WORKFLOW.ACTION.EDIT) { @@ -292,12 +307,13 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat // Don't compare params: the edit screen may carry "Add agent" seed params, so a strict param // match would miss it and REPLACE would mount a fresh edit screen that wipes the unsaved draft. Navigation.goBack(backTo, {compareParams: false}); - }, [isInitialCreationFlow, route.params.policyID, firstApprover, approvalWorkflow?.action, approvalWorkflow?.members, policy?.employeeList]); + }, [isInitialCreationFlow, route.params.policyID, firstApprover, approvalWorkflow?.action, dropUnconfirmedStagedMembers]); // Fall back to goBack — plain Navigation.goBack() closes the modal after a refresh. const onBackButtonPress = () => { const {backTo} = route.params as WorkspaceSplitNavigatorParamList[typeof SCREENS.WORKSPACE.WORKFLOWS_APPROVALS_EXPENSES_FROM]; if (backTo) { + dropUnconfirmedStagedMembers(); Navigation.goBack(backTo); return; } @@ -306,13 +322,13 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat const nextStep = useCallback(() => { const existingMembers: Member[] = []; - const usersToInvite: Array<{email: string; accountID?: number}> = []; + const usersToInvite: Array<{email: string; accountID?: number; displayName?: string; avatar?: AvatarSource}> = []; for (const member of selectedMembers) { if (!member.login) { continue; } - const isPolicyMember = policy?.employeeList?.[member.login]; + const isPolicyMember = policy?.employeeList?.[normalizeLogin(member.login)]; if (isPolicyMember) { existingMembers.push({ displayName: member.text ?? '', @@ -323,9 +339,14 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat // This is a non-workspace member that needs to be invited const iconId = member.icons?.at(0)?.id; const accountID = typeof iconId === 'number' ? iconId : undefined; + // Carry the picker's display name and avatar so they survive the round-trip through + // approvalWorkflow.members. Without this the invited user reverts to showing their email + // and a fallback avatar after returning from the invite step or on the confirm page. usersToInvite.push({ email: member.login, accountID, + displayName: member.text, + avatar: member.icons?.at(0)?.source, }); } } @@ -340,9 +361,9 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat const allMembers: Member[] = [ ...normalizedExistingMembers, ...usersToInvite.map((user) => ({ - displayName: user.email, + displayName: user.displayName ?? user.email, email: user.email, - avatar: undefined, + avatar: typeof user.avatar === 'string' ? user.avatar : undefined, })), ]; setApprovalWorkflowMembers(allMembers); @@ -432,7 +453,7 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat email: member.login, avatar: typeof iconSource === 'string' ? iconSource : undefined, }); - if (policy?.employeeList?.[member.login]) { + if (policy?.employeeList?.[normalizeLogin(member.login)]) { continue; } const iconId = member.icons?.at(0)?.id; From 1e7e1357576c3c6174e1aa14cb2c62aa595a733e Mon Sep 17 00:00:00 2001 From: Rutika Pawar <183392827+twilight2294@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:01:08 +0530 Subject: [PATCH 2/4] Update WorkspaceWorkflowsApprovalsExpensesFromPage.tsx --- .../approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx index 5fcdaae1b22b..0abbefef792b 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx @@ -41,8 +41,8 @@ type WorkspaceWorkflowsApprovalsExpensesFromPageProps = WithPolicyAndFullscreenL // A user invited by phone is stored in the workflow with the raw login the admin typed, but the workspace // keys its members by the canonical SMS login (e164@expensify.sms). Normalize before any membership or -// dedup comparison so a freshly invited phone number isn't treated as both a pending invite and a separate -// member (which made it show twice and lose its selection on back). This is a no-op for email logins. +// duplicate comparison so a freshly invited phone number isn't treated as both a pending invite and a +// separate member (which made it show twice and lose its selection on back). This is a no-op for email logins. function normalizeLogin(login: string | null | undefined): string { return addSMSDomainIfPhoneNumber(login ?? ''); } From a6b58b67188eb0343bbd3118f5ffe49adc176ac7 Mon Sep 17 00:00:00 2001 From: Rutika Pawar <183392827+twilight2294@users.noreply.github.com> Date: Tue, 16 Jun 2026 08:15:46 +0000 Subject: [PATCH 3/4] address reviews --- ...orkspaceWorkflowsApprovalsApproverPage.tsx | 6 ++++- ...paceWorkflowsApprovalsExpensesFromPage.tsx | 23 +++++++++++-------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx index 309115f7f23c..7a05b1dc602e 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx @@ -14,6 +14,7 @@ import {isAnyHRReadOnlyWorkflowMode} from '@libs/HRUtils'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {WorkspaceSplitNavigatorParamList} from '@libs/Navigation/types'; +import {addSMSDomainIfPhoneNumber} from '@libs/PhoneNumber'; import {getDefaultApprover, getMemberAccountIDsForWorkspace, isExpensifyTeam, shouldFilterExpensifyTeam} from '@libs/PolicyUtils'; import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper'; import MemberRightIcon from '@pages/workspace/MemberRightIcon'; @@ -172,7 +173,10 @@ function WorkspaceWorkflowsApprovalsApproverPage({policy, personalDetails, isLoa return; } - const newSelectedEmail = approver?.login ?? ''; + // Canonicalize phone logins (e164@expensify.sms) so the approver email stored as submitsTo + // matches the canonical key the invite writes into employeeList. Without this, a phone approver + // invited offline stops resolving once online and the workflow disappears. No-op for emails. + const newSelectedEmail = addSMSDomainIfPhoneNumber(approver?.login ?? ''); const policyMemberEmailsToAccountIDs = getMemberAccountIDsForWorkspace(employeeList); const accountID = Number(newSelectedEmail ? policyMemberEmailsToAccountIDs[newSelectedEmail] : ''); const {avatar, displayName = newSelectedEmail} = personalDetails?.[accountID] ?? {}; diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx index 0abbefef792b..40897f7cce91 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx @@ -283,7 +283,7 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat // never confirmed, so leaving them in approvalWorkflow.members would carry an un-invited user into // the form and fail backend validation with "Approvals can only be set for members of the policy". // This must run on every back path, including when an explicit backTo is honored below. - const dropUnconfirmedStagedMembers = useCallback(() => { + const dropUnconfirmedStagedMembers = () => { // Going back means we're done with this expenses-from session, so any // hand-off to the invite-message page is no longer in flight. isHandingOffToInviteRef.current = false; @@ -293,7 +293,7 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat if (confirmedMembers.length !== stagedMembers.length) { setApprovalWorkflowMembers(confirmedMembers); } - }, [approvalWorkflow?.members, policy?.employeeList]); + }; const goBack = useCallback(() => { dropUnconfirmedStagedMembers(); @@ -328,22 +328,27 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat if (!member.login) { continue; } - const isPolicyMember = policy?.employeeList?.[normalizeLogin(member.login)]; + // Store the canonical SMS login (e164@expensify.sms) for phone members so it matches the key the + // invite writes into employeeList. If we kept the raw number, convertApprovalWorkflowToPolicyEmployees + // would index a separate, non-matching employee, so the member and submitsTo fall out of sync once + // online and the workflow gets dropped. No-op for email logins. + const memberLogin = normalizeLogin(member.login); + const isPolicyMember = policy?.employeeList?.[memberLogin]; if (isPolicyMember) { existingMembers.push({ displayName: member.text ?? '', avatar: member.icons?.at(0)?.source, - email: member.login, + email: memberLogin, }); } else { - // This is a non-workspace member that needs to be invited + // This is a non-workspace member that needs to be invited. + // Carry the picker's display name and avatar so they survive the round-trip through + // approvalWorkflow.members, otherwise the invited user reverts to their email and a + // fallback avatar after returning from the invite step or on the confirm page. const iconId = member.icons?.at(0)?.id; const accountID = typeof iconId === 'number' ? iconId : undefined; - // Carry the picker's display name and avatar so they survive the round-trip through - // approvalWorkflow.members. Without this the invited user reverts to showing their email - // and a fallback avatar after returning from the invite step or on the confirm page. usersToInvite.push({ - email: member.login, + email: memberLogin, accountID, displayName: member.text, avatar: member.icons?.at(0)?.source, From d7c5a648adfc0cc335d3e1090dd5b9b8410cac4d Mon Sep 17 00:00:00 2001 From: Rutika Pawar <183392827+twilight2294@users.noreply.github.com> Date: Tue, 16 Jun 2026 12:06:34 +0000 Subject: [PATCH 4/4] fix regression --- .../workspace/workflows/approvals/ApprovalWorkflowEditor.tsx | 5 ++++- .../WorkspaceWorkflowsApprovalsExpensesFromPage.tsx | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pages/workspace/workflows/approvals/ApprovalWorkflowEditor.tsx b/src/pages/workspace/workflows/approvals/ApprovalWorkflowEditor.tsx index f375078b68f6..aca9bdce7480 100644 --- a/src/pages/workspace/workflows/approvals/ApprovalWorkflowEditor.tsx +++ b/src/pages/workspace/workflows/approvals/ApprovalWorkflowEditor.tsx @@ -19,6 +19,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import Navigation from '@libs/Navigation/Navigation'; import {sortAlphabetically} from '@libs/OptionsListUtils'; import {isControlPolicy} from '@libs/PolicyUtils'; +import {getDefaultAvatarURL} from '@libs/UserAvatarUtils'; import {getApprovalLimitDescription} from '@libs/WorkflowUtils'; import CONST from '@src/CONST'; import ROUTES from '@src/ROUTES'; @@ -92,7 +93,9 @@ function ApprovalWorkflowEditor({approvalWorkflow, removeApprovalWorkflow, polic const memberPills = useMemo( () => sortedMembers.map((m) => ({ - avatar: m.avatar, + // A just-invited member is stored without an avatar, so fall back to the email-based default + // avatar instead of the generic fallback icon. + avatar: m.avatar ?? getDefaultAvatarURL({accountEmail: m.email}), displayName: m.displayName, email: m.email, })), diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx index 40897f7cce91..abead1019d05 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsExpensesFromPage.tsx @@ -283,7 +283,7 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat // never confirmed, so leaving them in approvalWorkflow.members would carry an un-invited user into // the form and fail backend validation with "Approvals can only be set for members of the policy". // This must run on every back path, including when an explicit backTo is honored below. - const dropUnconfirmedStagedMembers = () => { + const dropUnconfirmedStagedMembers = useCallback(() => { // Going back means we're done with this expenses-from session, so any // hand-off to the invite-message page is no longer in flight. isHandingOffToInviteRef.current = false; @@ -293,7 +293,7 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat if (confirmedMembers.length !== stagedMembers.length) { setApprovalWorkflowMembers(confirmedMembers); } - }; + }, [approvalWorkflow?.members, policy?.employeeList]); const goBack = useCallback(() => { dropUnconfirmedStagedMembers();