Ensure channels are sorted alphabetically

Co-authored-by: Mikayla <mikayla@zed.dev>

crates/client/src/channel_store.rs

@@ -14,7 +14,8 @@ pub type ChannelId = u64;
 pub type UserId = u64;
 
 pub struct ChannelStore {
-    channels: Vec<Arc<Channel>>,
+    channels_by_id: HashMap<ChannelId, Arc<Channel>>,
+    channel_paths: Vec<Vec<ChannelId>>,
     channel_invitations: Vec<Arc<Channel>>,
     channel_participants: HashMap<ChannelId, Vec<Arc<User>>>,
     channels_with_admin_privileges: HashSet<ChannelId>,
@@ -29,8 +30,6 @@ pub struct ChannelStore {
 pub struct Channel {
     pub id: ChannelId,
     pub name: String,
-    pub parent_id: Option<ChannelId>,
-    pub depth: usize,
 }
 
 pub struct ChannelMembership {
@@ -69,10 +68,11 @@ impl ChannelStore {
                 if matches!(status, Status::ConnectionLost | Status::SignedOut) {
                     if let Some(this) = this.upgrade(&cx) {
                         this.update(&mut cx, |this, cx| {
-                            this.channels.clear();
+                            this.channels_by_id.clear();
                             this.channel_invitations.clear();
                             this.channel_participants.clear();
                             this.channels_with_admin_privileges.clear();
+                            this.channel_paths.clear();
                             this.outgoing_invites.clear();
                             cx.notify();
                         });
@@ -83,8 +83,9 @@ impl ChannelStore {
             }
         });
         Self {
-            channels: vec![],
+            channels_by_id: HashMap::default(),
-            channel_invitations: vec![],
+            channel_invitations: Vec::default(),
+            channel_paths: Vec::default(),
             channel_participants: Default::default(),
             channels_with_admin_privileges: Default::default(),
             outgoing_invites: Default::default(),
@@ -95,31 +96,43 @@ impl ChannelStore {
         }
     }
 
-    pub fn channels(&self) -> &[Arc<Channel>] {
+    pub fn channel_count(&self) -> usize {
-        &self.channels
+        self.channel_paths.len()
+    }
+
+    pub fn channels(&self) -> impl '_ + Iterator<Item = (usize, &Arc<Channel>)> {
+        self.channel_paths.iter().map(move |path| {
+            let id = path.last().unwrap();
+            let channel = self.channel_for_id(*id).unwrap();
+            (path.len() - 1, channel)
+        })
+    }
+
+    pub fn channel_at_index(&self, ix: usize) -> Option<(usize, &Arc<Channel>)> {
+        let path = self.channel_paths.get(ix)?;
+        let id = path.last().unwrap();
+        let channel = self.channel_for_id(*id).unwrap();
+        Some((path.len() - 1, channel))
     }
 
     pub fn channel_invitations(&self) -> &[Arc<Channel>] {
         &self.channel_invitations
     }
 
-    pub fn channel_for_id(&self, channel_id: ChannelId) -> Option<Arc<Channel>> {
+    pub fn channel_for_id(&self, channel_id: ChannelId) -> Option<&Arc<Channel>> {
-        self.channels.iter().find(|c| c.id == channel_id).cloned()
+        self.channels_by_id.get(&channel_id)
     }
 
-    pub fn is_user_admin(&self, mut channel_id: ChannelId) -> bool {
+    pub fn is_user_admin(&self, channel_id: ChannelId) -> bool {
-        loop {
+        self.channel_paths.iter().any(|path| {
-            if self.channels_with_admin_privileges.contains(&channel_id) {
+            if let Some(ix) = path.iter().position(|id| *id == channel_id) {
-                return true;
+                path[..=ix]
-            }
+                    .iter()
-            if let Some(channel) = self.channel_for_id(channel_id) {
+                    .any(|id| self.channels_with_admin_privileges.contains(id))
-                if let Some(parent_id) = channel.parent_id {
+            } else {
-                    channel_id = parent_id;
+                false
-                    continue;
-                }
-            }
-            return false;
             }
+        })
     }
 
     pub fn channel_participants(&self, channel_id: ChannelId) -> &[Arc<User>] {
@@ -373,71 +386,80 @@ impl ChannelStore {
         payload: proto::UpdateChannels,
         cx: &mut ModelContext<ChannelStore>,
     ) {
-        self.channels
+        if !payload.remove_channel_invitations.is_empty() {
-            .retain(|channel| !payload.remove_channels.contains(&channel.id));
             self.channel_invitations
                 .retain(|channel| !payload.remove_channel_invitations.contains(&channel.id));
+        }
+        for channel in payload.channel_invitations {
+            match self
+                .channel_invitations
+                .binary_search_by_key(&channel.id, |c| c.id)
+            {
+                Ok(ix) => Arc::make_mut(&mut self.channel_invitations[ix]).name = channel.name,
+                Err(ix) => self.channel_invitations.insert(
+                    ix,
+                    Arc::new(Channel {
+                        id: channel.id,
+                        name: channel.name,
+                    }),
+                ),
+            }
+        }
+
+        let channels_changed = !payload.channels.is_empty() || !payload.remove_channels.is_empty();
+        if channels_changed {
+            if !payload.remove_channels.is_empty() {
+                self.channels_by_id
+                    .retain(|channel_id, _| !payload.remove_channels.contains(channel_id));
                 self.channel_participants
                     .retain(|channel_id, _| !payload.remove_channels.contains(channel_id));
                 self.channels_with_admin_privileges
                     .retain(|channel_id| !payload.remove_channels.contains(channel_id));
-
-        for channel in payload.channel_invitations {
-            if let Some(existing_channel) = self
-                .channel_invitations
-                .iter_mut()
-                .find(|c| c.id == channel.id)
-            {
-                let existing_channel = Arc::make_mut(existing_channel);
-                existing_channel.name = channel.name;
-                continue;
-            }
-
-            self.channel_invitations.insert(
-                0,
-                Arc::new(Channel {
-                    id: channel.id,
-                    name: channel.name,
-                    parent_id: None,
-                    depth: 0,
-                }),
-            );
             }
 
             for channel in payload.channels {
-            if let Some(existing_channel) = self.channels.iter_mut().find(|c| c.id == channel.id) {
+                if let Some(existing_channel) = self.channels_by_id.get_mut(&channel.id) {
                     let existing_channel = Arc::make_mut(existing_channel);
                     existing_channel.name = channel.name;
                     continue;
                 }
+                self.channels_by_id.insert(
+                    channel.id,
+                    Arc::new(Channel {
+                        id: channel.id,
+                        name: channel.name,
+                    }),
+                );
 
                 if let Some(parent_id) = channel.parent_id {
-                if let Some(ix) = self.channels.iter().position(|c| c.id == parent_id) {
+                    let mut ix = 0;
-                    let parent_channel = &self.channels[ix];
+                    while ix < self.channel_paths.len() {
-                    let depth = parent_channel.depth + 1;
+                        let path = &self.channel_paths[ix];
-                    self.channels.insert(
+                        if path.ends_with(&[parent_id]) {
-                        ix + 1,
+                            let mut new_path = path.clone();
-                        Arc::new(Channel {
+                            new_path.push(channel.id);
-                            id: channel.id,
+                            self.channel_paths.insert(ix + 1, new_path);
-                            name: channel.name,
+                            ix += 1;
-                            parent_id: Some(parent_id),
+                        }
-                            depth,
+                        ix += 1;
-                        }),
-                    );
                     }
                 } else {
-                self.channels.insert(
+                    self.channel_paths.push(vec![channel.id]);
-                    0,
-                    Arc::new(Channel {
-                        id: channel.id,
-                        name: channel.name,
-                        parent_id: None,
-                        depth: 0,
-                    }),
-                );
                 }
             }
 
+            self.channel_paths.sort_by(|a, b| {
+                let a = Self::channel_path_sorting_key(a, &self.channels_by_id);
+                let b = Self::channel_path_sorting_key(b, &self.channels_by_id);
+                a.cmp(b)
+            });
+            self.channel_paths.dedup();
+            self.channel_paths.retain(|path| {
+                path.iter()
+                    .all(|channel_id| self.channels_by_id.contains_key(channel_id))
+            });
+        }
+
         for permission in payload.channel_permissions {
             if permission.is_admin {
                 self.channels_with_admin_privileges
@@ -492,4 +514,12 @@ impl ChannelStore {
 
         cx.notify();
     }
+
+    fn channel_path_sorting_key<'a>(
+        path: &'a [ChannelId],
+        channels_by_id: &'a HashMap<ChannelId, Arc<Channel>>,
+    ) -> impl 'a + Iterator<Item = Option<&'a str>> {
+        path.iter()
+            .map(|id| Some(channels_by_id.get(id)?.name.as_str()))
+    }
 }

crates/client/src/channel_store_tests.rs

@@ -36,8 +36,8 @@ fn test_update_channels(cx: &mut AppContext) {
         &channel_store,
         &[
             //
-            (0, "a", false),
+            (0, "a".to_string(), false),
-            (0, "b", true),
+            (0, "b".to_string(), true),
         ],
         cx,
     );
@@ -64,15 +64,76 @@ fn test_update_channels(cx: &mut AppContext) {
     assert_channels(
         &channel_store,
         &[
-            (0, "a", false),
+            (0, "a".to_string(), false),
-            (1, "y", false),
+            (1, "y".to_string(), false),
-            (0, "b", true),
+            (0, "b".to_string(), true),
-            (1, "x", true),
+            (1, "x".to_string(), true),
         ],
         cx,
     );
 }
 
+#[gpui::test]
+fn test_dangling_channel_paths(cx: &mut AppContext) {
+    let http = FakeHttpClient::with_404_response();
+    let client = Client::new(http.clone(), cx);
+    let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http, cx));
+
+    let channel_store = cx.add_model(|cx| ChannelStore::new(client, user_store, cx));
+
+    update_channels(
+        &channel_store,
+        proto::UpdateChannels {
+            channels: vec![
+                proto::Channel {
+                    id: 0,
+                    name: "a".to_string(),
+                    parent_id: None,
+                },
+                proto::Channel {
+                    id: 1,
+                    name: "b".to_string(),
+                    parent_id: Some(0),
+                },
+                proto::Channel {
+                    id: 2,
+                    name: "c".to_string(),
+                    parent_id: Some(1),
+                },
+            ],
+            channel_permissions: vec![proto::ChannelPermission {
+                channel_id: 0,
+                is_admin: true,
+            }],
+            ..Default::default()
+        },
+        cx,
+    );
+    // Sanity check
+    assert_channels(
+        &channel_store,
+        &[
+            //
+            (0, "a".to_string(), true),
+            (1, "b".to_string(), true),
+            (2, "c".to_string(), true),
+        ],
+        cx,
+    );
+
+    update_channels(
+        &channel_store,
+        proto::UpdateChannels {
+            remove_channels: vec![1, 2],
+            ..Default::default()
+        },
+        cx,
+    );
+
+    // Make sure that the 1/2/3 path is gone
+    assert_channels(&channel_store, &[(0, "a".to_string(), true)], cx);
+}
+
 fn update_channels(
     channel_store: &ModelHandle<ChannelStore>,
     message: proto::UpdateChannels,
@@ -84,15 +145,20 @@ fn update_channels(
 #[track_caller]
 fn assert_channels(
     channel_store: &ModelHandle<ChannelStore>,
-    expected_channels: &[(usize, &str, bool)],
+    expected_channels: &[(usize, String, bool)],
     cx: &AppContext,
 ) {
-    channel_store.read_with(cx, |store, _| {
+    let actual = channel_store.read_with(cx, |store, _| {
-        let actual = store
+        store
             .channels()
-            .iter()
+            .map(|(depth, channel)| {
-            .map(|c| (c.depth, c.name.as_str(), store.is_user_admin(c.id)))
+                (
-            .collect::<Vec<_>>();
+                    depth,
-        assert_eq!(actual, expected_channels);
+                    channel.name.to_string(),
+                    store.is_user_admin(channel.id),
+                )
+            })
+            .collect::<Vec<_>>()
     });
+    assert_eq!(actual, expected_channels);
 }

crates/collab/src/tests/channel_tests.rs

@@ -3,8 +3,8 @@ use crate::{
     tests::{room_participants, RoomParticipants, TestServer},
 };
 use call::ActiveCall;
-use client::{Channel, ChannelMembership, User};
+use client::{ChannelId, ChannelMembership, ChannelStore, User};
-use gpui::{executor::Deterministic, TestAppContext};
+use gpui::{executor::Deterministic, ModelHandle, TestAppContext};
 use rpc::{proto, RECEIVE_TIMEOUT};
 use std::sync::Arc;
 
@@ -35,31 +35,28 @@ async fn test_core_channels(
         .unwrap();
 
     deterministic.run_until_parked();
-    client_a.channel_store().read_with(cx_a, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_a.channel_store(),
-            channels.channels(),
+        cx_a,
         &[
-                Arc::new(Channel {
+            ExpectedChannel {
                 id: channel_a_id,
                 name: "channel-a".to_string(),
-                    parent_id: None,
                 depth: 0,
-                }),
+                user_is_admin: true,
-                Arc::new(Channel {
+            },
+            ExpectedChannel {
                 id: channel_b_id,
                 name: "channel-b".to_string(),
-                    parent_id: Some(channel_a_id),
                 depth: 1,
-                })
+                user_is_admin: true,
-            ]
+            },
+        ],
     );
-        assert!(channels.is_user_admin(channel_a_id));
-        assert!(channels.is_user_admin(channel_b_id));
-    });
 
-    client_b
+    client_b.channel_store().read_with(cx_b, |channels, _| {
-        .channel_store()
+        assert!(channels.channels().collect::<Vec<_>>().is_empty())
-        .read_with(cx_b, |channels, _| assert_eq!(channels.channels(), &[]));
+    });
 
     // Invite client B to channel A as client A.
     client_a
@@ -78,17 +75,16 @@ async fn test_core_channels(
 
     // Client A sees that B has been invited.
     deterministic.run_until_parked();
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channel_invitations(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channel_invitations(),
+        cx_b,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
             id: channel_a_id,
             name: "channel-a".to_string(),
-                parent_id: None,
             depth: 0,
-            })]
+            user_is_admin: false,
-        )
+        }],
-    });
+    );
 
     let members = client_a
         .channel_store()
@@ -125,28 +121,25 @@ async fn test_core_channels(
     deterministic.run_until_parked();
 
     // Client B now sees that they are a member of channel A and its existing subchannels.
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channel_invitations(client_b.channel_store(), cx_b, &[]);
-        assert_eq!(channels.channel_invitations(), &[]);
+    assert_channels(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channels(),
+        cx_b,
         &[
-                Arc::new(Channel {
+            ExpectedChannel {
                 id: channel_a_id,
                 name: "channel-a".to_string(),
-                    parent_id: None,
+                user_is_admin: false,
                 depth: 0,
-                }),
+            },
-                Arc::new(Channel {
+            ExpectedChannel {
                 id: channel_b_id,
                 name: "channel-b".to_string(),
-                    parent_id: Some(channel_a_id),
+                user_is_admin: false,
                 depth: 1,
-                })
+            },
-            ]
+        ],
     );
-        assert!(!channels.is_user_admin(channel_a_id));
-        assert!(!channels.is_user_admin(channel_b_id));
-    });
 
     let channel_c_id = client_a
         .channel_store()
@@ -157,31 +150,30 @@ async fn test_core_channels(
         .unwrap();
 
     deterministic.run_until_parked();
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channels(),
+        cx_b,
         &[
-                Arc::new(Channel {
+            ExpectedChannel {
                 id: channel_a_id,
                 name: "channel-a".to_string(),
-                    parent_id: None,
+                user_is_admin: false,
                 depth: 0,
-                }),
+            },
-                Arc::new(Channel {
+            ExpectedChannel {
                 id: channel_b_id,
                 name: "channel-b".to_string(),
-                    parent_id: Some(channel_a_id),
+                user_is_admin: false,
                 depth: 1,
-                }),
+            },
-                Arc::new(Channel {
+            ExpectedChannel {
                 id: channel_c_id,
                 name: "channel-c".to_string(),
-                    parent_id: Some(channel_b_id),
+                user_is_admin: false,
                 depth: 2,
-                }),
+            },
-            ]
+        ],
-        )
+    );
-    });
 
     // Update client B's membership to channel A to be an admin.
     client_a
