-
Notifications
You must be signed in to change notification settings - Fork 246
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
fix(timeline): update responses after a UTD has been decrypted #4210
fix(timeline): update responses after a UTD has been decrypted #4210
Conversation
|
||
let first_item_again = | ||
assert_next_matches!(stream, VectorDiff::Set { index: 0, value } => value); | ||
assert_matches!(first_item_again.content(), TimelineItemContent::RedactedMessage); | ||
assert_matches!(first_item_again.original_json(), None); |
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.
The order of the assertion is changed here, because now we update the responses before the original event. This avoids a copy in the timeline code 👀 . You would be right calling this a code smell and asking for a change here, as I didn't bother too much (observers wouldn't notice a big difference).
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4210 +/- ##
==========================================
+ Coverage 84.88% 84.90% +0.02%
==========================================
Files 272 272
Lines 29133 29125 -8
==========================================
Hits 24730 24730
+ Misses 4403 4395 -8 ☔ View full report in Codecov by Sentry. |
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.
Thanks, I was confused for a bit longer than I'd like to admit, but I think that this looks good.
@@ -1190,8 +1190,34 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { | |||
} | |||
} | |||
|
|||
Flow::Remote { position: TimelineItemPosition::Update(idx), .. } => { | |||
Flow::Remote { | |||
event_id: decrypted_event_id, |
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.
Not really part of this PR but, oh boy, I feel like this could express that this is an UTD -> Decrypted transition a bit better.
Flow::Remote
is generally for remote echoes and TimlineItemPosition::Update
does not sound UTD specific at all.
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.
Good idea to rename this Update
; will do in a follow-up, thanks!
f8b231a
to
2665ea3
Compare
Fixes #4196.