fix: Make read receipts actually work and not explode everywhere #2481
No reviewers
Labels
No labels
Abandoned
Blocked
Bug
Changelog
Added
Changelog
Missing
Changelog
None
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/E2EE
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Matrix/T&S
Merge
Merge/Manual
Merge/Squash
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
bob the builder
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
continuwuation/continuwuity!2481
Loading…
Reference in a new issue
No description provided.
Delete branch "jade/compliant-read-receipts"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is
experimentalworking well on ellis.linkCloses #2436
Closes #741 ?
@ -59,0 +67,4 @@let mut is_dup = false;for key in keys {if let Ok(val) = self.readreceiptid_readreceipt.get(&key).await {I hate this code, might rewrite
@ -30,0 +36,6 @@return Err!(Request(Forbidden("You are not in this room.")));}if services.users.is_suspended(sender_user).await? {return Err!(Request(Forbidden("User is suspended.")));}You should still be allowed to mark messages as read even while suspended, otherwise notification counts and unread counts will just rise forever until the user is unsuspended
Technically /redact is also supposed to let you redact your own messages while suspended, I was just too lazy to special case self-redacts
Well this is running on production well ✅
c89ce0bbef66f320171e66f320171e9abbcc6f77sidenote, might drop
c17e789f9dbecause I'm not actually sure it's an improvement in UX in element x-based apps at least9abbcc6f7727de9f58a427de9f58a4f75c9a9739Dropped ✅
I think the force push broke this review but we'll see what happens
@ -48,2 +48,2 @@) {// Remove old entry) -> bool {let new_target = event.content.0.iter().find_map(|(ev_id, types)| {I think this could be a call to
user_receipt?Well, that does save a couple LOC
@ -59,0 +69,4 @@for key in keys {if let Ok(val) = self.readreceiptid_readreceipt.get(&key).await {if let Ok(existing) = serde_json::from_slice::<ReceiptEvent>(&val) {let existing_target = existing.content.0.iter().find_map(|(ev_id, types)| {ReceiptEventContentderefs to its innerBTreeMap, you don't need the.0here (also this could probably be auser_receiptcall)@ -59,0 +75,4 @@})});if let (Some(new_t), Some(old_t)) = (new_target, existing_target) {begging you to just shadow
new_targetandexisting_targetinstead of chopping letters off the variable namesHow about I just kill that entirely
@ -59,0 +76,4 @@});if let (Some(new_t), Some(old_t)) = (new_target, existing_target) {if new_t.1 == old_t.1 && new_t.2 == old_t.2 {these tuple accesses are not at all easy to understand without the benefit of an IDE
@ -59,0 +79,4 @@if new_t.1 == old_t.1 && new_t.2 == old_t.2 {if new_t.0 == old_t.0 {is_dup = true;} else {surely there's a way to do this that doesn't involve four consecutive
elsecases with the same content?I improved it a bit 🥺
527772eb8388441a409a