Skip to content
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.

Sign up for GitHub

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

Jump to bottom

crypto: Support Verified and VerificationViolation updates in IdentityStatusChanges streams #4154

Merged
merged 3 commits into from
Oct 25, 2024
Merged

crypto: Support Verified and VerificationViolation updates in IdentityStatusChanges streams #4154

merged 3 commits into from
Oct 25, 2024

Conversation

andybalaam
Copy link
Member

This is the Rust part of element-hq/element-meta#2492

@andybalaam andybalaam changed the base branch from main to andybalaam/dont-emit-verified-identities-at-start-of-stream October 18, 2024 14:51
@codecov Codecov
Copy link

codecov bot commented Oct 18, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 57.50000% with 17 lines in your changes missing coverage. Please review.

Project coverage is 84.76%. Comparing base (f8c23d8) to head (bb5e75f).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...x-sdk-crypto/src/identities/room_identity_state.rs 57.50% 17 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4154      +/-   ##
==========================================
- Coverage   84.81%   84.76%   -0.06%     
==========================================
  Files         269      269              
  Lines       28937    28955      +18     
==========================================
  Hits        24544    24544              
- Misses       4393     4411      +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andybalaam andybalaam force-pushed the andybalaam/dont-emit-verified-identities-at-start-of-stream branch from f0fb72e to 183a00a Compare October 22, 2024 11:25
@andybalaam andybalaam force-pushed the andybalaam/verified-support-in-room-identity-state branch from c767363 to 4262529 Compare October 22, 2024 11:40
Base automatically changed from andybalaam/dont-emit-verified-identities-at-start-of-stream to main October 22, 2024 11:40
@andybalaam andybalaam force-pushed the andybalaam/verified-support-in-room-identity-state branch from 4262529 to 45f4ce2 Compare October 22, 2024 11:41
@andybalaam andybalaam changed the title fix(crypto): Don't warn about verified users when subscribing to identity updates Support Verified and VerificationViolation updates in IdentityStatusChanges streams Oct 22, 2024
@andybalaam andybalaam marked this pull request as ready for review October 22, 2024 11:44
@andybalaam andybalaam requested review from a team as code owners October 22, 2024 11:44
@andybalaam andybalaam requested review from bnjbvr and BillCarsonFr and removed request for a team October 22, 2024 11:44
@bnjbvr bnjbvr changed the title Support Verified and VerificationViolation updates in IdentityStatusChanges streams crypto: Support Verified and VerificationViolation updates in IdentityStatusChanges streams Oct 22, 2024
bnjbvr
bnjbvr approved these changes Oct 22, 2024
View reviewed changes
Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

A bit overwhelmed by the test helpers, I find they obfuscate what's going on a lot, but I won't fight the change. Looked at the Rust logic only; please make sure to have a second pair of eyes checking the crypto changes are sane and the tests valid.

@andybalaam andybalaam requested a review from bnjbvr October 22, 2024 13:58
richvdh
richvdh approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Seems sensible to me, in the main

@andybalaam
refactor(crypto) Rename test functions to reflect wider name change
23969bc 
and simplify them slightly by combining the wrapper with the main
function. The separation used to be needed, but is not any more.
@andybalaam
refactor(crypto) Test RoomIdentityState by hard-coding identity states
a6dc54e
@andybalaam
feat(crypto) Support Verified and VerificationViolation updates in Id…
bb5e75f 
…entityStatusChanges streams
@andybalaam andybalaam force-pushed the andybalaam/verified-support-in-room-identity-state branch from 8d136e7 to bb5e75f Compare October 25, 2024 09:17
@andybalaam andybalaam enabled auto-merge (rebase) October 25, 2024 09:17
@andybalaam andybalaam merged commit 3558886 into main Oct 25, 2024
40 checks passed
@andybalaam andybalaam deleted the andybalaam/verified-support-in-room-identity-state branch October 25, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnjbvr bnjbvr bnjbvr approved these changes

@richvdh richvdh richvdh approved these changes

@BillCarsonFr BillCarsonFr Awaiting requested review from BillCarsonFr BillCarsonFr is a code owner automatically assigned from matrix-org/rust-crypto-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andybalaam @bnjbvr @richvdh