[PM-40530] feat: Hide the hide my email switch when enforced by SendControls policy - #7244
Conversation
github-actions
Bot
added
app:password-manager
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## PM-40528-send-controls-precedence-enforcement #7244 +/- ##
=================================================================================
+ Coverage 85.75% 85.99% +0.23%
=================================================================================
Files 937 1027 +90
Lines 67000 67782 +782
Branches 9794 9834 +40
=================================================================================
+ Hits 57453 58286 +833
+ Misses 6053 5976 -77
- Partials 3494 3520 +26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🤖 Bitwarden Claude Code ReviewOverall Assessment: REQUEST CHANGES Reviewed the hide-my-email toggle visibility change on the Add/Edit Send screen, the new Code Review Details
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (!shouldHideEmailAddressToggle) { | ||
| Spacer(modifier = Modifier.height(height = 8.dp)) | ||
| BitwardenSwitch( | ||
| modifier = Modifier | ||
| .testTag("SendHideEmailSwitch") | ||
| .fillMaxWidth() | ||
| .standardHorizontalMargin(), | ||
| label = stringResource(id = BitwardenString.hide_email), | ||
| isChecked = state.common.isHideEmailChecked, | ||
| onCheckedChange = addSendHandlers.onHideEmailToggle, | ||
| readOnly = isSendsRestrictedByPolicy, | ||
| enabled = state.common.isHideEmailChecked || | ||
| state.common.isHideEmailAddressEnabled, | ||
| cardStyle = CardStyle.Full, | ||
| ) | ||
| } |
There was a problem hiding this comment.
hideEmail on an existing send.
Details and fix
The retained enabled = isHideEmailChecked || isHideEmailAddressEnabled exists so that a send created before the policy took effect (with hideEmail = true) can still be turned off by the user, even while the policy forbids turning it on. With the flag on and disableHideEmail = true, that switch is now gone, but AddEditSendState.toSendView() still writes hideEmail = common.isHideEmailChecked (util/AddEditSendStateExtensions.kt:36), so editing that send silently re-saves the policy-violating value with no way for the user to comply.
Two possible fixes:
- Keep the switch visible when
isHideEmailCheckedis true (shouldHideEmailAddressTogglegains&& !isHideEmailChecked), preserving the legacy escape hatch, or - Coerce
isHideEmailChecked = falseinhandleEffectiveSendPolicyReceivewheneffectiveSendPolicy.disableHideEmailis true, so the hidden control cannot carry a restricted value into the save.
There was a problem hiding this comment.
That is the intended behaviour.
| /** | ||
| * Helper to determine if the "hide my email" toggle should be hidden entirely rather than | ||
| * simply disabled. The SendControls policy hides the toggle, while the legacy send options | ||
| * policy continues to only disable it. | ||
| */ | ||
| val shouldHideEmailAddressToggle: Boolean | ||
| get() = isSendControlsEnabled && | ||
| (viewState as? ViewState.Content)?.common?.isHideEmailAddressEnabled == false |
There was a problem hiding this comment.
❓ QUESTION: The gate is the feature flag, not the policy source, so legacy SendOptions orgs also lose the toggle and the notice.
Details
resolveEffectiveSendPolicy folds legacy SendOptions into the effective policy when the flag is on for orgs with no SendControls policy (PolicyManagerImpl.kt:239-250: remainingSendOptions.any { it.shouldDisableHideEmail ?: false }). Once the flag rolls out, those users get the hidden toggle and, via shouldDisplayPolicyWarning, no explanatory notice either — which is the opposite of what this KDoc describes ("the legacy send options policy continues to only disable it").
Is flag-based gating the intent for rollout? If so, the KDoc on both helpers reads as policy-source-based and could be reworded; if not, both helpers would need to key off whether a SendControls policy is actually in effect.
There was a problem hiding this comment.
This is working as intended because once the Flag is enabled it will also use the new Send Control policy.
There was a problem hiding this comment.
Pull request overview
Updates the Add/Edit Send experience so that when SendControls policy enforcement disables “Hide my email” (behind the pm-31885-send-controls flag), the toggle is removed entirely and the legacy “policies affecting Send options” notice is no longer shown.
Changes:
- Added
isSendControlsEnabled+shouldHideEmailAddressToggletoAddEditSendState, and propagate through screen/content composables to conditionally omit the “Hide my email” switch. - Observes effective send policy + SendControls feature-flag changes in
AddEditSendViewModelto keep UI state up to date while the screen is open. - Expanded unit + Compose UI tests to cover flag/policy combinations and dynamic updates.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt | Tracks SendControls flag in state, observes effective policy/flag flows, and adds state helpers to hide the toggle + suppress the legacy policy notice. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreen.kt | Passes new shouldHideEmailAddressToggle state through to content. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendContent.kt | Hides the “Hide my email” switch when instructed by state. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt | Adds coverage for SendControls flag and effective-policy updates impacting the new state fields/helpers. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreenTest.kt | Adds Compose UI assertions for legacy disabled-toggle behavior vs SendControls hidden-toggle behavior + notice suppression. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-40530
📔 Objective
When an org's SendControls policy sets disableHideEmail, the "Hide my email" switch on the Add/Edit Send screen now disappears completely instead of just being greyed out.
The "One or more organization policies are affecting your Send options" notice goes away too in that case. That banner exists to explain why a control is greyed out, and once the control isn't there at all it just leaves people wondering what was affected.
Both of those only apply when the pm-31885-send-controls flag is on. With the flag off, an org's legacy SendOptions policy still greys out the switch and still shows the notice, exactly as before. Each path has its own test so the old behaviour can't quietly drift.
Nothing needed to change on the View Send screen since it has no hide-email field to begin with.
Stacked on #7239 (PM-40528), which is where the effective send policy comes from, so this targets that branch rather than main.
📸 Screenshots
Flag OFF, Policy OFF:
Flag OFF, Policy ON:
Flag ON, Policy OFF:
Flag ON, Policy ON:
