Refactor to avoid some (mostly hypothetical) races

Conrad Irwin created

Tidy up added code to reduce duplicity of X and X_internals.

Change summary

crates/channel/src/channel_store_tests.rs              |  44 
crates/collab/src/db.rs                                |  36 
crates/collab/src/db/queries/buffers.rs                |   4 
crates/collab/src/db/queries/channels.rs               | 462 ++++---
crates/collab/src/db/queries/messages.rs               |   4 
crates/collab/src/db/queries/rooms.rs                  |  14 
crates/collab/src/db/tests/channel_tests.rs            | 705 ++++++-----
crates/collab/src/db/tests/message_tests.rs            |   8 
crates/collab/src/rpc.rs                               | 222 +--
crates/collab/src/tests/random_channel_buffer_tests.rs |   2 
crates/collab/src/tests/test_server.rs                 |  32 
crates/collab_ui/src/collab_panel.rs                   |   1 
crates/rpc/proto/zed.proto                             |   6 
13 files changed, 745 insertions(+), 795 deletions(-)

Detailed changes

crates/channel/src/channel_store_tests.rs 🔗

@@ -36,8 +36,8 @@ fn test_update_channels(cx: &mut AppContext) {
         &channel_store,
         &[
             //
-            (0, "a".to_string(), false),
-            (0, "b".to_string(), true),
+            (0, "a".to_string(), proto::ChannelRole::Member),
+            (0, "b".to_string(), proto::ChannelRole::Admin),
         ],
         cx,
     );
@@ -50,7 +50,7 @@ fn test_update_channels(cx: &mut AppContext) {
                     id: 3,
                     name: "x".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
-                    role: proto::ChannelRole::Member.into(),
+                    role: proto::ChannelRole::Admin.into(),
                 },
                 proto::Channel {
                     id: 4,
@@ -76,10 +76,10 @@ fn test_update_channels(cx: &mut AppContext) {
     assert_channels(
         &channel_store,
         &[
-            (0, "a".to_string(), false),
-            (1, "y".to_string(), false),
-            (0, "b".to_string(), true),
-            (1, "x".to_string(), true),
+            (0, "a".to_string(), proto::ChannelRole::Member),
+            (1, "y".to_string(), proto::ChannelRole::Member),
+            (0, "b".to_string(), proto::ChannelRole::Admin),
+            (1, "x".to_string(), proto::ChannelRole::Admin),
         ],
         cx,
     );
@@ -131,9 +131,9 @@ fn test_dangling_channel_paths(cx: &mut AppContext) {
         &channel_store,
         &[
             //
-            (0, "a".to_string(), true),
-            (1, "b".to_string(), true),
-            (2, "c".to_string(), true),
+            (0, "a".to_string(), proto::ChannelRole::Admin),
+            (1, "b".to_string(), proto::ChannelRole::Admin),
+            (2, "c".to_string(), proto::ChannelRole::Admin),
         ],
         cx,
     );
@@ -148,7 +148,11 @@ fn test_dangling_channel_paths(cx: &mut AppContext) {
     );
 
     // Make sure that the 1/2/3 path is gone
-    assert_channels(&channel_store, &[(0, "a".to_string(), true)], cx);
+    assert_channels(
+        &channel_store,
+        &[(0, "a".to_string(), proto::ChannelRole::Admin)],
+        cx,
+    );
 }
 
 #[gpui::test]
@@ -165,13 +169,17 @@ async fn test_channel_messages(cx: &mut TestAppContext) {
             id: channel_id,
             name: "the-channel".to_string(),
             visibility: proto::ChannelVisibility::Members as i32,
-            role: proto::ChannelRole::Admin.into(),
+            role: proto::ChannelRole::Member.into(),
         }],
         ..Default::default()
     });
     cx.foreground().run_until_parked();
     cx.read(|cx| {
-        assert_channels(&channel_store, &[(0, "the-channel".to_string(), false)], cx);
+        assert_channels(
+            &channel_store,
+            &[(0, "the-channel".to_string(), proto::ChannelRole::Member)],
+            cx,
+        );
     });
 
     let get_users = server.receive::<proto::GetUsers>().await.unwrap();
