Repository navigation
[CP Staging] Revert "Align workspace selector logic" - #102134
Conversation
|
@sobitneupane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
![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: e8c069f3a3
ℹ️ 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".
| const lockedPreferredPolicy = isRestrictedToPreferredPolicy ? groupPoliciesWithChatEnabled.find((policy) => policy?.id === preferredPolicyID) : undefined; | ||
| const isLockedToPreferredPolicy = !!lockedPreferredPolicy; | ||
| const defaultChatEnabledPolicy = lockedPreferredPolicy ?? getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array<OnyxEntry<OnyxTypes.Policy>>, activePolicy) ?? undefined; | ||
| const defaultChatEnabledPolicy = getDefaultChatEnabledPolicy(groupPoliciesWithChatEnabled as Array<OnyxEntry<OnyxTypes.Policy>>, activePolicy); |
There was a problem hiding this comment.
When a member's security group enables a restrictedPrimaryPolicyID and that eligible workspace differs from the active one, this normal resolver selects the active workspace (or no default) instead. The Create flow can consequently create the report in the active workspace or present a selector containing all eligible workspaces, rather than using the configured restricted workspace; restore the preferred-policy override before resolving the default.
Useful? React with 👍 / 👎.
| const chatEnabledPaidGroupPoliciesSelector = (policies: OnyxCollection<OnyxTypes.Policy>, currentUserLogin: string | undefined) => | ||
| getGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin); | ||
| getGroupPoliciesWhereReportCanBeCreated(policies, currentUserLogin).slice(0, 2); |
There was a problem hiding this comment.
When a user has three or more eligible workspaces and the active workspace is not among the first two collection values, this slice removes it before getDefaultChatEnabledPolicy runs. That resolver then has no default, so clicking Create opens the workspace selector instead of creating directly in the active workspace; retain the active workspace in the capped list (or avoid the cap).
Useful? React with 👍 / 👎.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
This comment has been minimized.
This comment has been minimized.
|
@lakchote @cristipaval patch fixing lint errors below I have no permissions to push as it's main repo not |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
🚧 lakchote has triggered a test Expensify/App build. You can view the workflow run here. |
…kspace-selector-logic [CP Staging] Revert "Align workspace selector logic" (cherry picked from commit c30f0c3) (cherry-picked to staging by lakchote)
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🔁 Filed a Staging retest request for deploy blockers #102130 after this PR was cherry-picked to staging. |
|
🚀 Cherry-picked to staging by https://github.com/lakchote in version: 9.4.92-8 🚀
|
No help site changes are requiredThis PR is a pure revert of internal workspace-resolution logic. It restores the exact behavior the help site already describes, so no draft PR was created. What this PR changes: it reverts Align workspace selector logic, which had (1) made Create report skip the workspace selector when a domain security group locks the user to a Preferred Workspace, and (2) kept the default workspace inside the FAB's two-workspace slice. Reverting puts both back to how they shipped before. Why no docs change:
UI verification (web)I confirmed the documented labels and flow against the live app rather than assuming:
One thing to watchdocs/articles/new-expensify/domains/Create-and-Manage-Domain-Groups.md:89 says a Preferred Workspace will "automatically route a group's expenses and reports to a specific workspace." After this revert, Create report does not honor the Preferred Workspace — that is the open bug in Creating new report via FAB automatically uses default WS, not the Preferred one. and Create report workspace selector ignores domain restrictions and misfires on the FAB. I did not change that line, because documenting a bug that is actively being fixed would be wrong. It should stay accurate once the fix lands correctly. If you'd rather add a temporary caveat there, reply and I'll open the draft PR. @mountiny — no help site PR was created, so there is nothing to mark |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.92-8 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Cherry-picked to staging by https://github.com/lakchote in version: 9.4.93-0 🚀
|
Confirmed again: no help site changes are requiredI re-ran this review after the revert deployed to production in 9.4.92-8. The conclusion is unchanged from my earlier comment on this PR, so I did not create a draft PR. Why no docs change:
I verified the documented labels against the live UI last run at this same commit — + Create (a Still worth a decision from youdocs/articles/new-expensify/domains/Create-and-Manage-Domain-Groups.md:89 says a Preferred Workspace will "automatically route a group's expenses and reports to a specific workspace." With this revert now in production, Create report does not honor the Preferred Workspace — the open bug in Creating new report via FAB automatically uses default WS, not the Preferred one. and Create report workspace selector ignores domain restrictions and misfires on the FAB. I left that line alone, because documenting a bug that is actively being fixed would make the article wrong again as soon as the fix lands. If you'd rather ship a temporary caveat there, reply and I'll open the draft PR. @mountiny — there is no help site PR to review or mark |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.93-3 🚀
Bundle Size Analysis (Sentry): |
…lign-workspace-selector-logic' + fixed
|
🚀 Cherry-picked to staging by https://github.com/lakchote in version: 9.5.15-1 🚀
|
|
No help site update is needed. This PR reverts Align workspace selector logic, and that change's docs PR ([HOLD] Docs updates for E/A#101430 clarify which workspace new reports open on) was closed without merging. The articles still say what this revert restores: "Select Create report to open a draft report on your default workspace" (Create-and-Submit-Reports.md:15). |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.15-3 🚀
Bundle Size Analysis (Sentry): |
Reverts #101430
Fixed issues
$ #102130