[Domain Control] [Release 4] Add Expensify Card Preferred Workspace Toggle to DomainGroupDetailsPage#88713
Conversation
…tails-page' into jakubstec/domains/security-group-details-page-group-permissions4
|
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 |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…urity-group-details-page-group-permissions4
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4238a4c01
ℹ️ 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 isDisabled = !isPreferredPolicyEnabled || !hasWorkspaceCard; | ||
| const isEnabled = !isDisabled && !!group?.overridePreferredPolicyWithCardPolicy; |
There was a problem hiding this comment.
Keep toggle state independent from disabled prerequisites
When prerequisites are missing (for example, a group had this setting enabled and later loses its preferred workspace/card eligibility), isDisabled becomes true and isEnabled is forced to false by !isDisabled && .... That makes ToggleSettingOptionRow render the switch as off even though group.overridePreferredPolicyWithCardPolicy may still be persisted as true, so the UI misrepresents the actual saved configuration. Use disabled only to prevent interaction, and derive isActive directly from the stored setting value.
Useful? React with 👍 / 👎.
…urity-group-details-page-group-permissions4
…urity-group-details-page-group-permissions4
| const [cardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`, { | ||
| selector: filterInactiveCards, | ||
| }); | ||
| const hasWorkspaceCard = !!preferredPolicy?.areExpensifyCardsEnabled && !isEmptyObject(cardsList); |
There was a problem hiding this comment.
We should check the cards associated with the domain (under the private_expensifyCardSettings_<domainAccountID> Onyx data key), rather than the cards associated with the policy.
A domain is considered to be using the Expensify Card as long as the domain card settings object is defined.
Pseudocode:
const cardSettings = useOnyx(`private_expensifyCardSettings_${domainAccountID}`);
const isUsingExpensifyCard = !!cardSettings?.paymentBankAccountID;Example:
Below is a screenshot of private_expensifyCardSettings_<domainAccountID> Onyx data returned from OpenDomainInitialPage API:
There was a problem hiding this comment.
A domain is considered to be using the Expensify Card as long as the domain card settings object is defined.
So can we just do
const isUsingExpensifyCard = !!cardSettings? Because for NestedExpensifyCardSettings ,cardSettings.paymentBankAccountID is undefined ( it lives under e.g. US.paymentBankAccountID )so i think checking cardSettings is just enough
There was a problem hiding this comment.
Agree, !!cardSettings is sufficient
…ecurity-group-details-page-group-permissions4
…locking modal for disabled toggle
|
@Expensify/design Hey, could you verify, what message do we want on blocking modal, when user tries to toggle disabled Screen.Recording.2026-05-06.at.11.23.51.mov |
|
That would be more of a @Expensify/marketing @jamesdeanexpensify request. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 206f5f5e61
ℹ️ 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".
| subtitle={translate('domain.groups.expensifyCardPreferredWorkspaceDescription')} | ||
| switchAccessibilityLabel={translate('domain.groups.expensifyCardPreferredWorkspace')} | ||
| shouldPlaceSubtitleBelowSwitch | ||
| disabled={isDisabled} |
There was a problem hiding this comment.
Allow disabling an already-enabled card override
When this override is already persisted as enabled and the group later loses a prerequisite (for example Preferred Workspace is turned off or the domain card settings disappear), this disables the switch entirely; Switch only runs disabledAction in that state, so admins can no longer turn the saved overridePreferredPolicyWithCardPolicy value off from this page and are left with a locked-on stale setting. Gate the disabled state only when trying to enable the setting (e.g. include !isActive) so an already-enabled override can still be cleared.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Should not be the case, because we will toggle off Expensify Card Preferred Workspace each time Preferred Workspace is set to false. But it might be a safe guard though.
…each time Preferred Workspace is set to false
…urity-group-details-page-group-permissions4
| onCloseError={() => clearDomainSecurityGroupSettingError(domainAccountID, groupID, 'overridePreferredPolicyWithCardPolicyErrors')} | ||
| /> | ||
| </View> | ||
| <ConfirmModal |
There was a problem hiding this comment.
fixed! I'll make the same change to PreferredWorkspaceToggle in follow-up PR regarding other matter, because it shares the same behavior
Screen.Recording.2026-05-07.at.10.18.50.mov
There was a problem hiding this comment.
#89888
We will mark ConfirmModal as deprecated soon!
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-05-07.11.19.11.movAndroid: mWeb Chrome2026-05-07.11.19.11.moviOS: HybridApp2026-05-07.11.10.52.moviOS: mWeb Safari2026-05-07.11.15.24.movMacOS: Chrome / Safari2026-05-07.09.39.04.mov |
|
@ZhenjaHorbach i've applied fixes! |
|
LGTM! |
…ecurity-group-details-page-group-permissions4
|
🚧 @mountiny 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@Expensify/design for this, do we normally use a blocking modal or tooltip? Happy to give some feedback on the copy either way, just caught it a bit late. |
|
I think in this case the group decided that an alert modal was most appropriate. Other times we simply use text below the locked toggle. |
|
How does this sound?
|
|
Thanks, that looks good to me, looking into that |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.69-0 🚀
Bundle Size Analysis (Sentry): |
|
@jamesdeanexpensify PR with your suggestion is here #90005 |
|
Thank you! |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.69-18 🚀
|
Per the marketing-approved copy in PR Expensify#88713, update the toggle label to sentence case and rewrite the disabled-state modal body so it clearly tells admins what they need to enable first. Co-authored-by: Cursor <cursoragent@cursor.com>
Explanation of Change
Add
Expensify Card Preferred Workspacesetting for Domain Security Group RHP.Fixed Issues
$ #80133
PROPOSAL:
Tests
Preconditions:
/domains/<domainAccountID>/groupsExpensify Card Preferred Workspaceto trueOffline tests
N/A
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
MacOS: Chrome / Safari
Screen.Recording.2026-05-06.at.11.14.50.mov
Screen.Recording.2026-05-06.at.11.23.51.mov