[PM-42110] feat: Make a copy of a policy-restricted Send - #7276
Merged
andrebispo5 merged 12 commits intoAug 20, 2026
Conversation
github-actions
Bot
added
app:password-manager
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7276 +/- ##
==========================================
+ Coverage 86.07% 86.30% +0.22%
==========================================
Files 1024 891 -133
Lines 67998 65479 -2519
Branches 9904 9857 -47
==========================================
- Hits 58530 56511 -2019
+ Misses 5940 5466 -474
+ Partials 3528 3502 -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:
|
Contributor
🤖 Bitwarden Claude Code ReviewOverall Assessment: REQUEST CHANGES Reviewed the new copy mode for the add/edit Send screen ( Code Review Details
|
There was a problem hiding this comment.
Pull request overview
This PR adds a “Make a copy” call-to-action to the SendControls policy restriction banner on View Send, and introduces a copy mode for the Add/Edit Send screen that pre-fills a new Send from a restricted one while ensuring the original is never modified.
Changes:
- Adds
ModeType.COPY/AddEditSendType.CopyItemand routes View Send → Add/Edit Send in copy mode via a newNavigateToCopyevent. - Implements copy-mode state derivation from
SendView(toCopyViewState) and factors access-type derivation intoSendView.toSendAuth(). - Adds/updates unit + Compose tests covering the new CTA visibility, click behavior, navigation, and copy-mode policy application.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/main/res/values/strings.xml | Adds the “Make a copy” string resource for the CTA. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/viewsend/ViewSendViewModel.kt | Adds action + event to trigger navigation into copy flow. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/viewsend/ViewSendScreen.kt | Wires the CTA into the policy restriction banner and handles NavigateToCopy → Add/Edit route (COPY). |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/viewsend/model/SendPolicyRestriction.kt | Adds isCopyable to drive whether the CTA is shown/enabled. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/model/AddEditSendType.kt | Introduces CopyItem(sendItemId) for copy mode. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendNavigation.kt | Adds ModeType.COPY and maps it to AddEditSendType.CopyItem. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt | Loads source Send for CopyItem, derives copy-mode content, and ensures Save creates a new Send. |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/util/SendViewExtensions.kt | Extracts SendView.toSendAuth() and reuses it from toViewState(). |
| app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/util/SendViewCopyExtensions.kt | Adds SendView.toCopyViewState() for copy-mode prefill behavior. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/viewsend/ViewSendViewModelTest.kt | Adds test for MakeACopyClick → NavigateToCopy. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/viewsend/ViewSendScreenTest.kt | Adds tests for CTA visibility rules, click dispatch, and navigation to COPY route. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/util/SendViewExtensionsTest.kt | Adds focused tests for toSendAuth() behavior. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/util/SendViewCopyExtensionsTest.kt | Adds coverage for copy-mode view-state mapping rules. |
| app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt | Adds copy-mode tests for enforced policy application and save behavior (create vs update). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
from
August 18, 2026 08:40
be17bcd to
d49520d
Compare
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
2 times, most recently
from
August 18, 2026 09:37
a952968 to
66274b6
Compare
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
from
August 18, 2026 19:51
66274b6 to
ac8af0a
Compare
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
from
August 18, 2026 19:55
ac8af0a to
60f9c9a
Compare
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
from
August 19, 2026 10:27
60f9c9a to
8a31213
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (3)
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/viewsend/ViewSendViewModel.kt:178
handleMakeACopyClick()unconditionally emitsNavigateToCopy. Since the copy CTA is only valid when the currentpolicyRestrictionis copyable, this should be guarded (e.g., return early unlessstate.policyRestriction?.isCopyable == true) to prevent accidental navigation to copy mode for non-copyable restrictions (file sends / type-not-allowed) or if the action is triggered from a future UI path.
private fun handleMakeACopyClick() {
sendEvent(ViewSendEvent.NavigateToCopy(sendType = state.sendType, sendId = state.sendId))
}
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/viewsend/ViewSendScreen.kt:263
- The nested use of
ithere makes thetakeIfcondition rely on Kotlin's lambda scoping rules (it.isCopyablerefers to the outerpolicyRestriction), which is easy to misread. Consider naming the outer lambda parameter (e.g.,policyRestriction?.let { restriction -> ... }) and usingtakeIf { restriction.isCopyable }or an explicitifto improve readability and reduce the risk of mistakes during refactors.
policyRestriction?.let {
BitwardenActionCard(
cardTitle = stringResource(id = BitwardenString.organization_policy_restriction),
cardSubtitle = it.message(),
actionButton = BitwardenButtonData(
label = BitwardenString.make_a_copy.asText(),
onClick = onMakeACopyClick,
)
.takeIf { _ -> it.isCopyable },
leadingContent = {
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt:560
toSendAuth()is computed twice when buildingsendAuthfor copy mode. SincetoSendAuth()maps and allocates a new list for recipient emails, it would be clearer and avoid duplicate work to compute it once into a localval currentAuth = toSendAuth()and then usecurrentAuthfor both thecurrent = ...argument and the fallback.
): AddEditSendState.ViewState.Content {
return when (state.addEditSendType) {
is AddEditSendType.CopyItem -> {
toCopyViewState(
deletionDate = clock.instant().plus(
state.enforcedDeletionHours?.toLong() ?: DEFAULT_DELETION_HOURS,
ChronoUnit.HOURS,
),
isHideEmailAddressEnabled = isHideEmailAddressEnabled,
sendAuth = state
.enforcedWhoCanAccess
?.toEnforcedSendAuth(current = toSendAuth())
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
from
August 19, 2026 10:55
d63dbf1 to
8a31213
Compare
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
from
August 19, 2026 14:53
8a31213 to
635bb1b
Compare
Base automatically changed from
PM-42108-add-policy-restriction-banner-to-view-send
to
main
August 19, 2026 15:50
| assertNull(common.expirationDate) | ||
| assertEquals("", common.passwordInput) | ||
| assertFalse(common.hasPassword) | ||
| assertFalse(common.isDeactivateChecked) |
Collaborator
There was a problem hiding this comment.
We should just assert the entire result
| sendAuth = SendAuth.None, | ||
| ) | ||
|
|
||
| assertEquals(COPY_DELETION_DATE, result.common.deletionDate) |
| /** | ||
| * The deletion window applied to a new Send when no deletion date is enforced by policy (7 days). | ||
| */ | ||
| internal const val DEFAULT_DELETION_HOURS: Long = 7 * 24L |
Collaborator
There was a problem hiding this comment.
Can we make this private now?
Contributor
Author
There was a problem hiding this comment.
It is being used in the extensions file and in the AddEditSendViewModel.
| sendAuth = sendAuth, | ||
| ) | ||
|
|
||
| assertEquals(sendAuth, result.common.sendAuth) |
Collaborator
There was a problem hiding this comment.
Can we make sure all these tests get updated to compare total state?
| copyModeState(sendAuth = SendAuth.Password) | ||
| .copy( | ||
| isSendControlsEnabled = true, | ||
| whoCanAccess = SendAccessTypeJson.PASSWORD_PROTECTED, |
Collaborator
There was a problem hiding this comment.
Instead of copying it, can we just pass these into the copyModeState function?
| .value | ||
|
|
||
| assertEquals(copyModeState(sendAuth = SendAuth.Password), state) | ||
| assertTrue(state.isAddMode) |
Collaborator
There was a problem hiding this comment.
Maybe change the isAddMode to isAddOrCopyMode?
Contributor
Author
There was a problem hiding this comment.
No need, in fact it is add mode prefilled, and if we went that route there would be a lot more to change like the name of the class AddCopyEditSend 😅
Collaborator
There was a problem hiding this comment.
Makes sense, I am a little confused how this works though:
val isAddMode: Boolean get() = addEditSendType !is AddEditSendType.EditItemWhy would this be true?
Contributor
Author
There was a problem hiding this comment.
This particular one may be a bit misleading let me rename it.
| * @param currentState The state being updated, which is the in-flight value rather than the | ||
| * [state] property so that the mapping sees the same policy data as the update it belongs to. | ||
| */ | ||
| private fun SendView.toCurrentModeViewState( |
Collaborator
There was a problem hiding this comment.
Should this go to the extensions file too?
Contributor
Author
There was a problem hiding this comment.
toAddEditViewState is already in the extensions file, this is just the bit that hands it the view model's clock, environmentRepo and isHideEmailAddressEnabled so the two call sites don't have to repeat them.
| * @param isHideEmailAddressEnabled Whether hiding the sender's email is currently allowed. | ||
| * @param sendAuth The access type to apply to the copy, which the policy may have forced. | ||
| */ | ||
| fun SendView.toCopyViewState( |
Collaborator
There was a problem hiding this comment.
Can't this go in the main SendViewExtensions file
Move `toAddEditViewState` out of the view model into `SendViewExtensions`, taking the `Clock`, base web send URL and hide-email allowance as parameters. `DEFAULT_DELETION_HOURS` and `toEnforcedSendAuth` move alongside it as `internal` so both the view model and the new extension can reach them. Tighten the tests the reviewer flagged: - `toSendAuth should carry over the original recipients` asserts the whole `SendAuth.Email` rather than only the email values, using the existing `UUID.randomUUID` static mock so `AuthEmail.id` is deterministic. - The `toCopyViewState` tests assert the entire `Content` instead of picking fields off it, and the deletion date test now uses a date distinct from the shared fixture so it is not a duplicate of the first test. - The copy mode view model tests drop the partial follow-up assertions that the complete-state assertion above them already covers. - Qualify `this.toSendAuth()` and break `.copy(` onto its own line to match the surrounding style.
Update the remaining `toCopyViewState` tests to assert the whole `ViewState.Content` rather than picking a single field off the result, so a regression in any other field is caught rather than passing silently. Give `copyModeState` the `isSendControlsEnabled`, `deletionHours` and `whoCanAccess` parameters the copy mode tests were applying through a trailing `.copy(`, so each test states its expected state in one call.
`isAddMode` covered both `AddItem` and `CopyItem`, but "add mode" already means the strict single-case check elsewhere in the codebase: the vault equivalent defines `isAddItemMode` as `is VaultAddEditType.AddItem` and gives the clone case its own `isCloneMode`. Reusing the term for the union, behind a `!is EditItem` negation that never names the cases it includes, left a reviewer asking why the property is true for a copy. Rename it to `isNewSend`, which is what every call site actually gates on: the deletion date chooser, the delete button, the overflow menu, the current access count, and adopting the policy enforced access type and deletion window. None of them distinguish `AddItem` from `CopyItem`. Define it with an exhaustive `when` so the `CopyItem` branch is stated at the definition instead of inferred from an inverted check. This matches `title` and `sourceSendItemIdOrNull` in the same file, and a future subtype now fails to compile rather than silently defaulting to new. Rename the `copy mode should be treated as add mode` test to `copy mode should be treated as a new send` to drop the retired vocabulary. The two remaining "add mode" test names keep it correctly, since both build the view model with the default `AddItem` type.
Extension files in this codebase are named after the receiver type (`CipherViewExtensions`, `AttachmentViewExtensions`, `AddEditSendStateExtensions`), and nothing else splits a second file out per operation. `toCopyViewState` is a `SendView` extension, so it belongs beside `toViewState` and `toAddEditViewState` rather than in its own `SendViewCopyExtensions` file. Move the function and its tests into `SendViewExtensions` and `SendViewExtensionsTest`, then delete the copy-specific files. The merged test fixtures drop the duplicate text-type constant, keeping `DEFAULT_TEXT_TYPE`, and rename `EXPECTED_COMMON` to `COPY_COMMON` so it reads distinctly next to `DEFAULT_COMMON` now that both live in one file.
andrebispo5
force-pushed
the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
from
August 20, 2026 17:14
8f00e07 to
df61e4c
Compare
| isSendsRestrictedByPolicy: Boolean, | ||
| shouldHideEmailAddressToggle: Boolean, | ||
| isAddMode: Boolean, | ||
| isNewSend: Boolean, |
david-livefront
approved these changes
Aug 20, 2026
andrebispo5
deleted the
PM-42110-make-a-copy-of-a-policy-restricted-send
branch
August 20, 2026 20:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-42110
📔 Objective
Adds a "Make a copy" action to the policy restriction banner. It opens the Add Send screen prefilled
from the restricted Send, with the fields the policy enforces already set, so the user can create a
compliant replacement.
Copy mode
ModeType.COPYandAddEditSendType.CopyItem. A copy loads like an edit does, then behaveslike an add for everything else: it saves with
createSend, so the original is never touchedexpiration
existing save-time error naming the allowed domains, which beats silently dropping recipients
The button
uploaded again) or a Send whose type the policy no longer allows
actionTextandonActionClickadded toBitwardenActionCardin [PM-42108] feat: Add policy restriction banner to View Send #7275Worth flagging
isAddModechanged from "is AddItem" to "is not EditItem". I traced all 14 consumers: theoverflow menu, delete button and current-access-count row all correctly hide for a copy, and both
policy paths now apply to copies, which is what you want if the policy changes while the form is
open
whenblocks overAddEditSendTypeare each handled explicitly rather than with anelse, so a future mode cannot slip throughNote for reviewers
Sixth in the stack: main <- #7266 <- #7269 <- #7272 <- #7275 <- this. Please review #7275 first.
The diff here is only the last six commits.
📸 Screenshots
Screen.Recording.2026-08-18.at.09.38.30.mov