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(conversations): create conversation list with last message #1437

Merged
merged 15 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
191 changes: 191 additions & 0 deletions bindings_ffi/src/mls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,26 @@ impl FfiConversations {
Ok(convo_list)
}

pub async fn list_conversations(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

default order is:
ORDER BY COALESCE(rm.sent_at_ns, g.created_at_ns) DESC

Copy link
Contributor

@nplasterer nplasterer Dec 21, 2024

Choose a reason for hiding this comment

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

I'm not sure I understand when this is supposed to be used instead of the list() function above? cc @mchenani

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I saw in android, we fetched the conversations and the get the last message one by one and map them together.
we can replace it with this.

&self,
) -> Result<Vec<Arc<FfiConversationListItem>>, GenericError> {
let inner = self.inner_client.as_ref();
let convo_list: Vec<Arc<FfiConversationListItem>> = inner
.list_conversations()?
.into_iter()
.map(|conversation_item| {
Arc::new(FfiConversationListItem {
conversation: conversation_item.group.into(),
last_message: conversation_item
.last_message
.map(|stored_message| stored_message.into()),
})
})
.collect();

Ok(convo_list)
}

pub async fn list_groups(
&self,
opts: FfiListConversationsOptions,
Expand Down Expand Up @@ -1142,6 +1162,13 @@ pub struct FfiConversation {
inner: MlsGroup<RustXmtpClient>,
}

#[derive(uniffi::Object)]
#[allow(unused_variables, dead_code)]
pub struct FfiConversationListItem {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

introduced this struct to avoid changing all the current functions!

conversation: FfiConversation,
last_message: Option<FfiMessage>,
}

impl From<MlsGroup<RustXmtpClient>> for FfiConversation {
fn from(mls_group: MlsGroup<RustXmtpClient>) -> FfiConversation {
FfiConversation { inner: mls_group }
Expand Down Expand Up @@ -2667,6 +2694,170 @@ mod tests {
assert!(stream_messages.is_closed());
}

#[tokio::test(flavor = "multi_thread", worker_threads = 5)]
async fn test_list_conversations_last_message() {
// Step 1: Setup test client Alix and bo
let alix = new_test_client().await;
let bo = new_test_client().await;

// Step 2: Create a group and add messages
let alix_conversations = alix.conversations();

// Create a group
let group = alix_conversations
.create_group(
vec![bo.account_address.clone()],
FfiCreateGroupOptions::default(),
)
.await
.unwrap();

// Add messages to the group
group
.send("First message".as_bytes().to_vec())
.await
.unwrap();
group
.send("Second message".as_bytes().to_vec())
.await
.unwrap();

// Step 3: Synchronize conversations
alix_conversations
.sync_all_conversations(None)
.await
.unwrap();

// Step 4: List conversations and verify
let conversations = alix_conversations.list_conversations().await.unwrap();

// Ensure the group is included
assert_eq!(conversations.len(), 1, "Alix should have exactly 1 group");

let last_message = conversations[0].last_message.as_ref().unwrap();
assert_eq!(
last_message.content,
"Second message".as_bytes().to_vec(),
"Last message content should be the most recent"
);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 5)]
async fn test_list_conversations_no_messages() {
// Step 1: Setup test clients Alix and Bo
let alix = new_test_client().await;
let bo = new_test_client().await;

let alix_conversations = alix.conversations();

// Step 2: Create a group with Bo but do not send messages
alix_conversations
.create_group(
vec![bo.account_address.clone()],
FfiCreateGroupOptions::default(),
)
.await
.unwrap();

// Step 3: Synchronize conversations
alix_conversations
.sync_all_conversations(None)
.await
.unwrap();

// Step 4: List conversations and verify
let conversations = alix_conversations.list_conversations().await.unwrap();

// Ensure the group is included
assert_eq!(conversations.len(), 1, "Alix should have exactly 1 group");

// Verify that the last_message is None
assert!(
conversations[0].last_message.is_none(),
"Last message should be None since no messages were sent"
);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 5)]
async fn test_conversation_list_ordering() {
// Step 1: Setup test client
let client = new_test_client().await;
let conversations_api = client.conversations();

// Step 2: Create Group A
let group_a = conversations_api
.create_group(vec![], FfiCreateGroupOptions::default())
.await
.unwrap();

// Step 3: Create Group B
let group_b = conversations_api
.create_group(vec![], FfiCreateGroupOptions::default())
.await
.unwrap();

// Step 4: Send a message to Group A
group_a
.send("Message to Group A".as_bytes().to_vec())
.await
.unwrap();

// Step 5: Create Group C
let group_c = conversations_api
.create_group(vec![], FfiCreateGroupOptions::default())
.await
.unwrap();

// Step 6: Synchronize conversations
conversations_api
.sync_all_conversations(None)
.await
.unwrap();

// Step 7: Fetch the conversation list
let conversations = conversations_api.list_conversations().await.unwrap();

// Step 8: Assert the correct order of conversations
assert_eq!(
conversations.len(),
3,
"There should be exactly 3 conversations"
);

// Verify the order: Group C, Group A, Group B
assert_eq!(
conversations[0].conversation.inner.group_id, group_c.inner.group_id,
"Group C should be the first conversation"
);
assert_eq!(
conversations[1].conversation.inner.group_id, group_a.inner.group_id,
"Group A should be the second conversation"
);
assert_eq!(
conversations[2].conversation.inner.group_id, group_b.inner.group_id,
"Group B should be the third conversation"
);

// Verify the last_message field for Group A and None for others
assert!(
conversations[0].last_message.is_none(),
"Group C should have no messages"
);
assert!(
conversations[1].last_message.is_some(),
"Group A should have a last message"
);
assert_eq!(
conversations[1].last_message.as_ref().unwrap().content,
"Message to Group A".as_bytes().to_vec(),
"Group A's last message content should match"
);
assert!(
conversations[2].last_message.is_none(),
"Group B should have no messages"
);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 5)]
async fn test_can_sync_all_groups() {
let alix = new_test_client().await;
Expand Down
1 change: 1 addition & 0 deletions xmtp_mls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ diesel = { workspace = true, features = [
"r2d2",
"returning_clauses_for_sqlite_3_35",
"sqlite",
"32-column-tables"
] }
dyn-clone.workspace = true
libsqlite3-sys = { workspace = true }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP VIEW IF EXISTS conversation_list;
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
CREATE VIEW conversation_list AS
WITH ranked_messages AS (
SELECT
gm.group_id,
gm.id AS message_id,
gm.decrypted_message_bytes,
gm.sent_at_ns,
gm.kind AS message_kind,
gm.sender_installation_id,
gm.sender_inbox_id,
gm.delivery_status,
gm.content_type,
gm.version_major,
gm.version_minor,
gm.authority_id,
ROW_NUMBER() OVER (PARTITION BY gm.group_id ORDER BY gm.sent_at_ns DESC) AS row_num
FROM
group_messages gm
WHERE
gm.kind = 1
)
SELECT
g.id AS id,
g.created_at_ns,
g.membership_state,
g.installations_last_checked,
g.added_by_inbox_id,
g.welcome_id,
g.dm_inbox_id,
g.rotated_at_ns,
g.conversation_type,
rm.message_id,
rm.decrypted_message_bytes,
rm.sent_at_ns,
rm.message_kind,
rm.sender_installation_id,
rm.sender_inbox_id,
rm.delivery_status,
rm.content_type,
rm.version_major,
rm.version_minor,
rm.authority_id
FROM
groups g
LEFT JOIN ranked_messages rm
ON g.id = rm.group_id AND rm.row_num = 1
ORDER BY COALESCE(rm.sent_at_ns, g.created_at_ns) DESC;
38 changes: 38 additions & 0 deletions xmtp_mls/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use xmtp_proto::xmtp::mls::api::v1::{
#[cfg(any(test, feature = "test-utils"))]
use crate::groups::device_sync::WorkerHandle;

use crate::groups::ConversationListItem;
use crate::{
api::ApiClientWrapper,
groups::{
Expand Down Expand Up @@ -669,6 +670,43 @@ where
.collect())
}

pub fn list_conversations(&self) -> Result<Vec<ConversationListItem<Self>>, ClientError> {
Ok(self
.store()
.conn()?
.fetch_conversation_list()?
.into_iter()
.map(|conversation_item| {
let message = conversation_item.message_id.and_then(|message_id| {
// Only construct StoredGroupMessage if all fields are Some
Some(StoredGroupMessage {
id: message_id,
group_id: conversation_item.id.clone(),
decrypted_message_bytes: conversation_item.decrypted_message_bytes?,
sent_at_ns: conversation_item.sent_at_ns?,
sender_installation_id: conversation_item.sender_installation_id?,
sender_inbox_id: conversation_item.sender_inbox_id?,
kind: conversation_item.kind?,
delivery_status: conversation_item.delivery_status?,
content_type: conversation_item.content_type?,
version_major: conversation_item.version_major?,
version_minor: conversation_item.version_minor?,
authority_id: conversation_item.authority_id?,
})
});

ConversationListItem {
group: MlsGroup::new(
self.clone(),
conversation_item.id,
conversation_item.created_at_ns,
),
last_message: message,
}
})
.collect())
}

/// Upload a Key Package to the network and publish the signed identity update
/// from the provided SignatureRequest
pub async fn register_identity(
Expand Down
5 changes: 5 additions & 0 deletions xmtp_mls/src/groups/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@ pub struct MlsGroup<C> {
mutex: Arc<Mutex<()>>,
}

pub struct ConversationListItem<C> {
pub group: MlsGroup<C>,
pub last_message: Option<StoredGroupMessage>,
}

#[derive(Default)]
pub struct GroupMetadataOptions {
pub name: Option<String>,
Expand Down
Loading