Repository navigation
fix: Report - Formula report field initial value {report:i} is not persisted & resets to {report:id} #84207
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
fix: Report - Formula report field initial value {report:i} is not persisted & resets to {report:id} #84207
Changes from all commits
d0979fb
9e3f922
e61b92b
6a0adb3
85175c3
564ca90
b70c6d9
bf17a99
c0d5726
0a97bdd
2bfac9e
bec0726
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import type ONYXKEYS from '@src/ONYXKEYS'; | |
| import type {InputID} from '@src/types/form/WorkspaceReportFieldForm'; | ||
| import type {PolicyReportField, PolicyReportFieldType} from '@src/types/onyx/Policy'; | ||
| import {addErrorMessage} from './ErrorUtils'; | ||
| import type {FormulaPart} from './Formula'; | ||
| import {isRequiredFulfilled} from './ValidationUtils'; | ||
|
|
||
| /** | ||
|
|
@@ -96,7 +97,7 @@ function hasFormulaPartsInInitialValue(initialValue?: string): boolean { | |
|
|
||
| // Dynamically require to avoid circular dependency with ReportActionsUtils | ||
| const {parse, FORMULA_PART_TYPES} = require('./Formula') as { | ||
| parse: (formula?: string) => Array<{type: string}>; | ||
| parse: (formula?: string) => FormulaPart[]; | ||
| FORMULA_PART_TYPES: {FREETEXT: string}; | ||
| }; | ||
| return parse(initialValue).some((part) => part.type !== FORMULA_PART_TYPES.FREETEXT); | ||
|
|
@@ -109,12 +110,112 @@ function isReportFieldNameExisting(fieldList: Record<string, PolicyReportField> | |
| return Object.values(fieldList ?? {}).some((reportField) => reportField.name.toLowerCase() === fieldName.toLowerCase()); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the list of unsupported {report:*} formula parts in the initial value. | ||
| * Used to validate formula report fields so unsupported tokens (e.g. {report:i}) are rejected with a clear error. | ||
| */ | ||
| function getUnsupportedReportFieldFormulaParts(initialValue?: string): string[] { | ||
| if (!initialValue || typeof initialValue !== 'string') { | ||
| return []; | ||
| } | ||
|
|
||
| // Dynamically require to avoid circular dependency with ReportActionsUtils | ||
| const {parse, FORMULA_PART_TYPES} = require('./Formula') as { | ||
| parse: (formula?: string) => FormulaPart[]; | ||
| FORMULA_PART_TYPES: {REPORT: string}; | ||
| }; | ||
|
|
||
| // cspell:ignore oldid | ||
| const supportedReportFields = new Set([ | ||
| 'id', | ||
| 'oldid', | ||
| 'title', | ||
| 'status', | ||
| 'displaystatus', | ||
| 'expensescount', | ||
| 'type', | ||
| 'startdate', | ||
| 'enddate', | ||
| 'total', | ||
| 'reimbursable', | ||
| 'currency', | ||
| 'policyname', | ||
| 'workspacename', | ||
| 'created', | ||
| 'approve', | ||
| 'submit', | ||
| 'autoreporting', | ||
| ]); | ||
|
|
||
| const supportedSubmitDirections = new Set(['from', 'to', 'date']); | ||
| const supportedSubmitPersonFields = new Set(['firstname', 'lastname', 'fullname', 'email', 'userid', 'customfield1', 'payrollid', 'customfield2']); | ||
| const supportedAutoReportingFields = new Set(['start', 'end']); | ||
|
|
||
| const unsupported: string[] = []; | ||
| const parts = parse(initialValue); | ||
|
|
||
| for (const part of parts) { | ||
| if (part.type !== FORMULA_PART_TYPES.REPORT) { | ||
| continue; | ||
| } | ||
|
|
||
| const [field, ...rest] = part.fieldPath ?? []; | ||
| const normalizedField = field?.trim().toLowerCase(); | ||
|
|
||
| if (!normalizedField || !supportedReportFields.has(normalizedField)) { | ||
| unsupported.push(part.definition); | ||
| continue; | ||
| } | ||
|
|
||
| if (normalizedField === 'submit') { | ||
| const direction = rest.at(0)?.trim().toLowerCase(); | ||
| if (!direction || !supportedSubmitDirections.has(direction)) { | ||
| unsupported.push(part.definition); | ||
| continue; | ||
| } | ||
|
|
||
| // report:submit:from and report:submit:to are valid without subfield. | ||
| if ((direction === 'from' || direction === 'to') && rest.length === 1) { | ||
| continue; | ||
| } | ||
|
|
||
| // report:submit:date is valid (format is optional and validated elsewhere) | ||
| if (direction === 'date') { | ||
| continue; | ||
| } | ||
|
|
||
| // report:submit:from:* and report:submit:to:* need a subfield | ||
| const submitField = rest.at(1)?.trim().toLowerCase(); | ||
| if (!submitField || !supportedSubmitPersonFields.has(submitField)) { | ||
|
Comment on lines
+188
to
+189
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This validation now treats Useful? React with 👍 / 👎. |
||
| unsupported.push(part.definition); | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| if (normalizedField === 'autoreporting') { | ||
| const subField = rest.at(0)?.trim().toLowerCase(); | ||
| if (!subField || !supportedAutoReportingFields.has(subField)) { | ||
| unsupported.push(part.definition); | ||
| } | ||
| } | ||
|
|
||
| if (normalizedField === 'approve') { | ||
| if (rest.at(0)?.trim().toLowerCase() !== 'date') { | ||
| unsupported.push(part.definition); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return unsupported; | ||
| } | ||
|
|
||
| export { | ||
| getReportFieldTypeTranslationKey, | ||
| getReportFieldAlternativeTextTranslationKey, | ||
| validateReportFieldListValueName, | ||
| generateFieldID, | ||
| getReportFieldInitialValue, | ||
| getUnsupportedReportFieldFormulaParts, | ||
| hasFormulaPartsInInitialValue, | ||
| isReportFieldNameExisting, | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we're missing some accepted formulas. Can you check here and test adding them?
https://help.expensify.com/articles/expensify-classic/spending-insights/Export-Expenses-And-Reports#formulas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @linhvovan29546
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we're only validating the formula here