-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat: [Auth Violations] Support isDismissed on newDot to hide violations. #54455
base: main
Are you sure you want to change the base?
Changes from 5 commits
6463ecf
6166bb7
2a179a2
cd59a56
5682ae0
1016065
a2d0da6
ee414e3
82877a6
18f9c6f
40c5559
3f4dd02
7eb5639
593f497
825c4be
969623b
486acb5
0a0cbc3
55f73fa
a5194c8
332efdf
1ef4be5
afe362c
6abedea
05747b2
d870956
2774880
078b3fc
3970b9d
a39a2f5
789b51d
f9e3e7e
93d33a2
156a3aa
d477565
ed89588
79b012f
65cb691
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 |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
import variables from '@styles/variables'; | ||
import * as IOU from '@userActions/IOU'; | ||
import * as TransactionActions from '@userActions/Transaction'; | ||
import CONST from '@src/CONST'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
import ROUTES from '@src/ROUTES'; | ||
import SCREENS from '@src/SCREENS'; | ||
|
@@ -50,35 +51,36 @@ | |
// eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth | ||
const {shouldUseNarrowLayout, isSmallScreenWidth} = useResponsiveLayout(); | ||
const route = useRoute(); | ||
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID ?? '-1'}`); | ||
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID ?? CONST.DEFAULT_NUMBER_ID}`); | ||
const [transaction] = useOnyx( | ||
`${ONYXKEYS.COLLECTION.TRANSACTION}${ | ||
ReportActionsUtils.isMoneyRequestAction(parentReportAction) ? ReportActionsUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID ?? -1 : -1 | ||
ReportActionsUtils.isMoneyRequestAction(parentReportAction) | ||
? ReportActionsUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID ?? CONST.DEFAULT_NUMBER_ID | ||
: CONST.DEFAULT_NUMBER_ID | ||
}`, | ||
); | ||
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); | ||
const [dismissedHoldUseExplanation, dismissedHoldUseExplanationResult] = useOnyx(ONYXKEYS.NVP_DISMISSED_HOLD_USE_EXPLANATION, {initialValue: true}); | ||
const isLoadingHoldUseExplained = isLoadingOnyxValue(dismissedHoldUseExplanationResult); | ||
const styles = useThemeStyles(); | ||
const theme = useTheme(); | ||
const {translate} = useLocalize(); | ||
const [shouldShowHoldMenu, setShouldShowHoldMenu] = useState(false); | ||
const isOnHold = TransactionUtils.isOnHold(transaction); | ||
const isDuplicate = TransactionUtils.isDuplicate(transaction?.transactionID ?? ''); | ||
const isDuplicate = TransactionUtils.isDuplicate(transaction?.transactionID); | ||
const reportID = report?.reportID; | ||
|
||
const isReportInRHP = route.name === SCREENS.SEARCH.REPORT_RHP; | ||
const shouldDisplaySearchRouter = !isReportInRHP || isSmallScreenWidth; | ||
|
||
const hasAllPendingRTERViolations = TransactionUtils.allHavePendingRTERViolation([transaction?.transactionID ?? '-1']); | ||
const hasAllPendingRTERViolations = TransactionUtils.allHavePendingRTERViolation(transaction?.transactionID ? [transaction?.transactionID] : []); | ||
|
||
const shouldShowBrokenConnectionViolation = TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID ?? '-1', parentReport, policy); | ||
const shouldShowBrokenConnectionViolation = TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID, parentReport, policy); | ||
|
||
const shouldShowMarkAsCashButton = | ||
hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!PolicyUtils.isPolicyAdmin(policy) || ReportUtils.isCurrentUserSubmitter(parentReport?.reportID ?? ''))); | ||
hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!PolicyUtils.isPolicyAdmin(policy) || ReportUtils.isCurrentUserSubmitter(parentReport?.reportID))); | ||
|
||
const markAsCash = useCallback(() => { | ||
TransactionActions.markAsCash(transaction?.transactionID ?? '-1', reportID ?? ''); | ||
TransactionActions.markAsCash(transaction?.transactionID, reportID); | ||
}, [reportID, transaction?.transactionID]); | ||
|
||
const isScanning = TransactionUtils.hasReceipt(transaction) && TransactionUtils.isReceiptBeingScanned(transaction); | ||
|
@@ -105,14 +107,14 @@ | |
icon: getStatusIcon(Expensicons.Hourglass), | ||
description: ( | ||
<BrokenConnectionDescription | ||
transactionID={transaction?.transactionID ?? '-1'} | ||
transactionID={transaction?.transactionID} | ||
report={report} | ||
policy={policy} | ||
/> | ||
), | ||
}; | ||
} | ||
if (TransactionUtils.hasPendingRTERViolation(TransactionUtils.getTransactionViolations(transaction?.transactionID ?? '-1', transactionViolations))) { | ||
if (TransactionUtils.hasPendingRTERViolation(TransactionUtils.getTransactionViolations(transaction?.transactionID))) { | ||
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. @rojiphil, could you please check the ESLint failures below? I have already resolved dozens of those but it keeps increasing everytime I push a commit. I want your help especially in resolving the warning just below this comment and when we use 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. In the case of calling The guidelines for the fix of the ESlint failures is already mentioned here. Please look for similar handling for ESlint failures elsewhere in code as that can help. But let me know if you get stuck otherwise.
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. @rojiphil thanks for the help 🙇🏻. |
||
return {icon: getStatusIcon(Expensicons.Hourglass), description: translate('iou.pendingMatchWithCreditCardDescription')}; | ||
} | ||
if (isScanning) { | ||
|
@@ -157,7 +159,7 @@ | |
shouldShowPinButton={false} | ||
report={{ | ||
...report, | ||
reportID: reportID ?? '', | ||
Check failure on line 162 in src/components/MoneyRequestHeader.tsx GitHub Actions / Changed files ESLint check
|
||
ownerAccountID: parentReport?.ownerAccountID, | ||
}} | ||
policy={policy} | ||
|
@@ -179,7 +181,7 @@ | |
text={translate('iou.reviewDuplicates')} | ||
style={[styles.p0, styles.ml2]} | ||
onPress={() => { | ||
Navigation.navigate(ROUTES.TRANSACTION_DUPLICATE_REVIEW_PAGE.getRoute(reportID ?? '', Navigation.getReportRHPActiveRoute())); | ||
Check failure on line 184 in src/components/MoneyRequestHeader.tsx GitHub Actions / Changed files ESLint check
|
||
}} | ||
/> | ||
)} | ||
|
@@ -201,7 +203,7 @@ | |
text={translate('iou.reviewDuplicates')} | ||
style={[styles.w100, styles.pr0]} | ||
onPress={() => { | ||
Navigation.navigate(ROUTES.TRANSACTION_DUPLICATE_REVIEW_PAGE.getRoute(reportID ?? '', Navigation.getReportRHPActiveRoute())); | ||
Check failure on line 206 in src/components/MoneyRequestHeader.tsx GitHub Actions / Changed files ESLint check
|
||
}} | ||
/> | ||
</View> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,7 +83,8 @@ function MoneyRequestPreviewContent({ | |
const transactionID = isMoneyRequestAction ? ReportActionsUtils.getOriginalMessage(action)?.IOUTransactionID : undefined; | ||
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`); | ||
const [walletTerms] = useOnyx(ONYXKEYS.WALLET_TERMS); | ||
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); | ||
const [allViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); | ||
const transactionViolations = TransactionUtils.getTransactionViolations(transaction?.transactionID); | ||
|
||
const sessionAccountID = session?.accountID; | ||
const managerID = iouReport?.managerID ?? CONST.DEFAULT_NUMBER_ID; | ||
|
@@ -117,9 +118,9 @@ function MoneyRequestPreviewContent({ | |
const isOnHold = TransactionUtils.isOnHold(transaction); | ||
const isSettlementOrApprovalPartial = !!iouReport?.pendingFields?.partial; | ||
const isPartialHold = isSettlementOrApprovalPartial && isOnHold; | ||
const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID, transactionViolations, true); | ||
const hasNoticeTypeViolations = TransactionUtils.hasNoticeTypeViolation(transaction?.transactionID, transactionViolations, true) && ReportUtils.isPaidGroupPolicy(iouReport); | ||
const hasWarningTypeViolations = TransactionUtils.hasWarningTypeViolation(transaction?.transactionID, transactionViolations, true); | ||
const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID, allViolations, true); | ||
const hasNoticeTypeViolations = TransactionUtils.hasNoticeTypeViolation(transaction?.transactionID, allViolations, true) && ReportUtils.isPaidGroupPolicy(iouReport); | ||
const hasWarningTypeViolations = TransactionUtils.hasWarningTypeViolation(transaction?.transactionID, allViolations, true); | ||
const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(transaction); | ||
const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); | ||
const isFetchingWaypointsFromServer = TransactionUtils.isFetchingWaypointsFromServer(transaction); | ||
|
@@ -134,11 +135,8 @@ function MoneyRequestPreviewContent({ | |
|
||
// Get transaction violations for given transaction id from onyx, find duplicated transactions violations and get duplicates | ||
const allDuplicates = useMemo( | ||
() => | ||
transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction?.transactionID}`]?.find( | ||
(violation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION, | ||
)?.data?.duplicates ?? [], | ||
[transaction?.transactionID, transactionViolations], | ||
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. Don't we have to keep 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.
Just updated the code, now we are using |
||
() => transactionViolations?.find((violation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION)?.data?.duplicates ?? [], | ||
[transactionViolations], | ||
); | ||
|
||
// Remove settled transactions from duplicates | ||
|
@@ -209,7 +207,7 @@ function MoneyRequestPreviewContent({ | |
} | ||
|
||
if (shouldShowRBR && transaction) { | ||
const violations = TransactionUtils.getTransactionViolations(transaction.transactionID, transactionViolations); | ||
const violations = TransactionUtils.getTransactionViolations(transaction.transactionID); | ||
if (shouldShowHoldMessage) { | ||
return `${message} ${CONST.DOT_SEPARATOR} ${translate('violations.hold')}`; | ||
} | ||
|
@@ -256,7 +254,7 @@ function MoneyRequestPreviewContent({ | |
if (TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID, iouReport, policy)) { | ||
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('violations.brokenConnection530Error')}; | ||
} | ||
if (TransactionUtils.hasPendingUI(transaction, TransactionUtils.getTransactionViolations(transaction?.transactionID, transactionViolations))) { | ||
if (TransactionUtils.hasPendingUI(transaction, TransactionUtils.getTransactionViolations(transaction?.transactionID))) { | ||
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('iou.pendingMatchWithCreditCard')}; | ||
} | ||
return {shouldShow: false}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,9 +71,9 @@ | |
const receiptFieldViolationNames: OnyxTypes.ViolationName[] = [CONST.VIOLATIONS.MODIFIED_AMOUNT, CONST.VIOLATIONS.MODIFIED_DATE]; | ||
|
||
const getTransactionID = (report: OnyxEntry<OnyxTypes.Report>, parentReportActions: OnyxEntry<OnyxTypes.ReportActions>) => { | ||
const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1']; | ||
const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? CONST.DEFAULT_NUMBER_ID]; | ||
Krishna2323 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const originalMessage = parentReportAction && ReportActionsUtils.isMoneyRequestAction(parentReportAction) ? ReportActionsUtils.getOriginalMessage(parentReportAction) : undefined; | ||
return originalMessage?.IOUTransactionID ?? -1; | ||
return originalMessage?.IOUTransactionID ?? undefined; | ||
}; | ||
|
||
function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = false, updatedTransaction, isFromReviewDuplicates = false}: MoneyRequestViewProps) { | ||
|
@@ -81,8 +81,8 @@ | |
const session = useSession(); | ||
const {isOffline} = useNetwork(); | ||
const {translate, toLocaleDigit} = useLocalize(); | ||
const parentReportID = report?.parentReportID ?? '-1'; | ||
Check failure on line 84 in src/components/ReportActionItem/MoneyRequestView.tsx GitHub Actions / Changed files ESLint check
|
||
const policyID = report?.policyID ?? '-1'; | ||
Check failure on line 85 in src/components/ReportActionItem/MoneyRequestView.tsx GitHub Actions / Changed files ESLint check
|
||
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`); | ||
const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReport?.parentReportID}`, { | ||
selector: (chatReportValue) => chatReportValue && {reportID: chatReportValue.reportID, errorFields: chatReportValue.errorFields}, | ||
|
@@ -95,14 +95,14 @@ | |
const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`, { | ||
canEvict: false, | ||
}); | ||
const [transactionViolations] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${getTransactionID(report, parentReportActions)}`); | ||
const transactionViolations = TransactionUtils.getTransactionViolations(getTransactionID(report, parentReportActions) ?? undefined); | ||
|
||
const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1']; | ||
Check failure on line 100 in src/components/ReportActionItem/MoneyRequestView.tsx GitHub Actions / Changed files ESLint check
|
||
const isTrackExpense = ReportUtils.isTrackExpenseReport(report); | ||
const moneyRequestReport = parentReport; | ||
const linkedTransactionID = useMemo(() => { | ||
const originalMessage = parentReportAction && ReportActionsUtils.isMoneyRequestAction(parentReportAction) ? ReportActionsUtils.getOriginalMessage(parentReportAction) : undefined; | ||
return originalMessage?.IOUTransactionID ?? '-1'; | ||
Check failure on line 105 in src/components/ReportActionItem/MoneyRequestView.tsx GitHub Actions / Changed files ESLint check
|
||
}, [parentReportAction]); | ||
|
||
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${linkedTransactionID}`); | ||
|
@@ -227,7 +227,7 @@ | |
if (newBillable === TransactionUtils.getBillable(transaction)) { | ||
return; | ||
} | ||
IOU.updateMoneyRequestBillable(transaction?.transactionID ?? '-1', report?.reportID ?? '-1', newBillable, policy, policyTagList, policyCategories); | ||
Check failure on line 230 in src/components/ReportActionItem/MoneyRequestView.tsx GitHub Actions / Changed files ESLint check
Check failure on line 230 in src/components/ReportActionItem/MoneyRequestView.tsx GitHub Actions / Changed files ESLint check
|
||
}, | ||
[transaction, report, policy, policyTagList, policyCategories], | ||
); | ||
|
@@ -323,7 +323,7 @@ | |
ROUTES.MONEY_REQUEST_STEP_DISTANCE.getRoute( | ||
CONST.IOU.ACTION.EDIT, | ||
iouType, | ||
transaction?.transactionID ?? '-1', | ||
Check failure on line 326 in src/components/ReportActionItem/MoneyRequestView.tsx GitHub Actions / Changed files ESLint check
|
||
report?.reportID ?? '-1', | ||
Navigation.getReportRHPActiveRoute(), | ||
), | ||
|
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.
Let us use
||
instead of??
as that will help address empty strings as well.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.
We use
??
everywhere in the app, otherwise we would need to disable eslint.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.
I think it’s fine to leave it as that is the more widely accepted pattern. Maybe this can be taken up later when it is considered in its entirety.