Skip to content

Fix custom report field disappearing for submitter after report approval #95337

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

Merged
JS00001 merged 5 commits into from
Jul 6, 2026
+119 −25
Merged

Fix custom report field disappearing for submitter after report approval #95337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dca57d4
Fix report fields disappearing for submitter after report approval
MelvinBot Jul 5, 2026
b58ae3f
Remove now-unused CONST import
MelvinBot Jul 5, 2026
42604eb
Add regression tests for report fields visibility after approval
MelvinBot Jul 5, 2026
ff93af8
Simplify report-field visibility conditions
MelvinBot Jul 5, 2026
7ddf814
Remove unneeded explanatory comments on report-field visibility
MelvinBot Jul 6, 2026
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: 2 additions & 11 deletions src/components/MoneyRequestReportView/MoneyRequestViewReportFields.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import {
getReportFieldMaps,
isGroupPolicyExpenseReport as isGroupPolicyExpenseReportUtils,
isInvoiceReport as isInvoiceReportUtils,
isReportFieldDisabled,
isReportFieldDisabledForUser,
isReportFieldOfTypeTitle,
shouldHideSingleReportField,
} from '@libs/ReportUtils';

Expand All @@ -41,9 +39,6 @@ type MoneyRequestViewReportFieldsProps = {
/** Policy that the report belongs to */
policy: OnyxEntry<Policy>;

/** Indicates whether the IOU report is a combined report */
isCombinedReport?: boolean;

/** Indicates whether we have any pending actions from parent component */
pendingAction?: PendingAction;
};
Expand Down Expand Up @@ -89,7 +84,7 @@ function ReportFieldView(reportField: EnrichedPolicyReportField, report: OnyxEnt
</OfflineWithFeedback>
);
}
function MoneyRequestViewReportFields({report, policy, isCombinedReport = false, pendingAction}: MoneyRequestViewReportFieldsProps) {
function MoneyRequestViewReportFields({report, policy, pendingAction}: MoneyRequestViewReportFieldsProps) {
const styles = useThemeStyles();
const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails();

Expand Down Expand Up @@ -121,14 +116,10 @@ function MoneyRequestViewReportFields({report, policy, isCombinedReport = false,
});
}, [policy, report, currentUserAccountID]);

const enabledReportFields = sortedPolicyReportFields.filter(
(reportField) => !isReportFieldDisabled(report, reportField, policy) || reportField.type === CONST.REPORT_FIELD_TYPES.FORMULA,
);
const isOnlyTitleFieldEnabled = enabledReportFields.length === 1 && isReportFieldOfTypeTitle(enabledReportFields.at(0));
const isGroupPolicyExpenseReport = isGroupPolicyExpenseReportUtils(report);
const isInvoiceReport = isInvoiceReportUtils(report);

const shouldDisplayReportFields = (isGroupPolicyExpenseReport || isInvoiceReport) && !!policy?.areReportFieldsEnabled && (!isOnlyTitleFieldEnabled || !isCombinedReport);
const shouldDisplayReportFields = (isGroupPolicyExpenseReport || isInvoiceReport) && !!policy?.areReportFieldsEnabled;

