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) => ( 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, })), @@ -133,7 +136,13 @@ function ApprovalWorkflowEditor({approvalWorkflow, removeApprovalWorkflow, polic const handleExpensesFromPress = useCallback(() => { 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/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 b3c4afb8e754..abead1019d05 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 +// 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 ?? ''); +} + 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,26 +322,36 @@ 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]; + // 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; usersToInvite.push({ - email: member.login, + email: memberLogin, accountID, + displayName: member.text, + avatar: member.icons?.at(0)?.source, }); } } @@ -340,9 +366,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 +458,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;