Skip to content

[PM-40528] feat: Wire SendControls precedence into DisableSend/SendOptions enforcement points - #7239

Merged
andrebispo5 merged 10 commits into
mainfrom
PM-40528-send-controls-precedence-enforcement
Aug 11, 2026
Merged

[PM-40528] feat: Wire SendControls precedence into DisableSend/SendOptions enforcement points#7239
andrebispo5 merged 10 commits into
mainfrom
PM-40528-send-controls-precedence-enforcement

Conversation

@andrebispo5

@andrebispo5 andrebispo5 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🎟️ 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-controls flag 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 querying DISABLE_SEND / SEND_OPTIONS directly:

  • VaultUnlockedNavBarViewModel (Sends tab visibility)
  • SendViewModel (policy warning banner)
  • VaultItemListingViewModel (policy warning banner)
  • AddEditSendViewModel (Hide my email toggle)

AddEditSendState also 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.

@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:feature Change Type - Feature Development labels Aug 4, 2026
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.23529% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.31%. Comparing base (eda4fa4) to head (aecf556).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...twarden/data/platform/manager/PolicyManagerImpl.kt 82.97% 0 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7239      +/-   ##
==========================================
+ Coverage   86.11%   86.31%   +0.20%     
==========================================
  Files         894      919      +25     
  Lines       65210    66222    +1012     
  Branches     9756     9780      +24     
==========================================
+ Hits        56155    57159    +1004     
- Misses       5560     5561       +1     
- Partials     3495     3502       +7     
Flag Coverage Δ
app-data 17.66% <57.35%> (-0.16%) ⬇️
app-ui-auth-tools 18.52% <22.05%> (-0.15%) ⬇️
app-ui-platform 16.44% <4.41%> (-0.02%) ⬇️
app-ui-vault 27.92% <4.41%> (+0.52%) ⬆️
authenticator 6.09% <0.00%> (+0.01%) ⬆️
lib-core-network-bridge 4.11% <0.00%> (+0.01%) ⬆️
lib-data-ui 1.20% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@andrebispo5
andrebispo5 marked this pull request as ready for review August 4, 2026 15:05
@andrebispo5
andrebispo5 requested review from a team and david-livefront as code owners August 4, 2026 15:05
Copilot AI review requested due to automatic review settings August 4, 2026 15:05
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new EffectiveSendPolicy model, the PolicyManager.getEffectiveSendPolicy() / getEffectiveSendPolicyFlow() precedence helper, and the four migrated enforcement points (VaultUnlockedNavBarViewModel, SendViewModel, VaultItemListingViewModel, AddEditSendViewModel). Flag-off behavior is byte-for-byte equivalent to the previous DISABLE_SEND / SEND_OPTIONS logic (including the getActivePolicies<SendOptions>() decode semantics), and the flag-on path only suppresses legacy policies for organizations that have an active, successfully-decoded SendControls policy, so undecodable or disabled SendControls policies safely fall back to legacy enforcement. I confirmed no remaining DISABLE_SEND / SEND_OPTIONS enforcement sites were missed, that the removed imports are unused, and that the new @Parcelize state fields use Parcelize-supported enum types.

Code Review Details
  • ♻️ : Flow test named for a feature-flag change never toggles the flag, leaving the flag-on flow path uncovered
    • app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PolicyManagerTest.kt:937

Also noted, no action required for this PR: when multiple organizations have active SendControls policies, whoCanAccess / allowedDomains / deletionHours / allowedSendTypes come from whichever policy happens to be first (PolicyManagerImpl.kt:240) — worth revisiting with most-restrictive resolution in the follow-up ticket that wires these into the UI.

}

@Test
fun `getEffectiveSendPolicyFlow should re-emit when the feature flag changes`() = runTest {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ 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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 EffectiveSendPolicy plus PolicyManager.getEffectiveSendPolicy() / getEffectiveSendPolicyFlow() and implemented precedence resolution in PolicyManagerImpl.
  • 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 AddEditSendState with 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.

Comment on lines +227 to +241
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

Comment on lines +936 to +938
@Test
fun `getEffectiveSendPolicyFlow should re-emit when the feature flag changes`() = runTest {
val userStateJson = mockk<UserStateJson> {
Comment on lines 323 to 326
@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 {
),
)

assertEquals(false, policyManager.getEffectiveSendPolicy().disableSend)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be validating the entire state

),
)

assertEquals(true, policyManager.getEffectiveSendPolicy().disableSend)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate the whole state

),
)

assertEquals(true, policyManager.getEffectiveSendPolicy().disableHideEmail)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate whole state here also

),
)

assertEquals(true, awaitItem().disableSend)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate whole state

mutableSendControlsFlagFlow.value = true

// Flag on, so the organization's SendControls policy supersedes its legacy one.
assertEquals(false, awaitItem().disableSend)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

.getEffectiveSendPolicyFlow()
.test {
// Flag off, so the organization's legacy DisableSend policy applies.
assertEquals(true, awaitItem().disableSend)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate whole state here also

@@ -328,18 +322,18 @@ class VaultUnlockedNavBarViewModelTest : BaseViewModelTest() {

@Suppress("MaxLineLength")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still need this suppress?

@aj-rosado aj-rosado left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@andrebispo5
andrebispo5 added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit f92ae63 Aug 11, 2026
23 checks passed
@andrebispo5
andrebispo5 deleted the PM-40528-send-controls-precedence-enforcement branch August 11, 2026 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app:password-manager Bitwarden Password Manager app context t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants