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

[No QA] [Direct Feeds] Broken connection violation #50793

Merged
merged 8 commits into from
Oct 17, 2024
Merged

[No QA] [Direct Feeds] Broken connection violation #50793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2f90dd1
Add hasBrokenConnectionViolation and getBrokenConnectionDescription f…
VickyStash Oct 14, 2024
7e7c40c
Merge branch 'refs/heads/main' into feature/50454-broken-connection-v…
VickyStash Oct 15, 2024
7bce745
Add broken connection violation warning display
VickyStash Oct 15, 2024
ba663d7
Fix message display
VickyStash Oct 15, 2024
d9316e8
Fix
VickyStash Oct 15, 2024
129025e
Update MoneyRequestPreviewContent component
VickyStash Oct 15, 2024
91e299d
Lint fixes
VickyStash Oct 15, 2024
630893f
Updated the checks
VickyStash Oct 16, 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
65 changes: 65 additions & 0 deletions src/components/BrokenConnectionDescription.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import React from 'react';
mountiny marked this conversation as resolved.
Show resolved Hide resolved
import type {OnyxEntry} from 'react-native-onyx';
mountiny marked this conversation as resolved.
Show resolved Hide resolved
import {useOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ReportUtils from '@libs/ReportUtils';
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;

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

/** Policy which the report is tied to */
policy: OnyxEntry<Policy>;
};

function BrokenConnectionDescription({transactionID, policy, report}: BrokenConnectionDescriptionProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const [transactionViolations] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

my concern here is that for a fraction of time useOnyx gives undefined value, hope this doesn't affect the violation fetch. We can only test this in production, so lets stay hopeful 🤞


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);
const isPolicyAdmin = PolicyUtils.isPolicyAdmin(policy);

if (!brokenConnection530Error && !brokenConnectionError) {
return '';
}

if (brokenConnection530Error) {
return translate('violations.brokenConnection530Error');
}

if (isPolicyAdmin && !ReportUtils.isCurrentUserSubmitter(report?.reportID ?? '')) {
return (
<>
{`${translate('violations.adminBrokenConnectionError')}`}
<TextLink
style={[styles.textLabelSupporting, styles.link]}
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policy?.id ?? '-1'))}
>{`${translate('workspace.common.companyCards')}`}</TextLink>
.
</>
);
}

if (ReportUtils.isReportApproved(report) || ReportUtils.isReportManuallyReimbursed(report) || (ReportUtils.isProcessingReport(report) && !PolicyUtils.isInstantSubmitEnabled(policy))) {
return translate('violations.memberBrokenConnectionError');
}

return `${translate('violations.memberBrokenConnectionError')} ${translate('violations.markAsCashToIgnore')}`;
}

BrokenConnectionDescription.displayName = 'BrokenConnectionDescription';

export default BrokenConnectionDescription;
32 changes: 26 additions & 6 deletions src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type * as OnyxTypes from '@src/types/onyx';
import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage';
import type IconAsset from '@src/types/utils/IconAsset';
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import BrokenConnectionDescription from './BrokenConnectionDescription';
import Button from './Button';
import ConfirmModal from './ConfirmModal';
import DelegateNoAccessModal from './DelegateNoAccessModal';
Expand Down Expand Up @@ -108,6 +109,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
const hasOnlyPendingTransactions = allTransactions.length > 0 && allTransactions.every((t) => TransactionUtils.isExpensifyCardTransaction(t) && TransactionUtils.isPending(t));
const transactionIDs = allTransactions.map((t) => t.transactionID);
const hasAllPendingRTERViolations = TransactionUtils.allHavePendingRTERViolation([transaction?.transactionID ?? '-1']);
const shouldShowBrokenConnectionViolation = TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID ?? '-1', moneyRequestReport, policy);
const hasOnlyHeldExpenses = ReportUtils.hasOnlyHeldExpenses(moneyRequestReport?.reportID ?? '');
const isPayAtEndExpense = TransactionUtils.isPayAtEndExpense(transaction);
const isArchivedReport = ReportUtils.isArchivedRoomWithID(moneyRequestReport?.reportID);
Expand All @@ -121,25 +123,31 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea

const onlyShowPayElsewhere = useMemo(() => !canIOUBePaid && getCanIOUBePaid(true), [canIOUBePaid, getCanIOUBePaid]);

const shouldShowMarkAsCashButton =
hasAllPendingRTERViolations ||
(shouldShowBrokenConnectionViolation && (!PolicyUtils.isPolicyAdmin(policy) || ReportUtils.isCurrentUserSubmitter(moneyRequestReport?.reportID ?? '')));

const shouldShowPayButton = canIOUBePaid || onlyShowPayElsewhere;

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

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

