[PM-32743] Add ability to create folders during import to orgs

[mcamirault]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-32743

📔 Objective

This PR adds the ability to create folders and folder relationships when importing into an organization, which is required to enable the ability to convert collections into folders when importing into My Items as described in the ticket

📸 Screenshots

[codecov]

Codecov Report

❌ Patch coverage is 75.51020% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.05%. Comparing base (5289a5f) to head (0054f83).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...tyFramework/Vault/Repositories/CipherRepository.cs	0.00%	4 Missing and 1 partial ⚠️
...ture.Dapper/Vault/Repositories/CipherRepository.cs	0.00%	3 Missing and 1 partial ⚠️
.../Core/Tools/ImportFeatures/ImportCiphersCommand.cs	89.65%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7568      +/-   ##
==========================================
+ Coverage   59.64%   64.05%   +4.40%     
==========================================
  Files        2101     2101              
  Lines       92666    92690      +24     
  Branches     8254     8257       +3     
==========================================
+ Hits        55270    59371    +4101     
+ Misses      35443    31277    -4166     
- Partials     1953     2042      +89     

☔ View full report in Codecov by Sentry.
📢 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.

[mcamirault]

This wasn't used as far as I can tell, removed as cleanup

[mcamirault]

@claude Review this PR

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the addition of folder import support to organization vault imports. Changes extend ImportIntoOrganizationalVaultAsync and ICipherRepository.CreateAsync to accept folders and folder relationships, refactor shared folder-processing logic into ProcessFolders, and add FoldersLimit/FolderRelationshipsLimit configuration. The folder-owner pattern (folders tied to the importing user, org ciphers referenced via the per-user Folders JSON map) matches the existing personal-import implementation, and all callers of the changed signatures were updated together.

Code Review Details

No findings. Existing review threads already covered the noteworthy areas: org/personal vault separation via UserId on the Cipher row, ProcessFolders rename, lock-file/limit handling for Folders/FolderRelationships, and test coverage for the new success path.

[claude]

Claude finished @mcamirault's task in 3m 11s —— View job

---

PR Review: PM-32743 — Add ability to create folders during import to orgs

• Read changed files and understand context
• Analyze code for bugs, security, and quality
• Verify test coverage adequacy
• Post review feedback

---

Overall the change is clean and well-targeted: extracting ProcessFolders removes duplication between individual and organizational imports, and the fix replacing cipher.UserId with importingUserId in the folder JSON is necessary (org ciphers have a null UserId, so the previous expression would have produced "":"<id>" for org imports). A few items below — none of them blockers.

🟡 Suggestion — Avoid an unnecessary DB roundtrip when there are no folders

src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs:208-210

private async Task<List<Folder>> ProcessFolders(...)
{
    var userfoldersIds = (await _folderRepository.GetManyByUserIdAsync(importingUserId))...

ImportIntoOrganizationalVaultAsync now always calls ProcessFolders, which always queries the user's existing folders — even when the request includes zero folders (which will be the common case for existing clients that don't yet send Folders/FolderRelationships). Consider an early return when folders.Count == 0 && !folderRelationships.Any() to avoid the extra GetManyByUserIdAsync call on every org import.

🟡 Suggestion — Test asserts new-folder count, but not the cipher↔folder mapping

test/Core.Test/Tools/ImportFeatures/ImportCiphersAsyncCommandTests.cs:175-178

The new assertion verifies that the already-existing folder is filtered out of newFolders. Nice. But the more interesting behavior in ProcessFolders — that cipher.Folders is populated as {"<importingUserId>":"<folderId>"} based on folderRelationships — isn't asserted. Worth adding an Arg.Is<List<CipherDetails>> check confirming the JSON contains the upper-cased importingUserId and corresponding folder Id (especially since the line changed from cipher.UserId → importingUserId in this PR, which is exactly what was broken for org ciphers).

🟢 Nit — Pre-existing typo carried forward

src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs:210

userfoldersIds → userFolderIds. Pre-existing, but since the line moved into a new method this is a low-cost rename.

🟢 Nit — Comment style

src/Core/Tools/ImportFeatures/ImportCiphersCommand.cs:212-213

//Assign id... / //Need to keep... should have a space after // per the repo's prevailing style. Pre-existing.

❓ Question — Behavior under OrganizationDataOwnership policy

ImportIntoOrganizationalVaultAsync does not check OrganizationDataOwnershipPolicyRequirement (only ImportIntoIndividualVaultAsync does). I agree with your earlier comment that ciphers end up on the organization (no UserId), so they don't land in a personal vault. However, the folders created here are inherently user-owned (Folder.UserId = importingUserId) and live in the importing user's personal vault — they're how the cipher↔folder JSON map resolves on the client.

When OrganizationDataOwnership is enabled, is it the intended behavior to still materialize user-owned folder rows in the importing user's personal vault as a side effect of an org import? If so, no change needed. If not, this might warrant a similar policy gate (or just skipping ProcessFolders when the policy is enabled). Worth a quick alignment with the policy owner before merging.

✅ Things I checked and are fine

• FolderWithIdRequestModel.Id being null → falls back to Guid.Empty, won't match userfoldersIds, gets a fresh GUID via SetNewId(). Safe.
• f.ToFolder(userId) forces UserId = importingUserId on every imported folder, so a malicious client can't create a folder owned by another user.
• BulkResourceCreationService.CreateFoldersAsync exists and writes to [dbo].[Folder].
• EF mapping (Mapper.Map<List<Folder>>) resolves to Bit.Infrastructure.EntityFramework.Vault.Models.Folder, consistent with the existing folder-bulk path.
• The integration test in CipherRepositoryTests passes [] as folders, exercising the !folders.Any() branch — good.
• New FoldersLimit/FolderRelationshipsLimit plumbed through appsettings.json, GlobalSettings, and the controller cap.
  • Branch: tools/pm-32743/import-collections-to-my-items-as-folders

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud