Fix regressions from [Rules Revamp] Release 2 - #95444
Conversation
…re disabled. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…edits. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…ange. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…ection. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…istic copy. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76ffee7ce0
ℹ️ 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".
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…ting until policy loads. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…ions Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…ions Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ae1fbf4fa
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Looks good from my end too 👍 |
|
@situchan over to you |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-08-03.at.3.31.15.PM.mov |
|
@situchan bump |
|
On it |
|
This is still reproducible |
|
Test step updated |
| useFocusEffect(() => { | ||
| fetchPolicyCategories(); | ||
| }); |
There was a problem hiding this comment.
I am not sure this tutorial considered auto memoization by React Compiler but do we not need useCallback here?
cc: @MelvinBot
There was a problem hiding this comment.
Correct — you don't need an explicit useCallback here. That React Navigation warning is written for codebases without React Compiler, and App runs the compiler on every src/ file.
Why: useFocusEffect re-runs its effect whenever the callback's reference changes. Without memoization an inline arrow is a new reference every render, so the effect would fire on every re-render while focused — that's what the docs are guarding against. React Compiler auto-memoizes the inline callback passed to useFocusEffect and fetchPolicyCategories based on their captured deps (areCategoriesEnabled, policyCategories, policyID), which is functionally equivalent to wrapping them in useCallback. So a manual useCallback would just be redundant. The compiler covers this file — the babel sources filter only excludes tests/ and node_modules/.
One caveat worth knowing: App runs two compiler implementations — oxc on web and babel-plugin-react-compiler on native — and they don't always memoize a file identically (REACT_COMPILER.md). If one memoizes this file and the other doesn't, the CI memoization divergence check will flag it, and the documented fix is to add the manual useCallback back. So: rely on the compiler by default, and only reach for useCallback if that check fails on this file.
| const styles = useThemeStyles(); | ||
| const {translate} = useLocalize(); | ||
| const illustrations = useMemoizedLazyIllustrations(['FolderOpen']); | ||
| const policyData = usePolicyData(policyID); |
There was a problem hiding this comment.
Minor perf: usePolicyData (computes reports + transactions + violations across all policy reports) is quite heavy for the empty state.
enablePolicyCategories only reads policyData.policy?.id and policyData.categories.
Since this component only renders when categories are disabled, impact is limited — but a lighter path (usePolicy + the categories useOnyx) would avoid the unnecessary derived work.
There was a problem hiding this comment.
enablePolicyCategories also passes policyData into pushTransactionAutoSelectionsOnyxData and pushTransactionViolationsOnyxData.
| const [form] = useOnyx(ONYXKEYS.FORMS.REQUIRE_FIELDS_RULE_FORM); | ||
| const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`); | ||
| const areCategoriesEnabled = !!policy?.areCategoriesEnabled; | ||
| const arePolicyCategoriesLoading = areCategoriesEnabled && policyCategories === undefined; |
There was a problem hiding this comment.
Bug: permanent spinner when offline
Test in non-default workspace. Clear cache and restart to clear categories.
Screen.Recording.2026-08-03.at.3.46.39.PM.mov
| const [form] = useOnyx(ONYXKEYS.FORMS.FLAG_FOR_REVIEW_RULE_FORM); | ||
| const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`); | ||
| const areCategoriesEnabled = !!policy?.areCategoriesEnabled; | ||
| const arePolicyCategoriesLoading = areCategoriesEnabled && policyCategories === undefined; |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@JS00001, could you please review and merge this? It would help avoid conflicts in the next PR. |
|
🚧 JS00001 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! 🧪🧪
|
|
@Krishna2323 @JS00001 @situchan To confirm, Is applause.expensifail.com domain enabled for rulesRevamp beta? |
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.4.50-0 🚀
|
|
🤖 I reviewed the changes in this PR against the help site files under Conclusion: No help site changes are required. This PR fixes two regressions in the Rules feature (plus a supporting refactor):
Both linked issues confirm this is a not-yet-in-production feature and the fixes are UI edge-case corrections — they don't add or change any configurable behavior. The relevant help article, Workspace Rules, documents Rules conceptually (expense/category/spend/tag rules) and doesn't describe category-picker contents or empty states at this granularity. Nothing documented there becomes inaccurate as a result of these changes, so no draft docs PR was created. If you'd like the broader Require fields / Flag for review workspace rules from the Rules Revamp documented on the help site (they aren't currently covered as top-level rules in the article), that would be a separate, larger docs effort — let me know and I can scope it. @Krishna2323, no linked help site PR was created because no docs changes are required. If you believe a help article should be updated to reflect this behavior, reply here and I'll create the draft PR. |
|
Deploy Blocker #97840 was identified to be related to this PR. |

Explanation of Change
Fixed Issues
$ #95305
$ #95358
PROPOSAL:
Tests
Prerequisites
rulesRevampbeta enabled#95305 — Category enable page when categories are disabled
#95358 — None not shown in category picker
Offline tests
QA Steps
Same as tests
Verify that no errors appear in the JS console
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
Monosnap.screencast.2026-07-30.20-33-47.mp4