fix: prevent report field value from referencing itself #72375

NJ-2020 · 2025-10-11T09:26:41Z

Explanation of Change

Add validation to prevent formulas from referencing themselves

Fixed Issues

$ #70775
PROPOSAL: #70775 (comment)

Tests

Create a workspace if needed
Go to workspace settings, Reports, enable report fields if needed

Initial/Default value

Self Reference

Create new report field named "A"
Attempt to set it's initial value to "{field:A}"
Verify an error is displayed and you can't save that value

Circular Reference

2 circular/references

Create new report field named "A"
Set it's initial value to "{field:B}" (make sure {field:B} or field B is not created yet)
Verify you can save/create the field
Create new report field named "B"
Attempt to set it's initial value to "{field:A}"
Verify an error is displayed and you can't save that value

More than 2 circular/references

Create new report field named "A"
Set it's initial value to "{field:B}" (make sure {field:B} or field B is not created yet)
Verify you can save/create the field
Create new report field named "B"
Set it's initial value to "{field:C}" (make sure {field:C} or field C is not created yet)
Create new report field named "C"
Attempt to set it's initial value to "{field:A}"
Verify an error is displayed and you can't save that value

No references

Create new report field named "A"
Set it's initial value to "{field:B}" (make sure {field:B} is created and the default/initial value is set to empty > "" or no formula circular field)
Verify you can save/create the field

Report field value / report expense

Go to workspace expenses chat
Select/Open any report
Edit the report field value
Type formula value that references itself/circular reference i.e A, A > A, A > B > A
Verify an error is displayed and you can't save that value

Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.2025-10-16.at.10.online-video-cutter.com.mp4

Android: mWeb Chrome

Screen.Recording.2025-10-16.at.09.49.55.mp4

iOS: Native

Screen.Recording.2025-10-16.at.10.44.52.mp4

iOS: mWeb Safari

Screen.Recording.2025-10-16.at.10.49.38.mp4

MacOS: Chrome / Safari

Screen.Recording.2025-10-16.at.09.25.18.mp4

MacOS: Desktop

Screen.Recording.2025-10-16.at.09.18.15.mp4

codecov · 2025-10-11T09:41:27Z

Codecov Report

❌ Patch coverage is 69.23077% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/libs/Formula.ts	81.81%	6 Missing ⚠️
src/pages/EditReportFieldText.tsx	0.00%	3 Missing ⚠️
...workspace/reports/ReportFieldsInitialValuePage.tsx	0.00%	2 Missing ⚠️
...pages/workspace/reports/CreateReportFieldsPage.tsx	0.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
src/pages/EditReportFieldPage.tsx	`0.00% <ø> (ø)`
...pages/workspace/reports/CreateReportFieldsPage.tsx	`0.00% <0.00%> (ø)`
...workspace/reports/ReportFieldsInitialValuePage.tsx	`0.00% <0.00%> (ø)`
src/pages/EditReportFieldText.tsx	`0.00% <0.00%> (ø)`
src/libs/Formula.ts	`84.16% <81.81%> (-0.35%)`	⬇️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

NJ-2020 · 2025-10-11T09:53:00Z

I'll add unit test soon

melvin-bot · 2025-10-16T04:17:07Z

@dubielzyk-expensify @DylanDylann One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

dubielzyk-expensify · 2025-10-20T00:17:49Z

Videos are looking good to me 👍

neil-marcellini · 2025-10-27T13:52:52Z

I have bumped in Slack for a C+ review

abzokhattab · 2025-10-27T15:35:19Z

Reviewing ...

abzokhattab · 2025-10-27T16:22:16Z

@NJ-2020 I get this error in the second test. Do you also have it?

Screen.Recording.2025-10-27.at.17.18.51.mov

Error: undefined was not found in the en locale

The above error occurred in the <WorkspaceReportFieldsPage> component.

Can you please merge from main? and see if you still have the issue?

…cing

melvin-bot · 2025-10-28T03:36:49Z

Hey, I noticed you changed src/languages/en.ts in a PR from a fork. If you want to automatically generate translations for other locales, you can run the Generate static translations GitHub workflow.

