-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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.
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
Follow ups to Broken Connection with direct feeds #57242
Changes from all commits
368294d
7f19faa
d17898b
0f20a32
a614465
7e6f6b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,17 @@ import { | |
isPolicyAdmin, | ||
} from '@libs/PolicyUtils'; | ||
import {getOriginalMessage, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils'; | ||
import {getReportTransactions, isOpenExpenseReport, isProcessingReport, isReportIDApproved, isSettled, isThread} from '@libs/ReportUtils'; | ||
import { | ||
getReportTransactions, | ||
isCurrentUserSubmitter, | ||
isOpenExpenseReport, | ||
isProcessingReport, | ||
isReportApproved, | ||
isReportIDApproved, | ||
isReportManuallyReimbursed, | ||
isSettled, | ||
isThread, | ||
} from '@libs/ReportUtils'; | ||
import type {IOURequestType} from '@userActions/IOU'; | ||
import CONST from '@src/CONST'; | ||
import type {IOUType} from '@src/CONST'; | ||
|
@@ -820,7 +830,7 @@ function shouldShowBrokenConnectionViolation( | |
// This should not be possible except in the case of incorrect type assertions. Generally TS should prevent this at compile time. | ||
throw new Error('Invalid argument combination. If a transactionIDList is passed in, then an OnyxCollection of violations is expected'); | ||
} | ||
violations = transactionOrIDList.flatMap((id) => transactionViolations?.[id] ?? []); | ||
violations = transactionOrIDList.flatMap((id) => transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []); | ||
} else { | ||
if (!Array.isArray(transactionViolations)) { | ||
// This should not be possible except in the case of incorrect type assertions. Generally TS should prevent this at compile time. | ||
|
@@ -830,7 +840,24 @@ function shouldShowBrokenConnectionViolation( | |
} | ||
|
||
const brokenConnectionViolations = violations.filter((violation) => isBrokenConnectionViolation(violation)); | ||
return brokenConnectionViolations.length > 0 && (!isPolicyAdmin(policy) || isOpenExpenseReport(report) || (isProcessingReport(report) && isInstantSubmitEnabled(policy))); | ||
|
||
if (brokenConnectionViolations.length > 0) { | ||
if (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report?.reportID)) { | ||
return true; | ||
} | ||
return isOpenExpenseReport(report) || (isProcessingReport(report) && isInstantSubmitEnabled(policy)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @VickyStash Could you help to explain why do we need to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DylanDylann Let me double check it to be sure! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DylanDylann That's mentioned in the doc:
|
||
} | ||
|
||
return false; | ||
} | ||
|
||
function checkIfShouldShowMarkAsCashButton(hasRTERVPendingViolation: boolean, shouldDisplayBrokenConnectionViolation: boolean, report: OnyxEntry<Report>, policy: OnyxEntry<Policy>) { | ||
if (hasRTERVPendingViolation) { | ||
return true; | ||
} | ||
return ( | ||
shouldDisplayBrokenConnectionViolation && (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report?.reportID)) && !isReportApproved({report}) && !isReportManuallyReimbursed(report) | ||
); | ||
} | ||
|
||
/** | ||
|
@@ -1523,6 +1550,7 @@ export { | |
isPerDiemRequest, | ||
isViolationDismissed, | ||
isBrokenConnectionViolation, | ||
checkIfShouldShowMarkAsCashButton, | ||
shouldShowRTERViolationMessage, | ||
}; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VickyStash Could you please write unit test covering the
shouldShowBrokenConnectionViolation
logic?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mountiny I've prepared a follow-up PR: #57542