@@ -195,35 +187,32 @@ async fn test_core_channels(
 
     // Observe that client B is now an admin of channel A, and that
     // their admin priveleges extend to subchannels of channel A.
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channel_invitations(client_b.channel_store(), cx_b, &[]);
-        assert_eq!(channels.channel_invitations(), &[]);
+    assert_channels(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channels(),
+        cx_b,
         &[
-                Arc::new(Channel {
+            ExpectedChannel {
                 id: channel_a_id,
                 name: "channel-a".to_string(),
-                    parent_id: None,
                 depth: 0,
-                }),
+                user_is_admin: true,
-                Arc::new(Channel {
+            },
+            ExpectedChannel {
                 id: channel_b_id,
                 name: "channel-b".to_string(),
-                    parent_id: Some(channel_a_id),
                 depth: 1,
-                }),
+                user_is_admin: true,
-                Arc::new(Channel {
+            },
+            ExpectedChannel {
                 id: channel_c_id,
                 name: "channel-c".to_string(),
-                    parent_id: Some(channel_b_id),
                 depth: 2,
-                }),
+                user_is_admin: true,
-            ]
+            },
+        ],
     );
 
-        assert!(channels.is_user_admin(channel_c_id))
-    });
-
     // Client A deletes the channel, deletion also deletes subchannels.
     client_a
         .channel_store()
@@ -234,30 +223,26 @@ async fn test_core_channels(
         .unwrap();
 
     deterministic.run_until_parked();
-    client_a.channel_store().read_with(cx_a, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_a.channel_store(),
-            channels.channels(),
+        cx_a,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
             id: channel_a_id,
             name: "channel-a".to_string(),
-                parent_id: None,
-
             depth: 0,
-            })]
+            user_is_admin: true,
-        )
+        }],
-    });
+    );
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channels(),
+        cx_b,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
             id: channel_a_id,
             name: "channel-a".to_string(),
-                parent_id: None,
-
             depth: 0,
-            })]
+            user_is_admin: true,
-        )
+        }],
-    });
+    );
 
     // Remove client B
     client_a
@@ -271,46 +256,38 @@ async fn test_core_channels(
     deterministic.run_until_parked();
 
     // Client A still has their channel
-    client_a.channel_store().read_with(cx_a, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_a.channel_store(),
-            channels.channels(),
+        cx_a,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
             id: channel_a_id,
             name: "channel-a".to_string(),
-                parent_id: None,
             depth: 0,
-            })]
+            user_is_admin: true,
-        )
+        }],
-    });
+    );
 
-    // Client B is gone
+    // Client B no longer has access to the channel
-    client_b
+    assert_channels(client_b.channel_store(), cx_b, &[]);
-        .channel_store()
-        .read_with(cx_b, |channels, _| assert_eq!(channels.channels(), &[]));
 
     // When disconnected, client A sees no channels.
     server.forbid_connections();
     server.disconnect_client(client_a.peer_id().unwrap());
     deterministic.advance_clock(RECEIVE_TIMEOUT + RECONNECT_TIMEOUT);
