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.

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

feat(ui): Add function to toggle reactions from timeline #2118

Merged
merged 41 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
14121a9
Add ability to toggle reactions in timeline
jonnyandrew Jun 16, 2023
8a00ea9
Handle homeserver response to reactions
jonnyandrew Jun 19, 2023
ae07265
Fix bug in boolean expression
jonnyandrew Jun 19, 2023
f16932f
Don't execute duplicate reaction requests [WIP]
jonnyandrew Jun 19, 2023
b93ee22
Refactor
jonnyandrew Jun 20, 2023
45ceb48
Move reaction test to its own file
jonnyandrew Jun 20, 2023
26147e8
Add test for timeline inner
jonnyandrew Jun 21, 2023
5492a8a
Refactor
jonnyandrew Jun 21, 2023
464fcc8
Add more tests
jonnyandrew Jun 21, 2023
b49cf95
Fix formatting
jonnyandrew Jun 21, 2023
0aa8e12
Remove redundant extension function
jonnyandrew Jun 21, 2023
bee44c0
Fix function visibility
jonnyandrew Jun 21, 2023
4394743
Improve `ReactionGroup::by_sender`
jonnyandrew Jun 21, 2023
4e0393c
Remove explicit lock drop
jonnyandrew Jun 21, 2023
0390afd
Remove unused import
jonnyandrew Jun 21, 2023
02c5aa9
Add docs to struct
jonnyandrew Jun 21, 2023
2097fb1
Fix clippy issue
jonnyandrew Jun 21, 2023
af266d9
Fix typo
jonnyandrew Jun 22, 2023
6d15747
Merge branch 'main' of github.com:matrix-org/matrix-rust-sdk into jon…
jonnyandrew Jun 22, 2023
20bbfbc
Merge branch 'main' into jonny/timeline-reactions
jonnyandrew Jun 23, 2023
bbd9690
Move attributes below docs
jonnyandrew Jun 23, 2023
599a530
Fix error naming
jonnyandrew Jun 23, 2023
1bd7727
Rework
jonnyandrew Jun 22, 2023
bd1ff3e
Inline private function
jonnyandrew Jun 24, 2023
77ef161
Remove unnecessary curly brackets
jonnyandrew Jun 24, 2023
52fe044
Fix clippy
jonnyandrew Jun 24, 2023
e91d60a
Fix name of parameters
jonnyandrew Jun 24, 2023
566e92d
Apply review suggestion
jonnyandrew Jun 26, 2023
d92f11f
Log error if redaction does nothing
jonnyandrew Jun 26, 2023
f0fa95b
Remove unncessary comment
jonnyandrew Jun 26, 2023
4fe2968
Combine map/filter_map
jonnyandrew Jun 26, 2023
400428a
Fix typo
jonnyandrew Jun 26, 2023
2092493
Improve readibility of function signature
jonnyandrew Jun 26, 2023
29b127d
Reduce indentation in multiline string
jonnyandrew Jun 26, 2023
69bb37a
Remove semicolon
jonnyandrew Jun 26, 2023
b1ce549
Remove struct contstructors
jonnyandrew Jun 26, 2023
3858e16
Move test helpers up a module
jonnyandrew Jun 26, 2023
f750b05
Refactor matches! conditional
jonnyandrew Jun 26, 2023
83623a1
Remove recursion limit attribute
jonnyandrew Jun 26, 2023
fa72257
Fix formatting
jonnyandrew Jun 26, 2023
291612c
Fix unused variable
jonnyandrew Jun 26, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/matrix-sdk-ui/src/timeline/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ impl TimelineBuilder {
event_handler_handles: handles,
room_update_join_handle,
}),
reaction_lock: Mutex::new(()),
};

#[cfg(feature = "e2e-encryption")]
Expand Down
4 changes: 2 additions & 2 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub(super) struct TimelineEventMetadata {
pub(super) is_highlighted: bool,
}