if (!shouldDisplayReportFields || !sortedPolicyReportFields.length) {
return null;
Expand Down
15 changes: 2 additions & 13 deletions src/components/ReportActionItem/MoneyReportView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ import {
isClosedExpenseReportWithNoExpenses as isClosedExpenseReportWithNoExpensesReportUtils,
isGroupPolicyExpenseReport as isGroupPolicyExpenseReportUtils,
isInvoiceReport as isInvoiceReportUtils,
isReportFieldDisabled,
isReportFieldDisabledForUser,
isReportFieldOfTypeTitle,
isSettled as isSettledReportUtils,
shouldHideSingleReportField,
} from '@libs/ReportUtils';
Expand All @@ -45,7 +43,6 @@ import AnimatedEmptyStateBackground from '@pages/inbox/report/AnimatedEmptyState

import variables from '@styles/variables';

import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import {clearReportFieldKeyErrors} from '@src/libs/actions/Report';
import {DYNAMIC_ROUTES} from '@src/ROUTES';
Expand Down Expand Up @@ -154,20 +151,12 @@ function MoneyReportView({
return {sortedPolicyReportFields: sorted, fieldValues: values, fieldsByName: byName};
}, [policy?.fieldList, report]);

const enabledReportFields = sortedPolicyReportFields.filter(
(reportField) => !isReportFieldDisabled(report, reportField, policy) || reportField.type === CONST.REPORT_FIELD_TYPES.FORMULA,
);
const isOnlyTitleFieldEnabled = enabledReportFields.length === 1 && isReportFieldOfTypeTitle(enabledReportFields.at(0));
const isOnlyTitleFieldEnabled = sortedPolicyReportFields.every(shouldHideSingleReportField);
const isClosedExpenseReportWithNoExpenses = isClosedExpenseReportWithNoExpensesReportUtils(report);
const isGroupPolicyExpenseReport = isGroupPolicyExpenseReportUtils(report);

@github-actions github-actions Bot Jul 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❌ CONSISTENCY-3 (docs)

After this change isOnlyTitleFieldEnabled is defined as sortedPolicyReportFields.every(shouldHideSingleReportField), which is exactly the expression still repeated inside shouldShowReportField on the line !sortedPolicyReportFields.every(shouldHideSingleReportField). That term now duplicates the derived isOnlyTitleFieldEnabled — and the same shouldShowReportField expression already includes !isOnlyTitleFieldEnabled, so the .every(...) call recomputes an identical predicate over the same array.

Reuse the derived value instead of recomputing it. Since shouldShowReportField already contains (!isCombinedReport || !isOnlyTitleFieldEnabled), the extra .every(...) term collapses to !isOnlyTitleFieldEnabled and can be dropped:

const isOnlyTitleFieldEnabled = sortedPolicyReportFields.every(shouldHideSingleReportField);
// ...
const shouldShowReportField =
    !isClosedExpenseReportWithNoExpenses &&
    (isGroupPolicyExpenseReport || isInvoiceReport) &&
    !!policy?.areReportFieldsEnabled &&
    (!isCombinedReport || !isOnlyTitleFieldEnabled) &&
    !isOnlyTitleFieldEnabled;

(the last two lines can be merged into a single !isOnlyTitleFieldEnabled).

Reviewed at: 42604eb | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

const isInvoiceReport = isInvoiceReportUtils(report);

const shouldShowReportField =
!isClosedExpenseReportWithNoExpenses &&
(isGroupPolicyExpenseReport || isInvoiceReport) &&
!!policy?.areReportFieldsEnabled &&
(!isCombinedReport || !isOnlyTitleFieldEnabled) &&
!sortedPolicyReportFields.every(shouldHideSingleReportField);
const shouldShowReportField = !isClosedExpenseReportWithNoExpenses && (isGroupPolicyExpenseReport || isInvoiceReport) && !!policy?.areReportFieldsEnabled && !isOnlyTitleFieldEnabled;

const hasPendingAction = transactions.some(getTransactionPendingAction);

Expand Down
116 changes: 115 additions & 1 deletion tests/ui/MoneyReportViewTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ jest.mock('@hooks/useLocalize', () =>
})),
);

jest.mock('@hooks/useScreenWrapperTransitionStatus', () => ({
__esModule: true,
default: () => ({
didScreenTransitionEnd: true,
}),
}));

jest.mock('@react-navigation/native', () => ({
...((): typeof NativeNavigation => jest.requireActual('@react-navigation/native'))(),
useNavigation: jest.fn(() => ({navigate: jest.fn(), addListener: jest.fn(() => jest.fn())})),
Expand Down Expand Up @@ -89,14 +96,15 @@ const seedReportAndTransactions = async (transactions: OnyxTypes.Transaction[],
await waitForBatchedUpdatesWithAct();
};

const renderMoneyReportView = (report: OnyxTypes.Report, policy: OnyxTypes.Policy | undefined = undefined) =>
const renderMoneyReportView = (report: OnyxTypes.Report, policy: OnyxTypes.Policy | undefined = undefined, isCombinedReport = false) =>
render(
<ComposeProviders components={[OnyxListItemProvider]}>
<MoneyReportView
report={report}
policy={policy}
shouldHideThreadDividerLine={false}
shouldShowAnimatedBackground={false}
isCombinedReport={isCombinedReport}
/>
</ComposeProviders>,
);
Expand Down Expand Up @@ -264,3 +272,109 @@ describe('MoneyReportView reimbursable/non-reimbursable breakdown rows', () => {
});
});
});