-    client_a.channel_store().read_with(cx_a, |channels, _| {
+    assert_channels(client_a.channel_store(), cx_a, &[]);
-        assert_eq!(channels.channels(), &[]);
-        assert!(!channels.is_user_admin(channel_a_id));
-    });
 
     server.allow_connections();
     deterministic.advance_clock(RECEIVE_TIMEOUT + RECONNECT_TIMEOUT);
-    client_a.channel_store().read_with(cx_a, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_a.channel_store(),
-            channels.channels(),
+        cx_a,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
             id: channel_a_id,
             name: "channel-a".to_string(),
-                parent_id: None,
             depth: 0,
-            })]
+            user_is_admin: true,
+        }],
     );
-        assert!(channels.is_user_admin(channel_a_id));
-    });
 }
 
 fn assert_participants_eq(participants: &[Arc<User>], expected_partitipants: &[u64]) {
@@ -404,20 +381,21 @@ async fn test_channel_room(
         );
     });
 
+    assert_channels(
+        client_b.channel_store(),
+        cx_b,
+        &[ExpectedChannel {
+            id: zed_id,
+            name: "zed".to_string(),
+            depth: 0,
+            user_is_admin: false,
+        }],
+    );
     client_b.channel_store().read_with(cx_b, |channels, _| {
         assert_participants_eq(
             channels.channel_participants(zed_id),
             &[client_a.user_id().unwrap()],
         );
-        assert_eq!(
-            channels.channels(),
-            &[Arc::new(Channel {
-                id: zed_id,
-                name: "zed".to_string(),
-                parent_id: None,
-                depth: 0,
-            })]
-        )
     });
 
     client_c.channel_store().read_with(cx_c, |channels, _| {
@@ -629,20 +607,17 @@ async fn test_permissions_update_while_invited(
 
     deterministic.run_until_parked();
 
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channel_invitations(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channel_invitations(),
+        cx_b,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
+            depth: 0,
             id: rust_id,
             name: "rust".to_string(),
-                parent_id: None,
+            user_is_admin: false,
-
+        }],
-                depth: 0,
-            })],
     );
-
+    assert_channels(client_b.channel_store(), cx_b, &[]);
-        assert_eq!(channels.channels(), &[],);
-    });
 
     // Update B's invite before they've accepted it
     client_a
@@ -655,20 +630,17 @@ async fn test_permissions_update_while_invited(
 
     deterministic.run_until_parked();
 
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channel_invitations(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channel_invitations(),
+        cx_b,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
+            depth: 0,
             id: rust_id,
             name: "rust".to_string(),
-                parent_id: None,
+            user_is_admin: false,
-
+        }],
-                depth: 0,
-            })],
     );
-
+    assert_channels(client_b.channel_store(), cx_b, &[]);
-        assert_eq!(channels.channels(), &[],);
-    });
 }
 
 #[gpui::test]
@@ -695,34 +667,78 @@ async fn test_channel_rename(
         .await
         .unwrap();
 
-    let rust_archive_id = rust_id;
     deterministic.run_until_parked();
 
     // Client A sees the channel with its new name.
-    client_a.channel_store().read_with(cx_a, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_a.channel_store(),
-            channels.channels(),
+        cx_a,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
-                id: rust_archive_id,
-                name: "rust-archive".to_string(),
-                parent_id: None,
-
             depth: 0,
-            })],
+            id: rust_id,
+            name: "rust-archive".to_string(),
+            user_is_admin: true,
+        }],
     );
-    });
 
     // Client B sees the channel with its new name.
-    client_b.channel_store().read_with(cx_b, |channels, _| {
+    assert_channels(
-        assert_eq!(
+        client_b.channel_store(),
-            channels.channels(),
+        cx_b,
-            &[Arc::new(Channel {
+        &[ExpectedChannel {
-                id: rust_archive_id,
-                name: "rust-archive".to_string(),
-                parent_id: None,
-
             depth: 0,
-            })],
+            id: rust_id,
+            name: "rust-archive".to_string(),
+            user_is_admin: false,
+        }],
     );
-    });
+}
+
+#[derive(Debug, PartialEq)]
+struct ExpectedChannel {
+    depth: usize,
+    id: ChannelId,
+    name: String,
+    user_is_admin: bool,
+}
+
+#[track_caller]
+fn assert_channel_invitations(
+    channel_store: &ModelHandle<ChannelStore>,
+    cx: &TestAppContext,
+    expected_channels: &[ExpectedChannel],
+) {
+    let actual = channel_store.read_with(cx, |store, _| {
+        store
+            .channel_invitations()
+            .iter()
+            .map(|channel| ExpectedChannel {
+                depth: 0,
+                name: channel.name.clone(),
+                id: channel.id,
+                user_is_admin: store.is_user_admin(channel.id),
+            })
+            .collect::<Vec<_>>()
+    });
+    assert_eq!(actual, expected_channels);
+}
+
+#[track_caller]
+fn assert_channels(
+    channel_store: &ModelHandle<ChannelStore>,
+    cx: &TestAppContext,
+    expected_channels: &[ExpectedChannel],
+) {
+    let actual = channel_store.read_with(cx, |store, _| {
+        store
+            .channels()
+            .map(|(depth, channel)| ExpectedChannel {
+                depth,
+                name: channel.name.clone(),
+                id: channel.id,
+                user_is_admin: store.is_user_admin(channel.id),
+            })
+            .collect::<Vec<_>>()
+    });
+    assert_eq!(actual, expected_channels);
 }

