-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fix issue: Home - Connect to accounting task is checked off when connection fails #87739
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
Changes from all commits
d1575b7
38fc883
0b93640
090f873
4d1329c
6249b4b
81349ae
08a3e9b
83b5c8c
d08c492
9d9285d
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 |
|---|---|---|
|
|
@@ -76,7 +76,7 @@ | |
|
|
||
| let allPolicies: OnyxCollection<Policy>; | ||
|
|
||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.POLICY, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => (allPolicies = value), | ||
|
|
@@ -692,22 +692,70 @@ | |
| } | ||
|
|
||
| /** | ||
| * Checks if a policy has any non-default rules configured. | ||
| * Defaults are: no approval/expense/coding rules and no custom rules text. | ||
| * Checks if a policy has any rules configured (structured rules, individual expense limits, or prohibited expenses). | ||
| */ | ||
| function hasNonDefaultRules(policy: OnyxEntry<Policy>): boolean { | ||
| function hasConfiguredRules(policy: OnyxEntry<Policy>): boolean { | ||
|
Contributor
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. Let's add tests for this util
Contributor
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. Do we have tests for this util?
Contributor
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. I think we should rewrite this method to do early returns - everytime we find that any of these values is non-default, we can return true early +1 lets add robust tests for this one, should be easy |
||
| if (!policy) { | ||
| return false; | ||
| } | ||
|
|
||
| const hasCustomRules = !!policy.customRules && policy.customRules.trim().length > 0; | ||
| if (!!policy.customRules && policy.customRules.trim().length > 0) { | ||
| return true; | ||
| } | ||
|
|
||
| const {rules} = policy; | ||
| const hasApprovalRules = !!rules?.approvalRules && rules.approvalRules.length > 0; | ||
| const hasExpenseRules = !!rules?.expenseRules && rules.expenseRules.length > 0; | ||
| const hasCodingRules = !!rules?.codingRules && Object.keys(rules.codingRules).length > 0; | ||
| if (!!rules?.approvalRules && rules.approvalRules.length > 0) { | ||
| return true; | ||
| } | ||
| if (!!rules?.expenseRules && rules.expenseRules.length > 0) { | ||
| return true; | ||
| } | ||
| if (!!rules?.codingRules && Object.keys(rules.codingRules).length > 0) { | ||
| return true; | ||
| } | ||
|
|
||
| if (!!policy.maxExpenseAmount && policy.maxExpenseAmount !== CONST.DISABLED_MAX_EXPENSE_VALUE && policy.maxExpenseAmount !== CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT) { | ||
| return true; | ||
| } | ||
| if (!!policy.maxExpenseAge && policy.maxExpenseAge !== CONST.DISABLED_MAX_EXPENSE_VALUE && policy.maxExpenseAge !== CONST.POLICY.DEFAULT_MAX_EXPENSE_AGE) { | ||
| return true; | ||
| } | ||
| if ( | ||
| !!policy.maxExpenseAmountNoReceipt && | ||
| policy.maxExpenseAmountNoReceipt !== CONST.DISABLED_MAX_EXPENSE_VALUE && | ||
| policy.maxExpenseAmountNoReceipt !== CONST.POLICY.DEFAULT_MAX_AMOUNT_NO_RECEIPT | ||
| ) { | ||
| return true; | ||
| } | ||
| if ( | ||
| !!policy.maxExpenseAmountNoItemizedReceipt && | ||
| policy.maxExpenseAmountNoItemizedReceipt !== CONST.DISABLED_MAX_EXPENSE_VALUE && | ||
| policy.maxExpenseAmountNoItemizedReceipt !== CONST.POLICY.DEFAULT_MAX_AMOUNT_NO_ITEMIZED_RECEIPT | ||
| ) { | ||
| return true; | ||
| } | ||
|
|
||
| if (policy.defaultBillable) { | ||
| return true; | ||
| } | ||
| if (policy.defaultReimbursable === false) { | ||
| return true; | ||
| } | ||
| if (policy.eReceipts) { | ||
| return true; | ||
| } | ||
| if (policy.requireCompanyCardsEnabled) { | ||
| return true; | ||
| } | ||
|
|
||
| return hasCustomRules || hasApprovalRules || hasExpenseRules || hasCodingRules; | ||
| const {prohibitedExpenses} = policy; | ||
| return ( | ||
| !!prohibitedExpenses && | ||
| Object.entries(CONST.POLICY.DEFAULT_PROHIBITED_EXPENSES).some(([key, defaultValue]) => { | ||
| const value = prohibitedExpenses[key as keyof typeof CONST.POLICY.DEFAULT_PROHIBITED_EXPENSES]; | ||
| return value !== undefined && value !== defaultValue; | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -2132,7 +2180,7 @@ | |
| getTagLists, | ||
| hasTags, | ||
| hasCustomCategories, | ||
| hasNonDefaultRules, | ||
| hasConfiguredRules, | ||
| getTaxByID, | ||
| getUnitRateValue, | ||
| getRateDisplayValue, | ||
|
|
||
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.
where is this used @WojtekBoman
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.
LGTM on these defaults. Note, this is only set on paid workspaces
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 checked the billable and reimbursable too 👍