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.

Sign up for GitHub

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(group): add message expiration to group metadata #1477

Merged
merged 12 commits into from
Jan 10, 2025
+243 −56
Merged

feat(group): add message expiration to group metadata #1477

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
20978da
feat(group): add message expiration to group metadata
mchenani Jan 7, 2025
8dbd684
fix missing fields
mchenani Jan 7, 2025
c179ee4
fix clippy
mchenani Jan 7, 2025
ba0a4b3
fix tests and add message expiration starts timestamp
mchenani Jan 9, 2025
cdef053
fix node tests
mchenani Jan 9, 2025
75553e0
fix fmt on nodes
mchenani Jan 9, 2025
57345c3
add dm permission tests
mchenani Jan 9, 2025
747e27b
fix fmt
mchenani Jan 9, 2025
c2d1d1c
fix tests
mchenani Jan 9, 2025
1b79975
Merge branch 'main' into mc/add-message-expiration-to-group-metadata
mchenani Jan 9, 2025
16abbdf
Merge branch 'main' into mc/add-message-expiration-to-group-metadata
mchenani Jan 9, 2025
90fc38e
Merge branch 'main' into mc/add-message-expiration-to-group-metadata
mchenani Jan 9, 2025
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
118 changes: 111 additions & 7 deletions bindings_ffi/src/mls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ pub struct FfiConversations {

#[derive(uniffi::Enum, Clone, Debug)]
pub enum FfiGroupPermissionsOptions {
AllMembers,
Default,
AdminOnly,
CustomPolicy,
}
Expand Down Expand Up @@ -783,12 +783,13 @@ pub struct FfiPermissionPolicySet {
pub update_group_description_policy: FfiPermissionPolicy,
pub update_group_image_url_square_policy: FfiPermissionPolicy,
pub update_group_pinned_frame_url_policy: FfiPermissionPolicy,
pub update_message_expiration_ms_policy: FfiPermissionPolicy,
}

impl From<PreconfiguredPolicies> for FfiGroupPermissionsOptions {
fn from(policy: PreconfiguredPolicies) -> Self {
match policy {
PreconfiguredPolicies::AllMembers => FfiGroupPermissionsOptions::AllMembers,
PreconfiguredPolicies::Default => FfiGroupPermissionsOptions::Default,
PreconfiguredPolicies::AdminsOnly => FfiGroupPermissionsOptions::AdminOnly,
}
}
Expand All @@ -814,6 +815,18 @@ impl TryFrom<FfiPermissionPolicySet> for PolicySet {
MetadataField::GroupPinnedFrameUrl.to_string(),
policy_set.update_group_pinned_frame_url_policy.try_into()?,
);
// MessageExpirationFromMillis follows the same policy as MessageExpirationMillis
metadata_permissions_map.insert(
MetadataField::MessageExpirationFromMillis.to_string(),
policy_set
.update_message_expiration_ms_policy
.clone()
.try_into()?,
);
metadata_permissions_map.insert(
MetadataField::MessageExpirationMillis.to_string(),
policy_set.update_message_expiration_ms_policy.try_into()?,
);

Ok(PolicySet {
add_member_policy: policy_set.add_member_policy.try_into()?,
Expand Down Expand Up @@ -872,8 +885,8 @@ impl FfiConversations {
let metadata_options = opts.clone().into_group_metadata_options();

let group_permissions = match opts.permissions {
Some(FfiGroupPermissionsOptions::AllMembers) => {
Some(xmtp_mls::groups::PreconfiguredPolicies::AllMembers.to_policy_set())
Some(FfiGroupPermissionsOptions::Default) => {
Some(xmtp_mls::groups::PreconfiguredPolicies::Default.to_policy_set())
}
Some(FfiGroupPermissionsOptions::AdminOnly) => {
Some(xmtp_mls::groups::PreconfiguredPolicies::AdminsOnly.to_policy_set())
Expand Down Expand Up @@ -1365,6 +1378,8 @@ pub struct FfiCreateGroupOptions {
pub group_description: Option<String>,
pub group_pinned_frame_url: Option<String>,
pub custom_permission_policy_set: Option<FfiPermissionPolicySet>,
pub message_expiration_from_ms: Option<i64>,
pub message_expiration_ms: Option<i64>,
}

impl FfiCreateGroupOptions {
Expand All @@ -1374,6 +1389,8 @@ impl FfiCreateGroupOptions {
image_url_square: self.group_image_url_square,
description: self.group_description,
pinned_frame_url: self.group_pinned_frame_url,
message_expiration_from_ms: self.message_expiration_from_ms,
message_expiration_ms: self.message_expiration_ms,
}
}
}
Expand Down Expand Up @@ -1963,6 +1980,9 @@ impl FfiGroupPermissions {
update_group_pinned_frame_url_policy: get_policy(
MetadataField::GroupPinnedFrameUrl.as_str(),
),
update_message_expiration_ms_policy: get_policy(
MetadataField::MessageExpirationMillis.as_str(),
),
})
}
}
Expand Down Expand Up @@ -2679,6 +2699,8 @@ mod tests {
group_description: Some("group description".to_string()),
group_pinned_frame_url: Some("pinned frame".to_string()),
custom_permission_policy_set: None,
message_expiration_from_ms: None,
message_expiration_ms: None,
},
)
.await
Expand Down Expand Up @@ -3889,7 +3911,7 @@ mod tests {
}

#[tokio::test(flavor = "multi_thread", worker_threads = 5)]
async fn test_permissions_show_expected_values() {
async fn test_group_permissions_show_expected_values() {
let alix = new_test_client().await;
let bo = new_test_client().await;
// Create admin_only group
Expand Down Expand Up @@ -3918,12 +3940,74 @@ mod tests {
update_group_description_policy: FfiPermissionPolicy::Admin,
update_group_image_url_square_policy: FfiPermissionPolicy::Admin,
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};
assert_eq!(alix_permission_policy_set, expected_permission_policy_set);

// Create all_members group
let all_members_options = FfiCreateGroupOptions {
permissions: Some(FfiGroupPermissionsOptions::Default),
..Default::default()
};
let alix_group_all_members = alix
.conversations()
.create_group(vec![bo.account_address.clone()], all_members_options)
.await
.unwrap();

// Verify we can read the expected permissions
let alix_permission_policy_set = alix_group_all_members
.group_permissions()
.unwrap()
.policy_set()
.unwrap();
let expected_permission_policy_set = FfiPermissionPolicySet {
add_member_policy: FfiPermissionPolicy::Allow,
remove_member_policy: FfiPermissionPolicy::Admin,
add_admin_policy: FfiPermissionPolicy::SuperAdmin,
remove_admin_policy: FfiPermissionPolicy::SuperAdmin,
update_group_name_policy: FfiPermissionPolicy::Allow,
update_group_description_policy: FfiPermissionPolicy::Allow,
update_group_image_url_square_policy: FfiPermissionPolicy::Allow,
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};
assert_eq!(alix_permission_policy_set, expected_permission_policy_set);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 5)]
async fn test_dm_permissions_show_expected_values() {
let alix = new_test_client().await;
let bo = new_test_client().await;

let alix_group_admin_only = alix
.conversations()
.create_dm(bo.account_address.clone())
.await
.unwrap();

// Verify we can read the expected permissions
let alix_permission_policy_set = alix_group_admin_only
.group_permissions()
.unwrap()
.policy_set()
.unwrap();
let expected_permission_policy_set = FfiPermissionPolicySet {
add_member_policy: FfiPermissionPolicy::Deny,
remove_member_policy: FfiPermissionPolicy::Deny,
add_admin_policy: FfiPermissionPolicy::Deny,
remove_admin_policy: FfiPermissionPolicy::Deny,
update_group_name_policy: FfiPermissionPolicy::Allow,
update_group_description_policy: FfiPermissionPolicy::Allow,
update_group_image_url_square_policy: FfiPermissionPolicy::Allow,
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow,
update_message_expiration_ms_policy: FfiPermissionPolicy::Allow,
};
assert_eq!(alix_permission_policy_set, expected_permission_policy_set);

// Create all_members group
let all_members_options = FfiCreateGroupOptions {
permissions: Some(FfiGroupPermissionsOptions::AllMembers),
permissions: Some(FfiGroupPermissionsOptions::Default),
..Default::default()
};
let alix_group_all_members = alix
Expand All @@ -3947,6 +4031,7 @@ mod tests {
update_group_description_policy: FfiPermissionPolicy::Allow,
update_group_image_url_square_policy: FfiPermissionPolicy::Allow,
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};
assert_eq!(alix_permission_policy_set, expected_permission_policy_set);
}
Expand Down Expand Up @@ -3980,6 +4065,7 @@ mod tests {
update_group_description_policy: FfiPermissionPolicy::Admin,
update_group_image_url_square_policy: FfiPermissionPolicy::Admin,
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};
assert_eq!(alix_group_permissions, expected_permission_policy_set);

Expand Down Expand Up @@ -4007,6 +4093,7 @@ mod tests {
update_group_description_policy: FfiPermissionPolicy::Admin,
update_group_image_url_square_policy: FfiPermissionPolicy::Allow,
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};
assert_eq!(alix_group_permissions, new_expected_permission_policy_set);

Expand Down Expand Up @@ -4060,6 +4147,7 @@ mod tests {
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin,
add_member_policy: FfiPermissionPolicy::Allow,
remove_member_policy: FfiPermissionPolicy::Deny,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};

let create_group_options = FfiCreateGroupOptions {
Expand All @@ -4069,6 +4157,8 @@ mod tests {
group_description: Some("A test group".to_string()),
group_pinned_frame_url: Some("https://example.com/frame.png".to_string()),
custom_permission_policy_set: Some(custom_permissions),
message_expiration_from_ms: None,
message_expiration_ms: None,
};

let alix_group = alix
Expand Down Expand Up @@ -4107,6 +4197,10 @@ mod tests {
group_permissions_policy_set.update_group_pinned_frame_url_policy,
FfiPermissionPolicy::Admin
);
assert_eq!(
group_permissions_policy_set.update_message_expiration_ms_policy,
FfiPermissionPolicy::Admin
);
assert_eq!(
group_permissions_policy_set.add_member_policy,
FfiPermissionPolicy::Allow
Expand Down Expand Up @@ -4170,6 +4264,7 @@ mod tests {
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin,
add_member_policy: FfiPermissionPolicy::Allow,
remove_member_policy: FfiPermissionPolicy::Deny,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};

let custom_permissions_valid = FfiPermissionPolicySet {
Expand All @@ -4181,6 +4276,7 @@ mod tests {
update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin,
add_member_policy: FfiPermissionPolicy::Allow,
remove_member_policy: FfiPermissionPolicy::Deny,
update_message_expiration_ms_policy: FfiPermissionPolicy::Admin,
};

let create_group_options_invalid_1 = FfiCreateGroupOptions {
Expand All @@ -4190,6 +4286,8 @@ mod tests {
group_description: Some("A test group".to_string()),
group_pinned_frame_url: Some("https://example.com/frame.png".to_string()),
custom_permission_policy_set: Some(custom_permissions_invalid_1),
message_expiration_from_ms: None,
message_expiration_ms: None,
};

let results_1 = alix
Expand All @@ -4203,12 +4301,14 @@ mod tests {
assert!(results_1.is_err());

let create_group_options_invalid_2 = FfiCreateGroupOptions {
permissions: Some(FfiGroupPermissionsOptions::AllMembers),
permissions: Some(FfiGroupPermissionsOptions::Default),
group_name: Some("Test Group".to_string()),
group_image_url_square: Some("https://example.com/image.png".to_string()),
group_description: Some("A test group".to_string()),
group_pinned_frame_url: Some("https://example.com/frame.png".to_string()),
custom_permission_policy_set: Some(custom_permissions_valid.clone()),
message_expiration_from_ms: None,
message_expiration_ms: None,
};

let results_2 = alix
Expand All @@ -4228,6 +4328,8 @@ mod tests {
group_description: Some("A test group".to_string()),
group_pinned_frame_url: Some("https://example.com/frame.png".to_string()),
custom_permission_policy_set: Some(custom_permissions_valid.clone()),
message_expiration_from_ms: None,
message_expiration_ms: None,
};

let results_3 = alix
Expand All @@ -4247,6 +4349,8 @@ mod tests {
group_description: Some("A test group".to_string()),
group_pinned_frame_url: Some("https://example.com/frame.png".to_string()),
custom_permission_policy_set: Some(custom_permissions_valid),
message_expiration_from_ms: None,
message_expiration_ms: None,
};

let results_4 = alix
Expand Down
29 changes: 16 additions & 13 deletions bindings_node/src/conversations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ pub struct CreateGroupOptions {
pub group_description: Option<String>,
pub group_pinned_frame_url: Option<String>,
pub custom_permission_policy_set: Option<PermissionPolicySet>,
pub message_expiration_from_ms: Option<i64>,
pub message_expiration_ms: Option<i64>,
}

impl CreateGroupOptions {
Expand All @@ -131,6 +133,8 @@ impl CreateGroupOptions {
image_url_square: self.group_image_url_square,
description: self.group_description,
pinned_frame_url: self.group_pinned_frame_url,
message_expiration_from_ms: self.message_expiration_from_ms,
message_expiration_ms: self.message_expiration_ms,
}
}
}
Expand All @@ -152,17 +156,16 @@ impl Conversations {
account_addresses: Vec<String>,
options: Option<CreateGroupOptions>,
) -> Result<Conversation> {
let options = match options {
Some(options) => options,
None => CreateGroupOptions {
permissions: None,
group_name: None,
group_image_url_square: None,
group_description: None,
group_pinned_frame_url: None,
custom_permission_policy_set: None,
},
};
let options = options.unwrap_or(CreateGroupOptions {
permissions: None,
group_name: None,
group_image_url_square: None,
group_description: None,
group_pinned_frame_url: None,
custom_permission_policy_set: None,
message_expiration_from_ms: None,
message_expiration_ms: None,
});

if let Some(GroupPermissionsOptions::CustomPolicy) = options.permissions {
if options.custom_permission_policy_set.is_none() {
Expand All @@ -177,8 +180,8 @@ impl Conversations {
let metadata_options = options.clone().into_group_metadata_options();

let group_permissions = match options.permissions {
Some(GroupPermissionsOptions::AllMembers) => {
Some(PreconfiguredPolicies::AllMembers.to_policy_set())
Some(GroupPermissionsOptions::Default) => {
Some(PreconfiguredPolicies::Default.to_policy_set())
}
Some(GroupPermissionsOptions::AdminOnly) => {
Some(PreconfiguredPolicies::AdminsOnly.to_policy_set())
Expand Down
12 changes: 10 additions & 2 deletions bindings_node/src/permissions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use xmtp_mls::groups::{

#[napi]
pub enum GroupPermissionsOptions {
AllMembers,
Default,
AdminOnly,
CustomPolicy,
}
Expand Down Expand Up @@ -161,12 +161,13 @@ pub struct PermissionPolicySet {
pub update_group_description_policy: PermissionPolicy,
pub update_group_image_url_square_policy: PermissionPolicy,
pub update_group_pinned_frame_url_policy: PermissionPolicy,
pub update_message_expiration_ms_policy: PermissionPolicy,
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 Added only one policy for both message_expiration_ms and message_expiration_from_ms. wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea I think that makes sense, doesnt make sense for someone to be able to change one without the other 👍

}

impl From<PreconfiguredPolicies> for GroupPermissionsOptions {
fn from(policy: PreconfiguredPolicies) -> Self {
match policy {
PreconfiguredPolicies::AllMembers => GroupPermissionsOptions::AllMembers,
PreconfiguredPolicies::Default => GroupPermissionsOptions::Default,
PreconfiguredPolicies::AdminsOnly => GroupPermissionsOptions::AdminOnly,
}
}
Expand Down Expand Up @@ -215,6 +216,9 @@ impl GroupPermissions {
update_group_pinned_frame_url_policy: get_policy(
XmtpMetadataField::GroupPinnedFrameUrl.as_str(),
),
update_message_expiration_ms_policy: get_policy(
XmtpMetadataField::MessageExpirationMillis.as_str(),
),
})
}
}
Expand All @@ -241,6 +245,10 @@ impl TryFrom<PermissionPolicySet> for PolicySet {
XmtpMetadataField::GroupPinnedFrameUrl.to_string(),
policy_set.update_group_pinned_frame_url_policy.try_into()?,
);
metadata_permissions_map.insert(
XmtpMetadataField::MessageExpirationMillis.to_string(),
policy_set.update_message_expiration_ms_policy.try_into()?,
);

Ok(PolicySet {
add_member_policy: policy_set.add_member_policy.try_into()?,
Expand Down
Loading
Loading