Skip to content

Conversation

@nick-livefront
Copy link
Contributor

@nick-livefront nick-livefront commented Oct 14, 2025

🎟️ Tracking

PM-26772

📔 Objective

When importing archived ciphers:

  • Into an organization, throw an error. Archived ciphers are not allowed in organizations
  • Into a personal vault, persist the archive date so the ciphers still appear in the archived vault.

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@nick-livefront nick-livefront requested review from a team as code owners October 14, 2025 03:09
@nick-livefront nick-livefront requested a review from r-tome October 14, 2025 03:09
@codecov
Copy link

codecov bot commented Oct 14, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 54.81%. Comparing base (42568b6) to head (27e396a).
⚠️ Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
...dminConsole/Helpers/BulkResourceCreationService.cs 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6452      +/-   ##
==========================================
+ Coverage   50.73%   54.81%   +4.07%     
==========================================
  Files        1866     1866              
  Lines       82705    82707       +2     
  Branches     7310     7320      +10     
==========================================
+ Hits        41961    45334    +3373     
+ Misses      39143    35692    -3451     
- Partials     1601     1681      +80     

☔ 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.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2025

Logo
Checkmarx One – Scan Summary & Details3ebf749e-f3f5-4dcf-a5ee-80affac37e85

Great job! No new security vulnerabilities introduced in this pull request


var ciphers = fixture.Build<CipherRequestModel>()
.With(_ => _.OrganizationId, orgId.ToString())
.With(_ => _.FolderId, Guid.NewGuid().ToString())
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ Setting the folderId on orgItems isn't necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

r-tome
r-tome previously approved these changes Oct 15, 2025
Copy link
Contributor

@r-tome r-tome left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines 111 to 112
.With(c => c.OrganizationId, Guid.NewGuid().ToString())
.With(c => c.FolderId, Guid.NewGuid().ToString())
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ The cipher when imported into an individual vault, won't have an orgId set and for the sake of the test a folderId is also not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to remove the organization Id. For the FolderId I had to set it to null otherwise the random guid that was generated would be parsed and result in an error being thrown. 27e396a

@nick-livefront nick-livefront merged commit b63fdfa into main Oct 22, 2025
44 of 45 checks passed
@nick-livefront nick-livefront deleted the vault/pm-266772/import-archived branch October 22, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants