[Hold Auth#21766] Show configured domain group preferred workspace to domain admins without policy access#90255
Open
rayane-d wants to merge 11 commits into
Open
[Hold Auth#21766] Show configured domain group preferred workspace to domain admins without policy access#90255rayane-d wants to merge 11 commits into
rayane-d wants to merge 11 commits into
Conversation
…Name field for fallback policy name.
…t policy access PreferredWorkspaceToggle resolved the workspace name from the local Onyx policy collection and fell back to the viewer's first admin policy when the configured policy wasn't there. Domain admins without access to the configured workspace saw their own first workspace as the title. Prefer the local policy name as before, but fall back to the server-supplied restrictedPrimaryPolicyName form the group onyx data instead of the viewer's first admin policy. The title now reflects the actual configured workspace regardless of whether the viewer has access to its policy data
…s no policies The preferred workspace selector was hidden when the viewing admin had no admin policies, even if a workspace was already configured for the group. Keep the selector visible whenever a server-supplied preferred policy name is available, even with an empty admin policy list.
Admins without any admin policies of their own and without a local Onyx copy of the configured policy could still tap the menu row and navigate to a picker that had nothing to offer them. Disable the row when the viewer has neither admin policies nor a local copy of the configured policy. The title still displays the configured workspace, only the navigation is gated
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
rayane-d
changed the title
rayane-d
changed the title
…k for preferredPolicyName. preferredPolicyName now comes exclusively from the policy Onyx collection, where BE merges the minimal data for non-policy-member domain admins.
rayane-d
changed the title
…es and is not an admin of the current preferred policy
Contributor
|
@codex review |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an incorrect “Preferred workspace” title shown to domain admins who aren’t members of the configured workspace by relying on a backend-provided minimal policy entry in the Onyx policy collection, and removing the old fallback to the viewer’s first admin policy.
Changes:
- Resolve the preferred workspace name solely from the configured policy’s Onyx entry (no fallback to the viewer’s first admin policy).
- Render the preferred workspace row when a configured name exists even if the viewer has no admin policies, and disable navigation when the viewer can’t change it.
Comments suppressed due to low confidence (1)
src/pages/domain/Groups/PreferredWorkspaceToggle.tsx:122
- The
disabledcondition(!hasAdminPolicies && !!preferredPolicyName)is redundant with the surrounding render guard(hasAdminPolicies || !!preferredPolicyName): whenhasAdminPoliciesis false,!!preferredPolicyNamemust already be true for this block to render. Simplifying this logic (e.g. disable when!hasAdminPoliciesif the row is shown) would make the intent clearer and reduce branching.
{(hasAdminPolicies || !!preferredPolicyName) && (
<OfflineWithFeedback
pendingAction={restrictedPrimaryPolicyIDPendingAction}
errors={restrictedPrimaryPolicyIDErrors}
onClose={() => clearDomainSecurityGroupSettingError(domainAccountID, groupID, 'restrictedPrimaryPolicyIDErrors')}
errorRowStyles={[styles.mh5]}
>
<MenuItemWithTopDescription
description={translate('domain.groups.preferredWorkspace')}
title={preferredPolicyName}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.DOMAIN_SECURITY_GROUPS_PREFERRED_WORKSPACE.getRoute(domainAccountID, groupID))}
disabled={!isEnabled || (!hasAdminPolicies && !!preferredPolicyName)}
/>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: dac134b545
ℹ️ 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".
melvin-bot
Bot
requested review from
heyjennahay and
inimaga
and removed request for
a team
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
The bug:
PreferredWorkspaceToggleresolved the workspace name from the local Onyx policy collection and fell back to the viewer's first admin policy. Domain admins without access to the configured workspace saw their own first workspace as the title - an unrelated workspace presented as if it were the configured one.The solution: The BE now returns a minimal
{avatarURL, id, name}entry into the policy Onyx collection for each referencedrestrictedPrimaryPolicyIDthe viewer isn't a member of. The existing local-policy lookup therefore returns the correct configured workspace's name for these policies, and the misleading "viewer's first admin policy" fallback can be removed.Render the menu item whenever there's an effective name to show (so domain admins without admin policies of their own can still see what's configured), and disable navigation when the viewer doesn't have full access to the configured policy.
Fixed Issues
Part of https://github.com/Expensify/Expensify/issues/634391
PROPOSAL:
Tests
Offline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as test
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