Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide approve/pay buttons if the report contains violations #51133

Merged
merged 14 commits into from
Nov 25, 2024
+21 −10
Merged

Hide approve/pay buttons if the report contains violations #51133

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c8084ff
Hide approve button if report has violations
abzokhattab Oct 19, 2024
3204148
Disable the pay button if the report has violaitons
abzokhattab Nov 2, 2024
f760f27
minor edit
abzokhattab Nov 2, 2024
38a5650
minor edit
abzokhattab Nov 2, 2024
7af0e6a
minor edit
abzokhattab Nov 2, 2024
6c716bb
minor edit
abzokhattab Nov 2, 2024
150d783
Merge remote-tracking branch 'origin/main' into disable-approve-butto…
abzokhattab Nov 7, 2024
f3b40b8
Merge remote-tracking branch 'origin/main' into disable-approve-butto…
abzokhattab Nov 19, 2024
91a0d0d
adding violations as an argument to the can approve and pay functions
abzokhattab Nov 19, 2024
29e5c22
minor edit
abzokhattab Nov 19, 2024
c3a9f27
Merge remote-tracking branch 'origin/main' into disable-approve-butto…
abzokhattab Nov 20, 2024
d53ec65
minor
abzokhattab Nov 20, 2024
68ce50b
Merge remote-tracking branch 'origin/main' into disable-approve-butto…
abzokhattab Nov 23, 2024
140bbaa
minor
abzokhattab Nov 23, 2024
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
10 changes: 7 additions & 3 deletions src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,11 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
const isPayAtEndExpense = TransactionUtils.isPayAtEndExpense(transaction);
const isArchivedReport = ReportUtils.isArchivedRoomWithID(moneyRequestReport?.reportID);
const [archiveReason] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${moneyRequestReport?.reportID ?? '-1'}`, {selector: ReportUtils.getArchiveReason});
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);

const getCanIOUBePaid = useCallback(
(onlyShowPayElsewhere = false) => IOU.canIOUBePaid(moneyRequestReport, chatReport, policy, transaction ? [transaction] : undefined, onlyShowPayElsewhere),
[moneyRequestReport, chatReport, policy, transaction],
(onlyShowPayElsewhere = false) => IOU.canIOUBePaid(moneyRequestReport, chatReport, policy, transaction ? [transaction] : undefined, transactionViolations, onlyShowPayElsewhere),
[moneyRequestReport, chatReport, policy, transaction, transactionViolations],
);
const canIOUBePaid = useMemo(() => getCanIOUBePaid(), [getCanIOUBePaid]);

Expand All @@ -135,7 +136,10 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea

const shouldShowPayButton = canIOUBePaid || onlyShowPayElsewhere;

const shouldShowApproveButton = useMemo(() => IOU.canApproveIOU(moneyRequestReport, policy) && !hasOnlyPendingTransactions, [moneyRequestReport, policy, hasOnlyPendingTransactions]);
const shouldShowApproveButton = useMemo(
() => IOU.canApproveIOU(moneyRequestReport, policy, transactionViolations) && !hasOnlyPendingTransactions,
[moneyRequestReport, policy, hasOnlyPendingTransactions, transactionViolations],
);

const shouldDisableApproveButton = shouldShowApproveButton && !ReportUtils.isAllowedToApproveExpenseReport(moneyRequestReport);

Expand Down
6 changes: 3 additions & 3 deletions src/components/ReportActionItem/ReportPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,14 @@ function ReportPreview({

const bankAccountRoute = ReportUtils.getBankAccountRoute(chatReport);
const getCanIOUBePaid = useCallback(
(onlyShowPayElsewhere = false) => IOU.canIOUBePaid(iouReport, chatReport, policy, allTransactions, onlyShowPayElsewhere),
[iouReport, chatReport, policy, allTransactions],
(onlyShowPayElsewhere = false) => IOU.canIOUBePaid(iouReport, chatReport, policy, allTransactions, transactionViolations, onlyShowPayElsewhere),
[iouReport, chatReport, policy, allTransactions, transactionViolations],
);

const canIOUBePaid = useMemo(() => getCanIOUBePaid(), [getCanIOUBePaid]);
const onlyShowPayElsewhere = useMemo(() => !canIOUBePaid && getCanIOUBePaid(true), [canIOUBePaid, getCanIOUBePaid]);
const shouldShowPayButton = isPaidAnimationRunning || canIOUBePaid || onlyShowPayElsewhere;
const shouldShowApproveButton = useMemo(() => IOU.canApproveIOU(iouReport, policy), [iouReport, policy]);
const shouldShowApproveButton = useMemo(() => IOU.canApproveIOU(iouReport, policy, transactionViolations), [iouReport, policy, transactionViolations]);

const shouldDisableApproveButton = shouldShowApproveButton && !ReportUtils.isAllowedToApproveExpenseReport(iouReport);

Expand Down
2 changes: 1 addition & 1 deletion src/libs/SearchUIUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTr
const chatReportRNVP = data[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.chatReportID}`] ?? undefined;

