Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
981b9b0
fix approve button not shown because of invisible brokenCardConnecti…
rayane-d Nov 10, 2025
c198f7b
fix empty report next step shown because of invisible brokenCardConn…
rayane-d Nov 10, 2025
5bb1c92
Just check if the violation has been dismissed by anyone If we don't …
rayane-d Nov 11, 2025
e3d73cb
Fix empty report next step message when the broken connection next st…
rayane-d Nov 11, 2025
b03bdb1
revert a change
rayane-d Nov 11, 2025
b5bf0ca
fix empty report next step when the BrokenConnection violation is dis…
rayane-d Nov 11, 2025
2f7c119
run prettier
rayane-d Nov 11, 2025
7cc00cc
add automated tests for isViolationDismissed
rayane-d Nov 11, 2025
4871b1f
pass current user email to isViolationDismissed in isDuplicate. becau…
rayane-d Nov 11, 2025
6924d6b
fix failing test
rayane-d Nov 11, 2025
9cc99ea
fix type errors
rayane-d Nov 11, 2025
2948ca9
Merge branch 'main' into fix-approve-button-not-shown-because-of-brok…
rayane-d Nov 11, 2025
3da5117
remove deprecatedCurrentUserEmail
rayane-d Nov 11, 2025
591fa96
Merge branch 'Expensify:main' into fix-approve-button-not-shown-becau…
rayane-d Nov 12, 2025
ea0371a
Merge branch 'Expensify:main' into fix-approve-button-not-shown-becau…
rayane-d Nov 12, 2025
114ab85
Merge branch 'main' into fix-approve-button-not-shown-because-of-brok…
rayane-d Nov 17, 2025
de6486d
Address review comments
rayane-d Nov 17, 2025
9b6a154
add comment
rayane-d Nov 17, 2025
1f79c59
clarify comments
rayane-d Nov 18, 2025
271273f
fix hasDuplicateTransactions references
rayane-d Nov 18, 2025
6ebfa13
prettier
rayane-d Nov 18, 2025
d9b1080
Merge branch 'Expensify:main' into fix-approve-button-not-shown-becau…
rayane-d Nov 18, 2025
e515eb6
Fix "Mark as cash" button displayed for invisible violation
rayane-d Nov 18, 2025
8c4fbf0
prettier
rayane-d Nov 18, 2025
0f55707
Apply suggestions from code review
rayane-d Nov 18, 2025
79a7c94
add more tests
rayane-d Nov 18, 2025
716050e
- pass reportTransactions into hasVisibleViolationsForUser
rayane-d Nov 19, 2025
b61de33
fix failing test
rayane-d Nov 19, 2025
e38bc13
Merge branch 'main' into fix-approve-button-not-shown-because-of-brok…
rayane-d Nov 19, 2025
9370cb1
fix merge conflicts
rayane-d Nov 19, 2025
bfcaeaa
Merge branch 'Expensify:main' into fix-approve-button-not-shown-becau…
rayane-d Nov 19, 2025
dc230ad
Merge branch 'Expensify:main' into fix-approve-button-not-shown-becau…
rayane-d Nov 19, 2025
82cfbd2
Merge branch 'main' into fix-approve-button-not-shown-because-of-brok…
rayane-d Nov 20, 2025
2719db5
fix lint-changed errors
rayane-d Nov 21, 2025
394a446
Merge branch 'main' into fix-approve-button-not-shown-because-of-brok…
rayane-d Nov 21, 2025
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
17 changes: 11 additions & 6 deletions src/components/MoneyReportHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,12 @@ function MoneyReportHeader({

// Check if there is pending rter violation in all transactionViolations with given transactionIDs.
// wrapped in useMemo to avoid unnecessary re-renders and for better performance (array operation inside of function)
const hasAllPendingRTERViolations = useMemo(() => allHavePendingRTERViolation(transactions, violations), [transactions, violations]);
const hasAllPendingRTERViolations = useMemo(
() => allHavePendingRTERViolation(transactions, violations, email ?? '', moneyRequestReport, policy),
[transactions, violations, email, moneyRequestReport, policy],
Comment thread
rayane-d marked this conversation as resolved.
Comment thread
rayane-d marked this conversation as resolved.
);
// Check if user should see broken connection violation warning.
const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, moneyRequestReport, policy, violations);
const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactions, moneyRequestReport, policy, violations, email ?? '');
const hasOnlyHeldExpenses = hasOnlyHeldExpensesReportUtils(moneyRequestReport?.reportID);
const isPayAtEndExpense = isPayAtEndExpenseTransactionUtils(transaction);
const isArchivedReport = useReportIsArchived(moneyRequestReport?.reportID);
Expand Down Expand Up @@ -415,7 +418,7 @@ function MoneyReportHeader({

const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE;

const hasDuplicates = hasDuplicateTransactions(moneyRequestReport?.reportID);
const hasDuplicates = hasDuplicateTransactions(email ?? '', moneyRequestReport, policy);
const shouldShowMarkAsResolved = isMarkAsResolvedAction(moneyRequestReport, transactionViolations);
const shouldShowStatusBar =
hasAllPendingRTERViolations ||
Expand Down Expand Up @@ -575,7 +578,9 @@ function MoneyReportHeader({
return {icon: getStatusIcon(expensifyIcons.Flag), description: translate('iou.duplicateTransaction', {isSubmitted: isProcessingReport(moneyRequestReport)})};
}

if (!!transaction?.transactionID && shouldShowBrokenConnectionViolation) {
// Show the broken connection violation message only if it's part of transactionViolations (i.e., visible to the user).
// This prevents displaying an empty message.
if (!!transaction?.transactionID && !!transactionViolations.length && shouldShowBrokenConnectionViolation) {
return {
icon: getStatusIcon(expensifyIcons.Hourglass),
description: (
Expand All @@ -599,7 +604,7 @@ function MoneyReportHeader({
};

const getFirstDuplicateThreadID = (transactionsList: OnyxTypes.Transaction[], allReportActions: OnyxTypes.ReportAction[]) => {
const duplicateTransaction = transactionsList.find((reportTransaction) => isDuplicate(reportTransaction));
const duplicateTransaction = transactionsList.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '', moneyRequestReport, policy));
if (!duplicateTransaction) {
return null;
}
Expand Down Expand Up @@ -890,7 +895,7 @@ function MoneyReportHeader({
onPress={() => {
let threadID = transactionThreadReportID ?? getFirstDuplicateThreadID(transactions, reportActions);
if (!threadID) {
const duplicateTransaction = transactions.find((reportTransaction) => isDuplicate(reportTransaction));
const duplicateTransaction = transactions.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '', moneyRequestReport, policy));
const transactionID = duplicateTransaction?.transactionID;
const iouAction = getIOUActionForReportID(moneyRequestReport?.reportID, transactionID);
const createdTransactionThreadReport = createTransactionThreadReport(moneyRequestReport, iouAction);
Expand Down
6 changes: 3 additions & 3 deletions src/components/MoneyRequestHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
const styles = useThemeStyles();
const theme = useTheme();
const {translate} = useLocalize();
const {login: currentUserLogin} = useCurrentUserPersonalDetails();
const {login: currentUserLogin, email} = useCurrentUserPersonalDetails();
const isOnHold = isOnHoldTransactionUtils(transaction);
const isDuplicate = isDuplicateTransactionUtils(transaction);
const isDuplicate = isDuplicateTransactionUtils(transaction, email ?? '', report, policy);
const reportID = report?.reportID;
const {removeTransaction, currentSearchHash} = useSearchContext();
const {isExpenseSplit} = getOriginalTransactionWithSplitInfo(transaction, originalTransaction);
Expand Down Expand Up @@ -183,7 +183,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
if (isExpensifyCardTransaction(transaction) && isPending(transaction)) {
return {icon: getStatusIcon(expensifyIcons.CreditCardHourglass), description: translate('iou.transactionPendingDescription')};
}
if (shouldShowBrokenConnectionViolation) {
if (!!transaction?.transactionID && !!transactionViolations.length && shouldShowBrokenConnectionViolation) {
return {
icon: getStatusIcon(expensifyIcons.Hourglass),
description: (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ function TransactionGroupListExpanded<TItem extends ListItem>({
<TransactionItemRow
report={transaction.report}
transactionItem={transaction}
violations={getTransactionViolations(transaction, violations, currentUserDetails.email ?? '')}
violations={getTransactionViolations(transaction, violations, currentUserDetails.email ?? '', transaction.report, transaction.policy)}
isSelected={!!transaction.isSelected}
dateColumnSize={dateColumnSize}
amountColumnSize={amountColumnSize}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function TransactionListItem<TItem extends ListItem>({
const transactionViolations = useMemo(() => {
return (violations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionItem.transactionID}`] ?? []).filter(
(violation: TransactionViolation) =>
!isViolationDismissed(transactionItem, violation, currentUserDetails.email ?? '') &&
!isViolationDismissed(transactionItem, violation, currentUserDetails.email ?? '', snapshotReport, snapshotPolicy) &&
shouldShowViolation(snapshotReport, snapshotPolicy, violation.name, currentUserDetails.email ?? '', false),
);
Comment thread
rayane-d marked this conversation as resolved.
}, [snapshotPolicy, snapshotReport, transactionItem, violations, currentUserDetails.email]);
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useTransactionViolations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function useTransactionViolations(transactionID?: string, shouldShowRterForSettl
mergeProhibitedViolations(
transactionViolations.filter(
(violation: TransactionViolation) =>
!isViolationDismissed(transaction, violation, currentUserDetails.email ?? '') &&
!isViolationDismissed(transaction, violation, currentUserDetails.email ?? '', iouReport, policy) &&
shouldShowViolation(iouReport, policy, violation.name, currentUserDetails.email ?? '', shouldShowRterForSettledReport),
),
),
Expand Down
7 changes: 5 additions & 2 deletions src/hooks/useTransactionsAndViolationsForReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import ONYXKEYS from '@src/ONYXKEYS';
import type {TransactionViolations} from '@src/types/onyx';
import type {ReportTransactionsAndViolations} from '@src/types/onyx/DerivedValues';
import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails';
import useOnyx from './useOnyx';

const DEFAULT_RETURN_VALUE: ReportTransactionsAndViolations = {transactions: {}, violations: {}};

function useTransactionsAndViolationsForReport(reportID?: string) {
const allReportsTransactionsAndViolations = useAllReportsTransactionsAndViolations();
const currentUserDetails = useCurrentUserPersonalDetails();
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {canBeMissing: true});
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`, {canBeMissing: true});

const {transactions, violations} = reportID ? (allReportsTransactionsAndViolations?.[reportID] ?? DEFAULT_RETURN_VALUE) : DEFAULT_RETURN_VALUE;

Expand All @@ -22,14 +25,14 @@ function useTransactionsAndViolationsForReport(reportID?: string) {

// This is our accumulator, it's okay to reassign
// eslint-disable-next-line no-param-reassign
filteredTransactionViolations[transactionViolationKey] = getTransactionViolations(transaction, violations, currentUserDetails.email ?? '') ?? [];
filteredTransactionViolations[transactionViolationKey] = getTransactionViolations(transaction, violations, currentUserDetails.email ?? '', report, policy) ?? [];
return filteredTransactionViolations;
},
{} as Record<string, TransactionViolations>,
);
Comment thread
rayane-d marked this conversation as resolved.
Comment thread
rayane-d marked this conversation as resolved.

return {transactions, violations: filteredViolations};
}, [transactions, violations, currentUserDetails?.email]);
}, [transactions, violations, currentUserDetails?.email, report, policy]);

return transactionsAndViolations;
}
Expand Down
53 changes: 35 additions & 18 deletions src/libs/ReportPreviewActionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function canSubmit(report: Report, violations: OnyxCollection<TransactionViolati
}

const hasAnyViolations = hasMissingSmartscanFields(report.reportID, transactions) || hasAnyViolationsUtil(report.reportID, violations);

const isAnyReceiptBeingScanned = transactions?.some((transaction) => isScanning(transaction));

const submitToAccountID = getSubmitToAccountID(policy, report);
Expand All @@ -56,22 +57,32 @@ function canSubmit(report: Report, violations: OnyxCollection<TransactionViolati
return isExpense && (isSubmitter || isManager || isAdmin) && isOpen && !hasAnyViolations && !isAnyReceiptBeingScanned && !!transactions && transactions.length > 0;
}

function canApprove(report: Report, violations: OnyxCollection<TransactionViolation[]>, policy?: Policy, transactions?: Transaction[], shouldConsiderViolations = true) {
function canApprove(
report: Report,
violations: OnyxCollection<TransactionViolation[]>,
currentUserEmail: string,
policy: Policy | undefined,
transactions: Transaction[],
shouldConsiderViolations = true,
) {
const currentUserID = getCurrentUserAccountID();
const isExpense = isExpenseReport(report);
const isProcessing = isProcessingReport(report);
const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false;
const managerID = report?.managerID ?? CONST.DEFAULT_NUMBER_ID;
const isApprovalEnabled = policy?.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL;
const managerID = report.managerID ?? CONST.DEFAULT_NUMBER_ID;
const isCurrentUserManager = managerID === currentUserID;
const hasAnyViolations = hasMissingSmartscanFields(report.reportID, transactions) || hasAnyViolationsUtil(report.reportID, violations);
const reportTransactions = transactions ?? getReportTransactions(report?.reportID);
const isAnyReceiptBeingScanned = transactions?.some((transaction) => isScanning(transaction));

// We should consider only visible violations for the approver, invisible violations should not block approval
const reportTransactions = transactions.length ? transactions : getReportTransactions(report?.reportID);
const hasAnyVisibleViolations =
hasMissingSmartscanFields(report.reportID, reportTransactions) || ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, reportTransactions);
const isAnyReceiptBeingScanned = reportTransactions?.some((transaction) => isScanning(transaction));

if (isAnyReceiptBeingScanned) {
return false;
}

if (!!transactions && transactions?.length > 0 && transactions.every((transaction) => isPending(transaction))) {
if (reportTransactions.length > 0 && reportTransactions.every((transaction) => isPending(transaction))) {
return false;
}

Expand All @@ -82,7 +93,7 @@ function canApprove(report: Report, violations: OnyxCollection<TransactionViolat
return false;
}

return isExpense && isProcessing && !!isApprovalEnabled && (!hasAnyViolations || !shouldConsiderViolations) && reportTransactions.length > 0 && isCurrentUserManager;
return isExpense && isProcessing && !!isApprovalEnabled && (!hasAnyVisibleViolations || !shouldConsiderViolations) && reportTransactions.length > 0 && isCurrentUserManager;
}

function canPay(
Expand Down Expand Up @@ -170,7 +181,14 @@ function canExport(report: Report, violations: OnyxCollection<TransactionViolati
return (isApproved || isReimbursed || isClosed) && !hasAnyViolations;
}

function canReview(report: Report, violations: OnyxCollection<TransactionViolation[]>, isReportArchived: boolean, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) {
function canReview(
report: Report,
violations: OnyxCollection<TransactionViolation[]>,
isReportArchived: boolean,
currentUserEmail: string,
policy: Policy | undefined,
transactions: Transaction[],
) {
const hasAnyViolations = hasMissingSmartscanFields(report.reportID, transactions) || hasAnyViolationsUtil(report.reportID, violations);
const hasVisibleViolations = hasAnyViolations && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, transactions);
const isSubmitter = isCurrentUserSubmitter(report);
Expand All @@ -181,17 +199,16 @@ function canReview(report: Report, violations: OnyxCollection<TransactionViolati
!hasVisibleViolations ||
isReimbursed ||
(!(isSubmitter && isOpen && policy?.areWorkflowsEnabled) &&
!canApprove(report, violations, policy, transactions, false) &&
!canApprove(report, violations, currentUserEmail, policy, transactions, false) &&
!canPay(report, violations, isReportArchived, policy, policy, false))
) {
return false;
}

// We handle RTER violations independently because those are not configured via policy workflows
const isAdmin = isPolicyAdmin(policy);
const transactionIDs = transactions?.map((transaction) => transaction.transactionID) ?? [];
const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactions, violations);
const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, report, policy, violations);
const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactions, violations, currentUserEmail, report, policy);
const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactions, report, policy, violations, currentUserEmail);

if (hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!isAdmin || isSubmitter) && !isReportApproved({report}) && !isReportManuallyReimbursed(report))) {
return true;
Expand All @@ -209,9 +226,9 @@ function getReportPreviewAction(
violations: OnyxCollection<TransactionViolation[]>,
isReportArchived: boolean,
currentUserEmail: string,
report?: Report,
policy?: Policy,
transactions?: Transaction[],
report: Report | undefined,
policy: Policy | undefined,
transactions: Transaction[],
invoiceReceiverPolicy?: Policy,
isPaidAnimationRunning?: boolean,
isApprovedAnimationRunning?: boolean,
Expand All @@ -231,7 +248,7 @@ function getReportPreviewAction(
if (isSubmittingAnimationRunning) {
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.SUBMIT;
}
if (isAddExpenseAction(report, transactions ?? [], isReportArchived)) {
if (isAddExpenseAction(report, transactions, isReportArchived)) {
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.ADD_EXPENSE;
}

Expand All @@ -244,7 +261,7 @@ function getReportPreviewAction(
if (canSubmit(report, violations, isReportArchived, policy, transactions)) {
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.SUBMIT;
}
if (canApprove(report, violations, policy, transactions)) {
if (canApprove(report, violations, currentUserEmail, policy, transactions)) {
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE;
}
if (canPay(report, violations, isReportArchived, policy, invoiceReceiverPolicy)) {
Expand Down
Loading
Loading