@@ -366,19 +374,13 @@ fn update_channels(
 #[track_caller]
 fn assert_channels(
     channel_store: &ModelHandle<ChannelStore>,
-    expected_channels: &[(usize, String, bool)],
+    expected_channels: &[(usize, String, proto::ChannelRole)],
     cx: &AppContext,
 ) {
     let actual = channel_store.read_with(cx, |store, _| {
         store
             .channel_dag_entries()
-            .map(|(depth, channel)| {
-                (
-                    depth,
-                    channel.name.to_string(),
-                    store.is_channel_admin(channel.id),
-                )
-            })
+            .map(|(depth, channel)| (depth, channel.name.to_string(), channel.role))
             .collect::<Vec<_>>()
     });
     assert_eq!(actual, expected_channels);

crates/collab/src/db.rs 🔗

@@ -428,6 +428,31 @@ pub struct NewUserResult {
     pub signup_device_id: Option<String>,
 }
 
+#[derive(Debug)]
+pub struct MoveChannelResult {
+    pub participants_to_update: HashMap<UserId, ChannelsForUser>,
+    pub participants_to_remove: HashSet<UserId>,
+    pub moved_channels: HashSet<ChannelId>,
+}
+
+#[derive(Debug)]
+pub struct RenameChannelResult {
+    pub channel: Channel,
+    pub participants_to_update: HashMap<UserId, Channel>,
+}
+
+#[derive(Debug)]
+pub struct CreateChannelResult {
+    pub channel: Channel,
+    pub participants_to_update: Vec<(UserId, ChannelsForUser)>,
+}
+
+#[derive(Debug)]
+pub struct SetChannelVisibilityResult {
+    pub participants_to_update: HashMap<UserId, ChannelsForUser>,
+    pub participants_to_remove: HashSet<UserId>,
+}
+
 #[derive(FromQueryResult, Debug, PartialEq, Eq, Hash)]
 pub struct Channel {
     pub id: ChannelId,
@@ -436,6 +461,17 @@ pub struct Channel {
     pub role: ChannelRole,
 }
 
+impl Channel {
+    pub fn to_proto(&self) -> proto::Channel {
+        proto::Channel {
+            id: self.id.to_proto(),
+            name: self.name.clone(),
+            visibility: self.visibility.into(),
+            role: self.role.into(),
+        }
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Hash)]
 pub struct ChannelMember {
     pub role: ChannelRole,

crates/collab/src/db/queries/buffers.rs 🔗

@@ -482,9 +482,7 @@ impl Database {
                 )
                 .await?;
 
-                channel_members = self
-                    .get_channel_participants_internal(channel_id, &*tx)
-                    .await?;
+                channel_members = self.get_channel_participants(channel_id, &*tx).await?;
                 let collaborators = self
                     .get_channel_buffer_collaborators_internal(channel_id, &*tx)
                     .await?;

crates/collab/src/db/queries/channels.rs 🔗

@@ -16,20 +16,39 @@ impl Database {
         .await
     }
 
+    #[cfg(test)]
     pub async fn create_root_channel(&self, name: &str, creator_id: UserId) -> Result<ChannelId> {
-        self.create_channel(name, None, creator_id).await
+        Ok(self
+            .create_channel(name, None, creator_id)
+            .await?
+            .channel
+            .id)
     }
 
-    pub async fn create_channel(
+    #[cfg(test)]
+    pub async fn create_sub_channel(
         &self,
         name: &str,
-        parent: Option<ChannelId>,
+        parent: ChannelId,
         creator_id: UserId,
     ) -> Result<ChannelId> {
+        Ok(self
+            .create_channel(name, Some(parent), creator_id)
+            .await?
+            .channel
+            .id)
+    }
+
+    pub async fn create_channel(
+        &self,
+        name: &str,
+        parent: Option<ChannelId>,
+        admin_id: UserId,
+    ) -> Result<CreateChannelResult> {
         let name = Self::sanitize_channel_name(name)?;
         self.transaction(move |tx| async move {
             if let Some(parent) = parent {
-                self.check_user_is_channel_admin(parent, creator_id, &*tx)
+                self.check_user_is_channel_admin(parent, admin_id, &*tx)
                     .await?;
             }
 
@@ -71,17 +90,34 @@ impl Database {
                 .await?;
             }
 
-            channel_member::ActiveModel {
-                id: ActiveValue::NotSet,
-                channel_id: ActiveValue::Set(channel.id),
-                user_id: ActiveValue::Set(creator_id),
-                accepted: ActiveValue::Set(true),
-                role: ActiveValue::Set(ChannelRole::Admin),
+            if parent.is_none() {
+                channel_member::ActiveModel {
+                    id: ActiveValue::NotSet,
+                    channel_id: ActiveValue::Set(channel.id),
+                    user_id: ActiveValue::Set(admin_id),
+                    accepted: ActiveValue::Set(true),
+                    role: ActiveValue::Set(ChannelRole::Admin),
+                }
+                .insert(&*tx)
+                .await?;
             }
-            .insert(&*tx)
-            .await?;
 
-            Ok(channel.id)
+            let participants_to_update = if let Some(parent) = parent {
+                self.participants_to_notify_for_channel_change(parent, &*tx)
+                    .await?
+            } else {
+                vec![]
+            };
+
+            Ok(CreateChannelResult {
+                channel: Channel {
+                    id: channel.id,
+                    visibility: channel.visibility,
+                    name: channel.name,
+                    role: ChannelRole::Admin,
+                },
+                participants_to_update,
+            })
         })
         .await
     }
@@ -132,7 +168,7 @@ impl Database {
                 && channel.as_ref().map(|c| c.visibility) == Some(ChannelVisibility::Public)
             {
                 let channel_id_to_join = self
-                    .public_path_to_channel_internal(channel_id, &*tx)
+                    .public_path_to_channel(channel_id, &*tx)
                     .await?
                     .first()
                     .cloned()
@@ -178,13 +214,17 @@ impl Database {
         &self,
         channel_id: ChannelId,
         visibility: ChannelVisibility,
-        user_id: UserId,
-    ) -> Result<channel::Model> {
+        admin_id: UserId,
+    ) -> Result<SetChannelVisibilityResult> {
         self.transaction(move |tx| async move {
-            self.check_user_is_channel_admin(channel_id, user_id, &*tx)
+            self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
                 .await?;
 
-            let channel = channel::ActiveModel {
+            let previous_members = self
+                .get_channel_participant_details_internal(channel_id, &*tx)
+                .await?;
+
+            channel::ActiveModel {
                 id: ActiveValue::Unchanged(channel_id),
                 visibility: ActiveValue::Set(visibility),
                 ..Default::default()
@@ -192,7 +232,40 @@ impl Database {
             .update(&*tx)
             .await?;
 
-            Ok(channel)
+            let mut participants_to_update: HashMap<UserId, ChannelsForUser> = self
+                .participants_to_notify_for_channel_change(channel_id, &*tx)
+                .await?
+                .into_iter()
+                .collect();
+
+            let mut participants_to_remove: HashSet<UserId> = HashSet::default();
+            match visibility {
+                ChannelVisibility::Members => {
+                    for member in previous_members {
+                        if member.role.can_only_see_public_descendants() {
+                            participants_to_remove.insert(member.user_id);
+                        }
+                    }
+                }
+                ChannelVisibility::Public => {
+                    if let Some(public_parent_id) =
+                        self.public_parent_channel_id(channel_id, &*tx).await?
+                    {
+                        let parent_updates = self
+                            .participants_to_notify_for_channel_change(public_parent_id, &*tx)
+                            .await?;
+
+                        for (user_id, channels) in parent_updates {
+                            participants_to_update.insert(user_id, channels);
+                        }
+                    }
+                }
+            }
+
+            Ok(SetChannelVisibilityResult {
+                participants_to_update,
+                participants_to_remove,
+            })
         })
         .await
     }
@@ -303,14 +376,14 @@ impl Database {
     pub async fn rename_channel(
         &self,
         channel_id: ChannelId,
-        user_id: UserId,
+        admin_id: UserId,
         new_name: &str,
-    ) -> Result<Channel> {
+    ) -> Result<RenameChannelResult> {
         self.transaction(move |tx| async move {
             let new_name = Self::sanitize_channel_name(new_name)?.to_string();
 
             let role = self
-                .check_user_is_channel_admin(channel_id, user_id, &*tx)
+                .check_user_is_channel_admin(channel_id, admin_id, &*tx)
                 .await?;
 
             let channel = channel::ActiveModel {
@@ -321,11 +394,31 @@ impl Database {
             .update(&*tx)
             .await?;
 
-            Ok(Channel {
-                id: channel.id,
-                name: channel.name,
-                visibility: channel.visibility,
-                role,
+            let participants = self
+                .get_channel_participant_details_internal(channel_id, &*tx)
+                .await?;
+
+            Ok(RenameChannelResult {
+                channel: Channel {
+                    id: channel.id,
+                    name: channel.name,
+                    visibility: channel.visibility,
+                    role,
+                },
+                participants_to_update: participants
+                    .iter()
+                    .map(|participant| {
+                        (
+                            participant.user_id,
+                            Channel {
+                                id: channel.id,
+                                name: new_name.clone(),
+                                visibility: channel.visibility,
+                                role: participant.role,
+                            },
+                        )
+                    })
+                    .collect(),
             })
         })
         .await
@@ -628,91 +721,83 @@ impl Database {
         })
     }
 
-    pub async fn get_channel_members(&self, id: ChannelId) -> Result<Vec<UserId>> {
-        self.transaction(|tx| async move { self.get_channel_participants_internal(id, &*tx).await })
-            .await
-    }
-
-    pub async fn participants_to_notify_for_channel_change(
+    async fn participants_to_notify_for_channel_change(
         &self,
         new_parent: ChannelId,
-        admin_id: UserId,
+        tx: &DatabaseTransaction,
     ) -> Result<Vec<(UserId, ChannelsForUser)>> {
-        self.transaction(|tx| async move {
-            let mut results: Vec<(UserId, ChannelsForUser)> = Vec::new();
+        let mut results: Vec<(UserId, ChannelsForUser)> = Vec::new();
 
-            let members = self
-                .get_channel_participant_details_internal(new_parent, admin_id, &*tx)
-                .await?;
+        let members = self
+            .get_channel_participant_details_internal(new_parent, &*tx)
+            .await?;
 
-            dbg!(&members);
+        dbg!(&members);
 
-            for member in members.iter() {
-                if !member.role.can_see_all_descendants() {
-                    continue;
-                }
-                results.push((
-                    member.user_id,
-                    self.get_user_channels(
-                        member.user_id,
-                        vec![channel_member::Model {
-                            id: Default::default(),
-                            channel_id: new_parent,
-                            user_id: member.user_id,
-                            role: member.role,
-                            accepted: true,
-                        }],
-                        &*tx,
-                    )
-                    .await?,
-                ))
+        for member in members.iter() {
+            if !member.role.can_see_all_descendants() {
+                continue;
             }
+            results.push((
+                member.user_id,
+                self.get_user_channels(
+                    member.user_id,
+                    vec![channel_member::Model {
+                        id: Default::default(),
+                        channel_id: new_parent,
+                        user_id: member.user_id,
+                        role: member.role,
+                        accepted: true,
+                    }],
+                    &*tx,
+                )
+                .await?,
+            ))
+        }
 
-            let public_parent = self
-                .public_path_to_channel_internal(new_parent, &*tx)
-                .await?
-                .last()
-                .copied();
+        let public_parent = self
+            .public_path_to_channel(new_parent, &*tx)
+            .await?
+            .last()
+            .copied();
 
-            let Some(public_parent) = public_parent else {
-                return Ok(results);
-            };
+        let Some(public_parent) = public_parent else {
+            return Ok(results);
+        };
 
-            // could save some time in the common case by skipping this if the
-            // new channel is not public and has no public descendants.
-            let public_members = if public_parent == new_parent {
-                members
-            } else {
-                self.get_channel_participant_details_internal(public_parent, admin_id, &*tx)
-                    .await?
-            };
+        // could save some time in the common case by skipping this if the
+        // new channel is not public and has no public descendants.
+        let public_members = if public_parent == new_parent {
+            members
+        } else {
+            self.get_channel_participant_details_internal(public_parent, &*tx)
+                .await?
+        };
 
-            dbg!(&public_members);
+        dbg!(&public_members);
 
-            for member in public_members {
-                if !member.role.can_only_see_public_descendants() {
-                    continue;
-                };
-                results.push((
+        for member in public_members {
+            if !member.role.can_only_see_public_descendants() {
+                continue;
+            };
+            results.push((
+                member.user_id,
+                self.get_user_channels(
                     member.user_id,
-                    self.get_user_channels(
-                        member.user_id,
-                        vec![channel_member::Model {
-                            id: Default::default(),
-                            channel_id: public_parent,
-                            user_id: member.user_id,
-                            role: member.role,
-                            accepted: true,
-                        }],
-                        &*tx,
-                    )
-                    .await?,
-                ))
-            }
+                    vec![channel_member::Model {
+                        id: Default::default(),
+                        channel_id: public_parent,
+                        user_id: member.user_id,
+                        role: member.role,
+                        accepted: true,
+                    }],
+                    &*tx,
+                )
+                .await?,
+            ))
+        }
 
-            Ok(results)
-        })
-        .await
+        Ok(results)
     }
 
     pub async fn set_channel_member_role(
@@ -748,15 +833,11 @@ impl Database {
         .await
     }
 
-    pub async fn get_channel_participant_details_internal(
+    async fn get_channel_participant_details_internal(
         &self,
         channel_id: ChannelId,
-        admin_id: UserId,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<ChannelMember>> {
-        self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
-            .await?;
-
         let channel_visibility = channel::Entity::find()
             .filter(channel::Column::Id.eq(channel_id))
             .one(&*tx)
@@ -851,8 +932,11 @@ impl Database {
     ) -> Result<Vec<proto::ChannelMember>> {
         let members = self
             .transaction(move |tx| async move {
+                self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
+                    .await?;
+
                 Ok(self
-                    .get_channel_participant_details_internal(channel_id, admin_id, &*tx)
+                    .get_channel_participant_details_internal(channel_id, &*tx)
                     .await?)
             })
             .await?;
@@ -863,25 +947,18 @@ impl Database {
             .collect())
     }
 
-    pub async fn get_channel_participants_internal(
+    pub async fn get_channel_participants(
         &self,
-        id: ChannelId,
+        channel_id: ChannelId,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<UserId>> {
-        let ancestor_ids = self.get_channel_ancestors(id, tx).await?;
-        let user_ids = channel_member::Entity::find()
-            .distinct()
-            .filter(
-                channel_member::Column::ChannelId
-                    .is_in(ancestor_ids.iter().copied())
-                    .and(channel_member::Column::Accepted.eq(true)),
-            )
-            .select_only()
-            .column(channel_member::Column::UserId)
-            .into_values::<_, QueryUserIds>()
-            .all(&*tx)
+        let participants = self
+            .get_channel_participant_details_internal(channel_id, &*tx)
             .await?;
-        Ok(user_ids)
+        Ok(participants
+            .into_iter()
+            .map(|member| member.user_id)
+            .collect())
     }
 
     pub async fn check_user_is_channel_admin(
@@ -951,18 +1028,12 @@ impl Database {
         Ok(row)
     }
 
-    // ordered from higher in tree to lower
-    // only considers one path to a channel
-    // includes the channel itself
-    pub async fn path_to_channel(&self, channel_id: ChannelId) -> Result<Vec<ChannelId>> {
-        self.transaction(move |tx| async move {
-            Ok(self.path_to_channel_internal(channel_id, &*tx).await?)
-        })
-        .await
-    }
-
-    pub async fn parent_channel_id(&self, channel_id: ChannelId) -> Result<Option<ChannelId>> {
-        let path = self.path_to_channel(channel_id).await?;
+    pub async fn parent_channel_id(
+        &self,
+        channel_id: ChannelId,
+        tx: &DatabaseTransaction,
+    ) -> Result<Option<ChannelId>> {
+        let path = self.path_to_channel(channel_id, &*tx).await?;
         if path.len() >= 2 {
             Ok(Some(path[path.len() - 2]))
         } else {
@@ -973,8 +1044,9 @@ impl Database {
     pub async fn public_parent_channel_id(
         &self,
         channel_id: ChannelId,
+        tx: &DatabaseTransaction,
     ) -> Result<Option<ChannelId>> {
-        let path = self.path_to_channel(channel_id).await?;
+        let path = self.public_path_to_channel(channel_id, &*tx).await?;
         if path.len() >= 2 && path.last().copied() == Some(channel_id) {
             Ok(Some(path[path.len() - 2]))
         } else {
@@ -982,7 +1054,7 @@ impl Database {
         }
     }
 
-    pub async fn path_to_channel_internal(
+    pub async fn path_to_channel(
         &self,
         channel_id: ChannelId,
         tx: &DatabaseTransaction,
@@ -1005,27 +1077,12 @@ impl Database {
             .collect())
     }
 
-    // ordered from higher in tree to lower
-    // only considers one path to a channel
-    // includes the channel itself
-    pub async fn public_path_to_channel(&self, channel_id: ChannelId) -> Result<Vec<ChannelId>> {
-        self.transaction(move |tx| async move {
-            Ok(self
-                .public_path_to_channel_internal(channel_id, &*tx)
-                .await?)
-        })
-        .await
-    }
-
-    // ordered from higher in tree to lower
-    // only considers one path to a channel
-    // includes the channel itself
-    pub async fn public_path_to_channel_internal(
+    pub async fn public_path_to_channel(
         &self,
         channel_id: ChannelId,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<ChannelId>> {
-        let ancestor_ids = self.path_to_channel_internal(channel_id, &*tx).await?;
+        let ancestor_ids = self.path_to_channel(channel_id, &*tx).await?;
 
         let rows = channel::Entity::find()
             .filter(channel::Column::Id.is_in(ancestor_ids.iter().copied()))
@@ -1151,27 +1208,6 @@ impl Database {
         Ok(channel_ids)
     }
 
-    // returns all ids of channels in the tree under this channel_id.
-    pub async fn get_channel_descendant_ids(
-        &self,
-        channel_id: ChannelId,
-    ) -> Result<HashSet<ChannelId>> {
-        self.transaction(|tx| async move {
-            let pairs = self.get_channel_descendants([channel_id], &*tx).await?;
-
-            let mut results: HashSet<ChannelId> = HashSet::default();
-            for ChannelEdge {
-                parent_id: _,
-                channel_id,
-            } in pairs
-            {
-                results.insert(ChannelId::from_proto(channel_id));
-            }
-            Ok(results)
-        })
-        .await
-    }
-
     // Returns the channel desendants as a sorted list of edges for further processing.
     // The edges are sorted such that you will see unknown channel ids as children
     // before you see them as parents.
@@ -1388,9 +1424,6 @@ impl Database {
         from: ChannelId,
     ) -> Result<()> {
         self.transaction(|tx| async move {
-            // Note that even with these maxed permissions, this linking operation
-            // is still insecure because you can't remove someone's permissions to a
-            // channel if they've linked the channel to one where they're an admin.
             self.check_user_is_channel_admin(channel, user, &*tx)
                 .await?;
 
@@ -1433,6 +1466,8 @@ impl Database {
             .await?
             == 0;
 
+        dbg!(is_stranded, &paths);
+
         // Make sure that there is always at least one path to the channel
         if is_stranded {
             let root_paths: Vec<_> = paths
@@ -1445,6 +1480,8 @@ impl Database {
                     }
                 })
                 .collect();
+
+            dbg!(is_stranded, &root_paths);
             channel_path::Entity::insert_many(root_paths)
                 .exec(&*tx)
                 .await?;
@@ -1453,49 +1490,64 @@ impl Database {
         Ok(())
     }
 
-    /// Move a channel from one parent to another, returns the
-    /// Channels that were moved for notifying clients
+    /// Move a channel from one parent to another
     pub async fn move_channel(
         &self,
-        user: UserId,
-        channel: ChannelId,
-        from: ChannelId,
-        to: ChannelId,
-    ) -> Result<ChannelGraph> {
-        if from == to {
-            return Ok(ChannelGraph {
-                channels: vec![],
-                edges: vec![],
-            });
-        }
-
+        channel_id: ChannelId,
+        old_parent_id: Option<ChannelId>,
+        new_parent_id: ChannelId,
+        admin_id: UserId,
+    ) -> Result<Option<MoveChannelResult>> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_admin(channel, user, &*tx)
+            self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
                 .await?;
 
-            let moved_channels = self.link_channel_internal(user, channel, to, &*tx).await?;
+            debug_assert_eq!(
+                self.parent_channel_id(channel_id, &*tx).await?,
+                old_parent_id
+            );
 
-            self.unlink_channel_internal(user, channel, from, &*tx)
+            if old_parent_id == Some(new_parent_id) {
+                return Ok(None);
+            }
+            let previous_participants = self
+                .get_channel_participant_details_internal(channel_id, &*tx)
                 .await?;
 
-            Ok(moved_channels)
-        })
-        .await
-    }
+            self.link_channel_internal(admin_id, channel_id, new_parent_id, &*tx)
+                .await?;
 
-    pub async fn assert_root_channel(&self, channel: ChannelId) -> Result<()> {
-        self.transaction(|tx| async move {
-            let path = channel_path::Entity::find()
-                .filter(channel_path::Column::ChannelId.eq(channel))
-                .one(&*tx)
+            if let Some(from) = old_parent_id {
+                self.unlink_channel_internal(admin_id, channel_id, from, &*tx)
+                    .await?;
+            }
+
+            let participants_to_update: HashMap<UserId, ChannelsForUser> = self
+                .participants_to_notify_for_channel_change(new_parent_id, &*tx)
                 .await?
-                .ok_or_else(|| anyhow!("no such channel found"))?;
+                .into_iter()
+                .collect();
 
-            let mut id_parts = path.id_path.trim_matches('/').split('/');
+            let mut moved_channels: HashSet<ChannelId> = HashSet::default();
+            moved_channels.insert(channel_id);
+            for edge in self.get_channel_descendants([channel_id], &*tx).await? {
+                moved_channels.insert(ChannelId::from_proto(edge.channel_id));
+            }
+
+            let mut participants_to_remove: HashSet<UserId> = HashSet::default();
+            for participant in previous_participants {
+                if participant.kind == proto::channel_member::Kind::AncestorMember {
+                    if !participants_to_update.contains_key(&participant.user_id) {
+                        participants_to_remove.insert(participant.user_id);
+                    }
+                }
+            }
 
-            (id_parts.next().is_some() && id_parts.next().is_none())
-                .then_some(())
-                .ok_or_else(|| anyhow!("channel is not a root channel").into())
+            Ok(Some(MoveChannelResult {
+                participants_to_remove,
+                participants_to_update,
+                moved_channels,
+            }))
         })
         .await
     }

crates/collab/src/db/queries/messages.rs 🔗

@@ -183,9 +183,7 @@ impl Database {
             )
             .await?;
 
-            let mut channel_members = self
-                .get_channel_participants_internal(channel_id, &*tx)
-                .await?;
+            let mut channel_members = self.get_channel_participants(channel_id, &*tx).await?;
             channel_members.retain(|member| !participant_user_ids.contains(member));
 
             Ok((

crates/collab/src/db/queries/rooms.rs 🔗

@@ -53,9 +53,7 @@ impl Database {
             let (channel_id, room) = self.get_channel_room(room_id, &tx).await?;
             let channel_members;
             if let Some(channel_id) = channel_id {
-                channel_members = self
-                    .get_channel_participants_internal(channel_id, &tx)
-                    .await?;
+                channel_members = self.get_channel_participants(channel_id, &tx).await?;
             } else {
                 channel_members = Vec::new();
 
@@ -423,9 +421,7 @@ impl Database {
         .await?;
 
         let room = self.get_room(room_id, &tx).await?;
-        let channel_members = self
-            .get_channel_participants_internal(channel_id, &tx)
-            .await?;
+        let channel_members = self.get_channel_participants(channel_id, &tx).await?;
         Ok(JoinRoom {
             room,
             channel_id: Some(channel_id),
@@ -724,8 +720,7 @@ impl Database {
 
             let (channel_id, room) = self.get_channel_room(room_id, &tx).await?;
             let channel_members = if let Some(channel_id) = channel_id {
-                self.get_channel_participants_internal(channel_id, &tx)
-                    .await?
+                self.get_channel_participants(channel_id, &tx).await?
             } else {
                 Vec::new()
             };
@@ -883,8 +878,7 @@ impl Database {
                 };
 
                 let channel_members = if let Some(channel_id) = channel_id {
-                    self.get_channel_participants_internal(channel_id, &tx)
-                        .await?
+                    self.get_channel_participants(channel_id, &tx).await?
                 } else {
                     Vec::new()
                 };

crates/collab/src/db/tests/channel_tests.rs 🔗

@@ -36,28 +36,28 @@ async fn test_channels(db: &Arc<Database>) {
         .await
         .unwrap();
 
-    let crdb_id = db.create_channel("crdb", Some(zed_id), a_id).await.unwrap();
+    let crdb_id = db.create_sub_channel("crdb", zed_id, a_id).await.unwrap();
     let livestreaming_id = db
-        .create_channel("livestreaming", Some(zed_id), a_id)
+        .create_sub_channel("livestreaming", zed_id, a_id)
         .await
         .unwrap();
     let replace_id = db
-        .create_channel("replace", Some(zed_id), a_id)
+        .create_sub_channel("replace", zed_id, a_id)
         .await
         .unwrap();
 
-    let mut members = db.get_channel_members(replace_id).await.unwrap();
+    let mut members = db
+        .transaction(|tx| async move { Ok(db.get_channel_participants(replace_id, &*tx).await?) })
+        .await
+        .unwrap();
     members.sort();
     assert_eq!(members, &[a_id, b_id]);
 
     let rust_id = db.create_root_channel("rust", a_id).await.unwrap();
-    let cargo_id = db
-        .create_channel("cargo", Some(rust_id), a_id)
-        .await
-        .unwrap();
+    let cargo_id = db.create_sub_channel("cargo", rust_id, a_id).await.unwrap();
 
     let cargo_ra_id = db
-        .create_channel("cargo-ra", Some(cargo_id), a_id)
+        .create_sub_channel("cargo-ra", cargo_id, a_id)
         .await
         .unwrap();
 
@@ -264,7 +264,7 @@ async fn test_channel_invites(db: &Arc<Database>) {
         .unwrap();
 
     let channel_1_3 = db
-        .create_channel("channel_3", Some(channel_1_1), user_1)
+        .create_sub_channel("channel_3", channel_1_1, user_1)
         .await
         .unwrap();
 
@@ -277,7 +277,7 @@ async fn test_channel_invites(db: &Arc<Database>) {
         &[
             proto::ChannelMember {
                 user_id: user_1.to_proto(),
-                kind: proto::channel_member::Kind::Member.into(),
+                kind: proto::channel_member::Kind::AncestorMember.into(),
                 role: proto::ChannelRole::Admin.into(),
             },
             proto::ChannelMember {
@@ -369,20 +369,17 @@ async fn test_db_channel_moving(db: &Arc<Database>) {
 
     let zed_id = db.create_root_channel("zed", a_id).await.unwrap();
 
-    let crdb_id = db.create_channel("crdb", Some(zed_id), a_id).await.unwrap();
+    let crdb_id = db.create_sub_channel("crdb", zed_id, a_id).await.unwrap();
 
-    let gpui2_id = db
-        .create_channel("gpui2", Some(zed_id), a_id)
-        .await
-        .unwrap();
+    let gpui2_id = db.create_sub_channel("gpui2", zed_id, a_id).await.unwrap();
 
     let livestreaming_id = db
-        .create_channel("livestreaming", Some(crdb_id), a_id)
+        .create_sub_channel("livestreaming", crdb_id, a_id)
         .await
         .unwrap();
 
     let livestreaming_dag_id = db
-        .create_channel("livestreaming_dag", Some(livestreaming_id), a_id)
+        .create_sub_channel("livestreaming_dag", livestreaming_id, a_id)
         .await
         .unwrap();
 
@@ -409,311 +406,311 @@ async fn test_db_channel_moving(db: &Arc<Database>) {
         .await
         .is_err());
 
-    // ========================================================================
-    // Make a link
-    db.link_channel(a_id, livestreaming_id, zed_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    //     /- gpui2
-    // zed -- crdb - livestreaming - livestreaming_dag
-    //    \---------/
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Create a new channel below a channel with multiple parents
-    let livestreaming_dag_sub_id = db
-        .create_channel("livestreaming_dag_sub", Some(livestreaming_dag_id), a_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    //     /- gpui2
-    // zed -- crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id
-    //    \---------/
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Test a complex DAG by making another link
-    let returned_channels = db
-        .link_channel(a_id, livestreaming_dag_sub_id, livestreaming_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    //    /- gpui2                /---------------------\
-    // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id
-    //    \--------/
-
-    // make sure we're getting just the new link
-    // Not using the assert_dag helper because we want to make sure we're returning the full data
-    pretty_assertions::assert_eq!(
-        returned_channels,
-        graph(
-            &[(
-                livestreaming_dag_sub_id,
-                "livestreaming_dag_sub",
-                ChannelRole::Admin
-            )],
-            &[(livestreaming_dag_sub_id, livestreaming_id)]
-        )
-    );
-
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Test a complex DAG by making another link
-    let returned_channels = db
-        .link_channel(a_id, livestreaming_id, gpui2_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    //    /- gpui2 -\             /---------------------\
-    // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub_id
-    //    \---------/
-
-    // Make sure that we're correctly getting the full sub-dag
-    pretty_assertions::assert_eq!(
-        returned_channels,
-        graph(
-            &[
-                (livestreaming_id, "livestreaming", ChannelRole::Admin),
-                (
-                    livestreaming_dag_id,
-                    "livestreaming_dag",
-                    ChannelRole::Admin
-                ),
-                (
-                    livestreaming_dag_sub_id,
-                    "livestreaming_dag_sub",
-                    ChannelRole::Admin
-                ),
-            ],
-            &[
-                (livestreaming_id, gpui2_id),
-                (livestreaming_dag_id, livestreaming_id),
-                (livestreaming_dag_sub_id, livestreaming_id),
-                (livestreaming_dag_sub_id, livestreaming_dag_id),
-            ]
-        )
-    );
-
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_id, Some(gpui2_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Test unlinking in a complex DAG by removing the inner link
-    db.unlink_channel(a_id, livestreaming_dag_sub_id, livestreaming_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    //    /- gpui2 -\
-    // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
-    //    \---------/
-
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(gpui2_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Test unlinking in a complex DAG by removing the inner link
-    db.unlink_channel(a_id, livestreaming_id, gpui2_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    //    /- gpui2
-    // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
-    //    \---------/
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Test moving DAG nodes by moving livestreaming to be below gpui2
-    db.move_channel(a_id, livestreaming_id, crdb_id, gpui2_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    //    /- gpui2 -- livestreaming - livestreaming_dag - livestreaming_dag_sub
-    // zed - crdb    /
-    //    \---------/
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(gpui2_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Deleting a channel should not delete children that still have other parents
-    db.delete_channel(gpui2_id, a_id).await.unwrap();
-
-    // DAG is now:
-    // zed - crdb
-    //    \- livestreaming - livestreaming_dag - livestreaming_dag_sub
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Unlinking a channel from it's parent should automatically promote it to a root channel
-    db.unlink_channel(a_id, crdb_id, zed_id).await.unwrap();
-
-    // DAG is now:
-    // crdb
-    // zed
-    //    \- livestreaming - livestreaming_dag - livestreaming_dag_sub
-
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, None),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // You should be able to move a root channel into a non-root channel
-    db.link_channel(a_id, crdb_id, zed_id).await.unwrap();
-
-    // DAG is now:
-    // zed - crdb
-    //    \- livestreaming - livestreaming_dag - livestreaming_dag_sub
-
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // ========================================================================
-    // Prep for DAG deletion test
-    db.link_channel(a_id, livestreaming_id, crdb_id)
-        .await
-        .unwrap();
-
-    // DAG is now:
-    // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub
-    //    \--------/
-
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
-        result.channels,
-        &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (livestreaming_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
-            (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
-        ],
-    );
-
-    // Deleting the parent of a DAG should delete the whole DAG:
-    db.delete_channel(zed_id, a_id).await.unwrap();
-    let result = db.get_channels_for_user(a_id).await.unwrap();
-
-    assert!(result.channels.is_empty())
+    //  // ========================================================================
+    //  // Make a link
+    //  db.link_channel(a_id, livestreaming_id, zed_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  //     /- gpui2
+    //  // zed -- crdb - livestreaming - livestreaming_dag
+    //  //    \---------/
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (gpui2_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(crdb_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Create a new channel below a channel with multiple parents
+    //  let livestreaming_dag_sub_id = db
+    //      .create_channel("livestreaming_dag_sub", Some(livestreaming_dag_id), a_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  //     /- gpui2
+    //  // zed -- crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id
+    //  //    \---------/
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (gpui2_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(crdb_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Test a complex DAG by making another link
+    //  let returned_channels = db
+    //      .link_channel(a_id, livestreaming_dag_sub_id, livestreaming_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  //    /- gpui2                /---------------------\
+    //  // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id
+    //  //    \--------/
+
+    //  // make sure we're getting just the new link
+    //  // Not using the assert_dag helper because we want to make sure we're returning the full data
+    //  pretty_assertions::assert_eq!(
+    //      returned_channels,
+    //      graph(
+    //          &[(
+    //              livestreaming_dag_sub_id,
+    //              "livestreaming_dag_sub",
+    //              ChannelRole::Admin
+    //          )],
+    //          &[(livestreaming_dag_sub_id, livestreaming_id)]
+    //      )
+    //  );
+
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (gpui2_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(crdb_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Test a complex DAG by making another link
+    //  let returned_channels = db
+    //      .link_channel(a_id, livestreaming_id, gpui2_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  //    /- gpui2 -\             /---------------------\
+    //  // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub_id
+    //  //    \---------/
+
+    //  // Make sure that we're correctly getting the full sub-dag
+    //  pretty_assertions::assert_eq!(
+    //      returned_channels,
+    //      graph(
+    //          &[
+    //              (livestreaming_id, "livestreaming", ChannelRole::Admin),
+    //              (
+    //                  livestreaming_dag_id,
+    //                  "livestreaming_dag",
+    //                  ChannelRole::Admin
+    //              ),
+    //              (
+    //                  livestreaming_dag_sub_id,
+    //                  "livestreaming_dag_sub",
+    //                  ChannelRole::Admin
+    //              ),
+    //          ],
+    //          &[
+    //              (livestreaming_id, gpui2_id),
+    //              (livestreaming_dag_id, livestreaming_id),
+    //              (livestreaming_dag_sub_id, livestreaming_id),
+    //              (livestreaming_dag_sub_id, livestreaming_dag_id),
+    //          ]
+    //      )
+    //  );
+
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (gpui2_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(crdb_id)),
+    //          (livestreaming_id, Some(gpui2_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Test unlinking in a complex DAG by removing the inner link
+    //  db.unlink_channel(a_id, livestreaming_dag_sub_id, livestreaming_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  //    /- gpui2 -\
+    //  // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
+    //  //    \---------/
+
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (gpui2_id, Some(zed_id)),
+    //          (livestreaming_id, Some(gpui2_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(crdb_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Test unlinking in a complex DAG by removing the inner link
+    //  db.unlink_channel(a_id, livestreaming_id, gpui2_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  //    /- gpui2
+    //  // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub
+    //  //    \---------/
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (gpui2_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(crdb_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Test moving DAG nodes by moving livestreaming to be below gpui2
+    //  db.move_channel(livestreaming_id, Some(crdb_id), gpui2_id, a_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  //    /- gpui2 -- livestreaming - livestreaming_dag - livestreaming_dag_sub
+    //  // zed - crdb    /
+    //  //    \---------/
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (gpui2_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(gpui2_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Deleting a channel should not delete children that still have other parents
+    //  db.delete_channel(gpui2_id, a_id).await.unwrap();
+
+    //  // DAG is now:
+    //  // zed - crdb
+    //  //    \- livestreaming - livestreaming_dag - livestreaming_dag_sub
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Unlinking a channel from it's parent should automatically promote it to a root channel
+    //  db.unlink_channel(a_id, crdb_id, zed_id).await.unwrap();
+
+    //  // DAG is now:
+    //  // crdb
+    //  // zed
+    //  //    \- livestreaming - livestreaming_dag - livestreaming_dag_sub
+
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, None),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // You should be able to move a root channel into a non-root channel
+    //  db.link_channel(a_id, crdb_id, zed_id).await.unwrap();
+
+    //  // DAG is now:
+    //  // zed - crdb
+    //  //    \- livestreaming - livestreaming_dag - livestreaming_dag_sub
+
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // ========================================================================
+    //  // Prep for DAG deletion test
+    //  db.link_channel(a_id, livestreaming_id, crdb_id)
+    //      .await
+    //      .unwrap();
+
+    //  // DAG is now:
+    //  // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub
+    //  //    \--------/
+
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+    //  assert_dag(
+    //      result.channels,
+    //      &[
+    //          (zed_id, None),
+    //          (crdb_id, Some(zed_id)),
+    //          (livestreaming_id, Some(zed_id)),
+    //          (livestreaming_id, Some(crdb_id)),
+    //          (livestreaming_dag_id, Some(livestreaming_id)),
+    //          (livestreaming_dag_sub_id, Some(livestreaming_dag_id)),
+    //      ],
+    //  );
+
+    //  // Deleting the parent of a DAG should delete the whole DAG:
+    //  db.delete_channel(zed_id, a_id).await.unwrap();
+    //  let result = db.get_channels_for_user(a_id).await.unwrap();
+
+    //  assert!(result.channels.is_empty())
 }
 
 test_both_dbs!(
@@ -740,12 +737,12 @@ async fn test_db_channel_moving_bugs(db: &Arc<Database>) {
     let zed_id = db.create_root_channel("zed", user_id).await.unwrap();
 
     let projects_id = db
-        .create_channel("projects", Some(zed_id), user_id)
+        .create_sub_channel("projects", zed_id, user_id)
         .await
         .unwrap();
 
     let livestreaming_id = db
-        .create_channel("livestreaming", Some(projects_id), user_id)
+        .create_sub_channel("livestreaming", projects_id, user_id)
         .await
         .unwrap();
 
@@ -753,25 +750,37 @@ async fn test_db_channel_moving_bugs(db: &Arc<Database>) {
 
     // Move to same parent should be a no-op
     assert!(db
-        .move_channel(user_id, projects_id, zed_id, zed_id)
+        .move_channel(projects_id, Some(zed_id), zed_id, user_id)
         .await
         .unwrap()
-        .is_empty());
-
-    // Stranding a channel should retain it's sub channels
-    db.unlink_channel(user_id, projects_id, zed_id)
-        .await
-        .unwrap();
+        .is_none());
 
     let result = db.get_channels_for_user(user_id).await.unwrap();
     assert_dag(
         result.channels,
         &[
             (zed_id, None),
-            (projects_id, None),
+            (projects_id, Some(zed_id)),
             (livestreaming_id, Some(projects_id)),
         ],
     );
+
+    // Stranding a channel should retain it's sub channels
+    // Commented out as we don't fix permissions when this happens yet.
+    //
+    // db.unlink_channel(user_id, projects_id, zed_id)
+    //     .await
+    //     .unwrap();
+
+    // let result = db.get_channels_for_user(user_id).await.unwrap();
+    // assert_dag(
+    //     result.channels,
+    //     &[
+    //         (zed_id, None),
+    //         (projects_id, None),
+    //         (livestreaming_id, Some(projects_id)),
+    //     ],
+    // );
 }
 
 test_both_dbs!(
@@ -787,11 +796,11 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
 
     let zed_channel = db.create_root_channel("zed", admin).await.unwrap();
     let active_channel = db
-        .create_channel("active", Some(zed_channel), admin)
+        .create_sub_channel("active", zed_channel, admin)
         .await
         .unwrap();
     let vim_channel = db
-        .create_channel("vim", Some(active_channel), admin)
+        .create_sub_channel("vim", active_channel, admin)
         .await
         .unwrap();
 
@@ -834,7 +843,7 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
         &[
             proto::ChannelMember {
                 user_id: admin.to_proto(),
-                kind: proto::channel_member::Kind::Member.into(),
+                kind: proto::channel_member::Kind::AncestorMember.into(),
                 role: proto::ChannelRole::Admin.into(),
             },
             proto::ChannelMember {
@@ -892,7 +901,7 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
         &[
             proto::ChannelMember {
                 user_id: admin.to_proto(),
-                kind: proto::channel_member::Kind::Member.into(),
+                kind: proto::channel_member::Kind::AncestorMember.into(),
                 role: proto::ChannelRole::Admin.into(),
             },
             proto::ChannelMember {
@@ -933,7 +942,7 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
         &[
             proto::ChannelMember {
                 user_id: admin.to_proto(),
-                kind: proto::channel_member::Kind::Member.into(),
+                kind: proto::channel_member::Kind::AncestorMember.into(),
                 role: proto::ChannelRole::Admin.into(),
             },
             proto::ChannelMember {
@@ -981,7 +990,7 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
         &[
             proto::ChannelMember {
                 user_id: admin.to_proto(),
-                kind: proto::channel_member::Kind::Member.into(),
+                kind: proto::channel_member::Kind::AncestorMember.into(),
                 role: proto::ChannelRole::Admin.into(),
             },
             proto::ChannelMember {
@@ -1016,17 +1025,17 @@ async fn test_user_joins_correct_channel(db: &Arc<Database>) {
     let zed_channel = db.create_root_channel("zed", admin).await.unwrap();
 
     let active_channel = db
-        .create_channel("active", Some(zed_channel), admin)
+        .create_sub_channel("active", zed_channel, admin)
         .await
         .unwrap();
 
     let vim_channel = db
-        .create_channel("vim", Some(active_channel), admin)
+        .create_sub_channel("vim", active_channel, admin)
         .await
         .unwrap();
 
     let vim2_channel = db
-        .create_channel("vim2", Some(vim_channel), admin)
+        .create_sub_channel("vim2", vim_channel, admin)
         .await
         .unwrap();
 
@@ -1043,11 +1052,15 @@ async fn test_user_joins_correct_channel(db: &Arc<Database>) {
         .unwrap();
 
     let most_public = db
-        .public_path_to_channel(vim_channel)
+        .transaction(|tx| async move {
+            Ok(db
+                .public_path_to_channel(vim_channel, &tx)
+                .await?
+                .first()
+                .cloned())
+        })
         .await
-        .unwrap()
-        .first()
-        .cloned();
+        .unwrap();
 
     assert_eq!(most_public, Some(zed_channel))
 }

crates/collab/src/db/tests/message_tests.rs 🔗

@@ -25,7 +25,7 @@ async fn test_channel_message_retrieval(db: &Arc<Database>) {
         .await
         .unwrap()
         .user_id;
-    let channel = db.create_channel("channel", None, user).await.unwrap();
+    let channel = db.create_root_channel("channel", user).await.unwrap();
 
     let owner_id = db.create_server("test").await.unwrap().0 as u32;
     db.join_channel_chat(channel, rpc::ConnectionId { owner_id, id: 0 }, user)
@@ -87,7 +87,7 @@ async fn test_channel_message_nonces(db: &Arc<Database>) {
         .await
         .unwrap()
         .user_id;
-    let channel = db.create_channel("channel", None, user).await.unwrap();
+    let channel = db.create_root_channel("channel", user).await.unwrap();
 
     let owner_id = db.create_server("test").await.unwrap().0 as u32;
 
@@ -151,9 +151,9 @@ async fn test_channel_message_new_notification(db: &Arc<Database>) {
         .unwrap()
         .user_id;
 
-    let channel_1 = db.create_channel("channel", None, user).await.unwrap();
+    let channel_1 = db.create_root_channel("channel", user).await.unwrap();
 
-    let channel_2 = db.create_channel("channel-2", None, user).await.unwrap();
+    let channel_2 = db.create_root_channel("channel-2", user).await.unwrap();
 
     db.invite_channel_member(channel_1, observer, user, ChannelRole::Member)
         .await

crates/collab/src/rpc.rs 🔗

@@ -3,8 +3,9 @@ mod connection_pool;
 use crate::{
     auth,
     db::{
-        self, BufferId, ChannelId, ChannelRole, ChannelVisibility, ChannelsForUser, Database,
-        MessageId, ProjectId, RoomId, ServerId, User, UserId,
+        self, BufferId, ChannelId, ChannelsForUser, CreateChannelResult, Database, MessageId,
+        MoveChannelResult, ProjectId, RenameChannelResult, RoomId, ServerId,
+        SetChannelVisibilityResult, User, UserId,
     },
     executor::Executor,
     AppState, Result,
@@ -590,7 +591,7 @@ impl Server {
                 let mut pool = this.connection_pool.lock();
                 pool.add_connection(connection_id, user_id, user.admin);
                 this.peer.send(connection_id, build_initial_contacts_update(contacts, &pool))?;
-                this.peer.send(connection_id, build_initial_channels_update(
+                this.peer.send(connection_id, build_channels_update(
                     channels_for_user,
                     channel_invites
                 ))?;
@@ -2202,31 +2203,21 @@ async fn create_channel(
     let db = session.db().await;
 
     let parent_id = request.parent_id.map(|id| ChannelId::from_proto(id));
-    let id = db
+    let CreateChannelResult {
+        channel,
+        participants_to_update,
+    } = db
         .create_channel(&request.name, parent_id, session.user_id)
         .await?;
 
     response.send(proto::CreateChannelResponse {
-        channel: Some(proto::Channel {
-            id: id.to_proto(),
-            name: request.name,
-            visibility: proto::ChannelVisibility::Members as i32,
-            role: proto::ChannelRole::Admin.into(),
-        }),
+        channel: Some(channel.to_proto()),
         parent_id: request.parent_id,
     })?;
 
-    let Some(parent_id) = parent_id else {
-        return Ok(());
-    };
-
-    let updates = db
-        .participants_to_notify_for_channel_change(parent_id, session.user_id)
-        .await?;
-
     let connection_pool = session.connection_pool().await;
-    for (user_id, channels) in updates {
-        let update = build_initial_channels_update(channels, vec![]);
+    for (user_id, channels) in participants_to_update {
+        let update = build_channels_update(channels, vec![]);
         for connection_id in connection_pool.user_connection_ids(user_id) {
             if user_id == session.user_id {
                 continue;
@@ -2340,49 +2331,21 @@ async fn set_channel_visibility(
     let channel_id = ChannelId::from_proto(request.channel_id);
     let visibility = request.visibility().into();
 
-    let previous_members = db
-        .get_channel_participant_details(channel_id, session.user_id)
-        .await?;
-
-    db.set_channel_visibility(channel_id, visibility, session.user_id)
+    let SetChannelVisibilityResult {
+        participants_to_update,
+        participants_to_remove,
+    } = db
+        .set_channel_visibility(channel_id, visibility, session.user_id)
         .await?;
 
-    let mut updates: HashMap<UserId, ChannelsForUser> = db
-        .participants_to_notify_for_channel_change(channel_id, session.user_id)
-        .await?
-        .into_iter()
-        .collect();
-
-    let mut participants_who_lost_access: HashSet<UserId> = HashSet::default();
-    match visibility {
-        ChannelVisibility::Members => {
-            for member in previous_members {
-                if ChannelRole::from(member.role()).can_only_see_public_descendants() {
-                    participants_who_lost_access.insert(UserId::from_proto(member.user_id));
-                }
-            }
-        }
-        ChannelVisibility::Public => {
-            if let Some(public_parent_id) = db.public_parent_channel_id(channel_id).await? {
-                let parent_updates = db
-                    .participants_to_notify_for_channel_change(public_parent_id, session.user_id)
-                    .await?;
-
-                for (user_id, channels) in parent_updates {
-                    updates.insert(user_id, channels);
-                }
-            }
-        }
-    }
-
     let connection_pool = session.connection_pool().await;
-    for (user_id, channels) in updates {
-        let update = build_initial_channels_update(channels, vec![]);
+    for (user_id, channels) in participants_to_update {
+        let update = build_channels_update(channels, vec![]);
         for connection_id in connection_pool.user_connection_ids(user_id) {
             session.peer.send(connection_id, update.clone())?;
         }
     }
-    for user_id in participants_who_lost_access {
+    for user_id in participants_to_remove {
         let update = proto::UpdateChannels {
             delete_channels: vec![channel_id.to_proto()],
             ..Default::default()
@@ -2416,7 +2379,7 @@ async fn set_channel_member_role(
     let mut update = proto::UpdateChannels::default();
     if channel_member.accepted {
         let channels = db.get_channel_for_user(channel_id, member_id).await?;
-        update = build_initial_channels_update(channels, vec![]);
+        update = build_channels_update(channels, vec![]);
     } else {
         let channel = db.get_channel(channel_id, session.user_id).await?;
         update.channel_invitations.push(proto::Channel {
@@ -2446,34 +2409,24 @@ async fn rename_channel(
 ) -> Result<()> {
     let db = session.db().await;
     let channel_id = ChannelId::from_proto(request.channel_id);
-    let channel = db
+    let RenameChannelResult {
+        channel,
+        participants_to_update,
+    } = db
         .rename_channel(channel_id, session.user_id, &request.name)
         .await?;
 
     response.send(proto::RenameChannelResponse {
-        channel: Some(proto::Channel {
-            id: channel.id.to_proto(),
-            name: channel.name.clone(),
-            visibility: channel.visibility.into(),
-            role: proto::ChannelRole::Admin.into(),
-        }),
+        channel: Some(channel.to_proto()),
     })?;
 
-    let members = db
-        .get_channel_participant_details(channel_id, session.user_id)
-        .await?;
-
     let connection_pool = session.connection_pool().await;
-    for member in members {
-        for connection_id in connection_pool.user_connection_ids(UserId::from_proto(member.user_id))
-        {
-            let mut update = proto::UpdateChannels::default();
-            update.channels.push(proto::Channel {
-                id: channel.id.to_proto(),
-                name: channel.name.clone(),
-                visibility: channel.visibility.into(),
-                role: member.role.into(),
-            });
+    for (user_id, channel) in participants_to_update {
+        for connection_id in connection_pool.user_connection_ids(user_id) {
+            let update = proto::UpdateChannels {
+                channels: vec![channel.to_proto()],
+                ..Default::default()
+            };
 
             session.peer.send(connection_id, update.clone())?;
         }
@@ -2493,25 +2446,12 @@ async fn link_channel(
     let channel_id = ChannelId::from_proto(request.channel_id);
     let to = ChannelId::from_proto(request.to);
 
-    // TODO: Remove this restriction once we have symlinks
-    db.assert_root_channel(channel_id).await?;
-
-    db.link_channel(session.user_id, channel_id, to).await?;
-
-    let member_updates = db
-        .participants_to_notify_for_channel_change(to, session.user_id)
+    let result = db
+        .move_channel(channel_id, None, to, session.user_id)
         .await?;
+    drop(db);
 
-    dbg!(&member_updates);
-
-    let connection_pool = session.connection_pool().await;
-
-    for (member_id, channels) in member_updates {
-        let update = build_initial_channels_update(channels, vec![]);
-        for connection_id in connection_pool.user_connection_ids(member_id) {
-            session.peer.send(connection_id, update.clone())?;
-        }
-    }
+    notify_channel_moved(result, session).await?;
 
     response.send(Ack {})?;
 
@@ -2537,64 +2477,46 @@ async fn move_channel(
     let from_parent = ChannelId::from_proto(request.from);
     let to = ChannelId::from_proto(request.to);
 
-    let previous_participants = db
-        .get_channel_participant_details(channel_id, session.user_id)
+    let result = db
+        .move_channel(channel_id, Some(from_parent), to, session.user_id)
         .await?;
+    drop(db);
 
-    debug_assert_eq!(db.parent_channel_id(channel_id).await?, Some(from_parent));
+    notify_channel_moved(result, session).await?;
 
-    let channels_to_send = db
-        .move_channel(session.user_id, channel_id, from_parent, to)
-        .await?;
+    response.send(Ack {})?;
+    Ok(())
+}
 
-    if channels_to_send.is_empty() {
-        response.send(Ack {})?;
+async fn notify_channel_moved(result: Option<MoveChannelResult>, session: Session) -> Result<()> {
+    let Some(MoveChannelResult {
+        participants_to_remove,
+        participants_to_update,
+        moved_channels,
+    }) = result
+    else {
         return Ok(());
-    }
-
-    let updates = db
-        .participants_to_notify_for_channel_change(to, session.user_id)
-        .await?;
-
-    let mut participants_who_lost_access: HashSet<UserId> = HashSet::default();
-    let mut channels_to_delete = db.get_channel_descendant_ids(channel_id).await?;
-    channels_to_delete.insert(channel_id);
-
-    for previous_participant in previous_participants.iter() {
-        let user_id = UserId::from_proto(previous_participant.user_id);
-        if previous_participant.kind() == proto::channel_member::Kind::AncestorMember {
-            participants_who_lost_access.insert(user_id);
-        }
-    }
+    };
+    let moved_channels: Vec<u64> = moved_channels.iter().map(|id| id.to_proto()).collect();
 
     let connection_pool = session.connection_pool().await;
-    for (user_id, channels) in updates {
-        let mut update = build_initial_channels_update(channels, vec![]);
-        update.delete_channels = channels_to_delete
-            .iter()
-            .map(|channel_id| channel_id.to_proto())
-            .collect();
-        participants_who_lost_access.remove(&user_id);
+    for (user_id, channels) in participants_to_update {
+        let mut update = build_channels_update(channels, vec![]);
+        update.delete_channels = moved_channels.clone();
         for connection_id in connection_pool.user_connection_ids(user_id) {
             session.peer.send(connection_id, update.clone())?;
         }
     }
 
-    for user_id in participants_who_lost_access {
+    for user_id in participants_to_remove {
         let update = proto::UpdateChannels {
-            delete_channels: channels_to_delete
-                .iter()
-                .map(|channel_id| channel_id.to_proto())
-                .collect(),
+            delete_channels: moved_channels.clone(),
             ..Default::default()
         };
         for connection_id in connection_pool.user_connection_ids(user_id) {
             session.peer.send(connection_id, update.clone())?;
         }
     }
-
-    response.send(Ack {})?;
-
     Ok(())
 }
 
@@ -2641,38 +2563,12 @@ async fn channel_membership_updated(
     channel_id: ChannelId,
     session: &Session,
 ) -> Result<(), crate::Error> {
-    let mut update = proto::UpdateChannels::default();
+    let result = db.get_channel_for_user(channel_id, session.user_id).await?;
+    let mut update = build_channels_update(result, vec![]);
     update
         .remove_channel_invitations
         .push(channel_id.to_proto());
 
-    let result = db.get_channel_for_user(channel_id, session.user_id).await?;
-    update.channels.extend(
-        result
-            .channels
-            .channels
-            .into_iter()
-            .map(|channel| proto::Channel {
-                id: channel.id.to_proto(),
-                visibility: channel.visibility.into(),
-                role: channel.role.into(),
-                name: channel.name,
-            }),
-    );
-    update.unseen_channel_messages = result.channel_messages;
-    update.unseen_channel_buffer_changes = result.unseen_buffer_changes;
-    update.insert_edge = result.channels.edges;
-    update
-        .channel_participants
-        .extend(
-            result
-                .channel_participants
-                .into_iter()
-                .map(|(channel_id, user_ids)| proto::ChannelParticipants {
-                    channel_id: channel_id.to_proto(),
-                    participant_user_ids: user_ids.into_iter().map(UserId::to_proto).collect(),
-                }),
-        );
     session.peer.send(session.connection_id, update)?;
     Ok(())
 }
@@ -3155,7 +3051,7 @@ fn to_tungstenite_message(message: AxumMessage) -> TungsteniteMessage {
     }
 }
 
-fn build_initial_channels_update(
+fn build_channels_update(
     channels: ChannelsForUser,
     channel_invites: Vec<db::Channel>,
 ) -> proto::UpdateChannels {

crates/collab/src/tests/random_channel_buffer_tests.rs 🔗

@@ -48,7 +48,7 @@ impl RandomizedTest for RandomChannelBufferTest {
         let db = &server.app_state.db;
         for ix in 0..CHANNEL_COUNT {
             let id = db
-                .create_channel(&format!("channel-{ix}"), None, users[0].user_id)
+                .create_root_channel(&format!("channel-{ix}"), users[0].user_id)
                 .await
                 .unwrap();
             for user in &users[1..] {

crates/collab/src/tests/test_server.rs 🔗

@@ -604,38 +604,6 @@ impl TestClient {
     ) -> WindowHandle<Workspace> {
         cx.add_window(|cx| Workspace::new(0, project.clone(), self.app_state.clone(), cx))
     }
-
-    pub async fn add_admin_to_channel(
-        &self,
-        user: (&TestClient, &mut TestAppContext),
-        channel: u64,
-        cx_self: &mut TestAppContext,
-    ) {
-        let (other_client, other_cx) = user;
-
-        cx_self
-            .read(ChannelStore::global)
-            .update(cx_self, |channel_store, cx| {
-                channel_store.invite_member(
-                    channel,
-                    other_client.user_id().unwrap(),
-                    ChannelRole::Admin,
-                    cx,
-                )
-            })
-            .await
-            .unwrap();
-
-        cx_self.foreground().run_until_parked();
-
-        other_cx
-            .read(ChannelStore::global)
-            .update(other_cx, |channel_store, _| {
-                channel_store.respond_to_channel_invite(channel, true)
-            })
-            .await
-            .unwrap();
-    }
 }
 
 impl Drop for TestClient {

crates/collab_ui/src/collab_panel.rs 🔗

@@ -2662,7 +2662,6 @@ impl CollabPanel {
                             location: path.clone(),
                         },
                     ),
-                    ContextMenuItem::Separator,
                     ContextMenuItem::action(
                         "Move this channel",
                         StartMoveChannelFor {

crates/rpc/proto/zed.proto 🔗

@@ -970,16 +970,10 @@ message UpdateChannels {
     repeated Channel channel_invitations = 5;
     repeated uint64 remove_channel_invitations = 6;
     repeated ChannelParticipants channel_participants = 7;
-    //repeated ChannelRoles channel_roles = 8;
     repeated UnseenChannelMessage unseen_channel_messages = 9;
     repeated UnseenChannelBufferChange unseen_channel_buffer_changes = 10;
 }
 
-//message ChannelRoles {
-//    ChannelRole role = 1;
-//    uint64 channel_id = 2;
-//}
-
 message UnseenChannelMessage {
     uint64 channel_id = 1;
     uint64 message_id = 2;