add test for members returning for non creator. remove unwrap

xmtp · May 29, 2024 · e74ce1a · e74ce1a
1 parent d5d57c9
commit e74ce1a
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 18 deletions.
diff --git a/xmtp_mls/src/groups/members.rs b/xmtp_mls/src/groups/members.rs
@@ -14,7 +14,7 @@ pub struct GroupMember {
     pub permission_level: PermissionLevel,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PermissionLevel {
     Member,
     Admin,
@@ -49,28 +49,25 @@ impl MlsGroup {
         let members = association_state_map
             .into_iter()
             .map(|association_state| {
-                let is_admin = self
-                    .is_admin(association_state.inbox_id().to_string())
-                    .unwrap();
-                let is_super_admin = self
-                    .is_super_admin(association_state.inbox_id().to_string())
-                    .unwrap();
+                let inbox_id_str = association_state.inbox_id().to_string();
+                let is_admin = self.is_admin(inbox_id_str.clone())?;
+                let is_super_admin = self.is_super_admin(inbox_id_str.clone())?;
                 let permission_level = if is_super_admin {
                     PermissionLevel::SuperAdmin
-                } else if is_admin && !is_super_admin {
+                } else if is_admin {
                     PermissionLevel::Admin
                 } else {
                     PermissionLevel::Member
                 };
 
-                GroupMember {
-                    inbox_id: association_state.inbox_id().to_string(),
+                Ok(GroupMember {
+                    inbox_id: inbox_id_str,
                     account_addresses: association_state.account_addresses(),
                     installation_ids: association_state.installation_ids(),
                     permission_level,
-                }
+                })
             })
-            .collect::<Vec<GroupMember>>();
+            .collect::<Result<Vec<GroupMember>, GroupError>>()?;
 
         Ok(members)
     }

diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs
@@ -925,9 +925,11 @@ mod tests {
         builder::ClientBuilder,
         codecs::{group_updated::GroupUpdatedCodec, ContentCodec},
         groups::{
-            build_group_membership_extension, group_membership::GroupMembership,
-            group_mutable_metadata::MetadataField, members::PermissionLevel, PreconfiguredPolicies,
-            UpdateAdminListType,
+            build_group_membership_extension,
+            group_membership::GroupMembership,
+            group_mutable_metadata::MetadataField,
+            members::{GroupMember, PermissionLevel},
+            PreconfiguredPolicies, UpdateAdminListType,
         },
         storage::{
             group_intent::IntentState,
@@ -1103,11 +1105,27 @@ mod tests {
         assert_eq!(bola_group_name, "New Group");
 
         // Check if both clients can see the members correctly
-        let amal_members = amal_group.members().unwrap();
-        let bola_members = bola_group.members().unwrap();
+        let amal_members: Vec<GroupMember> = amal_group.members().unwrap();
+        let bola_members: Vec<GroupMember> = bola_group.members().unwrap();
 
         assert_eq!(amal_members.len(), 2);
-        assert_eq!(bola_members.len(), 2); // failing here, see len == 0
+        assert_eq!(bola_members.len(), 2);
+
+        for member in &amal_members {
+            if member.inbox_id == amal.inbox_id() {
+                assert_eq!(
+                    member.permission_level,
+                    PermissionLevel::SuperAdmin,
+                    "Amal should be a super admin"
+                );
+            } else if member.inbox_id == bola.inbox_id() {
+                assert_eq!(
+                    member.permission_level,
+                    PermissionLevel::Member,
+                    "Bola should be a member"
+                );
+            }
+        }
     }
 
     // Amal and Bola will both try and add Charlie from the same epoch.