[PM-40528] feat: Wire SendControls precedence into DisableSend/SendOptions enforcement points - #7239
[PM-40528] feat: Wire SendControls precedence into DisableSend/SendOptions enforcement points#7239andrebispo5 wants to merge 7 commits into
Conversation
github-actions
Bot
added
app:password-manager
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7239 +/- ##
==========================================
- Coverage 86.11% 85.75% -0.36%
==========================================
Files 894 937 +43
Lines 65179 67000 +1821
Branches 9741 9794 +53
==========================================
+ Hits 56126 57453 +1327
- Misses 5574 6053 +479
- Partials 3479 3494 +15
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: APPROVE Reviewed the new Code Review Details
Also noted, no action required for this PR: when multiple organizations have active SendControls policies, |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| } | ||
|
|
||
| @Test | ||
| fun `getEffectiveSendPolicyFlow should re-emit when the feature flag changes`() = runTest { |
There was a problem hiding this comment.
♻️ DEBT: This test never changes the feature flag, so the name is misleading and the flag-on path of the flow stays uncovered.
Details and fix
mutableSendControlsFlagFlow stays false for the whole test; the only mutation is mutablePolicyFlow, so what is actually verified is re-emission on policy change. That leaves the isSendControlsEnabled = true branch of getEffectiveSendPolicyFlow() (the SendControls precedence path through combine) untested — consistent with the partials Codecov reports in PolicyManagerImpl.
Suggested fix: rename to `getEffectiveSendPolicyFlow should re-emit when the underlying policies change` and add a second test that toggles mutableSendControlsFlagFlow.value = true while SendControls and DisableSend policies are both active, asserting the emitted disableSend flips.
There was a problem hiding this comment.
Pull request overview
Centralizes Send policy precedence by introducing an EffectiveSendPolicy abstraction and PolicyManager helpers that resolve between the new SendControls (type 21) policy and legacy DisableSend/SendOptions policies (gated by the SendControls feature flag). Updates existing UI enforcement points and tests to rely on this precedence-resolved policy.
Changes:
- Added
EffectiveSendPolicyplusPolicyManager.getEffectiveSendPolicy()/getEffectiveSendPolicyFlow()and implemented precedence resolution inPolicyManagerImpl. - Updated Send-related UI enforcement points (Sends tab visibility, Send screens/banners, Hide my email toggle) to use the effective policy APIs.
- Updated/added unit and screen tests to mock/validate effective-policy behavior; extended
AddEditSendStatewith new (currently unused) effective-policy fields.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PolicyManager.kt | Adds effective Send policy APIs to the PolicyManager interface. |
| app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PolicyManagerImpl.kt | Implements effective Send policy resolution + flow combining feature flag and policy flows. |
| app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/EffectiveSendPolicy.kt | Introduces the precedence-resolved Send policy model. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModel.kt | Routes Sends tab visibility through effective Send policy. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModel.kt | Routes Send policy enforcement/banner through effective Send policy. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt | Uses effective policy for Hide my email enablement and stores new effective-policy fields in state. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt | Routes Send policy warning behavior through effective Send policy. |
| app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PolicyManagerTest.kt | Adds coverage for effective policy resolution and flow behavior. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/vaultunlockednavbar/VaultUnlockedNavBarViewModelTest.kt | Updates tests to drive Sends-disabled behavior via effective policy flow. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModelTest.kt | Updates mocks/assertions to use effective Send policy APIs. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreenTest.kt | Updates default state to include new effective-policy fields. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt | Updates tests for Hide my email enablement to use effective policy. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt | Updates mocks to use effective Send policy APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val decodedSendControls = sendControlsPolicies | ||
| .mapNotNull { policy -> | ||
| (policy.policyInformation as? PolicyInformation.SendControls) | ||
| ?.let { policy.organizationId to it } | ||
| } | ||
| val organizationIdsWithSendControls = decodedSendControls.map { it.first }.toSet() | ||
|
|
||
| val remainingDisableSendPolicies = disableSendPolicies | ||
| .filterNot { organizationIdsWithSendControls.contains(it.organizationId) } | ||
| val remainingSendOptions = sendOptionsPolicies | ||
| .filterNot { organizationIdsWithSendControls.contains(it.organizationId) } | ||
| .mapNotNull { it.policyInformation as? PolicyInformation.SendOptions } | ||
|
|
||
| val firstSendControls = decodedSendControls.firstOrNull()?.second | ||
|
|
| @Test | ||
| fun `getEffectiveSendPolicyFlow should re-emit when the feature flag changes`() = runTest { | ||
| val userStateJson = mockk<UserStateJson> { |
| @Suppress("MaxLineLength") | ||
| @Test | ||
| fun `DISABLE_SEND policy flow update with disabled policy should set areSendsDisabled to false`() = | ||
| fun `effective send policy flow update with disabled policy should set areSendsDisabled to false`() = | ||
| runTest { |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-40528
📔 Objective
Adds a single place that decides which Send policy actually applies — the new type-21 SendControls policy or the legacy DisableSend/SendOptions policies — and swaps every existing enforcement point over to it.
When the
pm-31885-send-controlsflag is off, or an org doesn't have an active SendControls policy, nothing changes. When it's on and an org has an active SendControls policy, that org's legacy DisableSend/SendOptions policies are ignored in favor of the SendControls values, while other orgs' legacy policies still apply as before.Four call sites now go through the new
PolicyManager.getEffectiveSendPolicy()/getEffectiveSendPolicyFlow()helper instead of queryingDISABLE_SEND/SEND_OPTIONSdirectly:VaultUnlockedNavBarViewModel(Sends tab visibility)SendViewModel(policy warning banner)VaultItemListingViewModel(policy warning banner)AddEditSendViewModel(Hide my email toggle)AddEditSendStatealso picks up four new fields (whoCanAccess,allowedDomains,deletionHours,allowedSendTypes) sourced from the effective policy. They aren't wired into any UI yet — a later ticket will build on them.