Please look at the code and make sure there are no malicious changes before running the workflow.

If you have the K2 extension, you can simply click: [this button]

NJ-2020 · 2025-10-28T03:44:46Z

Can you please merge from main? and see if you still have the issue?

@abzokhattab Done, fixed ☑️

Screen.Recording.2025-10-28.at.10.42.15.mov

DylanDylann · 2025-10-28T10:26:18Z

We just added a new formula type. Today I will check again to see if there is any conflict with our change

DylanDylann · 2025-10-29T07:39:19Z

src/pages/workspace/reports/ReportFieldsInitialValuePage.tsx


    const hasAccountingConnections = hasAccountingConnectionsPolicyUtils(policy);
    const reportField = policy?.fieldList?.[getReportFieldKey(reportFieldID)] ?? null;
+


Suggested change

DylanDylann · 2025-10-29T08:00:41Z

src/pages/workspace/reports/CreateReportFieldsPage.tsx

                });
            }

+            if (type === CONST.REPORT_FIELD_TYPES.TEXT && hasCircularReferences(formInitialValue, name, policy?.fieldList)) {


Also need to check hasCircularReferences for fomula type

Done ☑️

DylanDylann · 2025-10-29T08:43:55Z

src/libs/Formula.ts

+    if (!fieldList || isEmptyObject(fieldList)) {
+        return formulaValues.some((formula) => {
+            const fieldPath = parsePart(formula).fieldPath;
+            return fieldPath.includes(fieldName);
+        });
+    }


This looks redundant because we always have a default report field

DylanDylann · 2025-10-29T08:54:51Z

src/libs/Formula.ts

+            }
+
+            // Get the referenced field name (first element in fieldPath)
+            const referencedFieldName = part.fieldPath.at(0);


Suggested change

const referencedFieldName = part.fieldPath.at(0);

const referencedFieldName = part.fieldPath.at(0)?.trim();

It works if we enter {field:A} but doesn't work if we enter {field: A}

--> need to trim the value

👍, Done ☑️

DylanDylann · 2025-10-29T09:02:57Z

src/libs/Formula.ts

+            if (referencedField?.defaultValue) {
+                // Recursively check the referenced field
+                if (hasCircularReferencesRecursive(referencedField.defaultValue, referencedFieldName)) {
+                    visitedLists.delete(currentFieldName);


Why do we need to delete the value in Set?

The visitedLists Set is function-scoped and short‑lived; it’s garbage‑collected after hasCircularReferences returns. @NJ-2020 Please correct me if you intend to do that for some reason?

DylanDylann · 2025-10-29T09:04:51Z

src/libs/Formula.ts

+/**
+ * Check if the report field formula value is containing circular references, e.g example:  A -> A,  A->B->A,  A->B->C->A, etc
+ */
+function hasCircularReferences(fieldValue: string, fieldName: string, fieldList?: FieldList) {


Please add returned type

Suggested change

function hasCircularReferences(fieldValue: string, fieldName: string, fieldList?: FieldList) {

function hasCircularReferences(fieldValue: string, fieldName: string, fieldList?: FieldList): boolean {

Thanks, done ☑️

DylanDylann · 2025-10-29T09:07:40Z

src/libs/Formula.ts

+                return true;
+            }
+
+            const referencedField = Object.values(fieldList).find((field) => field.name === referencedFieldName);


We can optimize this by creating a Map outside hasCircularReferencesRecursive function

const fieldsByName = new Map<string, {name: string; defaultValue: string}>( Object.values(fieldList).map((field) => [field.name, field]), );

and in here we should use get method

const referencedField = fieldsByName.get(referencedFieldName);

This reduces per-lookup from O(n) to O(1).

Thanks, done ☑️

DylanDylann · 2025-10-29T09:08:17Z

@NJ-2020 Left some comment

cc @neil-marcellini

melvin-bot · 2025-11-01T06:22:35Z

Hey, I noticed you changed src/languages/en.ts in a PR from a fork. For security reasons, translations are not generated automatically for PRs from forks.

If you want to automatically generate translations for other locales, an Expensify employee will have to:

Look at the code and make sure there are no malicious changes.
Run the Generate static translations GitHub workflow. If you have write access and the K2 extension, you can simply click: [this button]

Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running:

npx ts-node ./scripts/generateTranslations.ts --help

Typically, you'd want to translate only what you changed by running npx ts-node ./scripts/generateTranslations.ts --compare-ref main

NJ-2020 · 2025-11-01T07:54:32Z

Confirming 🇪🇸 translation here: https://expensify.slack.com/archives/C01GTK53T8Q/p1761983344629779

NJ-2020 added 2 commits October 11, 2025 15:56

fix: prevent report field value from referencing itself

b98bef6

fix edit report field

e48d3c7

melvin-bot bot assigned NJ-2020 Oct 11, 2025

NJ-2020 mentioned this pull request Oct 11, 2025

[CRN-QA] report field initial value set to formula referencing itself #70775

Open

fix: eslint checks

3c801ca

fix typecheck

fc85204

NJ-2020 and others added 6 commits October 15, 2025 14:38

Merge branch 'Expensify:main' into fix/70775-circular-referencing

91104cc

fix/update circular reference check validation

e547506

revert unnecessary changes & fix eslint error

7efc692

Merge branch 'Expensify:main' into fix/70775-circular-referencing

2a19811

fix: add unit test & eslint warning

eebe262

fix: cspell

9b62fef

NJ-2020 marked this pull request as ready for review October 16, 2025 04:17

NJ-2020 requested a review from a team as a code owner October 16, 2025 04:17

melvin-bot bot requested review from DylanDylann and dubielzyk-expensify and removed request for a team October 16, 2025 04:17

Merge branch 'Expensify:main' into fix/70775-circular-referencing

1a5b7db

neil-marcellini requested review from abzokhattab and removed request for DylanDylann October 27, 2025 14:04

neil-marcellini requested review from DylanDylann and removed request for abzokhattab October 27, 2025 22:02

Merge remote-tracking branch 'origin' into fix/70775-circular-referen…

e747e5b

…cing

DylanDylann reviewed Oct 29, 2025

View reviewed changes

Merge branch 'Expensify:main' into fix/70775-circular-referencing

5c03c01

NJ-2020 added 2 commits November 1, 2025 14:45

fix some issues, translations, improvements

648b914

fix: prettier

074e0e1


		const hasAccountingConnections = hasAccountingConnectionsPolicyUtils(policy);
		const reportField = policy?.fieldList?.[getReportFieldKey(reportFieldID)] ?? null;

	const referencedFieldName = part.fieldPath.at(0);
	const referencedFieldName = part.fieldPath.at(0)?.trim();

	function hasCircularReferences(fieldValue: string, fieldName: string, fieldList?: FieldList) {
	function hasCircularReferences(fieldValue: string, fieldName: string, fieldList?: FieldList): boolean {

fix: prevent report field value from referencing itself #72375

Are you sure you want to change the base?

fix: prevent report field value from referencing itself #72375

Conversation

NJ-2020 commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov bot commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NJ-2020 commented Oct 11, 2025

Uh oh!

melvin-bot bot commented Oct 16, 2025

Uh oh!

dubielzyk-expensify commented Oct 20, 2025

Uh oh!

neil-marcellini commented Oct 27, 2025

Uh oh!

abzokhattab commented Oct 27, 2025

Uh oh!

abzokhattab commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melvin-bot bot commented Oct 28, 2025

Uh oh!

NJ-2020 commented Oct 28, 2025

Uh oh!

DylanDylann commented Oct 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DylanDylann commented Oct 29, 2025

Uh oh!

melvin-bot bot commented Nov 1, 2025

Uh oh!

NJ-2020 commented Nov 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

NJ-2020 commented Oct 11, 2025 •

edited

Loading

codecov bot commented Oct 11, 2025 •

edited

Loading

abzokhattab commented Oct 27, 2025 •

edited

Loading