crates/collab_ui/src/collab_panel.rs

@@ -194,7 +194,10 @@ enum ListEntry {
     IncomingRequest(Arc<User>),
     OutgoingRequest(Arc<User>),
     ChannelInvite(Arc<Channel>),
-    Channel(Arc<Channel>),
+    Channel {
+        channel: Arc<Channel>,
+        depth: usize,
+    },
     ChannelEditor {
         depth: usize,
     },
@@ -315,9 +318,10 @@ impl CollabPanel {
                                 cx,
                             )
                         }
-                        ListEntry::Channel(channel) => {
+                        ListEntry::Channel { channel, depth } => {
                             let channel_row = this.render_channel(
                                 &*channel,
+                                *depth,
                                 &theme.collab_panel,
                                 is_selected,
                                 cx,
@@ -438,7 +442,7 @@ impl CollabPanel {
                         if this.take_editing_state(cx) {
                             this.update_entries(false, cx);
                             this.selection = this.entries.iter().position(|entry| {
-                                if let ListEntry::Channel(channel) = entry {
+                                if let ListEntry::Channel { channel, .. } = entry {
                                     channel.id == *channel_id
                                 } else {
                                     false
@@ -621,17 +625,19 @@ impl CollabPanel {
         if self.include_channels_section(cx) {
             self.entries.push(ListEntry::Header(Section::Channels, 0));
 
-            let channels = channel_store.channels();
+            if channel_store.channel_count() > 0 || self.channel_editing_state.is_some() {
-            if !(channels.is_empty() && self.channel_editing_state.is_none()) {
                 self.match_candidates.clear();
                 self.match_candidates
-                    .extend(channels.iter().enumerate().map(|(ix, channel)| {
+                    .extend(
-                        StringMatchCandidate {
+                        channel_store
+                            .channels()
+                            .enumerate()
+                            .map(|(ix, (_, channel))| StringMatchCandidate {
                                 id: ix,
                                 string: channel.name.clone(),
                                 char_bag: channel.name.chars().collect(),
-                        }
+                            }),
-                    }));
+                    );
                 let matches = executor.block(match_strings(
                     &self.match_candidates,
                     &query,
@@ -652,26 +658,30 @@ impl CollabPanel {
                     }
                 }
                 for mat in matches {
-                    let channel = &channels[mat.candidate_id];
+                    let (depth, channel) =
+                        channel_store.channel_at_index(mat.candidate_id).unwrap();
 
                     match &self.channel_editing_state {
                         Some(ChannelEditingState::Create { parent_id, .. })
                             if *parent_id == Some(channel.id) =>
                         {
-                            self.entries.push(ListEntry::Channel(channel.clone()));
+                            self.entries.push(ListEntry::Channel {
-                            self.entries.push(ListEntry::ChannelEditor {
+                                channel: channel.clone(),
-                                depth: channel.depth + 1,
+                                depth,
                             });
+                            self.entries
+                                .push(ListEntry::ChannelEditor { depth: depth + 1 });
                         }
                         Some(ChannelEditingState::Rename { channel_id, .. })
                             if *channel_id == channel.id =>
                         {
-                            self.entries.push(ListEntry::ChannelEditor {
+                            self.entries.push(ListEntry::ChannelEditor { depth });
-                                depth: channel.depth,
-                            });
                         }
                         _ => {
-                            self.entries.push(ListEntry::Channel(channel.clone()));
+                            self.entries.push(ListEntry::Channel {
+                                channel: channel.clone(),
+                                depth,
+                            });
                         }
                     }
                 }
@@ -1497,6 +1507,7 @@ impl CollabPanel {
     fn render_channel(
         &self,
         channel: &Channel,
+        depth: usize,
         theme: &theme::CollabPanel,
         is_selected: bool,
         cx: &mut ViewContext<Self>,
@@ -1542,7 +1553,7 @@ impl CollabPanel {
                 .with_style(*theme.contact_row.style_for(is_selected, state))
                 .with_padding_left(
                     theme.contact_row.default_style().padding.left
-                        + theme.channel_indent * channel.depth as f32,
+                        + theme.channel_indent * depth as f32,
                 )
         })
         .on_click(MouseButton::Left, move |_, this, cx| {
@@ -1884,7 +1895,7 @@ impl CollabPanel {
                             });
                         }
                     }
-                    ListEntry::Channel(channel) => {
+                    ListEntry::Channel { channel, .. } => {
                         self.join_channel(channel.id, cx);
                     }
                     ListEntry::ContactPlaceholder => self.toggle_contact_finder(cx),
@@ -2031,7 +2042,7 @@ impl CollabPanel {
         if !channel_store.is_user_admin(action.channel_id) {
             return;
         }
-        if let Some(channel) = channel_store.channel_for_id(action.channel_id) {
+        if let Some(channel) = channel_store.channel_for_id(action.channel_id).cloned() {
             self.channel_editing_state = Some(ChannelEditingState::Rename {
                 channel_id: action.channel_id,
                 pending_name: None,
@@ -2058,7 +2069,7 @@ impl CollabPanel {
         self.selection
             .and_then(|ix| self.entries.get(ix))
             .and_then(|entry| match entry {
-                ListEntry::Channel(channel) => Some(channel),
+                ListEntry::Channel { channel, .. } => Some(channel),
                 _ => None,
             })
     }
@@ -2395,9 +2406,16 @@ impl PartialEq for ListEntry {
                     return peer_id_1 == peer_id_2;
                 }
             }
-            ListEntry::Channel(channel_1) => {
+            ListEntry::Channel {
-                if let ListEntry::Channel(channel_2) = other {
+                channel: channel_1,
-                    return channel_1.id == channel_2.id;
+                depth: depth_1,
+            } => {
+                if let ListEntry::Channel {
+                    channel: channel_2,
+                    depth: depth_2,
+                } = other
+                {
+                    return channel_1.id == channel_2.id && depth_1 == depth_2;
                 }
             }
             ListEntry::ChannelInvite(channel_1) => {