Merge branch 'master' into next

backend/canisters/community/CHANGELOG.md

@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Suppress notifications and @s for suspect messages ([#5006](https://github.com/open-chat-labs/open-chat/pull/5006))
 - Make Diamond membership gate check synchronous ([#5027](https://github.com/open-chat-labs/open-chat/pull/5027))
 - Auto join Diamond members to newly created Diamond gated channels ([#5028](https://github.com/open-chat-labs/open-chat/pull/5028))
+- Join community members to public channel if it has its gate removed ([#5033](https://github.com/open-chat-labs/open-chat/pull/5033))
 
 ### Fixed
 

backend/canisters/community/impl/src/model/members.rs

@@ -342,6 +342,7 @@ pub struct CommunityMemberInternal {
 impl CommunityMemberInternal {
     pub fn leave(&mut self, channel_id: ChannelId, now: TimestampMillis) {
         if self.channels.remove(&channel_id) {
+            self.channels_removed.retain(|c| c.value != channel_id);
             self.channels_removed.push(Timestamped::new(channel_id, now));
         }
     }

backend/canisters/community/impl/src/updates/update_channel.rs

@@ -4,6 +4,7 @@ use canister_tracing_macros::trace;
 use community_canister::update_channel::{Response::*, *};
 use group_chat_core::UpdateResult;
 use ic_cdk_macros::update;
+use types::OptionUpdate;
 
 #[update]
 #[trace]
@@ -44,9 +45,15 @@ fn update_channel_impl(mut args: Args, state: &mut RuntimeState) -> Response {
                 now,
             ) {
                 UpdateResult::Success(result) => {
-                    if result.newly_public && channel.chat.gate.is_none() {
-                        for m in state.data.members.iter_mut() {
-                            join_channel_unchecked(channel, m, true, now);
+                    if channel.chat.is_public.value && channel.chat.gate.is_none() {
+                        // If the channel has just been made public or had its gate removed, join
+                        // existing community members to the channel
+                        if result.newly_public || matches!(result.gate_update, OptionUpdate::SetToNone) {
+                            for m in state.data.members.iter_mut() {
+                                if !m.channels_removed.iter().any(|c| c.value == channel.id) {
+                                    join_channel_unchecked(channel, m, true, now);
+                                }
+                            }
                         }
                     }
 

backend/integration_tests/src/client/community.rs

@@ -41,7 +41,7 @@ pub mod happy_path {
     use candid::Principal;
     use pocket_ic::PocketIc;
     use types::{
-        ChannelId, CommunityCanisterChannelSummary, CommunityCanisterCommunitySummary,
+        AccessGate, ChannelId, CommunityCanisterChannelSummary, CommunityCanisterCommunitySummary,
         CommunityCanisterCommunitySummaryUpdates, CommunityId, CommunityRole, EventIndex, EventsResponse,
         MessageContentInitial, MessageId, MessageIndex, Rules, TextContent, TimestampMillis, UserId,
     };
@@ -77,6 +77,38 @@ pub mod happy_path {
         }
     }
 
+    pub fn create_gated_channel(
+        env: &mut PocketIc,
+        sender: Principal,
+        community_id: CommunityId,
+        is_public: bool,
+        name: String,
+        gate: AccessGate,
+    ) -> ChannelId {
+        let response = super::create_channel(
+            env,
+            sender,
+            community_id.into(),
+            &community_canister::create_channel::Args {
+                is_public,
+                name: name.clone(),
+                description: format!("{name}_description"),
+                rules: Rules::default(),
+                subtype: None,
+                avatar: None,
+                history_visible_to_new_joiners: is_public,
+                permissions_v2: None,
+                events_ttl: None,
+                gate: Some(gate),
+            },
+        );
+
+        match response {
+            community_canister::create_channel::Response::Success(result) => result.channel_id,
+            response => panic!("'create_channel' error: {response:?}"),
+        }
+    }
+
     pub fn leave_channel(env: &mut PocketIc, sender: Principal, community_id: CommunityId, channel_id: ChannelId) {
         let response = super::leave_channel(
             env,

backend/integration_tests/src/communities/join_community_tests.rs

@@ -7,7 +7,7 @@ use pocket_ic::PocketIc;
 use std::collections::HashSet;
 use std::ops::Deref;
 use test_case::test_case;
-use types::{AccessGate, CommunityId, Empty, MessageContent, Rules};
+use types::{AccessGate, CommunityId, Empty, MessageContent};
 
 #[test]
 fn join_public_community_succeeds() {
@@ -273,30 +273,15 @@ fn user_joined_to_all_public_channels(diamond_member: bool) {
     let channel1 = client::community::happy_path::create_channel(env, user1.principal, community_id, true, random_string());
     let channel2 = client::community::happy_path::create_channel(env, user1.principal, community_id, true, random_string());
     let channel3 = client::community::happy_path::create_channel(env, user1.principal, community_id, false, random_string());
-    let channel4_response = client::community::create_channel(
+    let channel4 = client::community::happy_path::create_gated_channel(
         env,
         user1.principal,
-        community_id.into(),
-        &community_canister::create_channel::Args {
-            is_public: true,
-            name: random_string(),
-            description: random_string(),
-            rules: Rules::default(),
-            subtype: None,
-            avatar: None,
-            history_visible_to_new_joiners: true,
-            permissions_v2: None,
-            events_ttl: None,
-            gate: Some(AccessGate::DiamondMember),
-        },
+        community_id,
+        true,
+        random_string(),
+        AccessGate::DiamondMember,
     );
 
-    let channel4 = if let community_canister::create_channel::Response::Success(result) = channel4_response {
-        result.channel_id
-    } else {
-        panic!()
-    };
-
     let community_summary =
         client::local_user_index::happy_path::join_community(env, user.principal, canister_ids.local_user_index, community_id);
 

backend/integration_tests/src/communities/update_channel_tests.rs

@@ -2,10 +2,12 @@ use crate::env::ENV;
 use crate::rng::random_string;
 use crate::{client, TestEnv};
 use std::ops::Deref;
-use types::OptionUpdate;
+use test_case::test_case;
+use types::{AccessGate, OptionUpdate};
 
-#[test]
-fn make_private_channel_public_succeeds() {
+#[test_case(true)]
+#[test_case(false)]
+fn members_added_if_channel_made_public_or_gate_removed(make_public: bool) {
     let mut wrapper = ENV.deref().get();
     let TestEnv {
         env,
@@ -15,17 +17,47 @@ fn make_private_channel_public_succeeds() {
     } = wrapper.env();
 
     let user1 = client::register_diamond_user(env, canister_ids, *controller);
-    let user2 = client::local_user_index::happy_path::register_user(env, canister_ids.local_user_index);
+    let user2 = client::register_diamond_user(env, canister_ids, *controller);
+    let user3 = client::local_user_index::happy_path::register_user(env, canister_ids.local_user_index);
 
-    let community_id = client::user::happy_path::create_community(env, &user1, &random_string(), true, vec!["abc".to_string()]);
-    let channel_id =
-        client::community::happy_path::create_channel(env, user1.principal, community_id, false, "xyz".to_string());
+    let community_id = client::user::happy_path::create_community(env, &user1, &random_string(), true, vec![random_string()]);
+
+    let channel_id = if make_public {
+        client::community::happy_path::create_channel(env, user1.principal, community_id, false, random_string())
+    } else {
+        client::community::happy_path::create_gated_channel(
+            env,
+            user1.principal,
+            community_id,
+            true,
+            random_string(),
+            AccessGate::DiamondMember,
+        )
+    };
+
+    client::local_user_index::happy_path::invite_users_to_channel(
+        env,
+        user1.principal,
+        canister_ids.local_user_index,
+        community_id,
+        channel_id,
+        vec![user2.user_id],
+    );
+    client::local_user_index::happy_path::join_channel(
+        env,
+        user2.principal,
+        canister_ids.local_user_index,
+        community_id,
+        channel_id,
+    );
 
     for i in 0..5 {
         client::community::happy_path::send_text_message(env, &user1, community_id, channel_id, None, i.to_string(), None);
     }
 
-    client::local_user_index::happy_path::join_community(env, user2.principal, canister_ids.local_user_index, community_id);
+    client::community::happy_path::leave_channel(env, user2.principal, community_id, channel_id);
+
+    client::local_user_index::happy_path::join_community(env, user3.principal, canister_ids.local_user_index, community_id);
     client::community::happy_path::update_channel(
         env,
         user1.principal,
@@ -38,15 +70,26 @@ fn make_private_channel_public_succeeds() {
             avatar: OptionUpdate::NoChange,
             permissions_v2: None,
             events_ttl: OptionUpdate::NoChange,
-            gate: OptionUpdate::NoChange,
-            public: Some(true),
+            gate: if !make_public { OptionUpdate::SetToNone } else { OptionUpdate::NoChange },
+            public: make_public.then_some(true),
         },
     );
 
-    // Check that user2 has been added to the channel and that it is now public
-    let channel_summary = client::community::happy_path::channel_summary(env, &user2, community_id, channel_id);
+    // Check that user2 has not been re-added to the channel
+    let user2_channel_summary = client::community::happy_path::summary(env, &user2, community_id);
 
-    assert!(channel_summary.is_public);
-    assert_eq!(channel_summary.min_visible_event_index, 7.into());
-    assert_eq!(channel_summary.min_visible_message_index, 5.into());
+    assert!(!user2_channel_summary.channels.iter().any(|c| c.channel_id == channel_id));
+
+    // Check that user3 has been added to the channel
+    let user3_channel_summary = client::community::happy_path::channel_summary(env, &user3, community_id, channel_id);
+
+    if make_public {
+        assert!(user3_channel_summary.is_public);
+        assert_eq!(user3_channel_summary.min_visible_event_index, 10.into());
+        assert_eq!(user3_channel_summary.min_visible_message_index, 5.into());
+    } else {
+        assert!(user3_channel_summary.gate.is_none());
+        assert_eq!(user3_channel_summary.min_visible_event_index, 0.into());
+        assert_eq!(user3_channel_summary.min_visible_message_index, 0.into());
+    }
 }

backend/integration_tests/src/diamond_membership_tests.rs

@@ -129,7 +129,10 @@ fn membership_renews_automatically_if_set_to_recurring(ledger_error: bool) {
         .is_active());
 
     let new_balance = client::icrc1::happy_path::balance_of(env, canister_ids.icp_ledger, user.user_id);
-    assert_eq!(new_balance, 1_000_000_000 - 40_000_000);
+    assert_eq!(
+        new_balance,
+        1_000_000_000 - (2 * DiamondMembershipPlanDuration::OneMonth.icp_price_e8s() as u128)
+    );
 }
 
 #[test_case(true)]
@@ -240,7 +243,10 @@ fn update_subscription_succeeds(disable: bool) {
         ));
 
         let new_balance = client::icrc1::happy_path::balance_of(env, canister_ids.icp_ledger, user.user_id);
-        assert_eq!(new_balance, 1_000_000_000 - 20_000_000);
+        assert_eq!(
+            new_balance,
+            1_000_000_000 - DiamondMembershipPlanDuration::OneMonth.icp_price_e8s() as u128
+        );
     } else {
         let one_year_millis = DiamondMembershipPlanDuration::OneYear.as_millis();
         assert_eq!(
@@ -253,6 +259,11 @@ fn update_subscription_succeeds(disable: bool) {
         ));
 
         let new_balance = client::icrc1::happy_path::balance_of(env, canister_ids.icp_ledger, user.user_id);
-        assert_eq!(new_balance, 1_000_000_000 - 170_000_000);
+        assert_eq!(
+            new_balance,
+            1_000_000_000
+                - (DiamondMembershipPlanDuration::OneMonth.icp_price_e8s()
+                    + DiamondMembershipPlanDuration::OneYear.icp_price_e8s()) as u128
+        );
     }
 }

backend/libraries/group_chat_core/src/lib.rs

@@ -1399,6 +1399,7 @@ impl GroupChatCore {
     ) -> UpdateSuccessResult {
         let mut result = UpdateSuccessResult {
             newly_public: false,
+            gate_update: OptionUpdate::NoChange,
             rules_version: None,
         };
 
@@ -1497,6 +1498,7 @@ impl GroupChatCore {
         if let Some(gate) = gate.expand() {
             if self.gate.value != gate {
                 self.gate = Timestamped::new(gate.clone(), now);
+                result.gate_update = OptionUpdate::from_update(gate.clone());
 
                 events.push_main_event(
                     ChatEventInternal::GroupGateUpdated(Box::new(GroupGateUpdated {
@@ -1872,6 +1874,7 @@ pub enum UpdateResult {
 
 pub struct UpdateSuccessResult {
     pub newly_public: bool,
+    pub gate_update: OptionUpdate<AccessGate>,
     pub rules_version: Option<Version>,
 }
 

backend/libraries/types/src/diamond_membership.rs

@@ -62,7 +62,7 @@ impl DiamondMembershipPlanDuration {
             DiamondMembershipPlanDuration::OneMonth => 200_000_000,    // 2 CHAT
             DiamondMembershipPlanDuration::ThreeMonths => 500_000_000, // 5 CHAT
             DiamondMembershipPlanDuration::OneYear => 1_500_000_000,   // 15 CHAT
-            DiamondMembershipPlanDuration::Lifetime => 60_000_000_000, // 60 CHAT
+            DiamondMembershipPlanDuration::Lifetime => 6_000_000_000,  // 60 CHAT
         }
     }