Skip to content

[PM-35442] Json user repository not updating#268

Open
mzieniukbw wants to merge 3 commits into
km/pm-35442-key-connector-not-updating-keyfrom
km/pm-35442-user-repository-json-bug
Open

[PM-35442] Json user repository not updating#268
mzieniukbw wants to merge 3 commits into
km/pm-35442-key-connector-not-updating-keyfrom
km/pm-35442-user-repository-json-bug

Conversation

@mzieniukbw
Copy link
Copy Markdown
Contributor

@mzieniukbw mzieniukbw commented May 25, 2026

🎟️ Tracking

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

📔 Objective

JSON file repository UpdateAsync and DeleteAsync not persisting changes.
Root cause: CreateAsync stores records using JsonUserKeyModel (string ID) but UpdateAsync/DeleteAsync inherited from the base Repository class which passes a Guidto ReplaceOneAsync/DeleteOneAsync, which never matches.

Added integration tests for all user key repository implementations: JSON file, EF (Sqlite, Microsoft Sql server, PostgreSQL, MySql, MariaDb), Mongo.

📸 Screenshots

@mzieniukbw mzieniukbw requested a review from a team as a code owner May 25, 2026 20:59
@mzieniukbw mzieniukbw requested a review from Thomas-Avery May 25, 2026 20:59
@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.98%. Comparing base (f066fef) to head (98cc46b).

Additional details and impacted files
@@                              Coverage Diff                               @@
##           km/pm-35442-key-connector-not-updating-key     #268      +/-   ##
==============================================================================
+ Coverage                                       37.73%   42.98%   +5.24%     
==============================================================================
  Files                                              49       49              
  Lines                                            1823     1833      +10     
  Branches                                           99       99              
==============================================================================
+ Hits                                              688      788     +100     
+ Misses                                           1100     1010      -90     
  Partials                                           35       35              

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

Copy link
Copy Markdown

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

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

Nice addition on the integration tests.

Comment thread test/KeyConnector.Tests/Repositories/EntityFramework/UserKeyRepositoryTests.cs Outdated
Comment thread src/KeyConnector/Repositories/JsonFile/UserKeyRepository.cs Outdated
@mzieniukbw mzieniukbw requested a review from Thomas-Avery May 26, 2026 21:40
Thomas-Avery
Thomas-Avery previously approved these changes May 27, 2026
Copy link
Copy Markdown

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

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

LGTM

@mzieniukbw mzieniukbw marked this pull request as draft May 27, 2026 16:21
@mzieniukbw mzieniukbw force-pushed the km/pm-35442-user-repository-json-bug branch from 026baa3 to 98cc46b Compare May 27, 2026 16:22
@mzieniukbw mzieniukbw changed the base branch from main to km/pm-35442-key-connector-not-updating-key May 27, 2026 16:22
@mzieniukbw
Copy link
Copy Markdown
Contributor Author

@Thomas-Avery To ease QA testing, i decided to cherry-pick the commits on top of km/pm-35442-key-connector-not-updating-key. No other changes were made.

@mzieniukbw mzieniukbw marked this pull request as ready for review May 27, 2026 16:23
@sonarqubecloud
Copy link
Copy Markdown

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.

2 participants