[Payment due @rojiphil] Show rules feature as on when areRulesEnabled is undefined but categories have active rules#94691
Conversation
When areRulesEnabled is undefined on a corporate policy (common for accounts that configured category rules via OldDot before the flag existed), derive the enabled state from active per-category rule fields instead of defaulting to false. Explicitly false continues to always hide; true always shows. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e650790c84
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-authored-by: Cursor <cursoragent@cursor.com>
Address reviewer feedback by passing categories into arePolicyRulesEnabled and isPolicyFeatureEnabled call sites, extending hasConfiguredRules for Classic category rules, and adding comprehensive hasAnyCategoryRules tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Remove unintended submodule pointer change from this PR. Co-authored-by: Cursor <cursoragent@cursor.com>
arePolicyRulesEnabled gates on isControlPolicy (CORPORATE only), so tests that assert rules behavior must use CORPORATE rather than the TEAM default. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0b8fdf7f8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@MelvinBot review |
…apper Without categories, areRulesEnabled === undefined policies (migrated Classic workspaces) were correctly shown as Rules-enabled in the workspace nav and Getting Started UI, but navigating to any Rules route redirected back to More Features because AccessOrNotFoundWrapper called isPolicyFeatureEnabledUtil without categories, causing the undefined→hasAnyCategoryRules fallback to return false. Co-authored-by: Cursor <cursoragent@cursor.com>
mountiny
changed the title
- Merge duplicate @libs/PolicyUtils imports in PolicyUtilsTest.ts into a single import statement (import/no-duplicates) - Replace `as unknown as Policy` with createMock<Policy>() in arePolicyRulesEnabled tests to stay within the no-unsafe-type-assertion seatbelt count - Type policyCategories as PolicyCategories in GettingStartedSectionTest to avoid two extra `as never` casts that exceeded the seatbelt limit Co-authored-by: Cursor <cursoragent@cursor.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp94691-android-hybrid-001.mp4Android: mWeb Chrome94691-mweb-chrome-001.mp4iOS: HybridAppSkipped due to local build issues. It's fine since the implementation is not platform specific. iOS: mWeb Safari94691-mweb-safari-001.mp4MacOS: Chrome / Safari94691-web-chrome-007.mp494691-web-chrome-008.mp494691-web-chrome-009.mp4 |
melvin-bot
Bot
changed the title
|
🎯 @rojiphil, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
inimaga
deleted the
fix/category-rules-visible-for-undefined-areRulesEnabled
branch
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 inimaga has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/inimaga in version: 9.4.25-0 🚀
|
|
@rojiphil how to check areRulesEnabled was never explicitly set |
@kavimuru I tested this scenario after manually removing the I think this edge case happens for workspaces created in OD a long time ago where |
|
@mountiny Another related query. When category rules are configured in OD, the |
|
This PR failing because of the issue #95031 |
|
@rojiphil looking into that, there might be some other gaps because when the category settings is changed in Classic - the flag should change to true, so if that is not happening its still a bug |
|
@jponikarchuk only old workspaces would not have the flag set as far as I know |
|
@mountiny We used a new Expensify account |
|
@rojiphil can you reproduce the flag not changing when category rule is added in classic? It should be changing from what i can see in code |
|
@mountiny Doesn't look like it got fixed. The 94691-web-chrome-issue-001.mp4 |
|
🤖 Payment issue created: #95083 |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.4.25-2 🚀
Bundle Size Analysis (Sentry): |
7 participants
Explanation of Change
Adds
arePolicyRulesEnabled(policy, policyCategories?)toPolicyUtils.tsandhasAnyCategoryRules(categories?)toCategoryUtils.tsto implement consistent three-state logic for the Rules feature flag:areRulesEnabled === true→ always show Rules UIareRulesEnabled === false→ always hide Rules UIareRulesEnabled === undefined(corporate policy) → derive from active per-category rule fields set via OldDot Classic (maxExpenseAmount, receipt thresholds, required description/attendees,commentHint)This covers the edge case where a workspace admin configured category rules in OldDot without the
areRulesEnabledflag ever being set. NewDot was silently hiding all Rules UI for these users even though their rules were actively configured.Changes:
CategoryUtils.ts— newhasAnyCategoryRules(categories?)checks per-category rule fieldsPolicyUtils.ts— newarePolicyRulesEnabled(policy, policyCategories?)with three-state logic; team policies always returnfalse;isPolicyFeatureEnableddelegates to it forARE_RULES_ENABLED;hasConfiguredRulesextended to treat Classic category rules as configuredpolicyCategorieswhere available (workspace pages, card issue wizard,useGettingStartedItems,ModifiedExpenseMessage)AccessOrNotFoundWrapper.tsx— readspolicyCategoriesfrom Onyx and passes them toisPolicyFeatureEnabledUtil, so all ~20 Rules routes correctly gate access for migrated policies (without this, the sidebar could show Rules as enabled but navigating to/ruleswould bounce back to More Features)Fixed Issues
$ https://github.com/Expensify/Expensify/issues/652345
Tests
npm run test -- PolicyUtilsTest.tsand verify allarePolicyRulesEnabledandhasConfiguredRulescases passnpm run test -- CategoryUtilsTest.tsand verify allhasAnyCategoryRulescases passnpm run test -- useGettingStartedItems.test.ts GettingStartedSectionTest.tsx ModifiedExpenseMessageTest.tsand verify setup rules row and policy rules message behavior passareRulesEnabledundefined and at least one Classic category rule configured (e.g. max amount or required receipt), open Workspace → Categories → that category and verify the Category rules section is visible/rulesnow redirects to More FeaturesareRulesEnabled: false, verify Rules UI stays hidden even when category rule fields are presentareRulesEnabledundefined and Classic category rules; verify spend rules steps appear in the card wizardOffline tests
No offline-specific behavior changed. All rules visibility logic reads from Onyx policy and category data which is already available offline; no new network requests are made.
QA Steps
areRulesEnabledwas never explicitly set/rulesshould now redirect to More FeaturesPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari