Invalidate members on kick to prevent stale results #4382
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4382 +/- ##
==========================================
- Coverage 84.89% 84.86% -0.03%
==========================================
Files 272 272
Lines 29167 29149 -18
==========================================
- Hits 24761 24737 -24
- Misses 4406 4412 +6 ☔ View full report in Codecov by Sentry. |
| @@ -1333,6 +1333,10 @@ impl Room { | |||
| { reason: reason.map(ToOwned::to_owned) } | |||
| ); | |||
| self.client.send(request, None).await?; | |||
|
|
|||
| // Force future room members reload | |||
| self.mark_members_missing(); | |||
There was a problem hiding this comment.
Thanks, this looks sensible to me, but I'd like to double-check with @poljar .
In the meantime, I would have thought that if we need this in kick_user, we would also need it in ban_user and unban_user too. What do you think?
There was a problem hiding this comment.
Yes, you are right, its is almost certainly needed in ban_user also, I will update it there.
For unban_user they are probably not currently in the member list right, after an unban_user you would need to invite them (which would trigger an invalidate)? Unless I'm misunderstanding and the members list contains a list of unbaned users?
|
|
||
| // Force future room members reload | ||
| self.mark_members_missing(); | ||
|
|
There was a problem hiding this comment.
It would be great to have a test that checks this behaviour too.
|
Why is this needed, the next sync should give us the updated Surely we don't need to do |
|
Yeah, my intent here was to make the |
Well that depends on when you call the |
Mark the members list as 'invalid' when a user is kicked from a room, to allow a reload to happen the next time
members()is invoked.Signed-off-by: Daniel Salinas