#[derive(Clone)]
#[derive(Clone, Debug)]
pub(super) enum TimelineEventKind {
Message {
content: AnyMessageLikeEventContent,
Expand Down Expand Up @@ -516,7 +516,7 @@ impl<'a> TimelineEventHandler<'a> {
Some(event_item.with_kind(remote_event_item.with_reactions(reactions)))
});

if !self.result.items_updated > 0 {
if self.result.items_updated == 0 {
if let Some(reactions) = self.pending_reactions.get_mut(&rel.event_id) {
if !reactions.remove(&redacts) {
error!(
Expand Down
35 changes: 35 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/event_item/content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,20 @@ impl From<RoomEncryptedEventContent> for EncryptedMessage {
/// Value: The group of reactions.
pub type BundledReactions = IndexMap<String, ReactionGroup>;

pub trait BundledReactionsExt {
fn by_key(&self, key: &str) -> Option<&ReactionGroup>;
}

impl BundledReactionsExt for BundledReactions {
/// Get a reaction group by it's key
fn by_key(&self, key: &str) -> Option<&ReactionGroup> {
jplatte marked this conversation as resolved.
Show resolved Hide resolved
self.iter().find_map(|(k, v)| match k == key {
true => Some(v),
false => None,
})
}
}

// The long type after a long visibility specified trips up rustfmt currently.
// This works around. Report: https://github.com/rust-lang/rustfmt/issues/5703
type ReactionGroupInner = IndexMap<(Option<OwnedTransactionId>, Option<OwnedEventId>), OwnedUserId>;
Expand All @@ -402,6 +416,27 @@ impl ReactionGroup {
pub fn senders(&self) -> impl Iterator<Item = &UserId> {
self.values().map(AsRef::as_ref)
}

/// All reactions within this reaction group that were sent by the given
/// user Note that it is possible for a user to have sent multiple
/// reactions with the same key
jonnyandrew marked this conversation as resolved.
Show resolved Hide resolved
pub fn by_sender(
&self,
user_id: &UserId,
) -> impl Iterator<Item = (Option<&OwnedTransactionId>, Option<&OwnedEventId>)> {
let user_id = user_id.to_owned();
self.as_refs().filter_map(move |(k, v)| match v == &user_id {
true => Some(k),
false => None,
})
}
jonnyandrew marked this conversation as resolved.
Show resolved Hide resolved

pub fn as_refs(
&self,
) -> impl Iterator<Item = ((Option<&OwnedTransactionId>, Option<&OwnedEventId>), &OwnedUserId)>
{
self.iter().map(|(k, v)| ((k.0.as_ref(), k.1.as_ref()), v))
}
jplatte marked this conversation as resolved.
Show resolved Hide resolved
}

impl Deref for ReactionGroup {
Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/event_item/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ mod local;
mod remote;

pub use self::content::{
AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, InReplyToDetails,
MemberProfileChange, MembershipChange, Message, OtherState, ReactionGroup, RepliedToEvent,
RoomMembershipChange, Sticker, TimelineItemContent,
AnyOtherFullStateEventContent, BundledReactions, BundledReactionsExt, EncryptedMessage,
InReplyToDetails, MemberProfileChange, MembershipChange, Message, OtherState, ReactionGroup,
RepliedToEvent, RoomMembershipChange, Sticker, TimelineItemContent,
};
pub(super) use self::{
local::LocalEventTimelineItem,
Expand Down
116 changes: 116 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use ruma::{
fully_read::FullyReadEvent,
receipt::{Receipt, ReceiptThread, ReceiptType},
relation::Annotation,
room::redaction::RoomRedactionEventContent,
AnyMessageLikeEventContent, AnyRoomAccountDataEvent, AnySyncEphemeralRoomEvent,
},
push::Action,
Expand All @@ -58,6 +59,7 @@ use super::{
update_read_marker, Flow, HandleEventResult, TimelineEventHandler, TimelineEventKind,
TimelineEventMetadata, TimelineItemPosition,
},
reactions::ReactionToggleResult,
rfind_event_by_id, rfind_event_item,
traits::RoomDataProvider,
EventSendState, EventTimelineItem, InReplyToDetails, Message, Profile, RelativePosition,
Expand Down Expand Up @@ -266,6 +268,37 @@ impl<P: RoomDataProvider> TimelineInner<P> {
.handle_event(kind);
}

/// Handle the local redaction of an event.
#[instrument(skip_all)]
pub(super) async fn handle_local_redaction_event(
&self,
txn_id: OwnedTransactionId,
redacts: OwnedEventId,
content: RoomRedactionEventContent,
) {
// TODO: Refactor duplication with `handle_local_event`.
Copy link
Member

Choose a reason for hiding this comment

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

Looks simple enough to do now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I have not made this change so as to keep the diff as small as possible but can make a separate PR to follow up if that's ok?)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me.

let sender = self.room_data_provider.own_user_id().to_owned();
let sender_profile = self.room_data_provider.profile(&sender).await;
let event_meta = TimelineEventMetadata {
sender,
sender_profile,
timestamp: MilliSecondsSinceUnixEpoch::now(),
is_own_event: true,
// FIXME: Should we supply something here for encrypted rooms?
encryption_info: None,
read_receipts: Default::default(),
// An event sent by ourself is never matched against push rules.
is_highlighted: false,
};

let flow = Flow::Local { txn_id };
let kind = TimelineEventKind::Redaction { redacts, content };

let mut state = self.state.lock().await;
TimelineEventHandler::new(event_meta, flow, &mut state, self.track_read_receipts)
.handle_event(kind);
}

/// Update the send state of a local event represented by a transaction ID.
///
/// If no local event is found, a warning is raised.
Expand Down Expand Up @@ -337,6 +370,89 @@ impl<P: RoomDataProvider> TimelineInner<P> {
state.items.set(idx, Arc::new(new_item));
}

/// Update the send state of a local reaction represented by a transaction
/// ID.
///
/// If no local reaction is found, a warning is raised.
pub(super) async fn update_reaction_send_state(
&self,
annotation: &Annotation,
result: &ReactionToggleResult,
) {
let mut state = self.state.lock().await;
let (remote_echo_to_add, local_echo_to_remove) = match result {
ReactionToggleResult::AddSuccess { event_id, txn_id } => (Some(event_id), Some(txn_id)),
ReactionToggleResult::AddFailure { txn_id } => (None, Some(txn_id)),
ReactionToggleResult::RedactSuccess => return, // Nothing to do
ReactionToggleResult::RedactFailure { event_id } => (Some(event_id), None),
};

// TODO: Also handle pending reactions?
let related = rfind_event_item(&state.items, |it| {
it.event_id().is_some_and(|it| it == annotation.event_id)
});

let Some((idx, related)) = related else {
// Event isn't found at all.
warn!("Timeline item not found, can't update reaction ID");
return;
};
let Some(remote_related) = related.as_remote() else {
error!("inconsistent state: reaction received on a non-remote event item");
return;
};

let new_reactions = {
let mut reactions = remote_related.reactions.clone();
let reaction_group = reactions.entry(annotation.key.clone()).or_default();

// Remove the local echo from the related event.
if let Some(txn_id) = local_echo_to_remove {
if reaction_group.0.remove(&(Some(txn_id.to_owned()), None)).is_none() {
warn!(
"Tried to remove reaction by transaction ID, but didn't \
find matching reaction in the related event's reactions"
);
}
}
// Add the remote echo to the related event
if let Some(event_id) = remote_echo_to_add {
let own_user_id = self.room_data_provider.own_user_id().to_owned();
reaction_group.0.insert((None, Some(event_id.to_owned())), own_user_id);
};
if reaction_group.0.is_empty() {
reactions.remove(&annotation.key);
}

reactions
};
let new_related = related.with_kind(remote_related.with_reactions(new_reactions));

// Update the reactions stored in the timeline state
{
// Remove the local echo from reaction_map
// (should the local echo already be up-to-date after event handling?)
if let Some(txn_id) = local_echo_to_remove {
if state.reaction_map.remove(&(Some(txn_id.to_owned()), None)).is_none() {
warn!(
"Tried to remove reaction by transaction ID, but didn't \
find matching reaction in the reaction map"
);
}
}
// Add the remote echo to the reaction_map
if let Some(event_id) = remote_echo_to_add {
let own_user_id = self.room_data_provider.own_user_id().to_owned();
state
.reaction_map
.insert((None, Some(event_id.to_owned())), (own_user_id, annotation.clone()));
};
}

// Update the related item
state.items.set(idx, Arc::new(TimelineItem::Event(new_related)));
}

pub(super) async fn prepare_retry(
&self,
txn_id: &TransactionId,
Expand Down
Loading