Skip to content

Remove getPolicy usages from enabling more features and invoice company info #86074

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
cristipaval merged 5 commits into from
Mar 24, 2026
Merged

Remove getPolicy usages from enabling more features and invoice company info #86074

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
7c41a8f
removed getPolicy usages when updating invoice company name & web
bernhardoj Mar 23, 2026
e18de43
removed getPolicy usages when enabling more features
bernhardoj Mar 23, 2026
61713ce
fix test
bernhardoj Mar 23, 2026
ef0068a
fix typing
bernhardoj Mar 23, 2026
0b7f8f8
Merge branch 'main' into chore/66574-refactor-policy-to-use-useonyx-11
bernhardoj Mar 24, 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
83 changes: 38 additions & 45 deletions src/libs/actions/Policy/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@
};

const deprecatedAllPolicies: OnyxCollection<Policy> = {};
Onyx.connect({

Check warning on line 242 in src/libs/actions/Policy/Policy.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.POLICY,
callback: (val, key) => {
if (!key) {
Expand All @@ -255,7 +255,7 @@
});

let deprecatedAllReportActions: OnyxCollection<ReportActions>;
Onyx.connect({

Check warning on line 258 in src/libs/actions/Policy/Policy.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,
waitForCollectionCallback: true,
callback: (actions) => {
Expand All @@ -265,7 +265,7 @@

let deprecatedSessionEmail = '';
let deprecatedSessionAccountID = 0;
Onyx.connect({

Check warning on line 268 in src/libs/actions/Policy/Policy.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: (val) => {
deprecatedSessionEmail = val?.email ?? '';
Expand All @@ -274,7 +274,7 @@
});

let deprecatedAllPersonalDetails: OnyxEntry<PersonalDetailsList>;
Onyx.connect({

Check warning on line 277 in src/libs/actions/Policy/Policy.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: (val) => (deprecatedAllPersonalDetails = val),
});
Expand Down Expand Up @@ -4611,7 +4611,9 @@
API.writeWithNoDuplicatesEnableFeatureConflicts(WRITE_COMMANDS.ENABLE_POLICY_REPORT_FIELDS, parameters, onyxData);
}

function enablePolicyTaxes(policyID: string, enabled: boolean) {
function enablePolicyTaxes(policyID: string, enabled: true, currentTaxRates: TaxRatesWithDefault | undefined): void;
function enablePolicyTaxes(policyID: string, enabled: false): void;
function enablePolicyTaxes(policyID: string, enabled: boolean, currentTaxRates?: TaxRatesWithDefault) {
Comment on lines +4614 to +4616

@parasharrajat parasharrajat Mar 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need 3 types here?

@bernhardoj bernhardoj Mar 24, 2026

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.

The taxRates is only being used for shouldAddDefaultTaxRatesData and is only useful when enabled is true.

There are a few places where we call enablePolicyTaxes to disable the taxes. If we always pass the taxRates, we will unnecessarily subscribe to the onyx. (for example)

App/src/components/ConnectToQuickbooksOnlineFlow/index.native.tsx

Lines 17 to 30 in 0adae4c

function ConnectToQuickbooksOnlineFlow({policyID}: ConnectToQuickbooksOnlineFlowProps) {
const {translate} = useLocalize();
const webViewRef = useRef<WebView>(null);
const [isWebViewOpen, setIsWebViewOpen] = useState(false);
const [session] = useOnyx(ONYXKEYS.SESSION);
const authToken = session?.authToken ?? null;
useEffect(() => {
// Since QBO doesn't support Taxes, we should disable them from the LHN when connecting to QBO
enablePolicyTaxes(policyID, false);
setIsWebViewOpen(true);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const defaultTaxRates: TaxRatesWithDefault = CONST.DEFAULT_TAX;
const taxRatesData: OnyxData<typeof ONYXKEYS.COLLECTION.POLICY> = {
optimisticData: [
Expand Down Expand Up @@ -4661,15 +4663,12 @@
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
taxRates: undefined,
taxRates: null,
},
},
],
};
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
const shouldAddDefaultTaxRatesData = (!policy?.taxRates || isEmptyObject(policy.taxRates)) && enabled;
const shouldAddDefaultTaxRatesData = (!currentTaxRates || isEmptyObject(currentTaxRates)) && enabled;

const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.POLICY>> = [
{
Expand Down Expand Up @@ -4733,10 +4732,14 @@
}
}

function enablePolicyWorkflows(policyID: string, enabled: boolean) {
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
function enablePolicyWorkflows(
policyID: string,
enabled: boolean,
currentApprovalMode: Policy['approvalMode'],
currentAutoReporting: Policy['autoReporting'],
currentHarvesting: Policy['harvesting'],
currentReimbursementChoice: Policy['reimbursementChoice'],
) {
const onyxData: OnyxData<typeof ONYXKEYS.COLLECTION.POLICY> = {
optimisticData: [
{
Expand Down Expand Up @@ -4795,10 +4798,10 @@
areWorkflowsEnabled: !enabled,
...(!enabled
? {
approvalMode: policy?.approvalMode,
autoReporting: policy?.autoReporting,
harvesting: policy?.harvesting,
reimbursementChoice: policy?.reimbursementChoice,
approvalMode: currentApprovalMode,
autoReporting: currentAutoReporting,
harvesting: currentHarvesting,
reimbursementChoice: currentReimbursementChoice,
}
: {}),
pendingFields: {
Expand Down Expand Up @@ -4834,10 +4837,12 @@
maxExpenseAge: CONST.DISABLED_MAX_EXPENSE_VALUE,
};

function enablePolicyRules(policyID: string, enabled: boolean, shouldGoBack = true, policyData?: PolicyData) {
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
function enablePolicyRules(policy: OnyxEntry<Policy>, enabled: boolean, shouldGoBack = true, policyData?: PolicyData) {
if (!policy) {
return;
}

const policyID = policy.id;
const onyxData: OnyxData<typeof ONYXKEYS.COLLECTION.POLICY> = {
optimisticData: [
{
Expand Down Expand Up @@ -4912,10 +4917,7 @@
}
}

function enableDistanceRequestTax(policyID: string, customUnitName: string, customUnitID: string, attributes: Attributes) {
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
function enableDistanceRequestTax(policyID: string, customUnitName: string, customUnitID: string, attributes: Attributes, currentAttributes: Attributes | undefined) {
const onyxData: OnyxData<typeof ONYXKEYS.COLLECTION.POLICY> = {
optimisticData: [
{
Expand Down Expand Up @@ -4955,7 +4957,7 @@
value: {
customUnits: {
[customUnitID]: {
attributes: policy?.customUnits ? policy?.customUnits[customUnitID].attributes : null,
attributes: currentAttributes ?? null,
errorFields: {
taxEnabled: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'),
},
Expand Down Expand Up @@ -5306,10 +5308,12 @@
API.write(WRITE_COMMANDS.SET_POLICY_TAXES_FOREIGN_CURRENCY_DEFAULT, parameters, onyxData);
}

function upgradeToCorporate(policyID: string, featureName?: string) {
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
function upgradeToCorporate(policy: OnyxEntry<Policy>, featureName?: string) {
if (!policy) {
return;
}

const policyID = policy.id;
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.POLICY>> = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand Down Expand Up @@ -5365,10 +5369,7 @@
API.write(WRITE_COMMANDS.UPGRADE_TO_CORPORATE, parameters, {optimisticData, successData, failureData});
}

function downgradeToTeam(policyID: string) {
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
function downgradeToTeam(policyID: string, currentType: Policy['type'], currentIsAttendeeTrackingEnabled: Policy['isAttendeeTrackingEnabled']) {
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.POLICY>> = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand Down Expand Up @@ -5397,8 +5398,8 @@
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
isPendingDowngrade: false,
type: policy?.type,
isAttendeeTrackingEnabled: policy?.isAttendeeTrackingEnabled,
type: currentType,
isAttendeeTrackingEnabled: currentIsAttendeeTrackingEnabled,
},
},
];
Expand Down Expand Up @@ -6812,11 +6813,7 @@
}
}

function updateInvoiceCompanyName(policyID: string, companyName: string) {
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);

function updateInvoiceCompanyName(policyID: string, companyName: string, currentCompanyName: string | undefined) {
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.POLICY>> = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand Down Expand Up @@ -6852,7 +6849,7 @@
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
invoice: {
companyName: policy?.invoice?.companyName,
companyName: currentCompanyName,
pendingFields: {
companyName: null,
},
Expand All @@ -6869,11 +6866,7 @@
API.write(WRITE_COMMANDS.UPDATE_INVOICE_COMPANY_NAME, parameters, {optimisticData, successData, failureData});
}

function updateInvoiceCompanyWebsite(policyID: string, companyWebsite: string) {
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);

function updateInvoiceCompanyWebsite(policyID: string, companyWebsite: string, currentCompanyWebsite: string | undefined) {
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.POLICY>> = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand Down Expand Up @@ -6909,7 +6902,7 @@
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
invoice: {
companyWebsite: policy?.invoice?.companyWebsite,
companyWebsite: currentCompanyWebsite,
pendingFields: {
companyWebsite: null,
},
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Subscription/SubscriptionPlan/SubscriptionPlanCardActionButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function SubscriptionPlanCardActionButton({subscriptionPlan, isFromComparisonMod

if (planType === CONST.POLICY.TYPE.CORPORATE) {
if (canPerformUpgrade && !!policy?.id) {
upgradeToCorporate(policy.id);
upgradeToCorporate(policy);
closeComparisonModal?.();
return;
}
Expand Down
10 changes: 7 additions & 3 deletions src/pages/workspace/WorkspaceMoreFeaturesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
if (!policyID) {
return;
}
enablePolicyWorkflows(policyID, isEnabled);
enablePolicyWorkflows(policyID, isEnabled, policy?.approvalMode, policy?.autoReporting, policy?.harvesting, policy?.reimbursementChoice);
},
disabled: isSmartLimitEnabled,
disabledAction: onDisabledWorkflowPress,
Expand All @@ -326,7 +326,7 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.rules.alias, ROUTES.WORKSPACE_MORE_FEATURES.getRoute(policyID)));
return;
}
enablePolicyRules(policyID, isEnabled, undefined, policyData);
enablePolicyRules(policy, isEnabled, undefined, policyData);
},
onPress: () => {
if (!policyID) {
Expand Down Expand Up @@ -411,7 +411,11 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro
if (!policyID) {
return;
}
enablePolicyTaxes(policyID, isEnabled);
if (isEnabled) {
enablePolicyTaxes(policyID, true, policy?.taxRates);
return;
}
enablePolicyTaxes(policyID, false);
},
onPress: () => {
if (!policyID) {
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/distanceRates/PolicyDistanceRatesSettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ function PolicyDistanceRatesSettingsPage({route}: PolicyDistanceRatesSettingsPag
return;
}
const attributes = {...customUnit?.attributes, taxEnabled: isOn};
enableDistanceRequestTax(policyID, customUnit.name, customUnit.customUnitID, attributes);
enableDistanceRequestTax(policyID, customUnit.name, customUnit.customUnitID, attributes, customUnit.attributes);
};

return (
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/downgrade/WorkspaceDowngradePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function WorkspaceDowngradePage({route}: WorkspaceDowngradePageProps) {
setIsDowngradeWarningModalOpen(true);
return;
}
downgradeToTeam(policy.id);
downgradeToTeam(policy.id, policy.type, policy.isAttendeeTrackingEnabled);
};

const onClose = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/invoices/WorkspaceInvoicingDetailsName.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function WorkspaceInvoicingDetailsName({route}: WorkspaceInvoicingDetailsNamePro
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`);

const submit = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_INVOICES_COMPANY_NAME_FORM>) => {
updateInvoiceCompanyName(policyID, values[INPUT_IDS.COMPANY_NAME]);
updateInvoiceCompanyName(policyID, values[INPUT_IDS.COMPANY_NAME], policy?.invoice?.companyName);
Navigation.goBack();
};

Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/invoices/WorkspaceInvoicingDetailsWebsite.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function WorkspaceInvoicingDetailsWebsite({route}: WorkspaceInvoicingDetailsWebs

const submit = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_INVOICES_COMPANY_WEBSITE_FORM>) => {
const companyWebsite = Str.sanitizeURL(values[INPUT_IDS.COMPANY_WEBSITE], CONST.COMPANY_WEBSITE_DEFAULT_SCHEME);
updateInvoiceCompanyWebsite(policyID, companyWebsite);
updateInvoiceCompanyWebsite(policyID, companyWebsite, policy?.invoice?.companyWebsite);
Navigation.goBack();
};

Expand Down
7 changes: 3 additions & 4 deletions src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
return;
}

upgradeToCorporate(policy.id, feature?.name);
upgradeToCorporate(policy, feature?.name);
};

// useCallback is needed here because confirmUpgrade is passed as a prop to child components;
Expand Down Expand Up @@ -186,7 +186,7 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
}
break;
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.rules.id:
enablePolicyRules(policyID, true, false, policyDataRef.current);
enablePolicyRules(policy, true, false, policyDataRef.current);
break;
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.companyCards.id:
enableCompanyCards(policyID, true, false);
Expand All @@ -203,8 +203,7 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
categoryId,
feature,
perDiemCustomUnit?.customUnitID,
policy?.connections?.xero?.config,
policy?.connections?.xero?.data,
policy,
Comment thread
bernhardoj marked this conversation as resolved.

@cristipaval cristipaval Mar 23, 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.

Isn't this a performance concern? The policy object is very big and any change on it could trigger rerenders

@bernhardoj bernhardoj Mar 24, 2026

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 subscribe to the policy onyx, so it already re-renders anytime the policy changes, but yes, it will re-create this function too, same as other functions.

I pass the whole policy object to enablePolicyRules because it uses many properties of the policy and also we need to pass the whole policy to isControlPolicy.

App/src/libs/actions/Policy/Policy.ts

Line 4898 in 0adae4c

if (enabled && isControlPolicy(policy) && policy?.outputCurrency === CONST.CURRENCY.USD) {

@cristipaval cristipaval Mar 24, 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.

I see. This is a page the user doesn't visit often, so maybe it is fine for now. We'll refactor the whole thing if it becomes a real problem 👍

policyID,
qboConfig?.syncClasses,
qboConfig?.syncCustomers,
Expand Down
Loading
Loading