-
Notifications
You must be signed in to change notification settings - Fork 1k
[PM-40530] feat: Hide "hide my email switch" when enforced by SendControls policy #7244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9f98b6b
f1097f0
d43829d
5dd4e4b
43f5583
1b452eb
7889a70
984f8c3
3263197
1f71ad7
41931ff
c2e3343
686948f
252bbaf
349d537
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import android.net.Uri | |
| import android.os.Parcelable | ||
| import androidx.lifecycle.SavedStateHandle | ||
| import androidx.lifecycle.viewModelScope | ||
| import com.bitwarden.core.data.manager.model.FlagKey | ||
| import com.bitwarden.core.data.repository.model.DataState | ||
| import com.bitwarden.core.data.repository.util.takeUntilLoaded | ||
| import com.bitwarden.data.repository.util.baseWebSendUrl | ||
|
|
@@ -23,6 +24,7 @@ import com.bitwarden.ui.util.asText | |
| import com.bitwarden.ui.util.concat | ||
| import com.x8bit.bitwarden.data.auth.repository.AuthRepository | ||
| import com.x8bit.bitwarden.data.billing.manager.PremiumStateManager | ||
| import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager | ||
| import com.x8bit.bitwarden.data.platform.manager.PolicyManager | ||
| import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager | ||
| import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager | ||
|
|
@@ -51,6 +53,7 @@ import com.x8bit.bitwarden.ui.tools.feature.send.util.toSendUrl | |
| import dagger.hilt.android.lifecycle.HiltViewModel | ||
| import kotlinx.collections.immutable.persistentListOf | ||
| import kotlinx.collections.immutable.toImmutableList | ||
| import kotlinx.coroutines.flow.combine | ||
| import kotlinx.coroutines.flow.first | ||
| import kotlinx.coroutines.flow.launchIn | ||
| import kotlinx.coroutines.flow.map | ||
|
|
@@ -83,6 +86,7 @@ class AddEditSendViewModel @Inject constructor( | |
| private val clock: Clock, | ||
| private val clipboardManager: BitwardenClipboardManager, | ||
| private val environmentRepo: EnvironmentRepository, | ||
| private val featureFlagManager: FeatureFlagManager, | ||
| private val specialCircumstanceManager: SpecialCircumstanceManager, | ||
| private val vaultRepo: VaultRepository, | ||
| private val policyManager: PolicyManager, | ||
|
|
@@ -148,6 +152,7 @@ class AddEditSendViewModel @Inject constructor( | |
| dialogState = null, | ||
| baseWebSendUrl = environmentRepo.environment.baseWebSendUrl, | ||
| policyDisablesSend = effectiveSendPolicy.disableSend, | ||
| isSendControlsEnabled = featureFlagManager.getFeatureFlag(key = FlagKey.SendControls), | ||
| allowedDomains = effectiveSendPolicy.allowedDomains, | ||
| allowedSendTypes = effectiveSendPolicy.allowedSendTypes, | ||
| deletionHours = effectiveSendPolicy.deletionHours, | ||
|
|
@@ -182,6 +187,20 @@ class AddEditSendViewModel @Inject constructor( | |
| } | ||
| .onEach(::sendAction) | ||
| .launchIn(viewModelScope) | ||
|
|
||
| // The effective policy itself depends on the feature flag, so both are observed together | ||
| // to keep the derived state consistent whenever either one changes. | ||
| combine( | ||
| policyManager.getEffectiveSendPolicyFlow(), | ||
| featureFlagManager.getFeatureFlagFlow(key = FlagKey.SendControls), | ||
| ) { effectiveSendPolicy, isSendControlsEnabled -> | ||
| AddEditSendAction.Internal.EffectiveSendPolicyReceive( | ||
| effectiveSendPolicy = effectiveSendPolicy, | ||
| isSendControlsEnabled = isSendControlsEnabled, | ||
| ) | ||
| } | ||
| .onEach(::sendAction) | ||
| .launchIn(viewModelScope) | ||
| } | ||
|
|
||
| override fun handleAction(action: AddEditSendAction): Unit = when (action) { | ||
|
|
@@ -233,6 +252,10 @@ class AddEditSendViewModel @Inject constructor( | |
| handleRemovePasswordResultReceive(action) | ||
| } | ||
|
|
||
| is AddEditSendAction.Internal.EffectiveSendPolicyReceive -> { | ||
| handleEffectiveSendPolicyReceive(action) | ||
| } | ||
|
|
||
| is AddEditSendAction.Internal.SendDataReceive -> handleSendDataReceive(action) | ||
|
|
||
| is AddEditSendAction.Internal.GeneratorResultReceive -> { | ||
|
|
@@ -384,6 +407,31 @@ class AddEditSendViewModel @Inject constructor( | |
| } | ||
| } | ||
|
|
||
| private fun handleEffectiveSendPolicyReceive( | ||
| action: AddEditSendAction.Internal.EffectiveSendPolicyReceive, | ||
| ) { | ||
| val effectiveSendPolicy = action.effectiveSendPolicy | ||
| mutableStateFlow.update { currentState -> | ||
| currentState.copy( | ||
| policyDisablesSend = effectiveSendPolicy.disableSend, | ||
| isSendControlsEnabled = action.isSendControlsEnabled, | ||
| allowedDomains = effectiveSendPolicy.allowedDomains, | ||
| allowedSendTypes = effectiveSendPolicy.allowedSendTypes, | ||
| deletionHours = effectiveSendPolicy.deletionHours, | ||
| whoCanAccess = effectiveSendPolicy.whoCanAccess, | ||
| viewState = (currentState.viewState as? AddEditSendState.ViewState.Content) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use |
||
| ?.let { content -> | ||
| content.copy( | ||
| common = content.common.copy( | ||
| isHideEmailAddressEnabled = !effectiveSendPolicy.disableHideEmail, | ||
| ), | ||
| ) | ||
| } | ||
| ?: currentState.viewState, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Suppress("LongMethod") | ||
| private fun handleSendDataReceive(action: AddEditSendAction.Internal.SendDataReceive) { | ||
| when (val sendDataState = action.sendDataState) { | ||
|
|
@@ -905,6 +953,7 @@ data class AddEditSendState( | |
| val isShared: Boolean, | ||
| val baseWebSendUrl: String, | ||
| val policyDisablesSend: Boolean, | ||
| val isSendControlsEnabled: Boolean, | ||
| val allowedDomains: String?, | ||
| val allowedSendTypes: List<SendTypeJson>?, | ||
| val deletionHours: Int?, | ||
|
|
@@ -929,12 +978,24 @@ data class AddEditSendState( | |
| } | ||
|
|
||
| /** | ||
| * Helper to determine if the policy notice should be displayed. | ||
| * Helper to determine if the policy notice should be displayed. The notice is only relevant to | ||
| * the legacy send options policy, which disables the affected controls rather than hiding them. | ||
| * The SendControls policy removes those controls entirely, so there is nothing to explain. | ||
| */ | ||
| val shouldDisplayPolicyWarning: Boolean | ||
| get() = !policyDisablesSend && | ||
| !isSendControlsEnabled && | ||
| (viewState as? ViewState.Content)?.common?.isHideEmailAddressEnabled != true | ||
|
|
||
| /** | ||
| * 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 | ||
|
Comment on lines
+990
to
+997
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. β QUESTION: The gate is the feature flag, not the policy source, so legacy SendOptions orgs also lose the toggle and the notice. Details
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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is working as intended because once the Flag is enabled it will also use the new Send Control policy. |
||
|
|
||
| /** | ||
| * Helper to determine if the UI should display the content in add send mode. | ||
| */ | ||
|
|
@@ -1252,6 +1313,14 @@ sealed class AddEditSendAction { | |
| */ | ||
| data class CreateSendResultReceive(val result: CreateSendResult) : Internal() | ||
|
|
||
| /** | ||
| * Indicates an updated effective send policy has been received. | ||
| */ | ||
| data class EffectiveSendPolicyReceive( | ||
| val effectiveSendPolicy: EffectiveSendPolicy, | ||
| val isSendControlsEnabled: Boolean, | ||
| ) : Internal() | ||
|
|
||
| /** | ||
| * Indicates that the vault totp code result has been received. | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hideEmailon an existing send.Details and fix
The retained
enabled = isHideEmailChecked || isHideEmailAddressEnabledexists so that a send created before the policy took effect (withhideEmail = true) can still be turned off by the user, even while the policy forbids turning it on. With the flag on anddisableHideEmail = true, that switch is now gone, butAddEditSendState.toSendView()still writeshideEmail = 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:
isHideEmailCheckedis true (shouldHideEmailAddressTogglegains&& !isHideEmailChecked), preserving the legacy escape hatch, orisHideEmailChecked = falseinhandleEffectiveSendPolicyReceivewheneffectiveSendPolicy.disableHideEmailis true, so the hidden control cannot carry a restricted value into the save.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the intended behaviour.