Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 47 additions & 2 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import DotIndicatorMessage from '@components/DotIndicatorMessage';
import Icon from '@components/Icon';
import MenuItem from '@components/MenuItem';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import {ModalActions} from '@components/Modal/Global/ModalContext';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import {usePolicyCategories, usePolicyTags} from '@components/OnyxListItemProvider';
import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView';
Expand All @@ -17,6 +18,7 @@ import ViolationMessages from '@components/ViolationMessages';
import {useWideRHPState} from '@components/WideRHPContextProvider';
import useActiveRoute from '@hooks/useActiveRoute';
import useCardFeedErrors from '@hooks/useCardFeedErrors';
import useConfirmModal from '@hooks/useConfirmModal';
import {useCurrencyListActions} from '@hooks/useCurrencyList';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useEnvironment from '@hooks/useEnvironment';
Expand All @@ -35,7 +37,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import type {ViolationField} from '@hooks/useViolations';
import useViolations from '@hooks/useViolations';
import {updateMoneyRequestBillable, updateMoneyRequestReimbursable} from '@libs/actions/IOU/UpdateMoneyRequest';
import {updateMoneyRequestBillable, updateMoneyRequestReimbursable, updateMoneyRequestTaxRate} from '@libs/actions/IOU/UpdateMoneyRequest';
import initSplitExpense from '@libs/actions/SplitExpenses';
import {getIsMissingAttendeesViolation} from '@libs/AttendeeUtils';
import {getBrokenConnectionUrlToFixPersonalCard, getCompanyCardDescription} from '@libs/CardUtils';
Expand Down Expand Up @@ -177,6 +179,7 @@ function MoneyRequestView({
const {translate, toLocaleDigit} = useLocalize();
const {getCurrencySymbol} = useCurrencyListActions();
const {getReportRHPActiveRoute} = useActiveRoute();
const {showConfirmModal} = useConfirmModal();
const [lastVisitedPath] = useOnyx(ONYXKEYS.LAST_VISITED_PATH);

const {currentSearchResults} = useSearchStateContext();
Expand Down Expand Up @@ -437,7 +440,6 @@ function MoneyRequestView({
});
const shouldShowAttendees = shouldShowAttendeesTransactionUtils(iouType, policy);

const shouldShowTax = isTaxTrackingEnabled(isPolicyExpenseChat || isExpenseUnreported, policy, isDistanceRequest, isPerDiemRequest, isTimeRequest) || !!transaction?.taxName;
const tripID = getTripIDFromTransactionParentReportID(parentReport?.parentReportID);
const shouldShowViewTripDetails = hasReservationList(transaction) && !!tripID;

Expand All @@ -448,6 +450,10 @@ function MoneyRequestView({
// Need to return undefined when we have pendingAction to avoid the duplicate pending action
const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => (pendingAction ? undefined : transaction?.pendingFields?.[fieldPath]);

const isTaxEnabled = isTaxTrackingEnabled(isPolicyExpenseChat || isExpenseUnreported, policy, isDistanceRequest, isPerDiemRequest, isTimeRequest);
const shouldShowTaxDisabledAlert = !isTaxEnabled && !!transaction?.taxCode;
const shouldShowTax = isTaxEnabled || !!transaction?.taxName || shouldShowTaxDisabledAlert;
Comment on lines +454 to +455

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include all tax-data shapes when gating tax rows

shouldShowTaxDisabledAlert/shouldShowTax only look at transaction.taxCode and transaction.taxName, but this commit also made TAX_OUT_OF_POLICY fire when any tax data exists (taxCode or taxValue or taxAmount) in ViolationsUtils. In transactions that have taxValue/taxAmount without a taxCode, users can now get a tax violation while both tax rows stay hidden, so there is no in-UI path to clear the invalid tax fields.

Useful? React with 👍 / 👎.


let amountDescription = `${translate('iou.amount')}`;
let dateDescription = `${translate('common.date')}`;

Expand Down Expand Up @@ -634,6 +640,35 @@ function MoneyRequestView({
return '';
};

const showTaxDisabledAlert = () => {
showConfirmModal({
title: translate('iou.taxDisabledAlert.title'),
prompt: translate('iou.taxDisabledAlert.prompt'),
confirmText: translate('iou.taxDisabledAlert.confirmText'),
cancelText: translate('common.cancel'),
}).then(({action}) => {
if (action !== ModalActions.CONFIRM || !canEditTaxFields) {
return;
}

updateMoneyRequestTaxRate({
transactionID: transaction?.transactionID,
transactionThreadReport,
parentReport,
taxCode: '',
taxValue: '',
taxAmount: 0,
policy,
policyTagList,
policyCategories,
currentUserAccountIDParam,
currentUserEmailParam,
isASAPSubmitBetaEnabled,
parentReportNextStep,
});
});
};

const distanceCopyValue = !canEditDistance ? distanceToDisplay : undefined;
const distanceRateCopyValue = !canEditDistanceRate ? rateToDisplay : undefined;
const amountCopyValue = !canEditAmount ? amountTitle : undefined;
Expand Down Expand Up @@ -1078,6 +1113,11 @@ function MoneyRequestView({
shouldShowRightIcon={canEditTaxFields}
titleStyle={styles.flex1}
onPress={() => {
if (shouldShowTaxDisabledAlert) {
showTaxDisabledAlert();
return;
}

Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_TAX_RATE.getRoute(
CONST.IOU.ACTION.EDIT,
Expand All @@ -1104,6 +1144,11 @@ function MoneyRequestView({
shouldShowRightIcon={canEditTaxFields}
titleStyle={styles.flex1}
onPress={() => {
if (shouldShowTaxDisabledAlert) {
showTaxDisabledAlert();
return;
}

Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_TAX_AMOUNT.getRoute(
CONST.IOU.ACTION.EDIT,
Expand Down
5 changes: 5 additions & 0 deletions src/languages/de.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1622,6 +1622,11 @@ const translations: TranslationDeepObject<typeof en> = {
failedToApproveViaDEW: (reason: string) => `Genehmigung fehlgeschlagen. ${reason}`,
cannotDuplicateDistanceExpense:
'Sie können Entfernungsausgaben nicht über mehrere Arbeitsbereiche hinweg duplizieren, da sich die Sätze zwischen den Arbeitsbereichen unterscheiden können.',
taxDisabledAlert: {
title: 'Steuer deaktiviert',
prompt: 'Aktivieren Sie die Steuerverfolgung im Workspace, um die Ausgabendetails zu bearbeiten oder die Steuer aus dieser Ausgabe zu löschen.',
confirmText: 'Steuer löschen',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1668,6 +1668,11 @@ const translations = {
},
duplicateNonDefaultWorkspacePerDiemError: "You can't duplicate per diem expenses across workspaces because the rates may differ between workspaces.",
cannotDuplicateDistanceExpense: "You can't duplicate distance expenses across workspaces because the rates may differ between workspaces.",
taxDisabledAlert: {
title: 'Tax disabled',
prompt: 'Enable tax tracking on the workspace to edit the expense details or delete the tax from this expense.',
confirmText: 'Delete tax',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1555,6 +1555,11 @@ const translations: TranslationDeepObject<typeof en> = {
},
duplicateNonDefaultWorkspacePerDiemError: 'No puedes duplicar gastos de viáticos entre espacios de trabajo porque las tarifas pueden variar entre ellos.',
cannotDuplicateDistanceExpense: 'No puedes duplicar gastos de distancia entre espacios de trabajo porque las tasas pueden diferir entre espacios de trabajo.',
taxDisabledAlert: {
title: 'Impuesto deshabilitado',
prompt: 'Habilita el seguimiento de impuestos en el espacio de trabajo para editar los detalles del gasto o eliminar el impuesto de este gasto.',
confirmText: 'Eliminar impuesto',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/fr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1627,6 +1627,11 @@ const translations: TranslationDeepObject<typeof en> = {
`impossible d’approuver via les <a href="${CONST.CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL}">règles de l’espace de travail</a>. ${reason}`,
failedToApproveViaDEW: (reason: string) => `échec de l’approbation. ${reason}`,
cannotDuplicateDistanceExpense: 'Vous ne pouvez pas dupliquer des dépenses de distance entre espaces de travail, car les taux peuvent différer d’un espace de travail à l’autre.',
taxDisabledAlert: {
title: 'Taxe désactivée',
prompt: 'Activez le suivi des taxes dans l’espace de travail pour modifier les détails de la dépense ou supprimer la taxe de cette dépense.',
confirmText: 'Supprimer la taxe',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/it.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,11 @@ const translations: TranslationDeepObject<typeof en> = {
`approvazione non riuscita tramite le <a href="${CONST.CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL}">regole dello spazio di lavoro</a>. ${reason}`,
failedToApproveViaDEW: (reason: string) => `approvazione non riuscita. ${reason}`,
cannotDuplicateDistanceExpense: 'Non puoi duplicare le spese chilometriche tra diversi spazi di lavoro perché le tariffe potrebbero essere diverse.',
taxDisabledAlert: {
title: 'Imposta disattivata',
prompt: 'Abilita il monitoraggio delle imposte nello spazio di lavoro per modificare i dettagli della spesa o eliminare l’imposta da questa spesa.',
confirmText: 'Elimina imposta',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/ja.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,11 @@ const translations: TranslationDeepObject<typeof en> = {
failedToAutoApproveViaDEW: (reason: string) => `<a href="${CONST.CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL}">ワークスペースルール</a>で承認に失敗しました。${reason}`,
failedToApproveViaDEW: (reason: string) => `承認に失敗しました。${reason}`,
cannotDuplicateDistanceExpense: '距離精算はワークスペースごとにレートが異なる可能性があるため、ワークスペース間で複製することはできません。',
taxDisabledAlert: {
title: '税が無効です',
prompt: '経費の詳細を編集したり、この経費から税金を削除したりするには、ワークスペースで税金の追跡を有効にしてください。',
confirmText: '税を削除',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/nl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,11 @@ const translations: TranslationDeepObject<typeof en> = {
failedToAutoApproveViaDEW: (reason: string) => `goedkeuren via <a href="${CONST.CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL}">werkruimte­regels</a> is mislukt. ${reason}`,
failedToApproveViaDEW: (reason: string) => `goedkeuren mislukt. ${reason}`,
cannotDuplicateDistanceExpense: 'Je kunt afstandsvergoedingen niet dupliceren tussen werkruimtes, omdat de tarieven per werkruimte kunnen verschillen.',
taxDisabledAlert: {
title: 'Belasting uitgeschakeld',
prompt: 'Schakel belastingregistratie in voor de workspace om de onkostendetails te bewerken of de belasting uit deze onkostendeclaratie te verwijderen.',
confirmText: 'Belasting verwijderen',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/pl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,11 @@ const translations: TranslationDeepObject<typeof en> = {
`nie udało się zatwierdzić przez <a href="${CONST.CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL}">zasady w przestrzeni roboczej</a>. ${reason}`,
failedToApproveViaDEW: (reason: string) => `nie udało się zaakceptować. ${reason}`,
cannotDuplicateDistanceExpense: 'Nie możesz duplikować wydatków za przejazdy między przestrzeniami roboczymi, ponieważ stawki mogą się różnić między poszczególnymi przestrzeniami.',
taxDisabledAlert: {
title: 'Podatek wyłączony',
prompt: 'Włącz śledzenie podatku w przestrzeni roboczej, aby edytować szczegóły wydatku lub usunąć podatek z tego wydatku.',
confirmText: 'Usuń podatek',
},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 5 additions & 0 deletions src/languages/pt-BR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,11 @@ const translations: TranslationDeepObject<typeof en> = {
failedToAutoApproveViaDEW: (reason: string) => `falha ao aprovar pelas <a href="${CONST.CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL}">regras do workspace</a>. ${reason}`,
failedToApproveViaDEW: (reason: string) => `falha ao aprovar. ${reason}`,
cannotDuplicateDistanceExpense: 'Você não pode duplicar despesas de distância entre espaços de trabalho porque as tarifas podem ser diferentes entre eles.',
taxDisabledAlert: {
title: 'Imposto desativado',
prompt: 'Ative o acompanhamento de impostos no espaço de trabalho para editar os detalhes da despesa ou excluir o imposto desta despesa.',
confirmText: 'Excluir imposto',
},
},
transactionMerge: {
listPage: {
Expand Down
1 change: 1 addition & 0 deletions src/languages/zh-hans.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1568,6 +1568,7 @@ const translations: TranslationDeepObject<typeof en> = {
failedToAutoApproveViaDEW: (reason: string) => `未能通过<a href="${CONST.CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL}">工作区规则</a>批准。${reason}`,
failedToApproveViaDEW: (reason: string) => `批准失败。${reason}`,
cannotDuplicateDistanceExpense: '你无法在不同工作区之间复制里程报销,因为各个工作区的费率可能不同。',
taxDisabledAlert: {title: '税费已禁用', prompt: '请在工作区中启用税费跟踪,以便编辑此报销的详细信息或从该报销中删除税费。', confirmText: '删除税费'},
},
transactionMerge: {
listPage: {
Expand Down
5 changes: 0 additions & 5 deletions src/libs/MergeTransactionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,6 @@ function getMergeableDataAndConflictFields(
const isTargetValueEmpty = isEmptyMergeValue(targetValue);
const isSourceValueEmpty = isEmptyMergeValue(sourceValue);

// Temporarily skip merging tax value if either policy has tax tracking disabled until we handle in https://github.com/Expensify/App/issues/83157
if (field === 'taxValue' && (!targetTransactionPolicy?.tax?.trackingEnabled || !sourceTransactionPolicy?.tax?.trackingEnabled)) {
continue;
}

if (field === 'amount') {
// If target transaction is a card or split expense, always preserve the target transaction's amount and currency
// Card takes precedence over split expense
Expand Down
7 changes: 5 additions & 2 deletions src/libs/Violations/ViolationsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -655,11 +655,14 @@ const ViolationsUtils = {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_ATTENDEES});
}

if (isPolicyTrackTaxEnabled && !hasTaxOutOfPolicyViolation && !isTaxInPolicy) {
const hasTransactionTaxData = !!updatedTransaction.taxCode || !!updatedTransaction.taxValue || !!updatedTransaction.taxAmount;
const shouldAddTaxOutOfPolicy = isPolicyTrackTaxEnabled ? !isTaxInPolicy : hasTransactionTaxData;

if (!hasTaxOutOfPolicyViolation && shouldAddTaxOutOfPolicy) {
newTransactionViolations.push({name: CONST.VIOLATIONS.TAX_OUT_OF_POLICY, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true});
}

if (isPolicyTrackTaxEnabled && hasTaxOutOfPolicyViolation && isTaxInPolicy) {
if (hasTaxOutOfPolicyViolation && !shouldAddTaxOutOfPolicy) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.TAX_OUT_OF_POLICY});
}
return {
Expand Down
46 changes: 46 additions & 0 deletions tests/ui/MoneyRequestViewTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,52 @@ describe('MoneyRequestView edit fields', () => {
});
});

it('should show tax fields when tax tracking is disabled but transaction has tax data', async () => {
const threadReport = {
...LHNTestUtils.getFakeReport(),
parentReportID: expenseReportID,
parentReportActionID,
};

await setupTestData();

await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {
taxCode: 'TAX_10',
taxAmount: 500,
taxValue: '10%',
});
});
await waitForBatchedUpdatesWithAct();

renderMoneyRequestView(threadReport, {tax: {trackingEnabled: false}});
await waitForBatchedUpdatesWithAct();

await waitFor(() => {
expect(screen.getByTestId('menu-item-common.tax')).toBeOnTheScreen();
expect(screen.getByTestId('menu-item-iou.taxAmount')).toBeOnTheScreen();
});
});

it('should not show tax fields when tax tracking is disabled and transaction has no tax data', async () => {
const threadReport = {
...LHNTestUtils.getFakeReport(),
parentReportID: expenseReportID,
parentReportActionID,
};

await setupTestData();
await waitForBatchedUpdatesWithAct();

renderMoneyRequestView(threadReport, {tax: {trackingEnabled: false}});
await waitForBatchedUpdatesWithAct();

await waitFor(() => {
expect(screen.queryByTestId('menu-item-common.tax')).not.toBeOnTheScreen();
expect(screen.queryByTestId('menu-item-iou.taxAmount')).not.toBeOnTheScreen();
});
});

it('should show amount and merchant as readonly when report is settled', async () => {
const threadReport = {
...LHNTestUtils.getFakeReport(),
Expand Down
4 changes: 4 additions & 0 deletions tests/unit/MergeTransactionUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ describe('MergeTransactionUtils', () => {
tag: 'Same Tag',
billable: false,
attendees: [],
taxCode: undefined,
taxName: '',
taxPolicyID: undefined,
taxValue: undefined,
});
});

Expand Down
Loading
Loading