-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[PM-27884][PM-27886][PM-27885] - Add Cipher Archives #6578
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
Conversation
src/Infrastructure.EntityFramework/Vault/Models/CipherArchive.cs
Outdated
Show resolved
Hide resolved
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6578 +/- ##
==========================================
+ Coverage 55.04% 58.97% +3.93%
==========================================
Files 1934 1934
Lines 85581 85638 +57
Branches 7656 7660 +4
==========================================
+ Hits 47106 50508 +3402
+ Misses 36686 33258 -3428
- Partials 1789 1872 +83 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@withinfocus I've reached out to the vault team regarding your comment! |
nick-livefront
left a comment
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.
Nothing jumps out to me, feels like claude has given some good feedback!
I agree with missing tests here but integration tests feel like they will be coming downstream with the follow up work
bde0ab4 to
80d1c3a
Compare
|
@jrmccannon Sorry just need one last approval. Ty! |
bea16c3
rkac-bw
left a comment
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.
LGTM

🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-27884
https://bitwarden.atlassian.net/browse/PM-27885
https://bitwarden.atlassian.net/browse/PM-27886
📔 Objective
This is the first step in the process of migrating the cipher archiveDate to use the Cipher.Archives data column
Key Changes
Updated the following:
Notes
Testing Notes
Testing was done with archiving/unarchiving via the web client without issue.
📸 Screenshots
⏰ Reminders before review
🦮 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