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.

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

Merged
merged 42 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
6463ecf
feat: [Auth Violations] Support isDismissed on newDot to hide violati…
Krishna2323 Dec 23, 2024
6166bb7
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 5, 2025
2a179a2
fix eslint.
Krishna2323 Jan 5, 2025
cd59a56
fix eslint.
Krishna2323 Jan 5, 2025
5682ae0
fix eslint.
Krishna2323 Jan 5, 2025
1016065
fix eslint.
Krishna2323 Jan 6, 2025
a2d0da6
fix ESLint.
Krishna2323 Jan 6, 2025
ee414e3
fix ESLint.
Krishna2323 Jan 6, 2025
82877a6
Merge branch 'Expensify:main' into krishna2323/issue/53398
Krishna2323 Jan 9, 2025
18f9c6f
fix ESlint issues.
Krishna2323 Jan 9, 2025
40c5559
fix ESlint issues.
Krishna2323 Jan 9, 2025
3f4dd02
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 13, 2025
7eb5639
Merge branch 'Expensify:main' into krishna2323/issue/53398
Krishna2323 Jan 14, 2025
593f497
fix ESlint.
Krishna2323 Jan 15, 2025
825c4be
fix ESlint.
Krishna2323 Jan 15, 2025
969623b
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 15, 2025
486acb5
revert unnecessary changes.
Krishna2323 Jan 15, 2025
0a0cbc3
fix ESLint.
Krishna2323 Jan 15, 2025
55f73fa
fix ESLint.
Krishna2323 Jan 15, 2025
a5194c8
minor update.
Krishna2323 Jan 15, 2025
332efdf
fix merge conflicts.
Krishna2323 Jan 16, 2025
1ef4be5
minor import fix.
Krishna2323 Jan 16, 2025
afe362c
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 16, 2025
6abedea
remove duplicate imports.
Krishna2323 Jan 16, 2025
05747b2
minor fix.
Krishna2323 Jan 16, 2025
d870956
fix typescript issue.
Krishna2323 Jan 16, 2025
2774880
minor updates.
Krishna2323 Jan 20, 2025
078b3fc
fix merge conflicts.
Krishna2323 Jan 20, 2025
3970b9d
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 20, 2025
a39a2f5
minor import names update.
Krishna2323 Jan 20, 2025
789b51d
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 21, 2025
f9e3e7e
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 24, 2025
93d33a2
add unit tests for violation util functions.
Krishna2323 Jan 24, 2025
156a3aa
minor NAB fixes.
Krishna2323 Jan 28, 2025
d477565
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 28, 2025
ed89588
fix ESLint.
Krishna2323 Jan 28, 2025
79b012f
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 29, 2025
65cb691
minor fix.
Krishna2323 Jan 29, 2025
55e01d5
Merge branch 'main' into krishna2323/issue/53398
Krishna2323 Jan 30, 2025
e79c90b
fix TS issues.
Krishna2323 Jan 30, 2025
32bf0c8
minor update.
Krishna2323 Feb 3, 2025
5e20ecc
Merge branch 'Expensify:main' into krishna2323/issue/53398
Krishna2323 Feb 3, 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
13 changes: 8 additions & 5 deletions src/components/BrokenConnectionDescription.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
import React from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import {isInstantSubmitEnabled, isPolicyAdmin as isPolicyAdminPolicyUtils} from '@libs/PolicyUtils';
import {isCurrentUserSubmitter, isProcessingReport, isReportApproved, isReportManuallyReimbursed} from '@libs/ReportUtils';
import {getTransactionViolations} from '@libs/TransactionUtils';
import Navigation from '@navigation/Navigation';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Policy, Report} from '@src/types/onyx';
import TextLink from './TextLink';

type BrokenConnectionDescriptionProps = {
/** Transaction id of the corresponding report */
transactionID: string | undefined;

/** Current report */
report: OnyxEntry<Report>;

Expand All @@ -26,7 +24,7 @@ type BrokenConnectionDescriptionProps = {
function BrokenConnectionDescription({transactionID, policy, report}: BrokenConnectionDescriptionProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const [transactionViolations] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID ?? CONST.DEFAULT_NUMBER_ID}`);
const transactionViolations = getTransactionViolations(transactionID);

const brokenConnection530Error = transactionViolations?.find((violation) => violation.data?.rterType === CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION_530);
const brokenConnectionError = transactionViolations?.find((violation) => violation.data?.rterType === CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION);
Expand All @@ -46,7 +44,12 @@ function BrokenConnectionDescription({transactionID, policy, report}: BrokenConn
{`${translate('violations.adminBrokenConnectionError')}`}
<TextLink
style={[styles.textLabelSupporting, styles.link]}
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policy?.id))}
onPress={() => {
if (!policy?.id) {
return;
}
Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policy?.id));
}}
>{`${translate('workspace.common.companyCards')}`}</TextLink>
.
</>
Expand Down
9 changes: 4 additions & 5 deletions src/components/MoneyRequestHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import {markAsCash as markAsCashUtil} from '@libs/actions/Transaction';
import Navigation from '@libs/Navigation/Navigation';
import {isPolicyAdmin} from '@libs/PolicyUtils';
import {getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils';
Expand All @@ -25,7 +26,6 @@ import {
shouldShowBrokenConnectionViolation as shouldShowBrokenConnectionViolationTransactionUtils,
} from '@libs/TransactionUtils';
import variables from '@styles/variables';
import {markAsCash as markAsCashAction} from '@userActions/Transaction';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -61,13 +61,12 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
// 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}`);
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID ?? CONST.DEFAULT_NUMBER_ID}`);
Copy link
Contributor

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.

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 use ?? everywhere in the app, otherwise we would need to disable eslint.

Copy link
Contributor

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.

const [transaction] = useOnyx(
`${ONYXKEYS.COLLECTION.TRANSACTION}${
isMoneyRequestAction(parentReportAction) ? 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 [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
const isLoadingHoldUseExplained = isLoadingOnyxValue(dismissedHoldUseExplanationResult);
Expand All @@ -88,7 +87,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
const shouldShowMarkAsCashButton = hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!isPolicyAdmin(policy) || isCurrentUserSubmitter(parentReport?.reportID)));

