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
9 changes: 6 additions & 3 deletions src/libs/Formula.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const FORMULA_PART_TYPES = {
* Extract formula parts from a formula string, handling nested braces and escapes
* Based on OldDot Formula.extract method
*/
function extract(formula: string, opener = '{', closer = '}'): string[] {
function extract(formula?: string, opener = '{', closer = '}'): string[] {
if (!formula || typeof formula !== 'string') {
return [];
}
Expand Down Expand Up @@ -79,7 +79,7 @@ function extract(formula: string, opener = '{', closer = '}'): string[] {
* Parse a formula string into an array of formula parts
* Based on OldDot Formula.parse method
*/
function parse(formula: string): FormulaPart[] {
function parse(formula?: string): FormulaPart[] {
if (!formula || typeof formula !== 'string') {
return [];
}
Expand Down Expand Up @@ -191,10 +191,13 @@ function parsePart(definition: string): FormulaPart {
/**
* Compute the value of a formula given a context
*/
function compute(formula: string, context: FormulaContext): string {
function compute(formula?: string, context?: FormulaContext): string {
if (!formula || typeof formula !== 'string') {
return '';
}
if (!context) {
return '';
}

const parts = parse(formula);
let result = '';
Expand Down
11 changes: 4 additions & 7 deletions src/libs/OptimisticReportNames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ function shouldComputeReportName(report: Report, context: UpdateContext): boolea
}

// Only compute names for expense reports with policies that have title fields
// Check if the policy has a title field with a formula
const titleField = getTitleFieldFromRNVP(report.reportID, context);
if (!titleField?.defaultValue) {
// Check if the report has a title field with a formula in rNVP
const reportTitleField = getTitleFieldFromRNVP(report.reportID, context);
if (!reportTitleField?.defaultValue) {
return false;
}
return true;
Expand Down Expand Up @@ -195,13 +195,10 @@ function computeReportNameIfNeeded(report: Report | undefined, incomingUpdate: O
}

const titleField = getTitleFieldFromRNVP(targetReport.reportID, context);
if (!titleField?.defaultValue) {
return null;
}

// Quick check: see if the update might affect the report name
const updateType = determineObjectTypeByKey(incomingUpdate.key);
const formula = titleField.defaultValue;
const formula = titleField?.defaultValue;
const formulaParts = parse(formula);

let transaction: Transaction | undefined;
Expand Down
135 changes: 135 additions & 0 deletions src/libs/ReportTitleUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/**
* This file should only be used in context of optimistic report name updates.
* We're using direct Onyx connection and this can lead to stale component's state if used in wrong context.
*/
import type {OnyxUpdate} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Beta, BetaConfiguration, Policy, Report, ReportNameValuePairs} from '@src/types/onyx';
import Permissions from './Permissions';
import {getTitleReportField, isChatReport} from './ReportUtils';

let allReportNameValuePairs: Record<string, ReportNameValuePairs> = {};
let betas: Beta[] = [];
let betaConfiguration: BetaConfiguration = {};

/**
* We use Onyx.connectWithoutView because we do not use this in React components and this logic is not tied directly to the UI.
* We need up to date report name value pairs of reports to correctly determine if further updates to report's titles should be made.
* It wouldn't be possible without connection directly to Onyx.
*/
Onyx.connectWithoutView({

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.

NAB: Where will we be calling the report title utils from? Will we be calling them from various action functions throughout the app, such as when a report is created or moved? Or will we always be calling them from within the optimistic report names module, and identifying these same actions based on the Onyx updates? If it's always from the module, I would rather pass in a context param and set up these connections in the main context. If it's spread throughout the app then it's ok to have separate connect. NAB as we can always change the approach as we continue forward. I think my preference is to call these utils from the action functions directly, so this looks good. I want to make sure we both have the same plan in mind. Please send a link of your answer via DM so I don't miss it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We are on the same side. We'll be calling these functions directly from action functions like updateReportName from @src/actions/Report.ts

key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS,
waitForCollectionCallback: true,
callback: (val) => {
allReportNameValuePairs = (val as Record<string, ReportNameValuePairs>) ?? {};
},
});
Onyx.connectWithoutView({
key: ONYXKEYS.BETAS,
callback: (val) => {
betas = val ?? [];
},
});
Onyx.connectWithoutView({
key: ONYXKEYS.BETA_CONFIGURATION,
callback: (val) => {
betaConfiguration = val ?? {};
},
});

/**
* Get the title field from report name value pairs
*/
function getTitleFieldFromRNVP(reportID: string) {
const reportNameValuePairs = allReportNameValuePairs[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportID}`];
// eslint-disable-next-line @typescript-eslint/naming-convention
return reportNameValuePairs?.expensify_text_title;
}

/**
* Update title field in report's rNVP to match the policy's title field configuration
* This is the JavaScript equivalent of the backend updateTitleFieldToMatchPolicy function
*/
function updateTitleFieldToMatchPolicy(reportID: string, policy?: Policy): OnyxUpdate[] {
if (!Permissions.isBetaEnabled(CONST.BETAS.AUTH_AUTO_REPORT_TITLE, betas, betaConfiguration)) {
return [];
}
if (!reportID || !policy) {
return [];
}

// Get the policy's title field configuration
const reportTitleField = getTitleReportField(policy.fieldList ?? {});

// Early return if policy doesn't have a title field
if (!reportTitleField) {
return [];
}

// Create the update to set/update the title field in rNVP
const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportID}`,
value: {
// eslint-disable-next-line @typescript-eslint/naming-convention
expensify_text_title: reportTitleField,
},
},
];

return optimisticData;
}

/**
* Remove title field from report's rNVP when report is manually renamed to indicate that the manual name should be preserved, and the custom report name formula should no longer update the name.
*/
function removeTitleFieldFromReport(reportID: string): OnyxUpdate[] {
if (!Permissions.isBetaEnabled(CONST.BETAS.AUTH_AUTO_REPORT_TITLE, betas, betaConfiguration)) {
return [];
}
if (!reportID) {
return [];
}

const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportID}`,
value: {
// eslint-disable-next-line @typescript-eslint/naming-convention
expensify_text_title: null,
},
},
];

return optimisticData;
}

/**
* Check if a report should have its title field updated based on conditions
*/
function shouldUpdateTitleField(report: Report): boolean {
// todo: this should be more sophisticated function. check for iou etc
if (!report) {
return false;
}
// Skip chat reports
if (isChatReport(report)) {
return false;
}

// Skip if report has statement card ID (backend check: !getValue(db, reportID, NVP_STATEMENT_CARD_ID).empty())
// This would need to be implemented based on how statement card IDs are stored in the frontend

const reportTitleField = getTitleFieldFromRNVP(report.reportID);
if (!reportTitleField) {
return false;
}

return true;
}

export {getTitleFieldFromRNVP, removeTitleFieldFromReport, shouldUpdateTitleField, updateTitleFieldToMatchPolicy};
3 changes: 3 additions & 0 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
import Pusher from '@libs/Pusher';
import type {UserIsLeavingRoomEvent, UserIsTypingEvent} from '@libs/Pusher/types';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import {updateTitleFieldToMatchPolicy} from '@libs/ReportTitleUtils';
import type {OptimisticAddCommentReportAction, OptimisticChatReport, SelfDMParameters} from '@libs/ReportUtils';
import {
buildOptimisticAddCommentReportAction,
Expand Down Expand Up @@ -281,7 +282,7 @@
let currentUserAccountID = -1;
let currentUserEmail: string | undefined;

Onyx.connect({

Check warning on line 285 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
// When signed out, val is undefined
Expand All @@ -294,13 +295,13 @@
},
});

Onyx.connect({

Check warning on line 298 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.CONCIERGE_REPORT_ID,
callback: (value) => (conciergeReportID = value),
});

let preferredSkinTone: number = CONST.EMOJI_DEFAULT_SKIN_TONE;
Onyx.connect({

Check warning on line 304 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PREFERRED_EMOJI_SKIN_TONE,
callback: (value) => {
preferredSkinTone = EmojiUtils.getPreferredSkinToneIndex(value);
Expand All @@ -310,7 +311,7 @@
// map of reportID to all reportActions for that report
const allReportActions: OnyxCollection<ReportActions> = {};

Onyx.connect({

Check warning on line 314 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
callback: (actions, key) => {
if (!key || !actions) {
Expand All @@ -322,14 +323,14 @@
});

let allTransactionViolations: OnyxCollection<TransactionViolations> = {};
Onyx.connect({

Check warning on line 326 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
waitForCollectionCallback: true,
callback: (value) => (allTransactionViolations = value),
});

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 333 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -339,7 +340,7 @@

let isNetworkOffline = false;
let networkStatus: NetworkStatus;
Onyx.connect({

Check warning on line 343 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NETWORK,
callback: (value) => {
isNetworkOffline = value?.isOffline ?? false;
Expand All @@ -348,7 +349,7 @@
});

let allPersonalDetails: OnyxEntry<PersonalDetailsList> = {};
Onyx.connect({

Check warning on line 352 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand All @@ -356,7 +357,7 @@
});

let account: OnyxEntry<Account> = {};
Onyx.connect({

Check warning on line 360 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.ACCOUNT,
callback: (value) => {
account = value ?? {};
Expand All @@ -364,7 +365,7 @@
});

const draftNoteMap: OnyxCollection<string> = {};
Onyx.connect({

Check warning on line 368 in src/libs/actions/Report.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.PRIVATE_NOTES_DRAFT,
callback: (value, key) => {
if (!key) {
Expand Down Expand Up @@ -2790,6 +2791,8 @@
},
];

optimisticData.push(...updateTitleFieldToMatchPolicy(reportID, policy));

const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand Down
156 changes: 156 additions & 0 deletions tests/unit/ReportTitleUtilsTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import Onyx from 'react-native-onyx';
import {getTitleFieldFromRNVP, removeTitleFieldFromReport, shouldUpdateTitleField, updateTitleFieldToMatchPolicy} from '@libs/ReportTitleUtils';
// eslint-disable-next-line no-restricted-syntax -- disabled because we need ReportUtils to mock
import * as ReportUtils from '@libs/ReportUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Policy, PolicyReportField, Report, ReportNameValuePairs} from '@src/types/onyx';
import type CollectionDataSet from '@src/types/utils/CollectionDataSet';

// Mock dependencies
jest.mock('@libs/ReportUtils', () => ({
getTitleReportField: jest.fn(),
isChatReport: jest.fn(),
}));
jest.mock('@libs/Permissions', () => ({
isBetaEnabled: jest.fn().mockReturnValue(true),
}));

const mockedReportUtils = ReportUtils as jest.Mocked<typeof ReportUtils>;

describe('ReportTitleUtils', () => {
beforeAll(async () => {
const mergedCollection: CollectionDataSet<typeof ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS> = {};
mergedCollection[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}12345`] = {
// eslint-disable-next-line @typescript-eslint/naming-convention
expensify_text_title: {
defaultValue: 'Report {report:total}',
},
} as unknown as ReportNameValuePairs;
await Onyx.merge(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, mergedCollection);
});

beforeEach(() => {
jest.clearAllMocks();
});

describe('getTitleFieldFromRNVP', () => {
const mockReportID = '12345';
const mockTitleField = {
defaultValue: 'Report {report:total}',
};

it('should return title field when RNVP exists with title field', () => {
const result = getTitleFieldFromRNVP(mockReportID);

expect(result).toEqual(mockTitleField);
});

it('should return undefined when RNVP is undefined', () => {
const result = getTitleFieldFromRNVP('55555');

expect(result).toBeUndefined();
});
});

describe('updateTitleFieldToMatchPolicy', () => {
const mockReportID = '12345';
const mockTitleField: PolicyReportField = {
defaultValue: 'Test report Title',
} as unknown as PolicyReportField;

it('should return optimistic update when valid inputs provided', () => {
const mockPolicy: Policy = {
id: 'policy123',
fieldList: {
// eslint-disable-next-line @typescript-eslint/naming-convention
text_title: mockTitleField,
},
} as unknown as Policy;

mockedReportUtils.getTitleReportField.mockReturnValue(mockTitleField);

const result = updateTitleFieldToMatchPolicy(mockReportID, mockPolicy);

expect(mockedReportUtils.getTitleReportField).toHaveBeenCalledWith(mockPolicy.fieldList);
expect(result).toHaveLength(1);
expect(result.at(0)).toEqual({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${mockReportID}`,
value: {
// eslint-disable-next-line @typescript-eslint/naming-convention
expensify_text_title: mockTitleField,
},
});
});

it('should return empty array when policy is undefined', () => {
const result = updateTitleFieldToMatchPolicy(mockReportID, undefined);

expect(result).toEqual([]);
expect(mockedReportUtils.getTitleReportField).not.toHaveBeenCalled();
});
});

describe('removeTitleFieldFromReport', () => {
const mockReportID = '12345';

it('should return optimistic update with null value for valid reportID', () => {
const result = removeTitleFieldFromReport(mockReportID);

expect(result).toHaveLength(1);
expect(result.at(0)).toEqual({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${mockReportID}`,
value: {
// eslint-disable-next-line @typescript-eslint/naming-convention
expensify_text_title: null,
},
});
});
});

describe('shouldUpdateTitleField', () => {
const mockReportID = '12345';

it('should return false when report is chat report', () => {
const mockReport: Report = {
reportID: mockReportID,
type: CONST.REPORT.TYPE.CHAT,
} as Report;

mockedReportUtils.isChatReport.mockReturnValue(true);

const result = shouldUpdateTitleField(mockReport);

expect(mockedReportUtils.isChatReport).toHaveBeenCalledWith(mockReport);
expect(result).toBe(false);
});

it('should return false when report has no title field in RNVP', () => {
const mockReport: Report = {
reportID: '5555',
type: CONST.REPORT.TYPE.EXPENSE,
} as Report;

mockedReportUtils.isChatReport.mockReturnValue(false);

const result = shouldUpdateTitleField(mockReport);

expect(result).toBe(false);
});

it('should return true when non-chat report has title field', () => {
const mockReport: Report = {
reportID: mockReportID,
type: CONST.REPORT.TYPE.EXPENSE,
} as Report;

mockedReportUtils.isChatReport.mockReturnValue(false);

const result = shouldUpdateTitleField(mockReport);

expect(result).toBe(true);
});
});
});
Loading