if (
IOU.canIOUBePaid(report, chatReport, policy, allReportTransactions, false, chatReportRNVP, invoiceReceiverPolicy) &&
IOU.canIOUBePaid(report, chatReport, policy, allReportTransactions, undefined, false, chatReportRNVP, invoiceReceiverPolicy) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any util that returns transaction violations? So we can get and pass here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use oynx.connect to link to the transaction violations at the top of the file. However, from a performance perspective, I thought it would be better to make it optional and use the existing one in the IOU file as a fallback.

let me know if you think we should use the oynx.connect

Copy link
Contributor

Choose a reason for hiding this comment

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

No worry. I think it's fine for now. Can you do a quick test to ensure everything still good?

!ReportUtils.hasOnlyHeldExpenses(report.reportID, allReportTransactions)
) {
return CONST.SEARCH.ACTION_TYPES.PAY;
Expand Down
13 changes: 10 additions & 3 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6784,6 +6784,7 @@ function sendMoneyWithWallet(report: OnyxEntry<OnyxTypes.Report>, amount: number
function canApproveIOU(
iouReport: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Report> | SearchReport,
policy: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Policy> | SearchPolicy,
violations?: OnyxCollection<OnyxTypes.TransactionViolation[]>,
chatReportRNVP?: OnyxTypes.ReportNameValuePairs,
) {
// Only expense reports can be approved
Expand All @@ -6804,6 +6805,8 @@ function canApproveIOU(
const iouSettled = ReportUtils.isSettled(iouReport?.reportID);
const reportNameValuePairs = chatReportRNVP ?? ReportUtils.getReportNameValuePairs(iouReport?.reportID);
const isArchivedReport = ReportUtils.isArchivedRoom(iouReport, reportNameValuePairs);
const allViolations = violations ?? allTransactionViolations;
const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allViolations);
let isTransactionBeingScanned = false;
const reportTransactions = TransactionUtils.getAllReportTransactions(iouReport?.reportID);
for (const transaction of reportTransactions) {
Expand All @@ -6816,14 +6819,15 @@ function canApproveIOU(
}
}

return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !isTransactionBeingScanned;
return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !isTransactionBeingScanned && !hasViolations;
}

function canIOUBePaid(
iouReport: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Report> | SearchReport,
chatReport: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Report> | SearchReport,
policy: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Policy> | SearchPolicy,
transactions?: OnyxTypes.Transaction[] | SearchTransaction[],
violations?: OnyxCollection<OnyxTypes.TransactionViolation[]>,
onlyShowPayElsewhere = false,
chatReportRNVP?: OnyxTypes.ReportNameValuePairs,
invoiceReceiverPolicy?: SearchPolicy,
Expand Down Expand Up @@ -6870,7 +6874,9 @@ function canIOUBePaid(

const {reimbursableSpend} = ReportUtils.getMoneyRequestSpendBreakdown(iouReport);
const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : ReportUtils.canBeAutoReimbursed(iouReport, policy);
const shouldBeApproved = canApproveIOU(iouReport, policy);
const shouldBeApproved = canApproveIOU(iouReport, policy, violations);
const allViolations = violations ?? allTransactionViolations;
abzokhattab marked this conversation as resolved.
Show resolved Hide resolved
const hasViolations = ReportUtils.hasViolations(iouReport?.reportID ?? '-1', allViolations);

const isPayAtEndExpenseReport = ReportUtils.isPayAtEndExpenseReport(iouReport?.reportID, transactions);
return (
Expand All @@ -6882,6 +6888,7 @@ function canIOUBePaid(
!isChatReportArchived &&
!isAutoReimbursable &&
!shouldBeApproved &&
!hasViolations &&
!isPayAtEndExpenseReport
);
}
Expand All @@ -6892,7 +6899,7 @@ function getIOUReportActionToApproveOrPay(chatReport: OnyxEntry<OnyxTypes.Report
return Object.values(chatReportActions).find((action) => {
const iouReport = ReportUtils.getReportOrDraftReport(action.childReportID ?? '-1');
const policy = PolicyUtils.getPolicy(iouReport?.policyID);
const shouldShowSettlementButton = canIOUBePaid(iouReport, chatReport, policy) || canApproveIOU(iouReport, policy);
const shouldShowSettlementButton = canIOUBePaid(iouReport, chatReport, policy, undefined, allTransactionViolations) || canApproveIOU(iouReport, policy, allTransactionViolations);
return action.childReportID?.toString() !== excludedIOUReportID && action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW && shouldShowSettlementButton;
});
}
Expand Down
Loading