const markAsCash = useCallback(() => {
markAsCashAction(transaction?.transactionID, reportID);
markAsCashUtil(transaction?.transactionID, reportID);
}, [reportID, transaction?.transactionID]);

const isScanning = hasReceipt(transaction) && isReceiptBeingScanned(transaction);
Expand Down Expand Up @@ -122,7 +121,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
),
};
}
if (hasPendingRTERViolation(getTransactionViolations(transaction?.transactionID, transactionViolations))) {
if (hasPendingRTERViolation(getTransactionViolations(transaction?.transactionID))) {
return {icon: getStatusIcon(Expensicons.Hourglass), description: translate('iou.pendingMatchWithCreditCardDescription')};
}
if (isScanning) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ function MoneyRequestPreviewContent({
const transactionID = isMoneyRequestAction ? 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 = getTransactionViolations(transaction?.transactionID);

const sessionAccountID = session?.accountID;
const managerID = iouReport?.managerID ?? CONST.DEFAULT_NUMBER_ID;
Expand Down Expand Up @@ -145,9 +146,9 @@ function MoneyRequestPreviewContent({
const isOnHold = isOnHoldTransactionUtils(transaction);
const isSettlementOrApprovalPartial = !!iouReport?.pendingFields?.partial;
const isPartialHold = isSettlementOrApprovalPartial && isOnHold;
const hasViolations = hasViolationTransactionUtils(transaction?.transactionID, transactionViolations, true);
const hasNoticeTypeViolations = hasNoticeTypeViolationTransactionUtils(transaction?.transactionID, transactionViolations, true) && isPaidGroupPolicy(iouReport);
const hasWarningTypeViolations = hasWarningTypeViolationTransactionUtils(transaction?.transactionID, transactionViolations, true);
const hasViolations = hasViolationTransactionUtils(transaction?.transactionID, allViolations, true);
const hasNoticeTypeViolations = hasNoticeTypeViolationTransactionUtils(transaction?.transactionID, allViolations, true) && isPaidGroupPolicy(iouReport);
const hasWarningTypeViolations = hasWarningTypeViolationTransactionUtils(transaction?.transactionID, allViolations, true);
const hasFieldErrors = hasMissingSmartscanFields(transaction);
const isDistanceRequest = isDistanceRequestTransactionUtils(transaction);
const isPerDiemRequest = isPerDiemRequestTransactionUtils(transaction);
Expand All @@ -163,11 +164,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],
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have to keep transactionID as dependency so that allDuplicates can be updated when it becomes available? Also, I think we can remove transactionViolations and use getTransactionViolations directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const transactionViolations = getTransactionViolations(transaction?.transactionID);

transactionViolations will we updated when transaction?.transactionID is changed.

Also, I think we can remove transactionViolations and use getTransactionViolations directly.

Just updated the code, now we are using transactionViolations in 3 places.

() => transactionViolations?.find((violation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION)?.data?.duplicates ?? [],
[transactionViolations],
);

// Remove settled transactions from duplicates
Expand Down Expand Up @@ -240,14 +238,13 @@ function MoneyRequestPreviewContent({
}

if (shouldShowRBR && transaction) {
const violations = getTransactionViolations(transaction.transactionID, transactionViolations);
if (shouldShowHoldMessage) {
return `${message} ${CONST.DOT_SEPARATOR} ${translate('violations.hold')}`;
}
const firstViolation = violations?.at(0);
const firstViolation = transactionViolations?.at(0);
if (firstViolation) {
const violationMessage = ViolationsUtils.getViolationTranslation(firstViolation, translate);
const violationsCount = violations?.filter((v) => v.type === CONST.VIOLATION_TYPES.VIOLATION).length ?? 0;
const violationsCount = transactionViolations?.filter((v) => v.type === CONST.VIOLATION_TYPES.VIOLATION).length ?? 0;
const isTooLong = violationsCount > 1 || violationMessage.length > 15;
const hasViolationsAndFieldErrors = violationsCount > 0 && hasFieldErrors;

Expand Down Expand Up @@ -287,7 +284,7 @@ function MoneyRequestPreviewContent({
if (shouldShowBrokenConnectionViolation(transaction ? [transaction.transactionID] : [], iouReport, policy)) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('violations.brokenConnection530Error')};
}
if (hasPendingUI(transaction, getTransactionViolations(transaction?.transactionID, transactionViolations))) {
if (hasPendingUI(transaction, transactionViolations)) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('iou.pendingMatchWithCreditCard')};
}
return {shouldShow: false};
Expand Down
5 changes: 3 additions & 2 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
getDistanceInMeters,
getTagForDisplay,
getTaxName,
getTransactionViolations,
hasMissingSmartscanFields,
hasReceipt as hasReceiptTransactionUtils,
hasReservationList,
Expand Down Expand Up @@ -133,7 +134,7 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals

const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${linkedTransactionID ?? CONST.DEFAULT_NUMBER_ID}`);
const [transactionBackup] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${linkedTransactionID ?? CONST.DEFAULT_NUMBER_ID}`);
const [transactionViolations] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${linkedTransactionID ?? CONST.DEFAULT_NUMBER_ID}`);
const transactionViolations = getTransactionViolations(linkedTransactionID);

const {
created: transactionDate,
Expand Down Expand Up @@ -698,7 +699,7 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals
{shouldShowTax && (
<OfflineWithFeedback pendingAction={getPendingFieldAction('taxCode')}>
<MenuItemWithTopDescription
title={taxRateTitle ?? ''}
title={taxRateTitle}
description={taxRatesDescription}
interactive={canEditTaxFields}
shouldShowRightIcon={canEditTaxFields}
Expand Down
4 changes: 2 additions & 2 deletions src/components/ReportActionItem/ReportPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ function ReportPreview({
const lastTransaction = transactions?.at(0);
const lastThreeReceipts = lastThreeTransactions.map((transaction) => ({...getThumbnailAndImageURIs(transaction), transaction}));
const transactionIDList = transactions?.map((reportTransaction) => reportTransaction.transactionID) ?? [];
const showRTERViolationMessage = numberOfRequests === 1 && hasPendingUI(lastTransaction, getTransactionViolations(lastTransaction?.transactionID, transactionViolations));
const showRTERViolationMessage = numberOfRequests === 1 && hasPendingUI(lastTransaction, getTransactionViolations(lastTransaction?.transactionID));
const shouldShowBrokenConnectionViolation = numberOfRequests === 1 && shouldShowBrokenConnectionViolationTransactionUtils(transactionIDList, iouReport, policy);
let formattedMerchant = numberOfRequests === 1 ? getMerchant(lastTransaction) : null;
const formattedDescription = numberOfRequests === 1 ? getDescription(lastTransaction) : null;
Expand All @@ -250,7 +250,7 @@ function ReportPreview({
const isArchived = isArchivedReportWithID(iouReport?.reportID);
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
const filteredTransactions = transactions?.filter((transaction) => transaction) ?? [];
const shouldShowSubmitButton = canSubmitReport(iouReport, policy, filteredTransactions, transactionViolations);
const shouldShowSubmitButton = canSubmitReport(iouReport, policy, filteredTransactions);

const shouldDisableSubmitButton = shouldShowSubmitButton && !isAllowedToSubmitDraftExpenseReport(iouReport);

Expand Down
4 changes: 2 additions & 2 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1614,8 +1614,8 @@ function wasActionTakenByCurrentUser(reportAction: OnyxInputOrEntry<ReportAction
/**
* Get IOU action for a reportID and transactionID
*/
function getIOUActionForReportID(reportID: string | undefined, transactionID: string): OnyxEntry<ReportAction> {
if (!reportID) {
function getIOUActionForReportID(reportID: string | undefined, transactionID: string | undefined): OnyxEntry<ReportAction> {
if (!reportID || !transactionID) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rojiphil, the change is still there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am referring to the next line where we returned undefined. This change got removed as part of the merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh, sorry. Updated.

return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's return undefined 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.

done

Copy link
Contributor

Choose a reason for hiding this comment

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

@Krishna2323 Why was this reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it was reverted because I tried to simplify solving merge conflicts the change is still there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add the code back? Otherwise the code LGTM.

}
const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
Expand Down
2 changes: 1 addition & 1 deletion src/libs/SearchUIUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTr
}

// We check for isAllowedToApproveExpenseReport because if the policy has preventSelfApprovals enabled, we disable the Submit action and in that case we want to show the View action instead
if (canSubmitReport(report, policy, allReportTransactions, allViolations) && isAllowedToApproveExpenseReport) {
if (canSubmitReport(report, policy, allReportTransactions) && isAllowedToApproveExpenseReport) {
return CONST.SEARCH.ACTION_TYPES.SUBMIT;
}

Expand Down
Loading