Don't show New workspace button when the restrict policy creation security group is enabled#72753
Conversation
…urity group is enabled
|
@eVoloshchak 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] |
Codecov Report❌ Patch coverage is
... and 1016 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
@eVoloshchak this one came up again today, can you review it? Thanks! |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / SafariScreen.Recording.2025-10-24.at.18.15.19.movMacOS: DesktopScreen.Recording.2025-10-24.at.18.34.28.mov |
@trjExpensify I have a question: What data we need to check for this restriction: App/src/hooks/usePreferredPolicy.ts Line 34 in a0b659d App/src/types/onyx/SecurityGroup.ts Lines 6 to 10 in a0b659d |
|
The |
|
@paulnjs, we can use |
@trjExpensify I mean which |
|
|
|
Are we all set here? Can @eVoloshchak complete the review? |
|
@eVoloshchak Please review again. |
|
@trjExpensify Can you help to test this PR with the real data to verify it works well? I'm just hardcoding the value for testing. |
@paulnjs, that's a job for C+, I have a private domain set up, will test this later today |
|
Thanks. |
|
@trjExpensify, are we supposed to show the onboarding to newly created users in a private domain? During the onboarding, depending on the option the user chooses, we either create a workspace for them or prompt them to create a new one
Same thing happens with the
Should these also be solved in this PR? |
This is being worked on already in this PR which you're a reviewer on haha. How did you get onboarding to show? That's what we were struggling with in that PR to move it forward without hardcoding it.
Yeah, this should be covered here. Don't show the |
Yeah, I remember it being an issue, so triggered it manually in this case, because it will show up when we fix it in #67613. I do think we shouldn't wait for that here, we can fix it right away, and QA can test this by triggering the onboarding manually using the console (won't work for native though)
This one is done
This one is not yet, thank you for clearing this up cc: @paulnjs |
Yes, that is correct
No, I meant we shouldn't wait for #67613 to proceed with this PR. As of now, the onboarding will not show for users in private domain, but when this is fixed in #67613, it will, and it will show a "Create Workspace" button to users (which we're trying to prevent) So I suggest we just move forward with this PR such that if onboarding does show, we have better handling of it for this case. But to test this QA will have to put the following into the console: @paulnjs, could you add that to the test steps please? |
The only possible action is "Skip", so there is no point in showing the modal at all.
|
Oh, why will it show when it has this domain restriction about creating workspaces in place? Looking at the PRs vid, we show them the |
|
Ah that's right, we already have that logic implemented. Thank you! |
dangrous
left a comment
There was a problem hiding this comment.
Cool! This works for me.
|
Actually - are we able to add any automated tests for this, confirming the buttons don't show when they're not supposed to? |
|
@dangrous The logic is in the UI so I'm not sure how can we write the automation test for this? |
|
@paulnjs there are a number of component tests in |
|
@dangrous I added the test. |
dd5f950 to
1839ae9
Compare
dangrous
left a comment
There was a problem hiding this comment.
Apologies, can we add one more test case - to confirm the button IS shown when enableRestrictedPolicyCreation is false? thanks!
|
@dangrous added. Please help to check it |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/dangrous in version: 9.2.41-0 🚀
|
|
This PR is failing because of issue ##73864 The issue is reproducible in: All [Attach an evidence of the PR failing with the issue] 507760974-f740041d-f34e-456a-9b59-bcf2720a215f.mp4 |
|
Ah yep we should do the same for the upgrade flow! As for QA, as long as the instructions in this particular issue still pass, then we're good here. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.2.41-6 🚀
|












Explanation of Change
Don't show New workspace button when the restrict policy creation security group is enabled
Fixed Issues
$ #72272
PROPOSAL: #72272 (comment)
Tests
New workspacebuttonNew workspacebuttonOffline tests
Same
QA Steps
Same as test
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
MacOS: Desktop