describe('MoneyReportView report fields visibility', () => {
const customFieldKey = `expensify_${'field_test'}` as const;

const buildTitleField = (): OnyxTypes.PolicyReportField => ({
fieldID: CONST.REPORT_FIELD_TITLE_FIELD_ID,
name: 'title',
type: CONST.REPORT_FIELD_TYPES.FORMULA,
target: 'expense',
orderWeight: 1,
deletable: false,
defaultValue: '{report:type} {report:startdate}',
value: 'Expense Report',
values: [],
keys: [],
externalIDs: [],
disabledOptions: [],
isTax: false,
});

const buildCustomTextField = (): OnyxTypes.PolicyReportField => ({
fieldID: 'field_test',
name: 'Test',
type: CONST.REPORT_FIELD_TYPES.TEXT,
target: 'expense',
orderWeight: 2,
deletable: true,
defaultValue: '',
value: '1',
values: [],
keys: [],
externalIDs: [],
disabledOptions: [],
isTax: false,
});

const buildReportFieldsPolicy = (fieldList: Record<string, OnyxTypes.PolicyReportField>): OnyxTypes.Policy =>
({
...LHNTestUtils.getFakePolicy(policyID, 'Policy'),
// A non-admin submitter: report fields become read-only (not editable) after approval.
role: CONST.POLICY.ROLE.USER,
type: CONST.POLICY.TYPE.TEAM,
outputCurrency: CONST.CURRENCY.USD,
areReportFieldsEnabled: true,
fieldList,
}) as OnyxTypes.Policy;

const seedReportFieldsPolicy = async (policy: OnyxTypes.Policy, report: OnyxTypes.Report) => {
await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policy);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, report);
});
await waitForBatchedUpdatesWithAct();
};

beforeAll(() => {
Onyx.init({keys: ONYXKEYS, evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS]});
});

afterEach(async () => {
await act(async () => {
await Onyx.clear();
});
});

it('keeps a custom report field visible for a non-admin submitter after the report is approved (single-expense combined view)', async () => {
const fieldList = {
[CONST.REPORT_FIELD_TITLE_FIELD_ID]: buildTitleField(),
[customFieldKey]: buildCustomTextField(),
};
const policy = buildReportFieldsPolicy(fieldList);
const approvedReport = buildExpenseReport({
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
statusNum: CONST.REPORT.STATUS_NUM.APPROVED,
fieldList,
});
await seedReportFieldsPolicy(policy, approvedReport);

renderMoneyReportView(approvedReport, policy, true);
await waitForBatchedUpdatesWithAct();

// The custom field (rendered read-only after approval) must still show for the submitter.
await waitFor(() => {
expect(screen.getByText('Test')).toBeOnTheScreen();
});
});

it('hides the report-fields section in the combined view when only the title field is configured', async () => {
const fieldList = {[CONST.REPORT_FIELD_TITLE_FIELD_ID]: buildTitleField()};
const policy = buildReportFieldsPolicy(fieldList);
const approvedReport = buildExpenseReport({
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
statusNum: CONST.REPORT.STATUS_NUM.APPROVED,
fieldList,
});
await seedReportFieldsPolicy(policy, approvedReport);

renderMoneyReportView(approvedReport, policy, true);
await waitForBatchedUpdatesWithAct();

// The title field is shown in the report header for a combined report, so the redundant section stays hidden.
await waitFor(() => {
expect(screen.queryByText('title')).not.toBeOnTheScreen();
});
});
});
Loading