Part 1 - [Rules Revamp] Clean-up and Polish #95289
Conversation
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…ange Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
Recordings will be added in 15m |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4d50e566a
ℹ️ 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".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safarinon-beta.movweb.mov |
|
🤖 Code review Overall this is a clean, well-scoped change. It gates the old Categories/Tags "Settings" affordances behind the DetailsRefactor looks correct. Beta gating is consistent. Categories hides Settings ( Minor (clarity, non-blocking): the General-tab billable row still computes its pending action inline as Coverage: Codecov flags a small drop with no new tests, which is expected for feature-flagged UI wiring; not a blocker, but worth a note if unit coverage is desired for the toggle branch. Recommend the human reviewer confirm the on-device screenshots/recordings for the offline toggle behavior (test steps in the PR body), since that path isn't covered by automated tests. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b6e35de10
ℹ️ 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".
|
🤖 Regression scan One likely regression and one edge-case, both gated behind the 1.
|
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: f9addb879c
ℹ️ 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".
|
🤖 Regression scan — 1 real regression found Regression: workspaces where the Rules feature isn't available lose access to "Require category", "Require tags", and "Track billable expenses" when the The old toggles are hidden based only on the beta flag, but their new home is under the Rules feature. So if the beta is on and a workspace can't reach Rules, those settings become orphaned — hidden in their old location and unreachable in the new one. Repro: Control workspace with Categories + Tags enabled but the Rules feature toggled OFF (More features → Rules off), account on the
The PR's test steps only cover a Control workspace with Rules enabled, so this path wasn't exercised. Suggested fix: gate the old-toggle hiding on Rules availability too, not just the beta — e.g. hide only when Why this triggers, and the broader (non-Control) case
The Categories/Tags settings toggles are not Control-gated, so a Collect workspace with Tags enabled + the beta on is the same bug at larger scale — the toggles disappear and there is no Rules section on Collect to relocate them to. Worth confirming whether these settings are meant to be reachable on Collect; if so, the Areas checked and cleared (no regression): Details
CC the PR author to confirm the intended behavior for Rules-disabled / non-Control workspaces. |
Intentional: |
|
cc: @luacmartins |
|
@luacmartins over to you |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 luacmartins 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/luacmartins in version: 9.4.29-0 🚀
|
Help site review — changes required ✅I reviewed the changes in this PR against Draft PR: #95529 (labeled WhyThis PR moves the Track billable expenses toggle and the billable default out of Tags > Settings into the new Billable expenses page under Rules. The article
|
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.4.31-0 🚀
|
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.31-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Fixed Issues
$ #94968
$ #94970
$ #94965
PROPOSAL:
Tests
Prerequisites
rulesRevampbeta on your account.1. Categories — Settings removed (#94970)
2. Tags settings — toggles removed (#94965)
3. Billable on Rules — new toggle (#94968)
4. Billable toggle off
5. Beta off — regression
rulesRevampbeta and reload the app.Offline tests
Track billable toggle (#94968)
rulesRevampbeta and go to Rules > General > Billable expenses.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_hybrid.mp4
Android: mWeb Chrome
android_mWeb.mp4
iOS: Native
ios_hybrid.mp4
iOS: mWeb Safari
ios_mWeb.mp4
MacOS: Chrome / Safari
web_chrome.mp4