Remove logic for multiple channel parents (#3162)

Max Brunsfeld created

This PR simplifies our state management for channels, and logic for
inheriting channel permissions, by removing the ability for channels to
have multiple parent channels.

Change summary

crates/channel/src/channel.rs                                                    |   4 
crates/channel/src/channel_store.rs                                              | 156 
crates/channel/src/channel_store/channel_index.rs                                | 120 
crates/channel/src/channel_store_tests.rs                                        |  32 
crates/collab/migrations.sqlite/20221109000000_test_schema.sql                   |  11 
crates/collab/migrations/20231024085546_move_channel_paths_to_channels_table.sql |  12 
crates/collab/src/db.rs                                                          |  17 
crates/collab/src/db/queries/buffers.rs                                          |  18 
crates/collab/src/db/queries/channels.rs                                         | 641 
crates/collab/src/db/queries/messages.rs                                         |  79 
crates/collab/src/db/queries/rooms.rs                                            |  43 
crates/collab/src/db/tables.rs                                                   |   1 
crates/collab/src/db/tables/channel.rs                                           |  22 
crates/collab/src/db/tables/channel_path.rs                                      |  15 
crates/collab/src/db/tests.rs                                                    |  29 
crates/collab/src/db/tests/channel_tests.rs                                      | 610 
crates/collab/src/rpc.rs                                                         |  62 
crates/collab/src/tests/channel_buffer_tests.rs                                  |  23 
crates/collab/src/tests/channel_tests.rs                                         | 198 
crates/collab/src/tests/random_channel_buffer_tests.rs                           |   4 
crates/collab_ui/src/collab_panel.rs                                             | 440 
crates/rpc/proto/zed.proto                                                       |  23 
crates/rpc/src/proto.rs                                                          |   4 
crates/theme/src/theme.rs                                                        |   1 
styles/src/style_tree/collab_panel.ts                                            |  10 
styles/src/style_tree/search.ts                                                  |   5 
26 files changed, 777 insertions(+), 1,803 deletions(-)

Detailed changes

crates/channel/src/channel.rs 🔗

@@ -11,9 +11,7 @@ pub use channel_chat::{
     mentions_to_proto, ChannelChat, ChannelChatEvent, ChannelMessage, ChannelMessageId,
     MessageParams,
 };
-pub use channel_store::{
-    Channel, ChannelData, ChannelEvent, ChannelId, ChannelMembership, ChannelPath, ChannelStore,
-};
+pub use channel_store::{Channel, ChannelEvent, ChannelId, ChannelMembership, ChannelStore};
 
 #[cfg(test)]
 mod channel_store_tests;

crates/channel/src/channel_store.rs 🔗

@@ -9,11 +9,10 @@ use db::RELEASE_CHANNEL;
 use futures::{channel::mpsc, future::Shared, Future, FutureExt, StreamExt};
 use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Task, WeakModelHandle};
 use rpc::{
-    proto::{self, ChannelEdge, ChannelVisibility},
+    proto::{self, ChannelVisibility},
     TypedEnvelope,
 };
-use serde_derive::{Deserialize, Serialize};
-use std::{borrow::Cow, hash::Hash, mem, ops::Deref, sync::Arc, time::Duration};
+use std::{mem, sync::Arc, time::Duration};
 use util::ResultExt;
 
 pub fn init(client: &Arc<Client>, user_store: ModelHandle<UserStore>, cx: &mut AppContext) {
@@ -27,7 +26,7 @@ pub const RECONNECT_TIMEOUT: Duration = Duration::from_secs(30);
 pub type ChannelId = u64;
 
 pub struct ChannelStore {
-    channel_index: ChannelIndex,
+    pub channel_index: ChannelIndex,
     channel_invitations: Vec<Arc<Channel>>,
     channel_participants: HashMap<ChannelId, Vec<Arc<User>>>,
     outgoing_invites: HashSet<(ChannelId, UserId)>,
@@ -42,8 +41,6 @@ pub struct ChannelStore {
     _update_channels: Task<()>,
 }
 
-pub type ChannelData = (Channel, ChannelPath);
-
 #[derive(Clone, Debug, PartialEq)]
 pub struct Channel {
     pub id: ChannelId,
@@ -52,6 +49,7 @@ pub struct Channel {
     pub role: proto::ChannelRole,
     pub unseen_note_version: Option<(u64, clock::Global)>,
     pub unseen_message_id: Option<u64>,
+    pub parent_path: Vec<u64>,
 }
 
 impl Channel {
@@ -78,9 +76,6 @@ impl Channel {
     }
 }
 
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)]
-pub struct ChannelPath(Arc<[ChannelId]>);
-
 pub struct ChannelMembership {
     pub user: Arc<User>,
     pub kind: proto::channel_member::Kind,
@@ -193,16 +188,6 @@ impl ChannelStore {
         self.client.clone()
     }
 
-    pub fn has_children(&self, channel_id: ChannelId) -> bool {
-        self.channel_index.iter().any(|path| {
-            if let Some(ix) = path.iter().position(|id| *id == channel_id) {
-                path.len() > ix + 1
-            } else {
-                false
-            }
-        })
-    }
-
     /// Returns the number of unique channels in the store
     pub fn channel_count(&self) -> usize {
         self.channel_index.by_id().len()
@@ -222,20 +207,19 @@ impl ChannelStore {
     }
 
     /// Iterate over all entries in the channel DAG
-    pub fn channel_dag_entries(&self) -> impl '_ + Iterator<Item = (usize, &Arc<Channel>)> {
-        self.channel_index.iter().map(move |path| {
-            let id = path.last().unwrap();
-            let channel = self.channel_for_id(*id).unwrap();
-            (path.len() - 1, channel)
-        })
+    pub fn ordered_channels(&self) -> impl '_ + Iterator<Item = (usize, &Arc<Channel>)> {
+        self.channel_index
+            .ordered_channels()
+            .iter()
+            .filter_map(move |id| {
+                let channel = self.channel_index.by_id().get(id)?;
+                Some((channel.parent_path.len(), channel))
+            })
     }
 
-    pub fn channel_dag_entry_at(&self, ix: usize) -> Option<(&Arc<Channel>, &ChannelPath)> {
-        let path = self.channel_index.get(ix)?;
-        let id = path.last().unwrap();
-        let channel = self.channel_for_id(*id).unwrap();
-
-        Some((channel, path))
+    pub fn channel_at_index(&self, ix: usize) -> Option<&Arc<Channel>> {
+        let channel_id = self.channel_index.ordered_channels().get(ix)?;
+        self.channel_index.by_id().get(channel_id)
     }
 
     pub fn channel_at(&self, ix: usize) -> Option<&Arc<Channel>> {
@@ -484,20 +468,19 @@ impl ChannelStore {
                 .ok_or_else(|| anyhow!("missing channel in response"))?;
             let channel_id = channel.id;
 
-            let parent_edge = if let Some(parent_id) = parent_id {
-                vec![ChannelEdge {
-                    channel_id: channel.id,
-                    parent_id,
-                }]
-            } else {
-                vec![]
-            };
+            // let parent_edge = if let Some(parent_id) = parent_id {
+            //     vec![ChannelEdge {
+            //         channel_id: channel.id,
+            //         parent_id,
+            //     }]
+            // } else {
+            //     vec![]
+            // };
 
             this.update(&mut cx, |this, cx| {
                 let task = this.update_channels(
                     proto::UpdateChannels {
                         channels: vec![channel],
-                        insert_edge: parent_edge,
                         ..Default::default()
                     },
                     cx,
@@ -515,53 +498,16 @@ impl ChannelStore {
         })
     }
 
-    pub fn link_channel(
-        &mut self,
-        channel_id: ChannelId,
-        to: ChannelId,
-        cx: &mut ModelContext<Self>,
-    ) -> Task<Result<()>> {
-        let client = self.client.clone();
-        cx.spawn(|_, _| async move {
-            let _ = client
-                .request(proto::LinkChannel { channel_id, to })
-                .await?;
-
-            Ok(())
-        })
-    }
-
-    pub fn unlink_channel(
-        &mut self,
-        channel_id: ChannelId,
-        from: ChannelId,
-        cx: &mut ModelContext<Self>,
-    ) -> Task<Result<()>> {
-        let client = self.client.clone();
-        cx.spawn(|_, _| async move {
-            let _ = client
-                .request(proto::UnlinkChannel { channel_id, from })
-                .await?;
-
-            Ok(())
-        })
-    }
-
     pub fn move_channel(
         &mut self,
         channel_id: ChannelId,
-        from: ChannelId,
-        to: ChannelId,
+        to: Option<ChannelId>,
         cx: &mut ModelContext<Self>,
     ) -> Task<Result<()>> {
         let client = self.client.clone();
         cx.spawn(|_, _| async move {
             let _ = client
-                .request(proto::MoveChannel {
-                    channel_id,
-                    from,
-                    to,
-                })
+                .request(proto::MoveChannel { channel_id, to })
                 .await?;
 
             Ok(())
@@ -956,6 +902,7 @@ impl ChannelStore {
                         name: channel.name,
                         unseen_note_version: None,
                         unseen_message_id: None,
+                        parent_path: channel.parent_path,
                     }),
                 ),
             }
@@ -963,8 +910,6 @@ impl ChannelStore {
 
         let channels_changed = !payload.channels.is_empty()
             || !payload.delete_channels.is_empty()
-            || !payload.insert_edge.is_empty()
-            || !payload.delete_edge.is_empty()
             || !payload.unseen_channel_messages.is_empty()
             || !payload.unseen_channel_buffer_changes.is_empty();
 
@@ -1022,14 +967,6 @@ impl ChannelStore {
                     unseen_channel_message.message_id,
                 );
             }
-
-            for edge in payload.insert_edge {
-                index.insert_edge(edge.channel_id, edge.parent_id);
-            }
-
-            for edge in payload.delete_edge {
-                index.delete_edge(edge.parent_id, edge.channel_id);
-            }
         }
 
         cx.notify();
@@ -1078,44 +1015,3 @@ impl ChannelStore {
         }))
     }
 }
-
-impl Deref for ChannelPath {
-    type Target = [ChannelId];
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl ChannelPath {
-    pub fn new(path: Arc<[ChannelId]>) -> Self {
-        debug_assert!(path.len() >= 1);
-        Self(path)
-    }
-
-    pub fn parent_id(&self) -> Option<ChannelId> {
-        self.0.len().checked_sub(2).map(|i| self.0[i])
-    }
-
-    pub fn channel_id(&self) -> ChannelId {
-        self.0[self.0.len() - 1]
-    }
-}
-
-impl From<ChannelPath> for Cow<'static, ChannelPath> {
-    fn from(value: ChannelPath) -> Self {
-        Cow::Owned(value)
-    }
-}
-
-impl<'a> From<&'a ChannelPath> for Cow<'a, ChannelPath> {
-    fn from(value: &'a ChannelPath) -> Self {
-        Cow::Borrowed(value)
-    }
-}
-
-impl Default for ChannelPath {
-    fn default() -> Self {
-        ChannelPath(Arc::from([]))
-    }
-}

crates/channel/src/channel_store/channel_index.rs 🔗

@@ -1,14 +1,11 @@
-use std::{ops::Deref, sync::Arc};
-
 use crate::{Channel, ChannelId};
 use collections::BTreeMap;
 use rpc::proto;
-
-use super::ChannelPath;
+use std::sync::Arc;
 
 #[derive(Default, Debug)]
 pub struct ChannelIndex {
-    paths: Vec<ChannelPath>,
+    channels_ordered: Vec<ChannelId>,
     channels_by_id: BTreeMap<ChannelId, Arc<Channel>>,
 }
 
@@ -17,8 +14,12 @@ impl ChannelIndex {
         &self.channels_by_id
     }
 
+    pub fn ordered_channels(&self) -> &[ChannelId] {
+        &self.channels_ordered
+    }
+
     pub fn clear(&mut self) {
-        self.paths.clear();
+        self.channels_ordered.clear();
         self.channels_by_id.clear();
     }
 
@@ -26,13 +27,13 @@ impl ChannelIndex {
     pub fn delete_channels(&mut self, channels: &[ChannelId]) {
         self.channels_by_id
             .retain(|channel_id, _| !channels.contains(channel_id));
-        self.paths
-            .retain(|path| !path.iter().any(|channel_id| channels.contains(channel_id)));
+        self.channels_ordered
+            .retain(|channel_id| !channels.contains(channel_id));
     }
 
     pub fn bulk_insert(&mut self) -> ChannelPathsInsertGuard {
         ChannelPathsInsertGuard {
-            paths: &mut self.paths,
+            channels_ordered: &mut self.channels_ordered,
             channels_by_id: &mut self.channels_by_id,
         }
     }
@@ -75,42 +76,15 @@ impl ChannelIndex {
     }
 }
 
-impl Deref for ChannelIndex {
-    type Target = [ChannelPath];
-
-    fn deref(&self) -> &Self::Target {
-        &self.paths
-    }
-}
-
 /// A guard for ensuring that the paths index maintains its sort and uniqueness
 /// invariants after a series of insertions
 #[derive(Debug)]
 pub struct ChannelPathsInsertGuard<'a> {
-    paths: &'a mut Vec<ChannelPath>,
+    channels_ordered: &'a mut Vec<ChannelId>,
     channels_by_id: &'a mut BTreeMap<ChannelId, Arc<Channel>>,
 }
 
 impl<'a> ChannelPathsInsertGuard<'a> {
-    /// Remove the given edge from this index. This will not remove the channel.
-    /// If this operation would result in a dangling edge, re-insert it.
-    pub fn delete_edge(&mut self, parent_id: ChannelId, channel_id: ChannelId) {
-        self.paths.retain(|path| {
-            !path
-                .windows(2)
-                .any(|window| window == [parent_id, channel_id])
-        });
-
-        // Ensure that there is at least one channel path in the index
-        if !self
-            .paths
-            .iter()
-            .any(|path| path.iter().any(|id| id == &channel_id))
-        {
-            self.insert_root(channel_id);
-        }
-    }
-
     pub fn note_changed(&mut self, channel_id: ChannelId, epoch: u64, version: &clock::Global) {
         insert_note_changed(&mut self.channels_by_id, channel_id, epoch, &version);
     }
@@ -141,6 +115,7 @@ impl<'a> ChannelPathsInsertGuard<'a> {
                     name: channel_proto.name,
                     unseen_note_version: None,
                     unseen_message_id: None,
+                    parent_path: channel_proto.parent_path,
                 }),
             );
             self.insert_root(channel_proto.id);
@@ -148,74 +123,35 @@ impl<'a> ChannelPathsInsertGuard<'a> {
         ret
     }
 
-    pub fn insert_edge(&mut self, channel_id: ChannelId, parent_id: ChannelId) {
-        let mut parents = Vec::new();
-        let mut descendants = Vec::new();
-        let mut ixs_to_remove = Vec::new();
-
-        for (ix, path) in self.paths.iter().enumerate() {
-            if path
-                .windows(2)
-                .any(|window| window[0] == parent_id && window[1] == channel_id)
-            {
-                // We already have this edge in the index
-                return;
-            }
-            if path.ends_with(&[parent_id]) {
-                parents.push(path);
-            } else if let Some(position) = path.iter().position(|id| id == &channel_id) {
-                if position == 0 {
-                    ixs_to_remove.push(ix);
-                }
-                descendants.push(path.split_at(position).1);
-            }
-        }
-
-        let mut new_paths = Vec::new();
-        for parent in parents.iter() {
-            if descendants.is_empty() {
-                let mut new_path = Vec::with_capacity(parent.len() + 1);
-                new_path.extend_from_slice(parent);
-                new_path.push(channel_id);
-                new_paths.push(ChannelPath::new(new_path.into()));
-            } else {
-                for descendant in descendants.iter() {
-                    let mut new_path = Vec::with_capacity(parent.len() + descendant.len());
-                    new_path.extend_from_slice(parent);
-                    new_path.extend_from_slice(descendant);
-                    new_paths.push(ChannelPath::new(new_path.into()));
-                }
-            }
-        }
-
-        for ix in ixs_to_remove.into_iter().rev() {
-            self.paths.swap_remove(ix);
-        }
-        self.paths.extend(new_paths)
-    }
-
     fn insert_root(&mut self, channel_id: ChannelId) {
-        self.paths.push(ChannelPath::new(Arc::from([channel_id])));
+        self.channels_ordered.push(channel_id);
     }
 }
 
 impl<'a> Drop for ChannelPathsInsertGuard<'a> {
     fn drop(&mut self) {
-        self.paths.sort_by(|a, b| {
-            let a = channel_path_sorting_key(a, &self.channels_by_id);
-            let b = channel_path_sorting_key(b, &self.channels_by_id);
+        self.channels_ordered.sort_by(|a, b| {
+            let a = channel_path_sorting_key(*a, &self.channels_by_id);
+            let b = channel_path_sorting_key(*b, &self.channels_by_id);
             a.cmp(b)
         });
-        self.paths.dedup();
+        self.channels_ordered.dedup();
     }
 }
 
 fn channel_path_sorting_key<'a>(
-    path: &'a [ChannelId],
+    id: ChannelId,
     channels_by_id: &'a BTreeMap<ChannelId, Arc<Channel>>,
-) -> impl 'a + Iterator<Item = Option<&'a str>> {
-    path.iter()
-        .map(|id| Some(channels_by_id.get(id)?.name.as_str()))
+) -> impl Iterator<Item = &str> {
+    let (parent_path, name) = channels_by_id
+        .get(&id)
+        .map_or((&[] as &[_], None), |channel| {
+            (channel.parent_path.as_slice(), Some(channel.name.as_str()))
+        });
+    parent_path
+        .iter()
+        .filter_map(|id| Some(channels_by_id.get(id)?.name.as_str()))
+        .chain(name)
 }
 
 fn insert_note_changed(

crates/channel/src/channel_store_tests.rs 🔗

@@ -20,12 +20,14 @@ fn test_update_channels(cx: &mut AppContext) {
                     name: "b".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
                     role: proto::ChannelRole::Admin.into(),
+                    parent_path: Vec::new(),
                 },
                 proto::Channel {
                     id: 2,
                     name: "a".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
                     role: proto::ChannelRole::Member.into(),
+                    parent_path: Vec::new(),
                 },
             ],
             ..Default::default()
@@ -51,22 +53,14 @@ fn test_update_channels(cx: &mut AppContext) {
                     name: "x".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
                     role: proto::ChannelRole::Admin.into(),
+                    parent_path: vec![1],
                 },
                 proto::Channel {
                     id: 4,
                     name: "y".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
                     role: proto::ChannelRole::Member.into(),
-                },
-            ],
-            insert_edge: vec![
-                proto::ChannelEdge {
-                    parent_id: 1,
-                    channel_id: 3,
-                },
-                proto::ChannelEdge {
-                    parent_id: 2,
-                    channel_id: 4,
+                    parent_path: vec![2],
                 },
             ],
             ..Default::default()
@@ -98,28 +92,21 @@ fn test_dangling_channel_paths(cx: &mut AppContext) {
                     name: "a".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
                     role: proto::ChannelRole::Admin.into(),
+                    parent_path: vec![],
                 },
                 proto::Channel {
                     id: 1,
                     name: "b".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
                     role: proto::ChannelRole::Admin.into(),
+                    parent_path: vec![0],
                 },
                 proto::Channel {
                     id: 2,
                     name: "c".to_string(),
                     visibility: proto::ChannelVisibility::Members as i32,
                     role: proto::ChannelRole::Admin.into(),
-                },
-            ],
-            insert_edge: vec![
-                proto::ChannelEdge {
-                    parent_id: 0,
-                    channel_id: 1,
-                },
-                proto::ChannelEdge {
-                    parent_id: 1,
-                    channel_id: 2,
+                    parent_path: vec![0, 1],
                 },
             ],
             ..Default::default()
@@ -170,6 +157,7 @@ async fn test_channel_messages(cx: &mut TestAppContext) {
             name: "the-channel".to_string(),
             visibility: proto::ChannelVisibility::Members as i32,
             role: proto::ChannelRole::Member.into(),
+            parent_path: vec![],
         }],
         ..Default::default()
     });
@@ -197,7 +185,7 @@ async fn test_channel_messages(cx: &mut TestAppContext) {
 
     // Join a channel and populate its existing messages.
     let channel = channel_store.update(cx, |store, cx| {
-        let channel_id = store.channel_dag_entries().next().unwrap().1.id;
+        let channel_id = store.ordered_channels().next().unwrap().1.id;
         store.open_channel_chat(channel_id, cx)
     });
     let join_channel = server.receive::<proto::JoinChannelChat>().await.unwrap();
@@ -384,7 +372,7 @@ fn assert_channels(
 ) {
     let actual = channel_store.read_with(cx, |store, _| {
         store
-            .channel_dag_entries()
+            .ordered_channels()
             .map(|(depth, channel)| (depth, channel.name.to_string(), channel.role))
             .collect::<Vec<_>>()
     });

crates/collab/migrations.sqlite/20221109000000_test_schema.sql 🔗

@@ -193,9 +193,12 @@ CREATE TABLE "channels" (
     "id" INTEGER PRIMARY KEY AUTOINCREMENT,
     "name" VARCHAR NOT NULL,
     "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
-    "visibility" VARCHAR NOT NULL
+    "visibility" VARCHAR NOT NULL,
+    "parent_path" TEXT
 );
 
+CREATE INDEX "index_channels_on_parent_path" ON "channels" ("parent_path");
+
 CREATE TABLE IF NOT EXISTS "channel_chat_participants" (
     "id" INTEGER PRIMARY KEY AUTOINCREMENT,
     "user_id" INTEGER NOT NULL REFERENCES users (id),
@@ -224,12 +227,6 @@ CREATE TABLE "channel_message_mentions" (
     PRIMARY KEY(message_id, start_offset)
 );
 
-CREATE TABLE "channel_paths" (
-    "id_path" TEXT NOT NULL PRIMARY KEY,
-    "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE
-);
-CREATE INDEX "index_channel_paths_on_channel_id" ON "channel_paths" ("channel_id");
-
 CREATE TABLE "channel_members" (
     "id" INTEGER PRIMARY KEY AUTOINCREMENT,
     "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE,

crates/collab/migrations/20231024085546_move_channel_paths_to_channels_table.sql 🔗

@@ -0,0 +1,12 @@
+ALTER TABLE channels ADD COLUMN parent_path TEXT;
+
+UPDATE channels
+SET parent_path = substr(
+    channel_paths.id_path,
+    2,
+    length(channel_paths.id_path) - length('/' || channel_paths.channel_id::text || '/')
+)
+FROM channel_paths
+WHERE channel_paths.channel_id = channels.id;
+
+CREATE INDEX "index_channels_on_parent_path" ON "channels" ("parent_path");

crates/collab/src/db.rs 🔗

@@ -13,7 +13,6 @@ use anyhow::anyhow;
 use collections::{BTreeMap, HashMap, HashSet};
 use dashmap::DashMap;
 use futures::StreamExt;
-use queries::channels::ChannelGraph;
 use rand::{prelude::StdRng, Rng, SeedableRng};
 use rpc::{
     proto::{self},
@@ -492,21 +491,33 @@ pub struct RemoveChannelMemberResult {
     pub notification_id: Option<NotificationId>,
 }
 
-#[derive(FromQueryResult, Debug, PartialEq, Eq, Hash)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 pub struct Channel {
     pub id: ChannelId,
     pub name: String,
     pub visibility: ChannelVisibility,
     pub role: ChannelRole,
+    pub parent_path: Vec<ChannelId>,
 }
 
 impl Channel {
+    fn from_model(value: channel::Model, role: ChannelRole) -> Self {
+        Channel {
+            id: value.id,
+            visibility: value.visibility,
+            name: value.clone().name,
+            role,
+            parent_path: value.ancestors().collect(),
+        }
+    }
+
     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(),
+            parent_path: self.parent_path.iter().map(|c| c.to_proto()).collect(),
         }
     }
 }
@@ -530,7 +541,7 @@ impl ChannelMember {
 
 #[derive(Debug, PartialEq)]
 pub struct ChannelsForUser {
-    pub channels: ChannelGraph,
+    pub channels: Vec<Channel>,
     pub channel_participants: HashMap<ChannelId, Vec<UserId>>,
     pub unseen_buffer_changes: Vec<proto::UnseenChannelBufferChange>,
     pub channel_messages: Vec<proto::UnseenChannelMessage>,

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

@@ -16,7 +16,8 @@ impl Database {
         connection: ConnectionId,
     ) -> Result<proto::JoinChannelBufferResponse> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_participant(channel_id, user_id, &tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_participant(&channel, user_id, &tx)
                 .await?;
 
             let buffer = channel::Model {
@@ -129,9 +130,11 @@ impl Database {
         self.transaction(|tx| async move {
             let mut results = Vec::new();
             for client_buffer in buffers {
-                let channel_id = ChannelId::from_proto(client_buffer.channel_id);
+                let channel = self
+                    .get_channel_internal(ChannelId::from_proto(client_buffer.channel_id), &*tx)
+                    .await?;
                 if self
-                    .check_user_is_channel_participant(channel_id, user_id, &*tx)
+                    .check_user_is_channel_participant(&channel, user_id, &*tx)
                     .await
                     .is_err()
                 {
@@ -139,9 +142,9 @@ impl Database {
                     continue;
                 }
 
-                let buffer = self.get_channel_buffer(channel_id, &*tx).await?;
+                let buffer = self.get_channel_buffer(channel.id, &*tx).await?;
                 let mut collaborators = channel_buffer_collaborator::Entity::find()
-                    .filter(channel_buffer_collaborator::Column::ChannelId.eq(channel_id))
+                    .filter(channel_buffer_collaborator::Column::ChannelId.eq(channel.id))
                     .all(&*tx)
                     .await?;
 
@@ -439,7 +442,8 @@ impl Database {
         Vec<proto::VectorClockEntry>,
     )> {
         self.transaction(move |tx| async move {
-            self.check_user_is_channel_member(channel_id, user, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_member(&channel, user, &*tx)
                 .await?;
 
             let buffer = buffer::Entity::find()
@@ -482,7 +486,7 @@ impl Database {
                 )
                 .await?;
 
-                channel_members = self.get_channel_participants(channel_id, &*tx).await?;
+                channel_members = self.get_channel_participants(&channel, &*tx).await?;
                 let collaborators = self
                     .get_channel_buffer_collaborators_internal(channel_id, &*tx)
                     .await?;

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

@@ -1,5 +1,6 @@
 use super::*;
-use rpc::proto::{channel_member::Kind, ChannelEdge};
+use rpc::proto::channel_member::Kind;
+use sea_orm::TryGetableMany;
 
 impl Database {
     #[cfg(test)]
@@ -42,55 +43,41 @@ impl Database {
     pub async fn create_channel(
         &self,
         name: &str,
-        parent: Option<ChannelId>,
+        parent_channel_id: 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, admin_id, &*tx)
+            let mut parent = None;
+
+            if let Some(parent_channel_id) = parent_channel_id {
+                let parent_channel = self.get_channel_internal(parent_channel_id, &*tx).await?;
+                self.check_user_is_channel_admin(&parent_channel, admin_id, &*tx)
                     .await?;
+                parent = Some(parent_channel);
             }
 
             let channel = channel::ActiveModel {
                 id: ActiveValue::NotSet,
                 name: ActiveValue::Set(name.to_string()),
                 visibility: ActiveValue::Set(ChannelVisibility::Members),
+                parent_path: ActiveValue::Set(
+                    parent
+                        .as_ref()
+                        .map_or(String::new(), |parent| parent.path()),
+                ),
             }
             .insert(&*tx)
             .await?;
 
-            if let Some(parent) = parent {
-                let sql = r#"
-                    INSERT INTO channel_paths
-                    (id_path, channel_id)
-                    SELECT
-                        id_path || $1 || '/', $2
-                    FROM
-                        channel_paths
-                    WHERE
-                        channel_id = $3
-                "#;
-                let channel_paths_stmt = Statement::from_sql_and_values(
-                    self.pool.get_database_backend(),
-                    sql,
-                    [
-                        channel.id.to_proto().into(),
-                        channel.id.to_proto().into(),
-                        parent.to_proto().into(),
-                    ],
-                );
-                tx.execute(channel_paths_stmt).await?;
+            let participants_to_update;
+            if let Some(parent) = &parent {
+                participants_to_update = self
+                    .participants_to_notify_for_channel_change(parent, &*tx)
+                    .await?;
             } else {
-                channel_path::Entity::insert(channel_path::ActiveModel {
-                    channel_id: ActiveValue::Set(channel.id),
-                    id_path: ActiveValue::Set(format!("/{}/", channel.id)),
-                })
-                .exec(&*tx)
-                .await?;
-            }
+                participants_to_update = vec![];
 
-            if parent.is_none() {
                 channel_member::ActiveModel {
                     id: ActiveValue::NotSet,
                     channel_id: ActiveValue::Set(channel.id),
@@ -100,22 +87,10 @@ impl Database {
                 }
                 .insert(&*tx)
                 .await?;
-            }
-
-            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,
-                },
+                channel: Channel::from_model(channel, ChannelRole::Admin),
                 participants_to_update,
             })
         })
@@ -130,20 +105,14 @@ impl Database {
         environment: &str,
     ) -> Result<(JoinRoom, Option<MembershipUpdated>, ChannelRole)> {
         self.transaction(move |tx| async move {
-            let mut accept_invite_result = None;
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            let mut role = self.channel_role_for_user(&channel, user_id, &*tx).await?;
 
-            let channel = channel::Entity::find()
-                .filter(channel::Column::Id.eq(channel_id))
-                .one(&*tx)
-                .await?;
-
-            let mut role = self
-                .channel_role_for_user(channel_id, user_id, &*tx)
-                .await?;
+            let mut accept_invite_result = None;
 
-            if role.is_none() && channel.is_some() {
+            if role.is_none() {
                 if let Some(invitation) = self
-                    .pending_invite_for_channel(channel_id, user_id, &*tx)
+                    .pending_invite_for_channel(&channel, user_id, &*tx)
                     .await?
                 {
                     // note, this may be a parent channel
@@ -156,31 +125,28 @@ impl Database {
                     .await?;
 
                     accept_invite_result = Some(
-                        self.calculate_membership_updated(channel_id, user_id, &*tx)
+                        self.calculate_membership_updated(&channel, user_id, &*tx)
                             .await?,
                     );
 
                     debug_assert!(
-                        self.channel_role_for_user(channel_id, user_id, &*tx)
-                            .await?
-                            == role
+                        self.channel_role_for_user(&channel, user_id, &*tx).await? == role
                     );
                 }
             }
-            if role.is_none()
-                && channel.as_ref().map(|c| c.visibility) == Some(ChannelVisibility::Public)
-            {
+
+            if channel.visibility == ChannelVisibility::Public {
                 role = Some(ChannelRole::Guest);
-                let channel_id_to_join = self
-                    .public_path_to_channel(channel_id, &*tx)
+                let channel_to_join = self
+                    .public_ancestors_including_self(&channel, &*tx)
                     .await?
                     .first()
                     .cloned()
-                    .unwrap_or(channel_id);
+                    .unwrap_or(channel.clone());
 
                 channel_member::Entity::insert(channel_member::ActiveModel {
                     id: ActiveValue::NotSet,
-                    channel_id: ActiveValue::Set(channel_id_to_join),
+                    channel_id: ActiveValue::Set(channel_to_join.id),
                     user_id: ActiveValue::Set(user_id),
                     accepted: ActiveValue::Set(true),
                     role: ActiveValue::Set(ChannelRole::Guest),
@@ -189,19 +155,15 @@ impl Database {
                 .await?;
 
                 accept_invite_result = Some(
-                    self.calculate_membership_updated(channel_id, user_id, &*tx)
+                    self.calculate_membership_updated(&channel_to_join, user_id, &*tx)
                         .await?,
                 );
 
-                debug_assert!(
-                    self.channel_role_for_user(channel_id, user_id, &*tx)
-                        .await?
-                        == role
-                );
+                debug_assert!(self.channel_role_for_user(&channel, user_id, &*tx).await? == role);
             }
 
-            if channel.is_none() || role.is_none() || role == Some(ChannelRole::Banned) {
-                Err(anyhow!("no such channel, or not allowed"))?
+            if role.is_none() || role == Some(ChannelRole::Banned) {
+                Err(anyhow!("not allowed"))?
             }
 
             let live_kit_room = format!("channel-{}", nanoid::nanoid!(30));
@@ -209,7 +171,7 @@ impl Database {
                 .get_or_create_channel_room(channel_id, &live_kit_room, environment, &*tx)
                 .await?;
 
-            self.join_channel_room_internal(channel_id, room_id, user_id, connection, &*tx)
+            self.join_channel_room_internal(room_id, user_id, connection, &*tx)
                 .await
                 .map(|jr| (jr, accept_invite_result, role.unwrap()))
         })
@@ -223,23 +185,21 @@ impl Database {
         admin_id: UserId,
     ) -> Result<SetChannelVisibilityResult> {
         self.transaction(move |tx| async move {
-            self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+
+            self.check_user_is_channel_admin(&channel, admin_id, &*tx)
                 .await?;
 
             let previous_members = self
-                .get_channel_participant_details_internal(channel_id, &*tx)
+                .get_channel_participant_details_internal(&channel, &*tx)
                 .await?;
 
-            channel::ActiveModel {
-                id: ActiveValue::Unchanged(channel_id),
-                visibility: ActiveValue::Set(visibility),
-                ..Default::default()
-            }
-            .update(&*tx)
-            .await?;
+            let mut model = channel.into_active_model();
+            model.visibility = ActiveValue::Set(visibility);
+            let channel = model.update(&*tx).await?;
 
             let mut participants_to_update: HashMap<UserId, ChannelsForUser> = self
-                .participants_to_notify_for_channel_change(channel_id, &*tx)
+                .participants_to_notify_for_channel_change(&channel, &*tx)
                 .await?
                 .into_iter()
                 .collect();
@@ -249,10 +209,10 @@ impl Database {
             match visibility {
                 ChannelVisibility::Members => {
                     let all_descendents: Vec<ChannelId> = self
-                        .get_channel_descendants(vec![channel_id], &*tx)
+                        .get_channel_descendants_including_self(vec![channel_id], &*tx)
                         .await?
                         .into_iter()
-                        .map(|edge| ChannelId::from_proto(edge.channel_id))
+                        .map(|channel| channel.id)
                         .collect();
 
                     channels_to_remove = channel::Entity::find()
@@ -268,6 +228,7 @@ impl Database {
                         .collect();
 
                     channels_to_remove.push(channel_id);
+
                     for member in previous_members {
                         if member.role.can_only_see_public_descendants() {
                             participants_to_remove.insert(member.user_id);
@@ -275,11 +236,9 @@ impl Database {
                     }
                 }
                 ChannelVisibility::Public => {
-                    if let Some(public_parent_id) =
-                        self.public_parent_channel_id(channel_id, &*tx).await?
-                    {
+                    if let Some(public_parent) = self.public_parent_channel(&channel, &*tx).await? {
                         let parent_updates = self
-                            .participants_to_notify_for_channel_change(public_parent_id, &*tx)
+                            .participants_to_notify_for_channel_change(&public_parent, &*tx)
                             .await?;
 
                         for (user_id, channels) in parent_updates {
@@ -304,39 +263,12 @@ impl Database {
         user_id: UserId,
     ) -> Result<(Vec<ChannelId>, Vec<UserId>)> {
         self.transaction(move |tx| async move {
-            self.check_user_is_channel_admin(channel_id, user_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_admin(&channel, user_id, &*tx)
                 .await?;
 
-            // Don't remove descendant channels that have additional parents.
-            let mut channels_to_remove: HashSet<ChannelId> = HashSet::default();
-            channels_to_remove.insert(channel_id);
-
-            let graph = self.get_channel_descendants([channel_id], &*tx).await?;
-            for edge in graph.iter() {
-                channels_to_remove.insert(ChannelId::from_proto(edge.channel_id));
-            }
-
-            {
-                let mut channels_to_keep = channel_path::Entity::find()
-                    .filter(
-                        channel_path::Column::ChannelId
-                            .is_in(channels_to_remove.iter().copied())
-                            .and(
-                                channel_path::Column::IdPath
-                                    .not_like(&format!("%/{}/%", channel_id)),
-                            ),
-                    )
-                    .stream(&*tx)
-                    .await?;
-                while let Some(row) = channels_to_keep.next().await {
-                    let row = row?;
-                    channels_to_remove.remove(&row.channel_id);
-                }
-            }
-
-            let channel_ancestors = self.get_channel_ancestors(channel_id, &*tx).await?;
             let members_to_notify: Vec<UserId> = channel_member::Entity::find()
-                .filter(channel_member::Column::ChannelId.is_in(channel_ancestors))
+                .filter(channel_member::Column::ChannelId.is_in(channel.ancestors_including_self()))
                 .select_only()
                 .column(channel_member::Column::UserId)
                 .distinct()
@@ -344,25 +276,19 @@ impl Database {
                 .all(&*tx)
                 .await?;
 
+            let channels_to_remove = self
+                .get_channel_descendants_including_self(vec![channel.id], &*tx)
+                .await?
+                .into_iter()
+                .map(|channel| channel.id)
+                .collect::<Vec<_>>();
+
             channel::Entity::delete_many()
                 .filter(channel::Column::Id.is_in(channels_to_remove.iter().copied()))
                 .exec(&*tx)
                 .await?;
 
-            // Delete any other paths that include this channel
-            let sql = r#"
-                    DELETE FROM channel_paths
-                    WHERE
-                        id_path LIKE '%' || $1 || '%'
-                "#;
-            let channel_paths_stmt = Statement::from_sql_and_values(
-                self.pool.get_database_backend(),
-                sql,
-                [channel_id.to_proto().into()],
-            );
-            tx.execute(channel_paths_stmt).await?;
-
-            Ok((channels_to_remove.into_iter().collect(), members_to_notify))
+            Ok((channels_to_remove, members_to_notify))
         })
         .await
     }
@@ -375,7 +301,8 @@ impl Database {
         role: ChannelRole,
     ) -> Result<InviteMemberResult> {
         self.transaction(move |tx| async move {
-            self.check_user_is_channel_admin(channel_id, inviter_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_admin(&channel, inviter_id, &*tx)
                 .await?;
 
             channel_member::ActiveModel {
@@ -388,17 +315,7 @@ impl Database {
             .insert(&*tx)
             .await?;
 
-            let channel = channel::Entity::find_by_id(channel_id)
-                .one(&*tx)
-                .await?
-                .unwrap();
-
-            let channel = Channel {
-                id: channel.id,
-                visibility: channel.visibility,
-                name: channel.name,
-                role,
-            };
+            let channel = Channel::from_model(channel, role);
 
             let notifications = self
                 .create_notification(
@@ -440,40 +357,27 @@ impl Database {
         self.transaction(move |tx| async move {
             let new_name = Self::sanitize_channel_name(new_name)?.to_string();
 
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
             let role = self
-                .check_user_is_channel_admin(channel_id, admin_id, &*tx)
+                .check_user_is_channel_admin(&channel, admin_id, &*tx)
                 .await?;
 
-            let channel = channel::ActiveModel {
-                id: ActiveValue::Unchanged(channel_id),
-                name: ActiveValue::Set(new_name.clone()),
-                ..Default::default()
-            }
-            .update(&*tx)
-            .await?;
+            let mut model = channel.into_active_model();
+            model.name = ActiveValue::Set(new_name.clone());
+            let channel = model.update(&*tx).await?;
 
             let participants = self
-                .get_channel_participant_details_internal(channel_id, &*tx)
+                .get_channel_participant_details_internal(&channel, &*tx)
                 .await?;
 
             Ok(RenameChannelResult {
-                channel: Channel {
-                    id: channel.id,
-                    name: channel.name,
-                    visibility: channel.visibility,
-                    role,
-                },
+                channel: Channel::from_model(channel.clone(), 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,
-                            },
+                            Channel::from_model(channel.clone(), participant.role),
                         )
                     })
                     .collect(),
@@ -489,6 +393,8 @@ impl Database {
         accept: bool,
     ) -> Result<RespondToChannelInvite> {
         self.transaction(move |tx| async move {
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+
             let membership_update = if accept {
                 let rows_affected = channel_member::Entity::update_many()
                     .set(channel_member::ActiveModel {
@@ -510,7 +416,7 @@ impl Database {
                 }
 
                 Some(
-                    self.calculate_membership_updated(channel_id, user_id, &*tx)
+                    self.calculate_membership_updated(&channel, user_id, &*tx)
                         .await?,
                 )
             } else {
@@ -554,53 +460,26 @@ impl Database {
 
     async fn calculate_membership_updated(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         user_id: UserId,
         tx: &DatabaseTransaction,
     ) -> Result<MembershipUpdated> {
-        let mut channel_to_refresh = channel_id;
-        let mut removed_channels: Vec<ChannelId> = Vec::new();
-
-        // if the user was previously a guest of a parent public channel they may have seen this
-        // channel (or its descendants) in the tree already.
-        // Now they have new permissions, the graph of channels needs updating from that point.
-        if let Some(public_parent) = self.public_parent_channel_id(channel_id, &*tx).await? {
-            if self
-                .channel_role_for_user(public_parent, user_id, &*tx)
-                .await?
-                == Some(ChannelRole::Guest)
-            {
-                channel_to_refresh = public_parent;
-            }
-        }
-
-        // remove all descendant channels from the user's tree
-        removed_channels.append(
-            &mut self
-                .get_channel_descendants(vec![channel_to_refresh], &*tx)
-                .await?
-                .into_iter()
-                .map(|edge| ChannelId::from_proto(edge.channel_id))
-                .collect(),
-        );
-
-        let new_channels = self
-            .get_user_channels(user_id, Some(channel_to_refresh), &*tx)
-            .await?;
-
-        // We only add the current channel to "moved" if the user has lost access,
-        // otherwise it would be made a root channel on the client.
-        if !new_channels
-            .channels
-            .channels
-            .iter()
-            .any(|c| c.id == channel_id)
-        {
-            removed_channels.push(channel_id);
-        }
+        let new_channels = self.get_user_channels(user_id, Some(channel), &*tx).await?;
+        let removed_channels = self
+            .get_channel_descendants_including_self(vec![channel.id], &*tx)
+            .await?
+            .into_iter()
+            .filter_map(|channel| {
+                if !new_channels.channels.iter().any(|c| c.id == channel.id) {
+                    Some(channel.id)
+                } else {
+                    None
+                }
+            })
+            .collect::<Vec<_>>();
 
         Ok(MembershipUpdated {
-            channel_id,
+            channel_id: channel.id,
             new_channels,
             removed_channels,
         })
@@ -613,7 +492,8 @@ impl Database {
         admin_id: UserId,
     ) -> Result<RemoveChannelMemberResult> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_admin(&channel, admin_id, &*tx)
                 .await?;
 
             let result = channel_member::Entity::delete_many()
@@ -631,7 +511,7 @@ impl Database {
 
             Ok(RemoveChannelMemberResult {
                 membership_update: self
-                    .calculate_membership_updated(channel_id, member_id, &*tx)
+                    .calculate_membership_updated(&channel, member_id, &*tx)
                     .await?,
                 notification_id: self
                     .remove_notification(
@@ -673,11 +553,9 @@ impl Database {
 
             let channels = channels
                 .into_iter()
-                .map(|channel| Channel {
-                    id: channel.id,
-                    name: channel.name,
-                    visibility: channel.visibility,
-                    role: role_for_channel[&channel.id],
+                .filter_map(|channel| {
+                    let role = *role_for_channel.get(&channel.id)?;
+                    Some(Channel::from_model(channel, role))
                 })
                 .collect();
 
@@ -698,15 +576,9 @@ impl Database {
     pub async fn get_user_channels(
         &self,
         user_id: UserId,
-        parent_channel_id: Option<ChannelId>,
+        ancestor_channel: Option<&channel::Model>,
         tx: &DatabaseTransaction,
     ) -> Result<ChannelsForUser> {
-        // note: we could (maybe) win some efficiency here when parent_channel_id
-        // is set by getting just the role for that channel, then getting descendants
-        // with roles attached; but that's not as straightforward as it sounds
-        // because we need to calculate the path to the channel to make the query
-        // efficient, which currently requires an extra round trip to the database.
-        // Fix this later...
         let channel_memberships = channel_member::Entity::find()
             .filter(
                 channel_member::Column::UserId
@@ -716,117 +588,65 @@ impl Database {
             .all(&*tx)
             .await?;
 
-        let mut edges = self
-            .get_channel_descendants(channel_memberships.iter().map(|m| m.channel_id), &*tx)
+        let descendants = self
+            .get_channel_descendants_including_self(
+                channel_memberships.iter().map(|m| m.channel_id),
+                &*tx,
+            )
             .await?;
 
-        let mut role_for_channel: HashMap<ChannelId, (ChannelRole, bool)> = HashMap::default();
-
+        let mut roles_by_channel_id: HashMap<ChannelId, ChannelRole> = HashMap::default();
         for membership in channel_memberships.iter() {
-            let included =
-                parent_channel_id.is_none() || membership.channel_id == parent_channel_id.unwrap();
-            role_for_channel.insert(membership.channel_id, (membership.role, included));
+            roles_by_channel_id.insert(membership.channel_id, membership.role);
         }
 
-        for ChannelEdge {
-            parent_id,
-            channel_id,
-        } in edges.iter()
-        {
-            let parent_id = ChannelId::from_proto(*parent_id);
-            let channel_id = ChannelId::from_proto(*channel_id);
-            debug_assert!(role_for_channel.get(&parent_id).is_some());
-            let (parent_role, parent_included) = role_for_channel[&parent_id];
-
-            if let Some((existing_role, included)) = role_for_channel.get(&channel_id) {
-                role_for_channel.insert(
-                    channel_id,
-                    (existing_role.max(parent_role), *included || parent_included),
-                );
-            } else {
-                role_for_channel.insert(
-                    channel_id,
-                    (
-                        parent_role,
-                        parent_included
-                            || parent_channel_id.is_none()
-                            || Some(channel_id) == parent_channel_id,
-                    ),
-                );
-            }
-        }
-
-        let mut channels: Vec<Channel> = Vec::new();
-        let mut channels_to_remove: HashSet<u64> = HashSet::default();
-
-        let mut rows = channel::Entity::find()
-            .filter(channel::Column::Id.is_in(role_for_channel.keys().copied()))
-            .stream(&*tx)
-            .await?;
+        let mut visible_channel_ids: HashSet<ChannelId> = HashSet::default();
 
-        while let Some(row) = rows.next().await {
-            let channel = row?;
-            let (role, included) = role_for_channel[&channel.id];
-
-            if !included
-                || role == ChannelRole::Banned
-                || role == ChannelRole::Guest && channel.visibility != ChannelVisibility::Public
-            {
-                channels_to_remove.insert(channel.id.0 as u64);
-                continue;
-            }
-
-            channels.push(Channel {
-                id: channel.id,
-                name: channel.name,
-                visibility: channel.visibility,
-                role,
-            });
-        }
-        drop(rows);
-
-        if !channels_to_remove.is_empty() {
-            // Note: this code assumes each channel has one parent.
-            // If there are multiple valid public paths to a channel,
-            // e.g.
-            // If both of these paths are present (* indicating public):
-            // - zed* -> projects -> vim*
-            // - zed* -> conrad -> public-projects* -> vim*
-            // Users would only see one of them (based on edge sort order)
-            let mut replacement_parent: HashMap<u64, u64> = HashMap::default();
-            for ChannelEdge {
-                parent_id,
-                channel_id,
-            } in edges.iter()
-            {
-                if channels_to_remove.contains(channel_id) {
-                    replacement_parent.insert(*channel_id, *parent_id);
+        let channels: Vec<Channel> = descendants
+            .into_iter()
+            .filter_map(|channel| {
+                let parent_role = channel
+                    .parent_id()
+                    .and_then(|parent_id| roles_by_channel_id.get(&parent_id));
+
+                let role = if let Some(parent_role) = parent_role {
+                    let role = if let Some(existing_role) = roles_by_channel_id.get(&channel.id) {
+                        existing_role.max(*parent_role)
+                    } else {
+                        *parent_role
+                    };
+                    roles_by_channel_id.insert(channel.id, role);
+                    role
+                } else {
+                    *roles_by_channel_id.get(&channel.id)?
+                };
+
+                let can_see_parent_paths = role.can_see_all_descendants()
+                    || role.can_only_see_public_descendants()
+                        && channel.visibility == ChannelVisibility::Public;
+                if !can_see_parent_paths {
+                    return None;
                 }
-            }
 
-            let mut new_edges: Vec<ChannelEdge> = Vec::new();
-            'outer: for ChannelEdge {
-                mut parent_id,
-                channel_id,
-            } in edges.iter()
-            {
-                if channels_to_remove.contains(channel_id) {
-                    continue;
-                }
-                while channels_to_remove.contains(&parent_id) {
-                    if let Some(new_parent_id) = replacement_parent.get(&parent_id) {
-                        parent_id = *new_parent_id;
-                    } else {
-                        continue 'outer;
+                visible_channel_ids.insert(channel.id);
+
+                if let Some(ancestor) = ancestor_channel {
+                    if !channel
+                        .ancestors_including_self()
+                        .any(|id| id == ancestor.id)
+                    {
+                        return None;
                     }
                 }
-                new_edges.push(ChannelEdge {
-                    parent_id,
-                    channel_id: *channel_id,
-                })
-            }
-            edges = new_edges;
-        }
+
+                let mut channel = Channel::from_model(channel, role);
+                channel
+                    .parent_path
+                    .retain(|id| visible_channel_ids.contains(&id));
+
+                Some(channel)
+            })
+            .collect();
 
         #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)]
         enum QueryUserIdsAndChannelIds {
@@ -861,7 +681,7 @@ impl Database {
             .await?;
 
         Ok(ChannelsForUser {
-            channels: ChannelGraph { channels, edges },
+            channels,
             channel_participants,
             unseen_buffer_changes: channel_buffer_changes,
             channel_messages: unseen_messages,
@@ -870,7 +690,7 @@ impl Database {
 
     async fn participants_to_notify_for_channel_change(
         &self,
-        new_parent: ChannelId,
+        new_parent: &channel::Model,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<(UserId, ChannelsForUser)>> {
         let mut results: Vec<(UserId, ChannelsForUser)> = Vec::new();
@@ -890,11 +710,10 @@ impl Database {
             ))
         }
 
-        let public_parent = self
-            .public_path_to_channel(new_parent, &*tx)
-            .await?
-            .last()
-            .copied();
+        let public_parents = self
+            .public_ancestors_including_self(new_parent, &*tx)
+            .await?;
+        let public_parent = public_parents.last();
 
         let Some(public_parent) = public_parent else {
             return Ok(results);
@@ -931,7 +750,8 @@ impl Database {
         role: ChannelRole,
     ) -> Result<SetMemberRoleResult> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_admin(&channel, admin_id, &*tx)
                 .await?;
 
             let membership = channel_member::Entity::find()
@@ -951,24 +771,16 @@ impl Database {
             update.role = ActiveValue::Set(role);
             let updated = channel_member::Entity::update(update).exec(&*tx).await?;
 
-            if !updated.accepted {
-                let channel = channel::Entity::find_by_id(channel_id)
-                    .one(&*tx)
-                    .await?
-                    .unwrap();
-
-                return Ok(SetMemberRoleResult::InviteUpdated(Channel {
-                    id: channel.id,
-                    visibility: channel.visibility,
-                    name: channel.name,
-                    role,
-                }));
+            if updated.accepted {
+                Ok(SetMemberRoleResult::MembershipUpdated(
+                    self.calculate_membership_updated(&channel, for_user, &*tx)
+                        .await?,
+                ))
+            } else {
+                Ok(SetMemberRoleResult::InviteUpdated(Channel::from_model(
+                    channel, role,
+                )))
             }
-
-            Ok(SetMemberRoleResult::MembershipUpdated(
-                self.calculate_membership_updated(channel_id, for_user, &*tx)
-                    .await?,
-            ))
         })
         .await
     }
@@ -980,12 +792,13 @@ impl Database {
     ) -> Result<Vec<proto::ChannelMember>> {
         let (role, members) = self
             .transaction(move |tx| async move {
+                let channel = self.get_channel_internal(channel_id, &*tx).await?;
                 let role = self
-                    .check_user_is_channel_participant(channel_id, user_id, &*tx)
+                    .check_user_is_channel_participant(&channel, user_id, &*tx)
                     .await?;
                 Ok((
                     role,
-                    self.get_channel_participant_details_internal(channel_id, &*tx)
+                    self.get_channel_participant_details_internal(&channel, &*tx)
                         .await?,
                 ))
             })
@@ -1016,16 +829,9 @@ impl Database {
 
     async fn get_channel_participant_details_internal(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<ChannelMember>> {
-        let channel_visibility = channel::Entity::find()
-            .filter(channel::Column::Id.eq(channel_id))
-            .one(&*tx)
-            .await?
-            .map(|channel| channel.visibility)
-            .unwrap_or(ChannelVisibility::Members);
-
         #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)]
         enum QueryMemberDetails {
             UserId,
@@ -1035,16 +841,14 @@ impl Database {
             Visibility,
         }
 
-        let tx = tx;
-        let ancestor_ids = self.get_channel_ancestors(channel_id, &*tx).await?;
         let mut stream = channel_member::Entity::find()
             .left_join(channel::Entity)
-            .filter(channel_member::Column::ChannelId.is_in(ancestor_ids.iter().copied()))
+            .filter(channel_member::Column::ChannelId.is_in(channel.ancestors_including_self()))
             .select_only()
             .column(channel_member::Column::UserId)
             .column(channel_member::Column::Role)
             .column_as(
-                channel_member::Column::ChannelId.eq(channel_id),
+                channel_member::Column::ChannelId.eq(channel.id),
                 QueryMemberDetails::IsDirectMember,
             )
             .column(channel_member::Column::Accepted)
@@ -1072,7 +876,7 @@ impl Database {
 
             if channel_role == ChannelRole::Guest
                 && visibility != ChannelVisibility::Public
-                && channel_visibility != ChannelVisibility::Public
+                && channel.visibility != ChannelVisibility::Public
             {
                 continue;
             }
@@ -1108,11 +912,11 @@ impl Database {
 
     pub async fn get_channel_participants(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<UserId>> {
         let participants = self
-            .get_channel_participant_details_internal(channel_id, &*tx)
+            .get_channel_participant_details_internal(channel, &*tx)
             .await?;
         Ok(participants
             .into_iter()
@@ -1122,11 +926,11 @@ impl Database {
 
     pub async fn check_user_is_channel_admin(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         user_id: UserId,
         tx: &DatabaseTransaction,
     ) -> Result<ChannelRole> {
-        let role = self.channel_role_for_user(channel_id, user_id, tx).await?;
+        let role = self.channel_role_for_user(channel, user_id, tx).await?;
         match role {
             Some(ChannelRole::Admin) => Ok(role.unwrap()),
             Some(ChannelRole::Member)
@@ -1140,11 +944,11 @@ impl Database {
 
     pub async fn check_user_is_channel_member(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         user_id: UserId,
         tx: &DatabaseTransaction,
     ) -> Result<ChannelRole> {
-        let channel_role = self.channel_role_for_user(channel_id, user_id, tx).await?;
+        let channel_role = self.channel_role_for_user(channel, user_id, tx).await?;
         match channel_role {
             Some(ChannelRole::Admin) | Some(ChannelRole::Member) => Ok(channel_role.unwrap()),
             Some(ChannelRole::Banned) | Some(ChannelRole::Guest) | None => Err(anyhow!(
@@ -1155,11 +959,11 @@ impl Database {
 
     pub async fn check_user_is_channel_participant(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         user_id: UserId,
         tx: &DatabaseTransaction,
     ) -> Result<ChannelRole> {
-        let role = self.channel_role_for_user(channel_id, user_id, tx).await?;
+        let role = self.channel_role_for_user(channel, user_id, tx).await?;
         match role {
             Some(ChannelRole::Admin) | Some(ChannelRole::Member) | Some(ChannelRole::Guest) => {
                 Ok(role.unwrap())
@@ -1172,14 +976,12 @@ impl Database {
 
     pub async fn pending_invite_for_channel(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         user_id: UserId,
         tx: &DatabaseTransaction,
     ) -> Result<Option<channel_member::Model>> {
-        let channel_ids = self.get_channel_ancestors(channel_id, tx).await?;
-
         let row = channel_member::Entity::find()
-            .filter(channel_member::Column::ChannelId.is_in(channel_ids))
+            .filter(channel_member::Column::ChannelId.is_in(channel.ancestors_including_self()))
             .filter(channel_member::Column::UserId.eq(user_id))
             .filter(channel_member::Column::Accepted.eq(false))
             .one(&*tx)
@@ -1188,88 +990,39 @@ impl Database {
         Ok(row)
     }
 
-    pub async fn parent_channel_id(
+    pub async fn public_parent_channel(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         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 {
-            Ok(None)
+    ) -> Result<Option<channel::Model>> {
+        let mut path = self.public_ancestors_including_self(channel, &*tx).await?;
+        if path.last().unwrap().id == channel.id {
+            path.pop();
         }
+        Ok(path.pop())
     }
 
-    pub async fn public_parent_channel_id(
+    pub async fn public_ancestors_including_self(
         &self,
-        channel_id: ChannelId,
-        tx: &DatabaseTransaction,
-    ) -> Result<Option<ChannelId>> {
-        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 {
-            Ok(path.last().copied())
-        }
-    }
-
-    pub async fn path_to_channel(
-        &self,
-        channel_id: ChannelId,
-        tx: &DatabaseTransaction,
-    ) -> Result<Vec<ChannelId>> {
-        let arbitary_path = channel_path::Entity::find()
-            .filter(channel_path::Column::ChannelId.eq(channel_id))
-            .order_by(channel_path::Column::IdPath, sea_orm::Order::Desc)
-            .one(tx)
-            .await?;
-
-        let Some(path) = arbitary_path else {
-            return Ok(vec![]);
-        };
-
-        Ok(path
-            .id_path
-            .trim_matches('/')
-            .split('/')
-            .map(|id| ChannelId::from_proto(id.parse().unwrap()))
-            .collect())
-    }
-
-    pub async fn public_path_to_channel(
-        &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         tx: &DatabaseTransaction,
-    ) -> Result<Vec<ChannelId>> {
-        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()))
+    ) -> Result<Vec<channel::Model>> {
+        let visible_channels = channel::Entity::find()
+            .filter(channel::Column::Id.is_in(channel.ancestors_including_self()))
             .filter(channel::Column::Visibility.eq(ChannelVisibility::Public))
+            .order_by_asc(channel::Column::ParentPath)
             .all(&*tx)
             .await?;
 
-        let mut visible_channels: HashSet<ChannelId> = HashSet::default();
-
-        for row in rows {
-            visible_channels.insert(row.id);
-        }
-
-        Ok(ancestor_ids
-            .into_iter()
-            .filter(|id| visible_channels.contains(id))
-            .collect())
+        Ok(visible_channels)
     }
 
     pub async fn channel_role_for_user(
         &self,
-        channel_id: ChannelId,
+        channel: &channel::Model,
         user_id: UserId,
         tx: &DatabaseTransaction,
     ) -> Result<Option<ChannelRole>> {
-        let channel_ids = self.get_channel_ancestors(channel_id, tx).await?;
-
         #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)]
         enum QueryChannelMembership {
             ChannelId,

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

@@ -1,5 +1,4 @@
 use super::*;
-use futures::Stream;
 use rpc::Notification;
 use sea_orm::TryInsertResult;
 use time::OffsetDateTime;
@@ -12,7 +11,8 @@ impl Database {
         user_id: UserId,
     ) -> Result<()> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_participant(channel_id, user_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_participant(&channel, user_id, &*tx)
                 .await?;
             channel_chat_participant::ActiveModel {
                 id: ActiveValue::NotSet,
@@ -80,7 +80,8 @@ impl Database {
         before_message_id: Option<MessageId>,
     ) -> Result<Vec<proto::ChannelMessage>> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_participant(channel_id, user_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_participant(&channel, user_id, &*tx)
                 .await?;
 
             let mut condition =
@@ -94,7 +95,7 @@ impl Database {
                 .filter(condition)
                 .order_by_desc(channel_message::Column::Id)
                 .limit(count as u64)
-                .stream(&*tx)
+                .all(&*tx)
                 .await?;
 
             self.load_channel_messages(rows, &*tx).await
@@ -111,27 +112,23 @@ impl Database {
             let rows = channel_message::Entity::find()
                 .filter(channel_message::Column::Id.is_in(message_ids.iter().copied()))
                 .order_by_desc(channel_message::Column::Id)
-                .stream(&*tx)
+                .all(&*tx)
                 .await?;
 
-            let mut channel_ids = HashSet::<ChannelId>::default();
-            let messages = self
-                .load_channel_messages(
-                    rows.map(|row| {
-                        row.map(|row| {
-                            channel_ids.insert(row.channel_id);
-                            row
-                        })
-                    }),
-                    &*tx,
-                )
-                .await?;
+            let mut channels = HashMap::<ChannelId, channel::Model>::default();
+            for row in &rows {
+                channels.insert(
+                    row.channel_id,
+                    self.get_channel_internal(row.channel_id, &*tx).await?,
+                );
+            }
 
-            for channel_id in channel_ids {
-                self.check_user_is_channel_member(channel_id, user_id, &*tx)
+            for (_, channel) in channels {
+                self.check_user_is_channel_participant(&channel, user_id, &*tx)
                     .await?;
             }
 
+            let messages = self.load_channel_messages(rows, &*tx).await?;
             Ok(messages)
         })
         .await
@@ -139,26 +136,26 @@ impl Database {
 
     async fn load_channel_messages(
         &self,
-        mut rows: impl Send + Unpin + Stream<Item = Result<channel_message::Model, sea_orm::DbErr>>,
+        rows: Vec<channel_message::Model>,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<proto::ChannelMessage>> {
-        let mut messages = Vec::new();
-        while let Some(row) = rows.next().await {
-            let row = row?;
-            let nonce = row.nonce.as_u64_pair();
-            messages.push(proto::ChannelMessage {
-                id: row.id.to_proto(),
-                sender_id: row.sender_id.to_proto(),
-                body: row.body,
-                timestamp: row.sent_at.assume_utc().unix_timestamp() as u64,
-                mentions: vec![],
-                nonce: Some(proto::Nonce {
-                    upper_half: nonce.0,
-                    lower_half: nonce.1,
-                }),
-            });
-        }
-        drop(rows);
+        let mut messages = rows
+            .into_iter()
+            .map(|row| {
+                let nonce = row.nonce.as_u64_pair();
+                proto::ChannelMessage {
+                    id: row.id.to_proto(),
+                    sender_id: row.sender_id.to_proto(),
+                    body: row.body,
+                    timestamp: row.sent_at.assume_utc().unix_timestamp() as u64,
+                    mentions: vec![],
+                    nonce: Some(proto::Nonce {
+                        upper_half: nonce.0,
+                        lower_half: nonce.1,
+                    }),
+                }
+            })
+            .collect::<Vec<_>>();
         messages.reverse();
 
         let mut mentions = channel_message_mention::Entity::find()
@@ -203,7 +200,8 @@ impl Database {
         nonce: u128,
     ) -> Result<CreatedChannelMessage> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_participant(channel_id, user_id, &*tx)
+            let channel = self.get_channel_internal(channel_id, &*tx).await?;
+            self.check_user_is_channel_participant(&channel, user_id, &*tx)
                 .await?;
 
             let mut rows = channel_chat_participant::Entity::find()
@@ -310,7 +308,7 @@ impl Database {
                 }
             }
 
-            let mut channel_members = self.get_channel_participants(channel_id, &*tx).await?;
+            let mut channel_members = self.get_channel_participants(&channel, &*tx).await?;
             channel_members.retain(|member| !participant_user_ids.contains(member));
 
             Ok(CreatedChannelMessage {
@@ -483,8 +481,9 @@ impl Database {
                 .await?;
 
             if result.rows_affected == 0 {
+                let channel = self.get_channel_internal(channel_id, &*tx).await?;
                 if self
-                    .check_user_is_channel_admin(channel_id, user_id, &*tx)
+                    .check_user_is_channel_admin(&channel, user_id, &*tx)
                     .await
                     .is_ok()
                 {

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

@@ -50,10 +50,10 @@ impl Database {
                     .map(|participant| participant.user_id),
             );
 
-            let (channel_id, room) = self.get_channel_room(room_id, &tx).await?;
+            let (channel, 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(channel_id, &tx).await?;
+            if let Some(channel) = &channel {
+                channel_members = self.get_channel_participants(channel, &tx).await?;
             } else {
                 channel_members = Vec::new();
 
@@ -69,7 +69,7 @@ impl Database {
 
             Ok(RefreshedRoom {
                 room,
-                channel_id,
+                channel_id: channel.map(|channel| channel.id),
                 channel_members,
                 stale_participant_user_ids,
                 canceled_calls_to_user_ids,
@@ -381,7 +381,6 @@ impl Database {
 
     pub(crate) async fn join_channel_room_internal(
         &self,
-        channel_id: ChannelId,
         room_id: RoomId,
         user_id: UserId,
         connection: ConnectionId,
@@ -420,11 +419,12 @@ impl Database {
         .exec(&*tx)
         .await?;
 
-        let room = self.get_room(room_id, &tx).await?;
-        let channel_members = self.get_channel_participants(channel_id, &tx).await?;
+        let (channel, room) = self.get_channel_room(room_id, &tx).await?;
+        let channel = channel.ok_or_else(|| anyhow!("no channel for room"))?;
+        let channel_members = self.get_channel_participants(&channel, &*tx).await?;
         Ok(JoinRoom {
             room,
-            channel_id: Some(channel_id),
+            channel_id: Some(channel.id),
             channel_members,
         })
     }
@@ -718,16 +718,16 @@ 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(channel_id, &tx).await?
+            let (channel, room) = self.get_channel_room(room_id, &tx).await?;
+            let channel_members = if let Some(channel) = &channel {
+                self.get_channel_participants(&channel, &tx).await?
             } else {
                 Vec::new()
             };
 
             Ok(RejoinedRoom {
                 room,
-                channel_id,
+                channel_id: channel.map(|channel| channel.id),
                 channel_members,
                 rejoined_projects,
                 reshared_projects,
@@ -869,7 +869,7 @@ impl Database {
                     .exec(&*tx)
                     .await?;
 
-                let (channel_id, room) = self.get_channel_room(room_id, &tx).await?;
+                let (channel, room) = self.get_channel_room(room_id, &tx).await?;
                 let deleted = if room.participants.is_empty() {
                     let result = room::Entity::delete_by_id(room_id).exec(&*tx).await?;
                     result.rows_affected > 0
@@ -877,14 +877,14 @@ impl Database {
                     false
                 };
 
-                let channel_members = if let Some(channel_id) = channel_id {
-                    self.get_channel_participants(channel_id, &tx).await?
+                let channel_members = if let Some(channel) = &channel {
+                    self.get_channel_participants(channel, &tx).await?
                 } else {
                     Vec::new()
                 };
                 let left_room = LeftRoom {
                     room,
-                    channel_id,
+                    channel_id: channel.map(|channel| channel.id),
                     channel_members,
                     left_projects,
                     canceled_calls_to_user_ids,
@@ -1072,7 +1072,7 @@ impl Database {
         &self,
         room_id: RoomId,
         tx: &DatabaseTransaction,
-    ) -> Result<(Option<ChannelId>, proto::Room)> {
+    ) -> Result<(Option<channel::Model>, proto::Room)> {
         let db_room = room::Entity::find_by_id(room_id)
             .one(tx)
             .await?
@@ -1181,9 +1181,16 @@ impl Database {
                 project_id: db_follower.project_id.to_proto(),
             });
         }
+        drop(db_followers);
+
+        let channel = if let Some(channel_id) = db_room.channel_id {
+            Some(self.get_channel_internal(channel_id, &*tx).await?)
+        } else {
+            None
+        };
 
         Ok((
-            db_room.channel_id,
+            channel,
             proto::Room {
                 id: db_room.id.to_proto(),
                 live_kit_room: db_room.live_kit_room,

crates/collab/src/db/tables.rs 🔗

@@ -8,7 +8,6 @@ pub mod channel_chat_participant;
 pub mod channel_member;
 pub mod channel_message;
 pub mod channel_message_mention;
-pub mod channel_path;
 pub mod contact;
 pub mod feature_flag;
 pub mod follower;

crates/collab/src/db/tables/channel.rs 🔗

@@ -8,6 +8,28 @@ pub struct Model {
     pub id: ChannelId,
     pub name: String,
     pub visibility: ChannelVisibility,
+    pub parent_path: String,
+}
+
+impl Model {
+    pub fn parent_id(&self) -> Option<ChannelId> {
+        self.ancestors().last()
+    }
+
+    pub fn ancestors(&self) -> impl Iterator<Item = ChannelId> + '_ {
+        self.parent_path
+            .trim_end_matches('/')
+            .split('/')
+            .filter_map(|id| Some(ChannelId::from_proto(id.parse().ok()?)))
+    }
+
+    pub fn ancestors_including_self(&self) -> impl Iterator<Item = ChannelId> + '_ {
+        self.ancestors().chain(Some(self.id))
+    }
+
+    pub fn path(&self) -> String {
+        format!("{}{}/", self.parent_path, self.id)
+    }
 }
 
 impl ActiveModelBehavior for ActiveModel {}

crates/collab/src/db/tables/channel_path.rs 🔗

@@ -1,15 +0,0 @@
-use crate::db::ChannelId;
-use sea_orm::entity::prelude::*;
-
-#[derive(Clone, Debug, Default, PartialEq, Eq, DeriveEntityModel)]
-#[sea_orm(table_name = "channel_paths")]
-pub struct Model {
-    #[sea_orm(primary_key)]
-    pub id_path: String,
-    pub channel_id: ChannelId,
-}
-
-impl ActiveModelBehavior for ActiveModel {}
-
-#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)]
-pub enum Relation {}

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

@@ -7,7 +7,6 @@ mod message_tests;
 use super::*;
 use gpui::executor::Background;
 use parking_lot::Mutex;
-use rpc::proto::ChannelEdge;
 use sea_orm::ConnectionTrait;
 use sqlx::migrate::MigrateDatabase;
 use std::sync::{
@@ -153,33 +152,17 @@ impl Drop for TestDb {
     }
 }
 
-/// The second tuples are (channel_id, parent)
-fn graph(
-    channels: &[(ChannelId, &'static str, ChannelRole)],
-    edges: &[(ChannelId, ChannelId)],
-) -> ChannelGraph {
-    let mut graph = ChannelGraph {
-        channels: vec![],
-        edges: vec![],
-    };
-
-    for (id, name, role) in channels {
-        graph.channels.push(Channel {
+fn channel_tree(channels: &[(ChannelId, &[ChannelId], &'static str, ChannelRole)]) -> Vec<Channel> {
+    channels
+        .iter()
+        .map(|(id, parent_path, name, role)| Channel {
             id: *id,
             name: name.to_string(),
             visibility: ChannelVisibility::Members,
             role: *role,
+            parent_path: parent_path.to_vec(),
         })
-    }
-
-    for (channel, parent) in edges {
-        graph.edges.push(ChannelEdge {
-            channel_id: channel.to_proto(),
-            parent_id: parent.to_proto(),
-        })
-    }
-
-    graph
+        .collect()
 }
 
 static GITHUB_USER_ID: AtomicI32 = AtomicI32::new(5);

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

@@ -1,18 +1,15 @@
-use std::sync::Arc;
-
 use crate::{
     db::{
-        queries::channels::ChannelGraph,
-        tests::{graph, new_test_connection, new_test_user, TEST_RELEASE_CHANNEL},
-        ChannelId, ChannelRole, Database, NewUserParams, RoomId,
+        tests::{channel_tree, new_test_connection, new_test_user, TEST_RELEASE_CHANNEL},
+        Channel, ChannelId, ChannelRole, Database, NewUserParams, RoomId,
     },
     test_both_dbs,
 };
-use collections::{HashMap, HashSet};
 use rpc::{
     proto::{self},
     ConnectionId,
 };
+use std::sync::Arc;
 
 test_both_dbs!(test_channels, test_channels_postgres, test_channels_sqlite);
 
@@ -44,7 +41,10 @@ async fn test_channels(db: &Arc<Database>) {
         .unwrap();
 
     let mut members = db
-        .transaction(|tx| async move { Ok(db.get_channel_participants(replace_id, &*tx).await?) })
+        .transaction(|tx| async move {
+            let channel = db.get_channel_internal(replace_id, &*tx).await?;
+            Ok(db.get_channel_participants(&channel, &*tx).await?)
+        })
         .await
         .unwrap();
     members.sort();
@@ -61,42 +61,41 @@ async fn test_channels(db: &Arc<Database>) {
     let result = db.get_channels_for_user(a_id).await.unwrap();
     assert_eq!(
         result.channels,
-        graph(
-            &[
-                (zed_id, "zed", ChannelRole::Admin),
-                (crdb_id, "crdb", ChannelRole::Admin),
-                (livestreaming_id, "livestreaming", ChannelRole::Admin),
-                (replace_id, "replace", ChannelRole::Admin),
-                (rust_id, "rust", ChannelRole::Admin),
-                (cargo_id, "cargo", ChannelRole::Admin),
-                (cargo_ra_id, "cargo-ra", ChannelRole::Admin)
-            ],
-            &[
-                (crdb_id, zed_id),
-                (livestreaming_id, zed_id),
-                (replace_id, zed_id),
-                (cargo_id, rust_id),
-                (cargo_ra_id, cargo_id),
-            ]
-        )
+        channel_tree(&[
+            (zed_id, &[], "zed", ChannelRole::Admin),
+            (crdb_id, &[zed_id], "crdb", ChannelRole::Admin),
+            (
+                livestreaming_id,
+                &[zed_id],
+                "livestreaming",
+                ChannelRole::Admin
+            ),
+            (replace_id, &[zed_id], "replace", ChannelRole::Admin),
+            (rust_id, &[], "rust", ChannelRole::Admin),
+            (cargo_id, &[rust_id], "cargo", ChannelRole::Admin),
+            (
+                cargo_ra_id,
+                &[rust_id, cargo_id],
+                "cargo-ra",
+                ChannelRole::Admin
+            )
+        ],)
     );
 
     let result = db.get_channels_for_user(b_id).await.unwrap();
     assert_eq!(
         result.channels,
-        graph(
-            &[
-                (zed_id, "zed", ChannelRole::Member),
-                (crdb_id, "crdb", ChannelRole::Member),
-                (livestreaming_id, "livestreaming", ChannelRole::Member),
-                (replace_id, "replace", ChannelRole::Member)
-            ],
-            &[
-                (crdb_id, zed_id),
-                (livestreaming_id, zed_id),
-                (replace_id, zed_id)
-            ]
-        )
+        channel_tree(&[
+            (zed_id, &[], "zed", ChannelRole::Member),
+            (crdb_id, &[zed_id], "crdb", ChannelRole::Member),
+            (
+                livestreaming_id,
+                &[zed_id],
+                "livestreaming",
+                ChannelRole::Member
+            ),
+            (replace_id, &[zed_id], "replace", ChannelRole::Member)
+        ],)
     );
 
     // Update member permissions
@@ -112,19 +111,17 @@ async fn test_channels(db: &Arc<Database>) {
     let result = db.get_channels_for_user(b_id).await.unwrap();
     assert_eq!(
         result.channels,
-        graph(
-            &[
-                (zed_id, "zed", ChannelRole::Admin),
-                (crdb_id, "crdb", ChannelRole::Admin),
-                (livestreaming_id, "livestreaming", ChannelRole::Admin),
-                (replace_id, "replace", ChannelRole::Admin)
-            ],
-            &[
-                (crdb_id, zed_id),
-                (livestreaming_id, zed_id),
-                (replace_id, zed_id)
-            ]
-        )
+        channel_tree(&[
+            (zed_id, &[], "zed", ChannelRole::Admin),
+            (crdb_id, &[zed_id], "crdb", ChannelRole::Admin),
+            (
+                livestreaming_id,
+                &[zed_id],
+                "livestreaming",
+                ChannelRole::Admin
+            ),
+            (replace_id, &[zed_id], "replace", ChannelRole::Admin)
+        ],)
     );
 
     // Remove a single channel
@@ -327,14 +324,10 @@ async fn test_channel_renames(db: &Arc<Database>) {
         .await
         .unwrap();
 
-    let zed_archive_id = zed_id;
-
-    let channel = db.get_channel(zed_archive_id, user_1).await.unwrap();
+    let channel = db.get_channel(zed_id, user_1).await.unwrap();
     assert_eq!(channel.name, "zed-archive");
 
-    let non_permissioned_rename = db
-        .rename_channel(zed_archive_id, user_2, "hacked-lol")
-        .await;
+    let non_permissioned_rename = db.rename_channel(zed_id, user_2, "hacked-lol").await;
     assert!(non_permissioned_rename.is_err());
 
     let bad_name_rename = db.rename_channel(zed_id, user_1, "#").await;
@@ -383,328 +376,16 @@ async fn test_db_channel_moving(db: &Arc<Database>) {
     //     /- gpui2
     // zed -- crdb - livestreaming - livestreaming_dag
     let result = db.get_channels_for_user(a_id).await.unwrap();
-    assert_dag(
+    assert_channel_tree(
         result.channels,
         &[
-            (zed_id, None),
-            (crdb_id, Some(zed_id)),
-            (gpui2_id, Some(zed_id)),
-            (livestreaming_id, Some(crdb_id)),
-            (livestreaming_dag_id, Some(livestreaming_id)),
+            (zed_id, &[]),
+            (crdb_id, &[zed_id]),
+            (livestreaming_id, &[zed_id, crdb_id]),
+            (livestreaming_dag_id, &[zed_id, crdb_id, livestreaming_id]),
+            (gpui2_id, &[zed_id]),
         ],
     );
-
-    // Attempt to make a cycle
-    assert!(db
-        .link_channel(a_id, zed_id, livestreaming_id)
-        .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(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!(
@@ -743,37 +424,32 @@ async fn test_db_channel_moving_bugs(db: &Arc<Database>) {
 
     // Move to same parent should be a no-op
     assert!(db
-        .move_channel(projects_id, Some(zed_id), zed_id, user_id)
+        .move_channel(projects_id, Some(zed_id), user_id)
         .await
         .unwrap()
         .is_none());
 
     let result = db.get_channels_for_user(user_id).await.unwrap();
-    assert_dag(
+    assert_channel_tree(
         result.channels,
         &[
-            (zed_id, None),
-            (projects_id, Some(zed_id)),
-            (livestreaming_id, Some(projects_id)),
+            (zed_id, &[]),
+            (projects_id, &[zed_id]),
+            (livestreaming_id, &[zed_id, 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)),
-    //     ],
-    // );
+    // Move the project channel to the root
+    db.move_channel(projects_id, None, user_id).await.unwrap();
+    let result = db.get_channels_for_user(user_id).await.unwrap();
+    assert_channel_tree(
+        result.channels,
+        &[
+            (zed_id, &[]),
+            (projects_id, &[]),
+            (livestreaming_id, &[projects_id]),
+        ],
+    );
 }
 
 test_both_dbs!(
@@ -788,44 +464,52 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
     let guest = new_test_user(db, "guest@example.com").await;
 
     let zed_channel = db.create_root_channel("zed", admin).await.unwrap();
-    let active_channel = db
+    let active_channel_id = db
         .create_sub_channel("active", zed_channel, admin)
         .await
         .unwrap();
-    let vim_channel = db
-        .create_sub_channel("vim", active_channel, admin)
+    let vim_channel_id = db
+        .create_sub_channel("vim", active_channel_id, admin)
         .await
         .unwrap();
 
-    db.set_channel_visibility(vim_channel, crate::db::ChannelVisibility::Public, admin)
+    db.set_channel_visibility(vim_channel_id, crate::db::ChannelVisibility::Public, admin)
         .await
         .unwrap();
-    db.invite_channel_member(active_channel, member, admin, ChannelRole::Member)
+    db.invite_channel_member(active_channel_id, member, admin, ChannelRole::Member)
         .await
         .unwrap();
-    db.invite_channel_member(vim_channel, guest, admin, ChannelRole::Guest)
+    db.invite_channel_member(vim_channel_id, guest, admin, ChannelRole::Guest)
         .await
         .unwrap();
 
-    db.respond_to_channel_invite(active_channel, member, true)
+    db.respond_to_channel_invite(active_channel_id, member, true)
         .await
         .unwrap();
 
     db.transaction(|tx| async move {
-        db.check_user_is_channel_participant(vim_channel, admin, &*tx)
-            .await
+        db.check_user_is_channel_participant(
+            &db.get_channel_internal(vim_channel_id, &*tx).await?,
+            admin,
+            &*tx,
+        )
+        .await
     })
     .await
     .unwrap();
     db.transaction(|tx| async move {
-        db.check_user_is_channel_participant(vim_channel, member, &*tx)
-            .await
+        db.check_user_is_channel_participant(
+            &db.get_channel_internal(vim_channel_id, &*tx).await?,
+            member,
+            &*tx,
+        )
+        .await
     })
     .await
     .unwrap();
 
     let mut members = db
-        .get_channel_participant_details(vim_channel, admin)
+        .get_channel_participant_details(vim_channel_id, admin)
         .await
         .unwrap();
 
@@ -852,38 +536,49 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
         ]
     );
 
-    db.respond_to_channel_invite(vim_channel, guest, true)
+    db.respond_to_channel_invite(vim_channel_id, guest, true)
         .await
         .unwrap();
 
     db.transaction(|tx| async move {
-        db.check_user_is_channel_participant(vim_channel, guest, &*tx)
-            .await
+        db.check_user_is_channel_participant(
+            &db.get_channel_internal(vim_channel_id, &*tx).await?,
+            guest,
+            &*tx,
+        )
+        .await
     })
     .await
     .unwrap();
 
     let channels = db.get_channels_for_user(guest).await.unwrap().channels;
-    assert_dag(channels, &[(vim_channel, None)]);
+    assert_channel_tree(channels, &[(vim_channel_id, &[])]);
     let channels = db.get_channels_for_user(member).await.unwrap().channels;
-    assert_dag(
+    assert_channel_tree(
         channels,
-        &[(active_channel, None), (vim_channel, Some(active_channel))],
+        &[
+            (active_channel_id, &[]),
+            (vim_channel_id, &[active_channel_id]),
+        ],
     );
 
-    db.set_channel_member_role(vim_channel, admin, guest, ChannelRole::Banned)
+    db.set_channel_member_role(vim_channel_id, admin, guest, ChannelRole::Banned)
         .await
         .unwrap();
     assert!(db
         .transaction(|tx| async move {
-            db.check_user_is_channel_participant(vim_channel, guest, &*tx)
-                .await
+            db.check_user_is_channel_participant(
+                &db.get_channel_internal(vim_channel_id, &*tx).await.unwrap(),
+                guest,
+                &*tx,
+            )
+            .await
         })
         .await
         .is_err());
 
     let mut members = db
-        .get_channel_participant_details(vim_channel, admin)
+        .get_channel_participant_details(vim_channel_id, admin)
         .await
         .unwrap();
 
@@ -910,7 +605,7 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
         ]
     );
 
-    db.remove_channel_member(vim_channel, guest, admin)
+    db.remove_channel_member(vim_channel_id, guest, admin)
         .await
         .unwrap();
 
@@ -924,7 +619,7 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
 
     // currently people invited to parent channels are not shown here
     let mut members = db
-        .get_channel_participant_details(vim_channel, admin)
+        .get_channel_participant_details(vim_channel_id, admin)
         .await
         .unwrap();
 
@@ -951,28 +646,42 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
         .unwrap();
 
     db.transaction(|tx| async move {
-        db.check_user_is_channel_participant(zed_channel, guest, &*tx)
-            .await
+        db.check_user_is_channel_participant(
+            &db.get_channel_internal(zed_channel, &*tx).await.unwrap(),
+            guest,
+            &*tx,
+        )
+        .await
     })
     .await
     .unwrap();
     assert!(db
         .transaction(|tx| async move {
-            db.check_user_is_channel_participant(active_channel, guest, &*tx)
-                .await
+            db.check_user_is_channel_participant(
+                &db.get_channel_internal(active_channel_id, &*tx)
+                    .await
+                    .unwrap(),
+                guest,
+                &*tx,
+            )
+            .await
         })
         .await
         .is_err(),);
 
     db.transaction(|tx| async move {
-        db.check_user_is_channel_participant(vim_channel, guest, &*tx)
-            .await
+        db.check_user_is_channel_participant(
+            &db.get_channel_internal(vim_channel_id, &*tx).await.unwrap(),
+            guest,
+            &*tx,
+        )
+        .await
     })
     .await
     .unwrap();
 
     let mut members = db
-        .get_channel_participant_details(vim_channel, admin)
+        .get_channel_participant_details(vim_channel_id, admin)
         .await
         .unwrap();
 
@@ -1000,9 +709,9 @@ async fn test_user_is_channel_participant(db: &Arc<Database>) {
     );
 
     let channels = db.get_channels_for_user(guest).await.unwrap().channels;
-    assert_dag(
+    assert_channel_tree(
         channels,
-        &[(zed_channel, None), (vim_channel, Some(zed_channel))],
+        &[(zed_channel, &[]), (vim_channel_id, &[zed_channel])],
     )
 }
 
@@ -1047,15 +756,20 @@ async fn test_user_joins_correct_channel(db: &Arc<Database>) {
     let most_public = db
         .transaction(|tx| async move {
             Ok(db
-                .public_path_to_channel(vim_channel, &tx)
+                .public_ancestors_including_self(
+                    &db.get_channel_internal(vim_channel, &*tx).await.unwrap(),
+                    &tx,
+                )
                 .await?
                 .first()
                 .cloned())
         })
         .await
-        .unwrap();
+        .unwrap()
+        .unwrap()
+        .id;
 
-    assert_eq!(most_public, Some(zed_channel))
+    assert_eq!(most_public, zed_channel)
 }
 
 test_both_dbs!(
@@ -1092,26 +806,14 @@ async fn test_guest_access(db: &Arc<Database>) {
 }
 
 #[track_caller]
-fn assert_dag(actual: ChannelGraph, expected: &[(ChannelId, Option<ChannelId>)]) {
-    let mut actual_map: HashMap<ChannelId, HashSet<ChannelId>> = HashMap::default();
-    for channel in actual.channels {
-        actual_map.insert(channel.id, HashSet::default());
-    }
-    for edge in actual.edges {
-        actual_map
-            .get_mut(&ChannelId::from_proto(edge.channel_id))
-            .unwrap()
-            .insert(ChannelId::from_proto(edge.parent_id));
-    }
-
-    let mut expected_map: HashMap<ChannelId, HashSet<ChannelId>> = HashMap::default();
-
-    for (child, parent) in expected {
-        let entry = expected_map.entry(*child).or_default();
-        if let Some(parent) = parent {
-            entry.insert(*parent);
-        }
-    }
-
-    pretty_assertions::assert_eq!(actual_map, expected_map)
+fn assert_channel_tree(actual: Vec<Channel>, expected: &[(ChannelId, &[ChannelId])]) {
+    let actual = actual
+        .iter()
+        .map(|channel| (channel.id, channel.parent_path.as_slice()))
+        .collect::<Vec<_>>();
+    pretty_assertions::assert_eq!(
+        actual,
+        expected.to_vec(),
+        "wrong channel ids and parent paths"
+    );
 }

crates/collab/src/rpc.rs 🔗

@@ -277,8 +277,6 @@ impl Server {
             .add_request_handler(get_channel_messages_by_id)
             .add_request_handler(get_notifications)
             .add_request_handler(mark_notification_as_read)
-            .add_request_handler(link_channel)
-            .add_request_handler(unlink_channel)
             .add_request_handler(move_channel)
             .add_request_handler(follow)
             .add_message_handler(unfollow)
@@ -2472,52 +2470,19 @@ async fn rename_channel(
     Ok(())
 }
 
-// TODO: Implement in terms of symlinks
-// Current behavior of this is more like 'Move root channel'
-async fn link_channel(
-    request: proto::LinkChannel,
-    response: Response<proto::LinkChannel>,
-    session: Session,
-) -> Result<()> {
-    let db = session.db().await;
-    let channel_id = ChannelId::from_proto(request.channel_id);
-    let to = ChannelId::from_proto(request.to);
-
-    let result = db
-        .move_channel(channel_id, None, to, session.user_id)
-        .await?;
-    drop(db);
-
-    notify_channel_moved(result, session).await?;
-
-    response.send(Ack {})?;
-
-    Ok(())
-}
-
-// TODO: Implement in terms of symlinks
-async fn unlink_channel(
-    _request: proto::UnlinkChannel,
-    _response: Response<proto::UnlinkChannel>,
-    _session: Session,
-) -> Result<()> {
-    Err(anyhow!("unimplemented").into())
-}
-
 async fn move_channel(
     request: proto::MoveChannel,
     response: Response<proto::MoveChannel>,
     session: Session,
 ) -> Result<()> {
-    let db = session.db().await;
     let channel_id = ChannelId::from_proto(request.channel_id);
-    let from_parent = ChannelId::from_proto(request.from);
-    let to = ChannelId::from_proto(request.to);
+    let to = request.to.map(ChannelId::from_proto);
 
-    let result = db
-        .move_channel(channel_id, Some(from_parent), to, session.user_id)
+    let result = session
+        .db()
+        .await
+        .move_channel(channel_id, to, session.user_id)
         .await?;
-    drop(db);
 
     notify_channel_moved(result, session).await?;
 
@@ -3244,18 +3209,12 @@ fn build_channels_update(
 ) -> proto::UpdateChannels {
     let mut update = proto::UpdateChannels::default();
 
-    for channel in channels.channels.channels {
-        update.channels.push(proto::Channel {
-            id: channel.id.to_proto(),
-            name: channel.name,
-            visibility: channel.visibility.into(),
-            role: channel.role.into(),
-        });
+    for channel in channels.channels {
+        update.channels.push(channel.to_proto());
     }
 
     update.unseen_channel_buffer_changes = channels.unseen_buffer_changes;
     update.unseen_channel_messages = channels.channel_messages;
-    update.insert_edge = channels.channels.edges;
 
     for (channel_id, participants) in channels.channel_participants {
         update
@@ -3267,12 +3226,7 @@ fn build_channels_update(
     }
 
     for channel in channel_invites {
-        update.channel_invitations.push(proto::Channel {
-            id: channel.id.to_proto(),
-            name: channel.name,
-            visibility: channel.visibility.into(),
-            role: channel.role.into(),
-        });
+        update.channel_invitations.push(channel.to_proto());
     }
 
     update

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

@@ -3,7 +3,7 @@ use crate::{
     tests::TestServer,
 };
 use call::ActiveCall;
-use channel::{Channel, ACKNOWLEDGE_DEBOUNCE_INTERVAL};
+use channel::ACKNOWLEDGE_DEBOUNCE_INTERVAL;
 use client::ParticipantIndex;
 use client::{Collaborator, UserId};
 use collab_ui::channel_view::ChannelView;
@@ -11,10 +11,7 @@ use collections::HashMap;
 use editor::{Anchor, Editor, ToOffset};
 use futures::future;
 use gpui::{executor::Deterministic, ModelHandle, TestAppContext, ViewContext};
-use rpc::{
-    proto::{self, PeerId},
-    RECEIVE_TIMEOUT,
-};
+use rpc::{proto::PeerId, RECEIVE_TIMEOUT};
 use serde_json::json;
 use std::{ops::Range, sync::Arc};
 
@@ -411,10 +408,7 @@ async fn test_channel_buffer_disconnect(
     deterministic.advance_clock(RECEIVE_TIMEOUT + RECONNECT_TIMEOUT);
 
     channel_buffer_a.update(cx_a, |buffer, cx| {
-        assert_eq!(
-            buffer.channel(cx).unwrap().as_ref(),
-            &channel(channel_id, "the-channel", proto::ChannelRole::Admin)
-        );
+        assert_eq!(buffer.channel(cx).unwrap().name, "the-channel");
         assert!(!buffer.is_connected());
     });
 
@@ -441,17 +435,6 @@ async fn test_channel_buffer_disconnect(
     });
 }
 
-fn channel(id: u64, name: &'static str, role: proto::ChannelRole) -> Channel {
-    Channel {
-        id,
-        role,
-        name: name.to_string(),
-        visibility: proto::ChannelVisibility::Members,
-        unseen_note_version: None,
-        unseen_message_id: None,
-    }
-}
-
 #[gpui::test]
 async fn test_rejoin_channel_buffer(
     deterministic: Arc<Deterministic>,

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

@@ -61,10 +61,7 @@ async fn test_core_channels(
     );
 
     client_b.channel_store().read_with(cx_b, |channels, _| {
-        assert!(channels
-            .channel_dag_entries()
-            .collect::<Vec<_>>()
-            .is_empty())
+        assert!(channels.ordered_channels().collect::<Vec<_>>().is_empty())
     });
 
     // Invite client B to channel A as client A.
@@ -1019,7 +1016,7 @@ async fn test_channel_link_notifications(
     client_a
         .channel_store()
         .update(cx_a, |channel_store, cx| {
-            channel_store.link_channel(vim_channel, active_channel, cx)
+            channel_store.move_channel(vim_channel, Some(active_channel), cx)
         })
         .await
         .unwrap();
@@ -1054,7 +1051,7 @@ async fn test_channel_link_notifications(
     client_a
         .channel_store()
         .update(cx_a, |channel_store, cx| {
-            channel_store.link_channel(helix_channel, vim_channel, cx)
+            channel_store.move_channel(helix_channel, Some(vim_channel), cx)
         })
         .await
         .unwrap();
@@ -1427,7 +1424,7 @@ async fn test_channel_moving(
     client_a
         .channel_store()
         .update(cx_a, |channel_store, cx| {
-            channel_store.move_channel(channel_d_id, channel_c_id, channel_b_id, cx)
+            channel_store.move_channel(channel_d_id, Some(channel_b_id), cx)
         })
         .await
         .unwrap();
@@ -1445,189 +1442,6 @@ async fn test_channel_moving(
             (channel_d_id, 2),
         ],
     );
-
-    // TODO: Restore this test once we have a way to make channel symlinks
-    // client_a
-    //     .channel_store()
-    //     .update(cx_a, |channel_store, cx| {
-    //         channel_store.link_channel(channel_d_id, channel_c_id, cx)
-    //     })
-    //     .await
-    //     .unwrap();
-
-    // // Current shape for A:
-    // //      /------\
-    // // a - b -- c -- d
-    // assert_channels_list_shape(
-    //     client_a.channel_store(),
-    //     cx_a,
-    //     &[
-    //         (channel_a_id, 0),
-    //         (channel_b_id, 1),
-    //         (channel_c_id, 2),
-    //         (channel_d_id, 3),
-    //         (channel_d_id, 2),
-    //     ],
-    // );
-    //
-    // let b_channels = server
-    //     .make_channel_tree(
-    //         &[
-    //             ("channel-mu", None),
-    //             ("channel-gamma", Some("channel-mu")),
-    //             ("channel-epsilon", Some("channel-mu")),
-    //         ],
-    //         (&client_b, cx_b),
-    //     )
-    //     .await;
-    // let channel_mu_id = b_channels[0];
-    // let channel_ga_id = b_channels[1];
-    // let channel_ep_id = b_channels[2];
-
-    // // Current shape for B:
-    // //    /- ep
-    // // mu -- ga
-    // assert_channels_list_shape(
-    //     client_b.channel_store(),
-    //     cx_b,
-    //     &[(channel_mu_id, 0), (channel_ep_id, 1), (channel_ga_id, 1)],
-    // );
-
-    // client_a
-    //     .add_admin_to_channel((&client_b, cx_b), channel_b_id, cx_a)
-    //     .await;
-
-    // // Current shape for B:
-    // //    /- ep
-    // // mu -- ga
-    // //  /---------\
-    // // b  -- c  -- d
-    // assert_channels_list_shape(
-    //     client_b.channel_store(),
-    //     cx_b,
-    //     &[
-    //         // New channels from a
-    //         (channel_b_id, 0),
-    //         (channel_c_id, 1),
-    //         (channel_d_id, 2),
-    //         (channel_d_id, 1),
-    //         // B's old channels
-    //         (channel_mu_id, 0),
-    //         (channel_ep_id, 1),
-    //         (channel_ga_id, 1),
-    //     ],
-    // );
-
-    // client_b
-    //     .add_admin_to_channel((&client_c, cx_c), channel_ep_id, cx_b)
-    //     .await;
-
-    // // Current shape for C:
-    // // - ep
-    // assert_channels_list_shape(client_c.channel_store(), cx_c, &[(channel_ep_id, 0)]);
-
-    // client_b
-    //     .channel_store()
-    //     .update(cx_b, |channel_store, cx| {
-    //         channel_store.link_channel(channel_b_id, channel_ep_id, cx)
-    //     })
-    //     .await
-    //     .unwrap();
-
-    // // Current shape for B:
-    // //              /---------\
-    // //    /- ep -- b  -- c  -- d
-    // // mu -- ga
-    // assert_channels_list_shape(
-    //     client_b.channel_store(),
-    //     cx_b,
-    //     &[
-    //         (channel_mu_id, 0),
-    //         (channel_ep_id, 1),
-    //         (channel_b_id, 2),
-    //         (channel_c_id, 3),
-    //         (channel_d_id, 4),
-    //         (channel_d_id, 3),
-    //         (channel_ga_id, 1),
-    //     ],
-    // );
-
-    // // Current shape for C:
-    // //        /---------\
-    // // ep -- b  -- c  -- d
-    // assert_channels_list_shape(
-    //     client_c.channel_store(),
-    //     cx_c,
-    //     &[
-    //         (channel_ep_id, 0),
-    //         (channel_b_id, 1),
-    //         (channel_c_id, 2),
-    //         (channel_d_id, 3),
-    //         (channel_d_id, 2),
-    //     ],
-    // );
-
-    // client_b
-    //     .channel_store()
-    //     .update(cx_b, |channel_store, cx| {
-    //         channel_store.link_channel(channel_ga_id, channel_b_id, cx)
-    //     })
-    //     .await
-    //     .unwrap();
-
-    // // Current shape for B:
-    // //              /---------\
-    // //    /- ep -- b  -- c  -- d
-    // //   /          \
-    // // mu ---------- ga
-    // assert_channels_list_shape(
-    //     client_b.channel_store(),
-    //     cx_b,
-    //     &[
-    //         (channel_mu_id, 0),
-    //         (channel_ep_id, 1),
-    //         (channel_b_id, 2),
-    //         (channel_c_id, 3),
-    //         (channel_d_id, 4),
-    //         (channel_d_id, 3),
-    //         (channel_ga_id, 3),
-    //         (channel_ga_id, 1),
-    //     ],
-    // );
-
-    // // Current shape for A:
-    // //      /------\
-    // // a - b -- c -- d
-    // //      \-- ga
-    // assert_channels_list_shape(
-    //     client_a.channel_store(),
-    //     cx_a,
-    //     &[
-    //         (channel_a_id, 0),
-    //         (channel_b_id, 1),
-    //         (channel_c_id, 2),
-    //         (channel_d_id, 3),
-    //         (channel_d_id, 2),
-    //         (channel_ga_id, 2),
-    //     ],
-    // );
-
-    // // Current shape for C:
-    // //        /-------\
-    // // ep -- b -- c -- d
-    // //        \-- ga
-    // assert_channels_list_shape(
-    //     client_c.channel_store(),
-    //     cx_c,
-    //     &[
-    //         (channel_ep_id, 0),
-    //         (channel_b_id, 1),
-    //         (channel_c_id, 2),
-    //         (channel_d_id, 3),
-    //         (channel_d_id, 2),
-    //         (channel_ga_id, 2),
-    //     ],
-    // );
 }
 
 #[derive(Debug, PartialEq)]
@@ -1667,7 +1481,7 @@ fn assert_channels(
 ) {
     let actual = channel_store.read_with(cx, |store, _| {
         store
-            .channel_dag_entries()
+            .ordered_channels()
             .map(|(depth, channel)| ExpectedChannel {
                 depth,
                 name: channel.name.clone(),
@@ -1689,7 +1503,7 @@ fn assert_channels_list_shape(
 
     let actual = channel_store.read_with(cx, |store, _| {
         store
-            .channel_dag_entries()
+            .ordered_channels()
             .map(|(depth, channel)| (channel.id, depth))
             .collect::<Vec<_>>()
     });

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

@@ -83,7 +83,7 @@ impl RandomizedTest for RandomChannelBufferTest {
             match rng.gen_range(0..100_u32) {
                 0..=29 => {
                     let channel_name = client.channel_store().read_with(cx, |store, cx| {
-                        store.channel_dag_entries().find_map(|(_, channel)| {
+                        store.ordered_channels().find_map(|(_, channel)| {
                             if store.has_open_channel_buffer(channel.id, cx) {
                                 None
                             } else {
@@ -131,7 +131,7 @@ impl RandomizedTest for RandomChannelBufferTest {
             ChannelBufferOperation::JoinChannelNotes { channel_name } => {
                 let buffer = client.channel_store().update(cx, |store, cx| {
                     let channel_id = store
-                        .channel_dag_entries()
+                        .ordered_channels()
                         .find(|(_, c)| c.name == channel_name)
                         .unwrap()
                         .1

crates/collab_ui/src/collab_panel.rs 🔗

@@ -9,7 +9,7 @@ use crate::{
 };
 use anyhow::Result;
 use call::ActiveCall;
-use channel::{Channel, ChannelData, ChannelEvent, ChannelId, ChannelPath, ChannelStore};
+use channel::{Channel, ChannelEvent, ChannelId, ChannelStore};
 use channel_modal::ChannelModal;
 use client::{
     proto::{self, PeerId},
@@ -55,17 +55,17 @@ use workspace::{
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 struct ToggleCollapse {
-    location: ChannelPath,
+    location: ChannelId,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 struct NewChannel {
-    location: ChannelPath,
+    location: ChannelId,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 struct RenameChannel {
-    location: ChannelPath,
+    channel_id: ChannelId,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
@@ -111,13 +111,6 @@ pub struct CopyChannelLink {
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 struct StartMoveChannelFor {
     channel_id: ChannelId,
-    parent_id: Option<ChannelId>,
-}
-
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
-struct StartLinkChannelFor {
-    channel_id: ChannelId,
-    parent_id: Option<ChannelId>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
@@ -125,8 +118,6 @@ struct MoveChannel {
     to: ChannelId,
 }
 
-type DraggedChannel = (Channel, Option<ChannelId>);
-
 actions!(
     collab_panel,
     [
@@ -163,14 +154,6 @@ impl_actions!(
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 struct ChannelMoveClipboard {
     channel_id: ChannelId,
-    parent_id: Option<ChannelId>,
-    intent: ClipboardIntent,
-}
-
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-enum ClipboardIntent {
-    Move,
-    // Link,
 }
 
 const COLLABORATION_PANEL_KEY: &'static str = "CollaborationPanel";
@@ -217,19 +200,15 @@ pub fn init(cx: &mut AppContext) {
          _: &mut ViewContext<CollabPanel>| {
             panel.channel_clipboard = Some(ChannelMoveClipboard {
                 channel_id: action.channel_id,
-                parent_id: action.parent_id,
-                intent: ClipboardIntent::Move,
             });
         },
     );
 
     cx.add_action(
         |panel: &mut CollabPanel, _: &StartMoveChannel, _: &mut ViewContext<CollabPanel>| {
-            if let Some((_, path)) = panel.selected_channel() {
+            if let Some(channel) = panel.selected_channel() {
                 panel.channel_clipboard = Some(ChannelMoveClipboard {
-                    channel_id: path.channel_id(),
-                    parent_id: path.parent_id(),
-                    intent: ClipboardIntent::Move,
+                    channel_id: channel.id,
                 })
             }
         },
@@ -237,29 +216,19 @@ pub fn init(cx: &mut AppContext) {
 
     cx.add_action(
         |panel: &mut CollabPanel, _: &MoveSelected, cx: &mut ViewContext<CollabPanel>| {
-            let clipboard = panel.channel_clipboard.take();
-            if let Some(((selected_channel, _), clipboard)) =
-                panel.selected_channel().zip(clipboard)
-            {
-                match clipboard.intent {
-                    ClipboardIntent::Move => panel.channel_store.update(cx, |channel_store, cx| {
-                        match clipboard.parent_id {
-                            Some(parent_id) => channel_store.move_channel(
-                                clipboard.channel_id,
-                                parent_id,
-                                selected_channel.id,
-                                cx,
-                            ),
-                            None => channel_store.link_channel(
-                                clipboard.channel_id,
-                                selected_channel.id,
-                                cx,
-                            ),
-                        }
-                        .detach_and_log_err(cx)
-                    }),
-                }
-            }
+            let Some(clipboard) = panel.channel_clipboard.take() else {
+                return;
+            };
+            let Some(selected_channel) = panel.selected_channel() else {
+                return;
+            };
+
+            panel
+                .channel_store
+                .update(cx, |channel_store, cx| {
+                    channel_store.move_channel(clipboard.channel_id, Some(selected_channel.id), cx)
+                })
+                .detach_and_log_err(cx)
         },
     );
 
@@ -267,16 +236,9 @@ pub fn init(cx: &mut AppContext) {
         |panel: &mut CollabPanel, action: &MoveChannel, cx: &mut ViewContext<CollabPanel>| {
             if let Some(clipboard) = panel.channel_clipboard.take() {
                 panel.channel_store.update(cx, |channel_store, cx| {
-                    match clipboard.parent_id {
-                        Some(parent_id) => channel_store.move_channel(
-                            clipboard.channel_id,
-                            parent_id,
-                            action.to,
-                            cx,
-                        ),
-                        None => channel_store.link_channel(clipboard.channel_id, action.to, cx),
-                    }
-                    .detach_and_log_err(cx)
+                    channel_store
+                        .move_channel(clipboard.channel_id, Some(action.to), cx)
+                        .detach_and_log_err(cx)
                 })
             }
         },
@@ -286,11 +248,11 @@ pub fn init(cx: &mut AppContext) {
 #[derive(Debug)]
 pub enum ChannelEditingState {
     Create {
-        location: Option<ChannelPath>,
+        location: Option<ChannelId>,
         pending_name: Option<String>,
     },
     Rename {
-        location: ChannelPath,
+        location: ChannelId,
         pending_name: Option<String>,
     },
 }
@@ -324,16 +286,23 @@ pub struct CollabPanel {
     list_state: ListState<Self>,
     subscriptions: Vec<Subscription>,
     collapsed_sections: Vec<Section>,
-    collapsed_channels: Vec<ChannelPath>,
-    drag_target_channel: Option<ChannelData>,
+    collapsed_channels: Vec<ChannelId>,
+    drag_target_channel: ChannelDragTarget,
     workspace: WeakViewHandle<Workspace>,
     context_menu_on_selected: bool,
 }
 
+#[derive(PartialEq, Eq)]
+enum ChannelDragTarget {
+    None,
+    Root,
+    Channel(ChannelId),
+}
+
 #[derive(Serialize, Deserialize)]
 struct SerializedCollabPanel {
     width: Option<f32>,
-    collapsed_channels: Option<Vec<ChannelPath>>,
+    collapsed_channels: Option<Vec<ChannelId>>,
 }
 
 #[derive(Debug)]
@@ -378,7 +347,7 @@ enum ListEntry {
     Channel {
         channel: Arc<Channel>,
         depth: usize,
-        path: ChannelPath,
+        has_children: bool,
     },
     ChannelNotes {
         channel_id: ChannelId,
@@ -513,14 +482,14 @@ impl CollabPanel {
                         ListEntry::Channel {
                             channel,
                             depth,
-                            path,
+                            has_children,
                         } => {
                             let channel_row = this.render_channel(
                                 &*channel,
                                 *depth,
-                                path.to_owned(),
                                 &theme,
                                 is_selected,
+                                *has_children,
                                 ix,
                                 cx,
                             );
@@ -615,7 +584,7 @@ impl CollabPanel {
                 workspace: workspace.weak_handle(),
                 client: workspace.app_state().client.clone(),
                 context_menu_on_selected: true,
-                drag_target_channel: None,
+                drag_target_channel: ChannelDragTarget::None,
                 list_state,
             };
 
@@ -879,7 +848,7 @@ impl CollabPanel {
             if channel_store.channel_count() > 0 || self.channel_editing_state.is_some() {
                 self.match_candidates.clear();
                 self.match_candidates
-                    .extend(channel_store.channel_dag_entries().enumerate().map(
+                    .extend(channel_store.ordered_channels().enumerate().map(
                         |(ix, (_, channel))| StringMatchCandidate {
                             id: ix,
                             string: channel.name.clone(),
@@ -901,48 +870,52 @@ impl CollabPanel {
                 }
                 let mut collapse_depth = None;
                 for mat in matches {
-                    let (channel, path) = channel_store
-                        .channel_dag_entry_at(mat.candidate_id)
-                        .unwrap();
-                    let depth = path.len() - 1;
+                    let channel = channel_store.channel_at_index(mat.candidate_id).unwrap();
+                    let depth = channel.parent_path.len();
 
-                    if collapse_depth.is_none() && self.is_channel_collapsed(path) {
+                    if collapse_depth.is_none() && self.is_channel_collapsed(channel.id) {
                         collapse_depth = Some(depth);
                     } else if let Some(collapsed_depth) = collapse_depth {
                         if depth > collapsed_depth {
                             continue;
                         }
-                        if self.is_channel_collapsed(path) {
+                        if self.is_channel_collapsed(channel.id) {
                             collapse_depth = Some(depth);
                         } else {
                             collapse_depth = None;
                         }
                     }
 
+                    let has_children = channel_store
+                        .channel_at_index(mat.candidate_id + 1)
+                        .map_or(false, |next_channel| {
+                            next_channel.parent_path.ends_with(&[channel.id])
+                        });
+
                     match &self.channel_editing_state {
                         Some(ChannelEditingState::Create {
-                            location: parent_path,
+                            location: parent_id,
                             ..
-                        }) if parent_path.as_ref() == Some(path) => {
+                        }) if *parent_id == Some(channel.id) => {
                             self.entries.push(ListEntry::Channel {
                                 channel: channel.clone(),
                                 depth,
-                                path: path.clone(),
+                                has_children: false,
                             });
                             self.entries
                                 .push(ListEntry::ChannelEditor { depth: depth + 1 });
                         }
                         Some(ChannelEditingState::Rename {
-                            location: parent_path,
+                            location: parent_id,
                             ..
-                        }) if parent_path == path => {
+                        }) if parent_id == &channel.id => {
                             self.entries.push(ListEntry::ChannelEditor { depth });
                         }
                         _ => {
                             self.entries.push(ListEntry::Channel {
                                 channel: channel.clone(),
                                 depth,
-                                path: path.clone(),
+                                has_children,
                             });
                         }
                     }
@@ -1484,6 +1457,7 @@ impl CollabPanel {
         let mut channel_link = None;
         let mut channel_tooltip_text = None;
         let mut channel_icon = None;
+        let mut is_dragged_over = false;
 
         let text = match section {
             Section::ActiveCall => {
@@ -1567,26 +1541,37 @@ impl CollabPanel {
                     cx,
                 ),
             ),
-            Section::Channels => Some(
-                MouseEventHandler::new::<AddChannel, _>(0, cx, |state, _| {
-                    render_icon_button(
-                        theme
-                            .collab_panel
-                            .add_contact_button
-                            .style_for(is_selected, state),
-                        "icons/plus.svg",
-                    )
-                })
-                .with_cursor_style(CursorStyle::PointingHand)
-                .on_click(MouseButton::Left, |_, this, cx| this.new_root_channel(cx))
-                .with_tooltip::<AddChannel>(
-                    0,
-                    "Create a channel",
-                    None,
-                    tooltip_style.clone(),
-                    cx,
-                ),
-            ),
+            Section::Channels => {
+                if cx
+                    .global::<DragAndDrop<Workspace>>()
+                    .currently_dragged::<Channel>(cx.window())
+                    .is_some()
+                    && self.drag_target_channel == ChannelDragTarget::Root
+                {
+                    is_dragged_over = true;
+                }
+
+                Some(
+                    MouseEventHandler::new::<AddChannel, _>(0, cx, |state, _| {
+                        render_icon_button(
+                            theme
+                                .collab_panel
+                                .add_contact_button
+                                .style_for(is_selected, state),
+                            "icons/plus.svg",
+                        )
+                    })
+                    .with_cursor_style(CursorStyle::PointingHand)
+                    .on_click(MouseButton::Left, |_, this, cx| this.new_root_channel(cx))
+                    .with_tooltip::<AddChannel>(
+                        0,
+                        "Create a channel",
+                        None,
+                        tooltip_style.clone(),
+                        cx,
+                    ),
+                )
+            }
             _ => None,
         };
 
@@ -1657,9 +1642,37 @@ impl CollabPanel {
                 .constrained()
                 .with_height(theme.collab_panel.row_height)
                 .contained()
-                .with_style(header_style.container)
+                .with_style(if is_dragged_over {
+                    theme.collab_panel.dragged_over_header
+                } else {
+                    header_style.container
+                })
         });
 
+        result = result
+            .on_move(move |_, this, cx| {
+                if cx
+                    .global::<DragAndDrop<Workspace>>()
+                    .currently_dragged::<Channel>(cx.window())
+                    .is_some()
+                {
+                    this.drag_target_channel = ChannelDragTarget::Root;
+                    cx.notify()
+                }
+            })
+            .on_up(MouseButton::Left, move |_, this, cx| {
+                if let Some((_, dragged_channel)) = cx
+                    .global::<DragAndDrop<Workspace>>()
+                    .currently_dragged::<Channel>(cx.window())
+                {
+                    this.channel_store
+                        .update(cx, |channel_store, cx| {
+                            channel_store.move_channel(dragged_channel.id, None, cx)
+                        })
+                        .detach_and_log_err(cx)
+                }
+            });
+
         if can_collapse {
             result = result
                 .with_cursor_style(CursorStyle::PointingHand)
@@ -1910,24 +1923,23 @@ impl CollabPanel {
         &self,
         channel: &Channel,
         depth: usize,
-        path: ChannelPath,
         theme: &theme::Theme,
         is_selected: bool,
+        has_children: bool,
         ix: usize,
         cx: &mut ViewContext<Self>,
     ) -> AnyElement<Self> {
         let channel_id = channel.id;
         let collab_theme = &theme.collab_panel;
-        let has_children = self.channel_store.read(cx).has_children(channel_id);
         let is_public = self
             .channel_store
             .read(cx)
             .channel_for_id(channel_id)
             .map(|channel| channel.visibility)
             == Some(proto::ChannelVisibility::Public);
-        let other_selected =
-            self.selected_channel().map(|channel| channel.0.id) == Some(channel.id);
-        let disclosed = has_children.then(|| !self.collapsed_channels.binary_search(&path).is_ok());
+        let other_selected = self.selected_channel().map(|channel| channel.id) == Some(channel.id);
+        let disclosed =
+            has_children.then(|| !self.collapsed_channels.binary_search(&channel.id).is_ok());
 
         let is_active = iife!({
             let call_channel = ActiveCall::global(cx)
@@ -1950,13 +1962,9 @@ impl CollabPanel {
         let mut is_dragged_over = false;
         if cx
             .global::<DragAndDrop<Workspace>>()
-            .currently_dragged::<DraggedChannel>(cx.window())
+            .currently_dragged::<Channel>(cx.window())
             .is_some()
-            && self
-                .drag_target_channel
-                .as_ref()
-                .filter(|(_, dragged_path)| path.starts_with(dragged_path))
-                .is_some()
+            && self.drag_target_channel == ChannelDragTarget::Channel(channel_id)
         {
             is_dragged_over = true;
         }
@@ -2139,7 +2147,7 @@ impl CollabPanel {
                 .disclosable(
                     disclosed,
                     Box::new(ToggleCollapse {
-                        location: path.clone(),
+                        location: channel.id.clone(),
                     }),
                 )
                 .with_id(ix)
@@ -2159,7 +2167,7 @@ impl CollabPanel {
                 )
         })
         .on_click(MouseButton::Left, move |_, this, cx| {
-            if this.drag_target_channel.take().is_none() {
+            if this.drag_target_channel == ChannelDragTarget::None {
                 if is_active {
                     this.open_channel_notes(&OpenChannelNotes { channel_id }, cx)
                 } else {
@@ -2168,55 +2176,40 @@ impl CollabPanel {
             }
         })
         .on_click(MouseButton::Right, {
-            let path = path.clone();
+            let channel = channel.clone();
             move |e, this, cx| {
-                this.deploy_channel_context_menu(Some(e.position), &path, ix, cx);
+                this.deploy_channel_context_menu(Some(e.position), &channel, ix, cx);
             }
         })
         .on_up(MouseButton::Left, move |_, this, cx| {
             if let Some((_, dragged_channel)) = cx
                 .global::<DragAndDrop<Workspace>>()
-                .currently_dragged::<DraggedChannel>(cx.window())
+                .currently_dragged::<Channel>(cx.window())
             {
-                this.channel_store.update(cx, |channel_store, cx| {
-                    match dragged_channel.1 {
-                        Some(parent_id) => channel_store.move_channel(
-                            dragged_channel.0.id,
-                            parent_id,
-                            channel_id,
-                            cx,
-                        ),
-                        None => channel_store.link_channel(dragged_channel.0.id, channel_id, cx),
-                    }
+                this.channel_store
+                    .update(cx, |channel_store, cx| {
+                        channel_store.move_channel(dragged_channel.id, Some(channel_id), cx)
+                    })
                     .detach_and_log_err(cx)
-                })
             }
         })
         .on_move({
             let channel = channel.clone();
-            let path = path.clone();
             move |_, this, cx| {
-                if let Some((_, _dragged_channel)) =
-                    cx.global::<DragAndDrop<Workspace>>()
-                        .currently_dragged::<DraggedChannel>(cx.window())
+                if let Some((_, dragged_channel)) = cx
+                    .global::<DragAndDrop<Workspace>>()
+                    .currently_dragged::<Channel>(cx.window())
                 {
-                    match &this.drag_target_channel {
-                        Some(current_target)
-                            if current_target.0 == channel && current_target.1 == path =>
-                        {
-                            return
-                        }
-                        _ => {
-                            this.drag_target_channel = Some((channel.clone(), path.clone()));
-                            cx.notify();
-                        }
+                    if channel.id != dragged_channel.id {
+                        this.drag_target_channel = ChannelDragTarget::Channel(channel.id);
                     }
+                    cx.notify()
                 }
             }
         })
-        .as_draggable(
-            (channel.clone(), path.parent_id()),
-            move |_, (channel, _), cx: &mut ViewContext<Workspace>| {
+        .as_draggable::<_, Channel>(
+            channel.clone(),
+            move |_, channel, cx: &mut ViewContext<Workspace>| {
                 let theme = &theme::current(cx).collab_panel;
 
                 Flex::<Workspace>::row()
@@ -2551,39 +2544,29 @@ impl CollabPanel {
     }
 
     fn has_subchannels(&self, ix: usize) -> bool {
-        self.entries
-            .get(ix)
-            .zip(self.entries.get(ix + 1))
-            .map(|entries| match entries {
-                (
-                    ListEntry::Channel {
-                        path: this_path, ..
-                    },
-                    ListEntry::Channel {
-                        path: next_path, ..
-                    },
-                ) => next_path.starts_with(this_path),
-                _ => false,
-            })
-            .unwrap_or(false)
+        self.entries.get(ix).map_or(false, |entry| {
+            if let ListEntry::Channel { has_children, .. } = entry {
+                *has_children
+            } else {
+                false
+            }
+        })
     }
 
     fn deploy_channel_context_menu(
         &mut self,
         position: Option<Vector2F>,
-        path: &ChannelPath,
+        channel: &Channel,
         ix: usize,
         cx: &mut ViewContext<Self>,
     ) {
         self.context_menu_on_selected = position.is_none();
 
-        let channel_name = self.channel_clipboard.as_ref().and_then(|channel| {
-            let channel_name = self
-                .channel_store
+        let clipboard_channel_name = self.channel_clipboard.as_ref().and_then(|clipboard| {
+            self.channel_store
                 .read(cx)
-                .channel_for_id(channel.channel_id)
-                .map(|channel| channel.name.clone())?;
-            Some(channel_name)
+                .channel_for_id(clipboard.channel_id)
+                .map(|channel| channel.name.clone())
         });
 
         self.context_menu.update(cx, |context_menu, cx| {
@@ -2607,7 +2590,7 @@ impl CollabPanel {
             ));
 
             if self.has_subchannels(ix) {
-                let expand_action_name = if self.is_channel_collapsed(&path) {
+                let expand_action_name = if self.is_channel_collapsed(channel.id) {
                     "Expand Subchannels"
                 } else {
                     "Collapse Subchannels"
@@ -2615,7 +2598,7 @@ impl CollabPanel {
                 items.push(ContextMenuItem::action(
                     expand_action_name,
                     ToggleCollapse {
-                        location: path.clone(),
+                        location: channel.id,
                     },
                 ));
             }
@@ -2623,61 +2606,52 @@ impl CollabPanel {
             items.push(ContextMenuItem::action(
                 "Open Notes",
                 OpenChannelNotes {
-                    channel_id: path.channel_id(),
+                    channel_id: channel.id,
                 },
             ));
 
             items.push(ContextMenuItem::action(
                 "Open Chat",
                 JoinChannelChat {
-                    channel_id: path.channel_id(),
+                    channel_id: channel.id,
                 },
             ));
 
             items.push(ContextMenuItem::action(
                 "Copy Channel Link",
                 CopyChannelLink {
-                    channel_id: path.channel_id(),
+                    channel_id: channel.id,
                 },
             ));
 
-            if self
-                .channel_store
-                .read(cx)
-                .is_channel_admin(path.channel_id())
-            {
-                let parent_id = path.parent_id();
-
+            if self.channel_store.read(cx).is_channel_admin(channel.id) {
                 items.extend([
                     ContextMenuItem::Separator,
                     ContextMenuItem::action(
                         "New Subchannel",
                         NewChannel {
-                            location: path.clone(),
+                            location: channel.id,
                         },
                     ),
                     ContextMenuItem::action(
                         "Rename",
                         RenameChannel {
-                            location: path.clone(),
+                            channel_id: channel.id,
                         },
                     ),
                     ContextMenuItem::action(
                         "Move this channel",
                         StartMoveChannelFor {
-                            channel_id: path.channel_id(),
-                            parent_id,
+                            channel_id: channel.id,
                         },
                     ),
                 ]);
 
-                if let Some(channel_name) = channel_name {
+                if let Some(channel_name) = clipboard_channel_name {
                     items.push(ContextMenuItem::Separator);
                     items.push(ContextMenuItem::action(
                         format!("Move '#{}' here", channel_name),
-                        MoveChannel {
-                            to: path.channel_id(),
-                        },
+                        MoveChannel { to: channel.id },
                     ));
                 }
 
@@ -2686,20 +2660,20 @@ impl CollabPanel {
                     ContextMenuItem::action(
                         "Invite Members",
                         InviteMembers {
-                            channel_id: path.channel_id(),
+                            channel_id: channel.id,
                         },
                     ),
                     ContextMenuItem::action(
                         "Manage Members",
                         ManageMembers {
-                            channel_id: path.channel_id(),
+                            channel_id: channel.id,
                         },
                     ),
                     ContextMenuItem::Separator,
                     ContextMenuItem::action(
                         "Delete",
                         RemoveChannel {
-                            channel_id: path.channel_id(),
+                            channel_id: channel.id,
                         },
                     ),
                 ]);
@@ -2870,11 +2844,7 @@ impl CollabPanel {
 
                     self.channel_store
                         .update(cx, |channel_store, cx| {
-                            channel_store.create_channel(
-                                &channel_name,
-                                location.as_ref().map(|location| location.channel_id()),
-                                cx,
-                            )
+                            channel_store.create_channel(&channel_name, *location, cx)
                         })
                         .detach();
                     cx.notify();
@@ -2891,7 +2861,7 @@ impl CollabPanel {
 
                     self.channel_store
                         .update(cx, |channel_store, cx| {
-                            channel_store.rename(location.channel_id(), &channel_name, cx)
+                            channel_store.rename(*location, &channel_name, cx)
                         })
                         .detach();
                     cx.notify();
@@ -2918,33 +2888,27 @@ impl CollabPanel {
         _: &CollapseSelectedChannel,
         cx: &mut ViewContext<Self>,
     ) {
-        let Some((_, path)) = self
-            .selected_channel()
-            .map(|(channel, parent)| (channel.id, parent))
-        else {
+        let Some(channel_id) = self.selected_channel().map(|channel| channel.id) else {
             return;
         };
 
-        if self.is_channel_collapsed(&path) {
+        if self.is_channel_collapsed(channel_id) {
             return;
         }
 
-        self.toggle_channel_collapsed(&path.clone(), cx);
+        self.toggle_channel_collapsed(channel_id, cx);
     }
 
     fn expand_selected_channel(&mut self, _: &ExpandSelectedChannel, cx: &mut ViewContext<Self>) {
-        let Some((_, path)) = self
-            .selected_channel()
-            .map(|(channel, parent)| (channel.id, parent))
-        else {
+        let Some(id) = self.selected_channel().map(|channel| channel.id) else {
             return;
         };
 
-        if !self.is_channel_collapsed(&path) {
+        if !self.is_channel_collapsed(id) {
             return;
         }
 
-        self.toggle_channel_collapsed(path.to_owned(), cx)
+        self.toggle_channel_collapsed(id, cx)
     }
 
     fn toggle_channel_collapsed_action(
@@ -2952,21 +2916,16 @@ impl CollabPanel {
         action: &ToggleCollapse,
         cx: &mut ViewContext<Self>,
     ) {
-        self.toggle_channel_collapsed(&action.location, cx);
+        self.toggle_channel_collapsed(action.location, cx);
     }
 
-    fn toggle_channel_collapsed<'a>(
-        &mut self,
-        path: impl Into<Cow<'a, ChannelPath>>,
-        cx: &mut ViewContext<Self>,
-    ) {
-        let path = path.into();
-        match self.collapsed_channels.binary_search(&path) {
+    fn toggle_channel_collapsed<'a>(&mut self, channel_id: ChannelId, cx: &mut ViewContext<Self>) {
+        match self.collapsed_channels.binary_search(&channel_id) {
             Ok(ix) => {
                 self.collapsed_channels.remove(ix);
             }
             Err(ix) => {
-                self.collapsed_channels.insert(ix, path.into_owned());
+                self.collapsed_channels.insert(ix, channel_id);
             }
         };
         self.serialize(cx);
@@ -2975,8 +2934,8 @@ impl CollabPanel {
         cx.focus_self();
     }
 
-    fn is_channel_collapsed(&self, path: &ChannelPath) -> bool {
-        self.collapsed_channels.binary_search(path).is_ok()
+    fn is_channel_collapsed(&self, channel_id: ChannelId) -> bool {
+        self.collapsed_channels.binary_search(&channel_id).is_ok()
     }
 
     fn leave_call(cx: &mut ViewContext<Self>) {
@@ -3039,16 +2998,16 @@ impl CollabPanel {
     }
 
     fn remove(&mut self, _: &Remove, cx: &mut ViewContext<Self>) {
-        if let Some((channel, _)) = self.selected_channel() {
+        if let Some(channel) = self.selected_channel() {
             self.remove_channel(channel.id, cx)
         }
     }
 
     fn rename_selected_channel(&mut self, _: &menu::SecondaryConfirm, cx: &mut ViewContext<Self>) {
-        if let Some((_, parent)) = self.selected_channel() {
+        if let Some(channel) = self.selected_channel() {
             self.rename_channel(
                 &RenameChannel {
-                    location: parent.to_owned(),
+                    channel_id: channel.id,
                 },
                 cx,
             );
@@ -3057,15 +3016,12 @@ impl CollabPanel {
 
     fn rename_channel(&mut self, action: &RenameChannel, cx: &mut ViewContext<Self>) {
         let channel_store = self.channel_store.read(cx);
-        if !channel_store.is_channel_admin(action.location.channel_id()) {
+        if !channel_store.is_channel_admin(action.channel_id) {
             return;
         }
-        if let Some(channel) = channel_store
-            .channel_for_id(action.location.channel_id())
-            .cloned()
-        {
+        if let Some(channel) = channel_store.channel_for_id(action.channel_id).cloned() {
             self.channel_editing_state = Some(ChannelEditingState::Rename {
-                location: action.location.to_owned(),
+                location: action.channel_id.to_owned(),
                 pending_name: None,
             });
             self.channel_name_editor.update(cx, |editor, cx| {
@@ -3085,22 +3041,18 @@ impl CollabPanel {
     }
 
     fn show_inline_context_menu(&mut self, _: &menu::ShowContextMenu, cx: &mut ViewContext<Self>) {
-        let Some((_, path)) = self.selected_channel() else {
+        let Some(channel) = self.selected_channel() else {
             return;
         };
 
-        self.deploy_channel_context_menu(None, &path.to_owned(), self.selection.unwrap(), cx);
+        self.deploy_channel_context_menu(None, &channel.clone(), self.selection.unwrap(), cx);
     }
 
-    fn selected_channel(&self) -> Option<(&Arc<Channel>, &ChannelPath)> {
+    fn selected_channel(&self) -> Option<&Arc<Channel>> {
         self.selection
             .and_then(|ix| self.entries.get(ix))
             .and_then(|entry| match entry {
-                ListEntry::Channel {
-                    channel,
-                    path: parent,
-                    ..
-                } => Some((channel, parent)),
+                ListEntry::Channel { channel, .. } => Some(channel),
                 _ => None,
             })
     }
@@ -3517,19 +3469,13 @@ impl PartialEq for ListEntry {
                 }
             }
             ListEntry::Channel {
-                channel: channel_1,
-                depth: depth_1,
-                path: parent_1,
+                channel: channel_1, ..
             } => {
                 if let ListEntry::Channel {
-                    channel: channel_2,
-                    depth: depth_2,
-                    path: parent_2,
+                    channel: channel_2, ..
                 } = other
                 {
-                    return channel_1.id == channel_2.id
-                        && depth_1 == depth_2
-                        && parent_1 == parent_2;
+                    return channel_1.id == channel_2.id;
                 }
             }
             ListEntry::ChannelNotes { channel_id } => {

crates/rpc/proto/zed.proto 🔗

@@ -171,8 +171,6 @@ message Envelope {
         AckChannelMessage ack_channel_message = 143;
         GetChannelMessagesById get_channel_messages_by_id = 144;
 
-        LinkChannel link_channel = 145;
-        UnlinkChannel unlink_channel = 146;
         MoveChannel move_channel = 147;
         SetChannelVisibility set_channel_visibility = 148;
 
@@ -972,8 +970,6 @@ message LspDiskBasedDiagnosticsUpdated {}
 
 message UpdateChannels {
     repeated Channel channels = 1;
-    repeated ChannelEdge insert_edge = 2;
-    repeated ChannelEdge delete_edge = 3;
     repeated uint64 delete_channels = 4;
     repeated Channel channel_invitations = 5;
     repeated uint64 remove_channel_invitations = 6;
@@ -993,11 +989,6 @@ message UnseenChannelBufferChange {
     repeated VectorClockEntry version = 3;
 }
 
-message ChannelEdge {
-    uint64 channel_id = 1;
-    uint64 parent_id = 2;
-}
-
 message ChannelPermission {
     uint64 channel_id = 1;
     ChannelRole role = 3;
@@ -1137,20 +1128,9 @@ message GetChannelMessagesById {
     repeated uint64 message_ids = 1;
 }
 
-message LinkChannel {
-    uint64 channel_id = 1;
-    uint64 to = 2;
-}
-
-message UnlinkChannel {
-    uint64 channel_id = 1;
-    uint64 from = 2;
-}
-
 message MoveChannel {
     uint64 channel_id = 1;
-    uint64 from = 2;
-    uint64 to = 3;
+    optional uint64 to = 2;
 }
 
 message JoinChannelBuffer {
@@ -1586,6 +1566,7 @@ message Channel {
     string name = 2;
     ChannelVisibility visibility = 3;
     ChannelRole role = 4;
+    repeated uint64 parent_path = 5;
 }
 
 message Contact {

crates/rpc/src/proto.rs 🔗

@@ -210,7 +210,6 @@ messages!(
     (LeaveChannelChat, Foreground),
     (LeaveProject, Foreground),
     (LeaveRoom, Foreground),
-    (LinkChannel, Foreground),
     (MarkNotificationRead, Foreground),
     (MoveChannel, Foreground),
     (OnTypeFormatting, Background),
@@ -263,7 +262,6 @@ messages!(
     (SynchronizeBuffersResponse, Foreground),
     (Test, Foreground),
     (Unfollow, Foreground),
-    (UnlinkChannel, Foreground),
     (UnshareProject, Foreground),
     (UpdateBuffer, Foreground),
     (UpdateBufferFile, Foreground),
@@ -327,7 +325,6 @@ request_messages!(
     (JoinRoom, JoinRoomResponse),
     (LeaveChannelBuffer, Ack),
     (LeaveRoom, Ack),
-    (LinkChannel, Ack),
     (MarkNotificationRead, Ack),
     (MoveChannel, Ack),
     (OnTypeFormatting, OnTypeFormattingResponse),
@@ -362,7 +359,6 @@ request_messages!(
     (ShareProject, ShareProjectResponse),
     (SynchronizeBuffers, SynchronizeBuffersResponse),
     (Test, Test),
-    (UnlinkChannel, Ack),
     (UpdateBuffer, Ack),
     (UpdateParticipantLocation, Ack),
     (UpdateProject, Ack),

crates/theme/src/theme.rs 🔗

@@ -250,6 +250,7 @@ pub struct CollabPanel {
     pub add_contact_button: Toggleable<Interactive<IconButton>>,
     pub add_channel_button: Toggleable<Interactive<IconButton>>,
     pub header_row: ContainedText,
+    pub dragged_over_header: ContainerStyle,
     pub subheader_row: Toggleable<Interactive<ContainedText>>,
     pub leave_call: Interactive<ContainedText>,
     pub contact_row: Toggleable<Interactive<ContainerStyle>>,

styles/src/style_tree/collab_panel.ts 🔗

@@ -210,6 +210,14 @@ export default function contacts_panel(): any {
                 right: SPACING,
             },
         },
+        dragged_over_header: {
+            margin: { top: SPACING },
+            padding: {
+                left: SPACING,
+                right: SPACING,
+            },
+            background: background(layer, "hovered"),
+        },
         subheader_row,
         leave_call: interactive({
             base: {
@@ -279,7 +287,7 @@ export default function contacts_panel(): any {
                 margin: {
                     left: CHANNEL_SPACING,
                 },
-            }
+            },
         },
         list_empty_label_container: {
             margin: {

styles/src/style_tree/search.ts 🔗

@@ -2,7 +2,6 @@ import { with_opacity } from "../theme/color"
 import { background, border, foreground, text } from "./components"
 import { interactive, toggleable } from "../element"
 import { useTheme } from "../theme"
-import { text_button } from "../component/text_button"
 
 const search_results = () => {
     const theme = useTheme()
@@ -36,7 +35,7 @@ export default function search(): any {
             left: 10,
             right: 4,
         },
-        margin: { right: SEARCH_ROW_SPACING }
+        margin: { right: SEARCH_ROW_SPACING },
     }
 
     const include_exclude_editor = {
@@ -378,7 +377,7 @@ export default function search(): any {
         modes_container: {
             padding: {
                 right: SEARCH_ROW_SPACING,
-            }
+            },
         },
         replace_icon: {
             icon: {