const shouldShowSubmitButton = !!moneyRequestReport && isDraft && reimbursableSpend !== 0 && !hasAllPendingRTERViolations;
const shouldShowSubmitButton = !!moneyRequestReport && isDraft && reimbursableSpend !== 0 && !hasAllPendingRTERViolations && !shouldShowBrokenConnectionViolation;

const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
const shouldShowExportIntegrationButton = !shouldShowPayButton && !shouldShowSubmitButton && connectedIntegration && isAdmin && ReportUtils.canBeExported(moneyRequestReport);

const shouldShowSettlementButton = (shouldShowPayButton || shouldShowApproveButton) && !hasAllPendingRTERViolations && !shouldShowExportIntegrationButton;
const shouldShowSettlementButton =
(shouldShowPayButton || shouldShowApproveButton) && !hasAllPendingRTERViolations && !shouldShowExportIntegrationButton && !shouldShowBrokenConnectionViolation;

const shouldDisableSubmitButton = shouldShowSubmitButton && !ReportUtils.isAllowedToSubmitDraftExpenseReport(moneyRequestReport);
const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE;
const shouldShowStatusBar = hasAllPendingRTERViolations || hasOnlyHeldExpenses || hasScanningReceipt || isPayAtEndExpense || hasOnlyPendingTransactions;
const shouldShowStatusBar =
hasAllPendingRTERViolations || shouldShowBrokenConnectionViolation || hasOnlyHeldExpenses || hasScanningReceipt || isPayAtEndExpense || hasOnlyPendingTransactions;
const shouldShowNextStep = !ReportUtils.isClosedExpenseReportWithNoExpenses(moneyRequestReport) && isFromPaidPolicy && !!nextStep?.message?.length && !shouldShowStatusBar;
const shouldShowAnyButton =
shouldShowSettlementButton || shouldShowApproveButton || shouldShowSubmitButton || shouldShowNextStep || hasAllPendingRTERViolations || shouldShowExportIntegrationButton;
shouldShowSettlementButton || shouldShowApproveButton || shouldShowSubmitButton || shouldShowNextStep || shouldShowMarkAsCashButton || shouldShowExportIntegrationButton;
const bankAccountRoute = ReportUtils.getBankAccountRoute(chatReport);
const formattedAmount = CurrencyUtils.convertToDisplayString(reimbursableSpend, moneyRequestReport?.currency);
const [nonHeldAmount, fullAmount] = ReportUtils.getNonHeldAndFullAmount(moneyRequestReport, policy);
Expand Down Expand Up @@ -228,6 +236,18 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
if (hasOnlyHeldExpenses) {
return {icon: getStatusIcon(Expensicons.Stopwatch), description: translate('iou.expensesOnHold')};
}
if (shouldShowBrokenConnectionViolation) {
return {
icon: getStatusIcon(Expensicons.Hourglass),
description: (
<BrokenConnectionDescription
transactionID={transaction?.transactionID ?? '-1'}
report={moneyRequestReport}
policy={policy}
/>
),
};
}
if (hasAllPendingRTERViolations) {
return {icon: getStatusIcon(Expensicons.Hourglass), description: translate('iou.pendingMatchWithCreditCardDescription')};
}
Expand Down Expand Up @@ -336,7 +356,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
/>
</View>
)}
{hasAllPendingRTERViolations && !shouldUseNarrowLayout && (
{shouldShowMarkAsCashButton && !shouldUseNarrowLayout && (
<View style={[styles.pv2]}>
<Button
success
Expand Down Expand Up @@ -384,7 +404,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
isDisabled={shouldDisableSubmitButton}
/>
)}
{hasAllPendingRTERViolations && shouldUseNarrowLayout && (
{shouldShowMarkAsCashButton && shouldUseNarrowLayout && (
<Button
success
text={translate('iou.markAsCash')}
Expand Down
24 changes: 22 additions & 2 deletions src/components/MoneyRequestHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import variables from '@styles/variables';
import * as IOU from '@userActions/IOU';
Expand All @@ -18,6 +20,7 @@ import ROUTES from '@src/ROUTES';
import type {Policy, Report, ReportAction} from '@src/types/onyx';
import type IconAsset from '@src/types/utils/IconAsset';
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import BrokenConnectionDescription from './BrokenConnectionDescription';
import Button from './Button';
import HeaderWithBackButton from './HeaderWithBackButton';
import Icon from './Icon';
Expand Down Expand Up @@ -61,6 +64,11 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre

const hasAllPendingRTERViolations = TransactionUtils.allHavePendingRTERViolation([transaction?.transactionID ?? '-1']);

const shouldShowBrokenConnectionViolation = TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID ?? '-1', parentReport, policy);

const shouldShowMarkAsCashButton =
hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!PolicyUtils.isPolicyAdmin(policy) || ReportUtils.isCurrentUserSubmitter(parentReport?.reportID ?? '')));

const markAsCash = useCallback(() => {
TransactionActions.markAsCash(transaction?.transactionID ?? '-1', reportID ?? '');
}, [reportID, transaction?.transactionID]);
Expand All @@ -84,6 +92,18 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
if (TransactionUtils.isExpensifyCardTransaction(transaction) && TransactionUtils.isPending(transaction)) {
return {icon: getStatusIcon(Expensicons.CreditCardHourglass), description: translate('iou.transactionPendingDescription')};
}
if (shouldShowBrokenConnectionViolation) {
return {
icon: getStatusIcon(Expensicons.Hourglass),
description: (
<BrokenConnectionDescription
transactionID={transaction?.transactionID ?? '-1'}
report={report}
policy={policy}
/>
),
};
}
if (TransactionUtils.hasPendingRTERViolation(TransactionUtils.getTransactionViolations(transaction?.transactionID ?? '-1', transactionViolations))) {
return {icon: getStatusIcon(Expensicons.Hourglass), description: translate('iou.pendingMatchWithCreditCardDescription')};
}
Expand Down Expand Up @@ -137,7 +157,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
shouldDisplaySearchRouter
onBackButtonPress={onBackButtonPress}
>
{hasAllPendingRTERViolations && !shouldUseNarrowLayout && (
{shouldShowMarkAsCashButton && !shouldUseNarrowLayout && (
<Button
success
text={translate('iou.markAsCash')}
Expand All @@ -156,7 +176,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
/>
)}
</HeaderWithBackButton>
{hasAllPendingRTERViolations && shouldUseNarrowLayout && (
{shouldShowMarkAsCashButton && shouldUseNarrowLayout && (
<View style={[styles.ph5, styles.pb3]}>
<Button
success
Expand Down
4 changes: 2 additions & 2 deletions src/components/MoneyRequestHeaderStatusBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type {ReactNode} from 'react';
import type {ReactElement, ReactNode} from 'react';
import React from 'react';
import {View} from 'react-native';
import useThemeStyles from '@hooks/useThemeStyles';
Expand All @@ -9,7 +9,7 @@ type MoneyRequestHeaderStatusBarProps = {
icon: ReactNode;

/** Banner Description */
description: string;
description: string | ReactElement;

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we save the description as string itself, we can convert the prop into string before passing ? (saying cause we don't want the BE to throw some weird error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@allgandalf could you clarify what BE errors do you mean?
This component just renders the description display.
Since for the admin, the broken connection violation text can include TextLink (link to company cards), it returned as a component.

App/src/components/MoneyRequestHeaderStatusBar.tsx

Lines 18 to 28 in 91e299d

function MoneyRequestHeaderStatusBar({icon, description, shouldStyleFlexGrow = true}: MoneyRequestHeaderStatusBarProps) {
const styles = useThemeStyles();
return (
<View style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, shouldStyleFlexGrow && styles.flexGrow1, styles.overflowHidden, styles.headerStatusBarContainer]}>
<View style={styles.mr2}>{icon}</View>
<View style={[styles.flexShrink1]}>
<Text style={[styles.textLabelSupporting]}>{description}</Text>
</View>
</View>
);
}

/** Whether we style flex grow */
shouldStyleFlexGrow?: boolean;
Expand Down
5 changes: 5 additions & 0 deletions src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import * as IOUUtils from '@libs/IOUUtils';
import Navigation from '@libs/Navigation/Navigation';
import type {TransactionDuplicateNavigatorParamList} from '@libs/Navigation/types';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ReceiptUtils from '@libs/ReceiptUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
Expand Down Expand Up @@ -77,6 +78,7 @@ function MoneyRequestPreviewContent({
const [session] = useOnyx(ONYXKEYS.SESSION);
const [iouReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${iouReportID || '-1'}`);

const policy = PolicyUtils.getPolicy(iouReport?.policyID);
const isMoneyRequestAction = ReportActionsUtils.isMoneyRequestAction(action);
const transactionID = isMoneyRequestAction ? ReportActionsUtils.getOriginalMessage(action)?.IOUTransactionID : '-1';
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
Expand Down Expand Up @@ -248,6 +250,9 @@ function MoneyRequestPreviewContent({
if (TransactionUtils.isPending(transaction)) {
return {shouldShow: true, messageIcon: Expensicons.CreditCardHourglass, messageDescription: translate('iou.transactionPending')};
}
if (TransactionUtils.shouldShowBrokenConnectionViolation(transaction?.transactionID ?? '-1', iouReport, policy)) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('violations.brokenConnection530Error')};
}
if (TransactionUtils.hasPendingUI(transaction, TransactionUtils.getTransactionViolations(transaction?.transactionID ?? '-1', transactionViolations))) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('iou.pendingMatchWithCreditCard')};
}
Expand Down
10 changes: 7 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 @@ -171,15 +171,16 @@ function ReportPreview({
const showRTERViolationMessage =
numberOfRequests === 1 &&
TransactionUtils.hasPendingUI(allTransactions.at(0), TransactionUtils.getTransactionViolations(allTransactions.at(0)?.transactionID ?? '-1', transactionViolations));

const shouldShowBrokenConnectionViolation =
numberOfRequests === 1 && TransactionUtils.shouldShowBrokenConnectionViolation(allTransactions.at(0)?.transactionID ?? '-1', iouReport, policy);
let formattedMerchant = numberOfRequests === 1 ? TransactionUtils.getMerchant(allTransactions.at(0)) : null;
const formattedDescription = numberOfRequests === 1 ? TransactionUtils.getDescription(allTransactions.at(0)) : null;

if (TransactionUtils.isPartialMerchant(formattedMerchant ?? '')) {
formattedMerchant = null;
}

const shouldShowSubmitButton = isOpenExpenseReport && reimbursableSpend !== 0 && !showRTERViolationMessage;
const shouldShowSubmitButton = isOpenExpenseReport && reimbursableSpend !== 0 && !showRTERViolationMessage && !shouldShowBrokenConnectionViolation;
const shouldDisableSubmitButton = shouldShowSubmitButton && !ReportUtils.isAllowedToSubmitDraftExpenseReport(iouReport);

// The submit button should be success green colour only if the user is submitter and the policy does not have Scheduled Submit turned on
Expand Down Expand Up @@ -335,7 +336,7 @@ function ReportPreview({

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

const shouldShowSettlementButton = (shouldShowPayButton || shouldShowApproveButton) && !showRTERViolationMessage;
const shouldShowSettlementButton = (shouldShowPayButton || shouldShowApproveButton) && !showRTERViolationMessage && !shouldShowBrokenConnectionViolation;

const shouldPromptUserToAddBankAccount = ReportUtils.hasMissingPaymentMethod(userWallet, iouReportID) || ReportUtils.hasMissingInvoiceBankAccount(iouReportID);
const shouldShowRBR = hasErrors;
Expand Down Expand Up @@ -377,6 +378,9 @@ function ReportPreview({
if (shouldShowPendingSubtitle) {
return {shouldShow: true, messageIcon: Expensicons.CreditCardHourglass, messageDescription: translate('iou.transactionPending')};
}
if (shouldShowBrokenConnectionViolation) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('violations.brokenConnection530Error')};
}
if (showRTERViolationMessage) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('iou.pendingMatchWithCreditCard')};
}
Expand Down
9 changes: 8 additions & 1 deletion src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4688,7 +4688,10 @@ const translations = {
return message;
},
reviewRequired: 'Review required',
rter: ({brokenBankConnection, email, isAdmin, isTransactionOlderThan7Days, member}: ViolationsRterParams) => {
rter: ({brokenBankConnection, email, isAdmin, isTransactionOlderThan7Days, member, rterType}: ViolationsRterParams) => {
if (rterType === CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION_530 || rterType === CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION) {
return '';
}
if (brokenBankConnection) {
return isAdmin
? `Can't auto-match receipt due to broken bank connection which ${email} needs to fix`
Copy link
Contributor

Choose a reason for hiding this comment

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

When will we use these translations? Should we remove then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The updates were implemented in this PR.
It looks like it makes the error to be shown next to the merchant field on the Money Request View for rter violation.

Expand All @@ -4700,6 +4703,10 @@ const translations = {

return '';
},
brokenConnection530Error: 'Receipt pending due to broken bank connection.',
adminBrokenConnectionError: 'Receipt pending due to broken bank connection. Please resolve in ',
memberBrokenConnectionError: 'Receipt pending due to broken bank connection. Please ask a workspace admin to resolve.',
markAsCashToIgnore: 'Mark as cash to ignore and request payment.',
smartscanFailed: 'Receipt scanning failed. Enter details manually.',
someTagLevelsRequired: ({tagName}: ViolationsTagOutOfPolicyParams = {}) => `Missing ${tagName ?? 'Tag'}`,
tagOutOfPolicy: ({tagName}: ViolationsTagOutOfPolicyParams = {}) => `${tagName ?? 'Tag'} no longer valid`,
Expand Down
Loading
Loading