Sync Role as part of channels

Conrad Irwin created

Begin to fix guest notifications

Change summary

crates/channel/src/channel_store.rs               |  38 --
crates/channel/src/channel_store/channel_index.rs |   2 
crates/channel/src/channel_store_tests.rs         |   2 
crates/collab/src/db.rs                           |   2 
crates/collab/src/db/ids.rs                       |   2 
crates/collab/src/db/queries/buffers.rs           |   4 
crates/collab/src/db/queries/channels.rs          | 107 ++++---
crates/collab/src/db/tests.rs                     |   8 
crates/collab/src/db/tests/channel_tests.rs       | 160 +++++-----
crates/collab/src/rpc.rs                          | 238 ++++++++++------
crates/collab/src/tests/channel_buffer_tests.rs   |   7 
crates/collab/src/tests/channel_tests.rs          |   5 
crates/collab_ui/src/chat_panel.rs                |   2 
crates/collab_ui/src/collab_panel.rs              |   8 
crates/rpc/proto/zed.proto                        |   2 
15 files changed, 322 insertions(+), 265 deletions(-)

Detailed changes

crates/channel/src/channel_store.rs 🔗

@@ -9,7 +9,7 @@ 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, ChannelPermission, ChannelRole, ChannelVisibility},
+    proto::{self, ChannelEdge, ChannelVisibility},
     TypedEnvelope,
 };
 use serde_derive::{Deserialize, Serialize};
@@ -30,7 +30,6 @@ pub struct ChannelStore {
     channel_index: ChannelIndex,
     channel_invitations: Vec<Arc<Channel>>,
     channel_participants: HashMap<ChannelId, Vec<Arc<User>>>,
-    channels_with_admin_privileges: HashSet<ChannelId>,
     outgoing_invites: HashSet<(ChannelId, UserId)>,
     update_channels_tx: mpsc::UnboundedSender<proto::UpdateChannels>,
     opened_buffers: HashMap<ChannelId, OpenedModelHandle<ChannelBuffer>>,
@@ -50,6 +49,7 @@ pub struct Channel {
     pub id: ChannelId,
     pub name: String,
     pub visibility: proto::ChannelVisibility,
+    pub role: proto::ChannelRole,
     pub unseen_note_version: Option<(u64, clock::Global)>,
     pub unseen_message_id: Option<u64>,
 }
@@ -164,7 +164,6 @@ impl ChannelStore {
             channel_invitations: Vec::default(),
             channel_index: ChannelIndex::default(),
             channel_participants: Default::default(),
-            channels_with_admin_privileges: Default::default(),
             outgoing_invites: Default::default(),
             opened_buffers: Default::default(),
             opened_chats: Default::default(),
@@ -419,16 +418,11 @@ impl ChannelStore {
             .spawn(async move { task.await.map_err(|error| anyhow!("{}", error)) })
     }
 
-    pub fn is_user_admin(&self, channel_id: ChannelId) -> bool {
-        self.channel_index.iter().any(|path| {
-            if let Some(ix) = path.iter().position(|id| *id == channel_id) {
-                path[..=ix]
-                    .iter()
-                    .any(|id| self.channels_with_admin_privileges.contains(id))
-            } else {
-                false
-            }
-        })
+    pub fn is_channel_admin(&self, channel_id: ChannelId) -> bool {
+        let Some(channel) = self.channel_for_id(channel_id) else {
+            return false;
+        };
+        channel.role == proto::ChannelRole::Admin
     }
 
     pub fn channel_participants(&self, channel_id: ChannelId) -> &[Arc<User>] {
@@ -469,10 +463,6 @@ impl ChannelStore {
                     proto::UpdateChannels {
                         channels: vec![channel],
                         insert_edge: parent_edge,
-                        channel_permissions: vec![ChannelPermission {
-                            channel_id,
-                            role: ChannelRole::Admin.into(),
-                        }],
                         ..Default::default()
                     },
                     cx,
@@ -881,7 +871,6 @@ impl ChannelStore {
         self.channel_index.clear();
         self.channel_invitations.clear();
         self.channel_participants.clear();
-        self.channels_with_admin_privileges.clear();
         self.channel_index.clear();
         self.outgoing_invites.clear();
         cx.notify();
@@ -927,6 +916,7 @@ impl ChannelStore {
                     Arc::new(Channel {
                         id: channel.id,
                         visibility: channel.visibility(),
+                        role: channel.role(),
                         name: channel.name,
                         unseen_note_version: None,
                         unseen_message_id: None,
@@ -947,8 +937,6 @@ impl ChannelStore {
                 self.channel_index.delete_channels(&payload.delete_channels);
                 self.channel_participants
                     .retain(|channel_id, _| !payload.delete_channels.contains(channel_id));
-                self.channels_with_admin_privileges
-                    .retain(|channel_id| !payload.delete_channels.contains(channel_id));
 
                 for channel_id in &payload.delete_channels {
                     let channel_id = *channel_id;
@@ -992,16 +980,6 @@ impl ChannelStore {
             }
         }
 
-        for permission in payload.channel_permissions {
-            if permission.role() == proto::ChannelRole::Admin {
-                self.channels_with_admin_privileges
-                    .insert(permission.channel_id);
-            } else {
-                self.channels_with_admin_privileges
-                    .remove(&permission.channel_id);
-            }
-        }
-
         cx.notify();
         if payload.channel_participants.is_empty() {
             return None;

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

@@ -125,6 +125,7 @@ impl<'a> ChannelPathsInsertGuard<'a> {
         if let Some(existing_channel) = self.channels_by_id.get_mut(&channel_proto.id) {
             let existing_channel = Arc::make_mut(existing_channel);
             existing_channel.visibility = channel_proto.visibility();
+            existing_channel.role = channel_proto.role();
             existing_channel.name = channel_proto.name;
         } else {
             self.channels_by_id.insert(
@@ -132,6 +133,7 @@ impl<'a> ChannelPathsInsertGuard<'a> {
                 Arc::new(Channel {
                     id: channel_proto.id,
                     visibility: channel_proto.visibility(),
+                    role: channel_proto.role(),
                     name: channel_proto.name,
                     unseen_note_version: None,
                     unseen_message_id: None,

crates/collab/src/db.rs 🔗

@@ -433,13 +433,13 @@ pub struct Channel {
     pub id: ChannelId,
     pub name: String,
     pub visibility: ChannelVisibility,
+    pub role: ChannelRole,
 }
 
 #[derive(Debug, PartialEq)]
 pub struct ChannelsForUser {
     pub channels: ChannelGraph,
     pub channel_participants: HashMap<ChannelId, Vec<UserId>>,
-    pub channels_with_admin_privileges: HashSet<ChannelId>,
     pub unseen_buffer_changes: Vec<proto::UnseenChannelBufferChange>,
     pub channel_messages: Vec<proto::UnseenChannelMessage>,
 }

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

@@ -82,7 +82,7 @@ id_type!(UserId);
 id_type!(ChannelBufferCollaboratorId);
 id_type!(FlagId);
 
-#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default)]
+#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default, Hash)]
 #[sea_orm(rs_type = "String", db_type = "String(None)")]
 pub enum ChannelRole {
     #[sea_orm(string_value = "admin")]

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

@@ -16,7 +16,7 @@ impl Database {
         connection: ConnectionId,
     ) -> Result<proto::JoinChannelBufferResponse> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_member(channel_id, user_id, &tx)
+            self.check_user_is_channel_participant(channel_id, user_id, &tx)
                 .await?;
 
             let buffer = channel::Model {
@@ -131,7 +131,7 @@ impl Database {
             for client_buffer in buffers {
                 let channel_id = ChannelId::from_proto(client_buffer.channel_id);
                 if self
-                    .check_user_is_channel_member(channel_id, user_id, &*tx)
+                    .check_user_is_channel_participant(channel_id, user_id, &*tx)
                     .await
                     .is_err()
                 {

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

@@ -132,9 +132,12 @@ impl Database {
                 && channel.as_ref().map(|c| c.visibility) == Some(ChannelVisibility::Public)
             {
                 let channel_id_to_join = self
-                    .most_public_ancestor_for_channel(channel_id, &*tx)
+                    .public_path_to_channel_internal(channel_id, &*tx)
                     .await?
+                    .first()
+                    .cloned()
                     .unwrap_or(channel_id);
+
                 role = Some(ChannelRole::Guest);
                 joined_channel_id = Some(channel_id_to_join);
 
@@ -306,7 +309,8 @@ impl Database {
         self.transaction(move |tx| async move {
             let new_name = Self::sanitize_channel_name(new_name)?.to_string();
 
-            self.check_user_is_channel_admin(channel_id, user_id, &*tx)
+            let role = self
+                .check_user_is_channel_admin(channel_id, user_id, &*tx)
                 .await?;
 
             let channel = channel::ActiveModel {
@@ -321,6 +325,7 @@ impl Database {
                 id: channel.id,
                 name: channel.name,
                 visibility: channel.visibility,
+                role,
             })
         })
         .await
@@ -398,6 +403,8 @@ impl Database {
 
     pub async fn get_channel_invites_for_user(&self, user_id: UserId) -> Result<Vec<Channel>> {
         self.transaction(|tx| async move {
+            let mut role_for_channel: HashMap<ChannelId, ChannelRole> = HashMap::default();
+
             let channel_invites = channel_member::Entity::find()
                 .filter(
                     channel_member::Column::UserId
@@ -407,14 +414,12 @@ impl Database {
                 .all(&*tx)
                 .await?;
 
+            for invite in channel_invites {
+                role_for_channel.insert(invite.channel_id, invite.role);
+            }
+
             let channels = channel::Entity::find()
-                .filter(
-                    channel::Column::Id.is_in(
-                        channel_invites
-                            .into_iter()
-                            .map(|channel_member| channel_member.channel_id),
-                    ),
-                )
+                .filter(channel::Column::Id.is_in(role_for_channel.keys().cloned()))
                 .all(&*tx)
                 .await?;
 
@@ -424,6 +429,7 @@ impl Database {
                     id: channel.id,
                     name: channel.name,
                     visibility: channel.visibility,
+                    role: role_for_channel[&channel.id],
                 })
                 .collect();
 
@@ -458,19 +464,22 @@ impl Database {
     ) -> Result<ChannelsForUser> {
         self.transaction(|tx| async move {
             let tx = tx;
-
-            let channel_membership = channel_member::Entity::find()
-                .filter(
-                    channel_member::Column::UserId
-                        .eq(user_id)
-                        .and(channel_member::Column::ChannelId.eq(channel_id))
-                        .and(channel_member::Column::Accepted.eq(true)),
-                )
-                .all(&*tx)
+            let role = self
+                .check_user_is_channel_participant(channel_id, user_id, &*tx)
                 .await?;
 
-            self.get_user_channels(user_id, channel_membership, &tx)
-                .await
+            self.get_user_channels(
+                user_id,
+                vec![channel_member::Model {
+                    id: Default::default(),
+                    channel_id,
+                    user_id,
+                    role,
+                    accepted: true,
+                }],
+                &tx,
+            )
+            .await
         })
         .await
     }
@@ -509,7 +518,6 @@ impl Database {
         }
 
         let mut channels: Vec<Channel> = Vec::new();
-        let mut channels_with_admin_privileges: HashSet<ChannelId> = HashSet::default();
         let mut channels_to_remove: HashSet<u64> = HashSet::default();
 
         let mut rows = channel::Entity::find()
@@ -532,11 +540,8 @@ impl Database {
                 id: channel.id,
                 name: channel.name,
                 visibility: channel.visibility,
+                role: role,
             });
-
-            if role == ChannelRole::Admin {
-                channels_with_admin_privileges.insert(channel.id);
-            }
         }
         drop(rows);
 
@@ -618,7 +623,6 @@ impl Database {
         Ok(ChannelsForUser {
             channels: ChannelGraph { channels, edges },
             channel_participants,
-            channels_with_admin_privileges,
             unseen_buffer_changes: channel_buffer_changes,
             channel_messages: unseen_messages,
         })
@@ -787,9 +791,10 @@ impl Database {
         channel_id: ChannelId,
         user_id: UserId,
         tx: &DatabaseTransaction,
-    ) -> Result<()> {
-        match self.channel_role_for_user(channel_id, user_id, tx).await? {
-            Some(ChannelRole::Admin) => Ok(()),
+    ) -> Result<ChannelRole> {
+        let role = self.channel_role_for_user(channel_id, user_id, tx).await?;
+        match role {
+            Some(ChannelRole::Admin) => Ok(role.unwrap()),
             Some(ChannelRole::Member)
             | Some(ChannelRole::Banned)
             | Some(ChannelRole::Guest)
@@ -818,10 +823,11 @@ impl Database {
         channel_id: ChannelId,
         user_id: UserId,
         tx: &DatabaseTransaction,
-    ) -> Result<()> {
-        match self.channel_role_for_user(channel_id, user_id, tx).await? {
+    ) -> Result<ChannelRole> {
+        let role = self.channel_role_for_user(channel_id, user_id, tx).await?;
+        match role {
             Some(ChannelRole::Admin) | Some(ChannelRole::Member) | Some(ChannelRole::Guest) => {
-                Ok(())
+                Ok(role.unwrap())
             }
             Some(ChannelRole::Banned) | None => Err(anyhow!(
                 "user is not a channel participant or channel does not exist"
@@ -847,12 +853,26 @@ impl Database {
         Ok(row)
     }
 
-    pub async fn most_public_ancestor_for_channel(
+    // ordered from higher in tree to lower
+    // only considers one path to a channel
+    // includes the channel itself
+    pub async fn public_path_to_channel(&self, channel_id: ChannelId) -> Result<Vec<ChannelId>> {
+        self.transaction(move |tx| async move {
+            Ok(self
+                .public_path_to_channel_internal(channel_id, &*tx)
+                .await?)
+        })
+        .await
+    }
+
+    // ordered from higher in tree to lower
+    // only considers one path to a channel
+    // includes the channel itself
+    pub async fn public_path_to_channel_internal(
         &self,
         channel_id: ChannelId,
         tx: &DatabaseTransaction,
-    ) -> Result<Option<ChannelId>> {
-        // Note: if there are many paths to a channel, this will return just one
+    ) -> 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)
@@ -860,7 +880,7 @@ impl Database {
             .await?;
 
         let Some(path) = arbitary_path else {
-            return Ok(None);
+            return Ok(vec![]);
         };
 
         let ancestor_ids: Vec<ChannelId> = path
@@ -882,13 +902,10 @@ impl Database {
             visible_channels.insert(row.id);
         }
 
-        for ancestor in ancestor_ids {
-            if visible_channels.contains(&ancestor) {
-                return Ok(Some(ancestor));
-            }
-        }
-
-        Ok(None)
+        Ok(ancestor_ids
+            .into_iter()
+            .filter(|id| visible_channels.contains(id))
+            .collect())
     }
 
     pub async fn channel_role_for_user(
@@ -1059,7 +1076,8 @@ impl Database {
     /// Returns the channel with the given ID
     pub async fn get_channel(&self, channel_id: ChannelId, user_id: UserId) -> Result<Channel> {
         self.transaction(|tx| async move {
-            self.check_user_is_channel_participant(channel_id, user_id, &*tx)
+            let role = self
+                .check_user_is_channel_participant(channel_id, user_id, &*tx)
                 .await?;
 
             let channel = channel::Entity::find_by_id(channel_id).one(&*tx).await?;
@@ -1070,6 +1088,7 @@ impl Database {
             Ok(Channel {
                 id: channel.id,
                 visibility: channel.visibility,
+                role,
                 name: channel.name,
             })
         })

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

@@ -149,17 +149,21 @@ impl Drop for TestDb {
 }
 
 /// The second tuples are (channel_id, parent)
-fn graph(channels: &[(ChannelId, &'static str)], edges: &[(ChannelId, ChannelId)]) -> ChannelGraph {
+fn graph(
+    channels: &[(ChannelId, &'static str, ChannelRole)],
+    edges: &[(ChannelId, ChannelId)],
+) -> ChannelGraph {
     let mut graph = ChannelGraph {
         channels: vec![],
         edges: vec![],
     };
 
-    for (id, name) in channels {
+    for (id, name, role) in channels {
         graph.channels.push(Channel {
             id: *id,
             name: name.to_string(),
             visibility: ChannelVisibility::Members,
+            role: *role,
         })
     }
 

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

@@ -8,45 +8,20 @@ use crate::{
     db::{
         queries::channels::ChannelGraph,
         tests::{graph, TEST_RELEASE_CHANNEL},
-        ChannelId, ChannelRole, Database, NewUserParams, RoomId, UserId,
+        ChannelId, ChannelRole, Database, NewUserParams, RoomId, ServerId, UserId,
     },
     test_both_dbs,
 };
 use std::sync::{
-    atomic::{AtomicI32, Ordering},
+    atomic::{AtomicI32, AtomicU32, Ordering},
     Arc,
 };
 
 test_both_dbs!(test_channels, test_channels_postgres, test_channels_sqlite);
 
 async fn test_channels(db: &Arc<Database>) {
-    let a_id = db
-        .create_user(
-            "user1@example.com",
-            false,
-            NewUserParams {
-                github_login: "user1".into(),
-                github_user_id: 5,
-                invite_count: 0,
-            },
-        )
-        .await
-        .unwrap()
-        .user_id;
-
-    let b_id = db
-        .create_user(
-            "user2@example.com",
-            false,
-            NewUserParams {
-                github_login: "user2".into(),
-                github_user_id: 6,
-                invite_count: 0,
-            },
-        )
-        .await
-        .unwrap()
-        .user_id;
+    let a_id = new_test_user(db, "user1@example.com").await;
+    let b_id = new_test_user(db, "user2@example.com").await;
 
     let zed_id = db.create_root_channel("zed", a_id).await.unwrap();
 
@@ -91,13 +66,13 @@ async fn test_channels(db: &Arc<Database>) {
         result.channels,
         graph(
             &[
-                (zed_id, "zed"),
-                (crdb_id, "crdb"),
-                (livestreaming_id, "livestreaming"),
-                (replace_id, "replace"),
-                (rust_id, "rust"),
-                (cargo_id, "cargo"),
-                (cargo_ra_id, "cargo-ra")
+                (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),
@@ -114,10 +89,10 @@ async fn test_channels(db: &Arc<Database>) {
         result.channels,
         graph(
             &[
-                (zed_id, "zed"),
-                (crdb_id, "crdb"),
-                (livestreaming_id, "livestreaming"),
-                (replace_id, "replace")
+                (zed_id, "zed", ChannelRole::Member),
+                (crdb_id, "crdb", ChannelRole::Member),
+                (livestreaming_id, "livestreaming", ChannelRole::Member),
+                (replace_id, "replace", ChannelRole::Member)
             ],
             &[
                 (crdb_id, zed_id),
@@ -142,10 +117,10 @@ async fn test_channels(db: &Arc<Database>) {
         result.channels,
         graph(
             &[
-                (zed_id, "zed"),
-                (crdb_id, "crdb"),
-                (livestreaming_id, "livestreaming"),
-                (replace_id, "replace")
+                (zed_id, "zed", ChannelRole::Admin),
+                (crdb_id, "crdb", ChannelRole::Admin),
+                (livestreaming_id, "livestreaming", ChannelRole::Admin),
+                (replace_id, "replace", ChannelRole::Admin)
             ],
             &[
                 (crdb_id, zed_id),
@@ -179,32 +154,8 @@ test_both_dbs!(
 async fn test_joining_channels(db: &Arc<Database>) {
     let owner_id = db.create_server("test").await.unwrap().0 as u32;
 
-    let user_1 = db
-        .create_user(
-            "user1@example.com",
-            false,
-            NewUserParams {
-                github_login: "user1".into(),
-                github_user_id: 5,
-                invite_count: 0,
-            },
-        )
-        .await
-        .unwrap()
-        .user_id;
-    let user_2 = db
-        .create_user(
-            "user2@example.com",
-            false,
-            NewUserParams {
-                github_login: "user2".into(),
-                github_user_id: 6,
-                invite_count: 0,
-            },
-        )
-        .await
-        .unwrap()
-        .user_id;
+    let user_1 = new_test_user(db, "user1@example.com").await;
+    let user_2 = new_test_user(db, "user2@example.com").await;
 
     let channel_1 = db.create_root_channel("channel_1", user_1).await.unwrap();
 
@@ -523,7 +474,11 @@ async fn test_db_channel_moving(db: &Arc<Database>) {
     pretty_assertions::assert_eq!(
         returned_channels,
         graph(
-            &[(livestreaming_dag_sub_id, "livestreaming_dag_sub")],
+            &[(
+                livestreaming_dag_sub_id,
+                "livestreaming_dag_sub",
+                ChannelRole::Admin
+            )],
             &[(livestreaming_dag_sub_id, livestreaming_id)]
         )
     );
@@ -560,9 +515,17 @@ async fn test_db_channel_moving(db: &Arc<Database>) {
         returned_channels,
         graph(
             &[
-                (livestreaming_id, "livestreaming"),
-                (livestreaming_dag_id, "livestreaming_dag"),
-                (livestreaming_dag_sub_id, "livestreaming_dag_sub"),
+                (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),
@@ -1080,15 +1043,48 @@ async fn test_user_joins_correct_channel(db: &Arc<Database>) {
         .unwrap();
 
     let most_public = db
-        .transaction(
-            |tx| async move { db.most_public_ancestor_for_channel(vim_channel, &*tx).await },
-        )
+        .public_path_to_channel(vim_channel)
         .await
-        .unwrap();
+        .unwrap()
+        .first()
+        .cloned();
 
     assert_eq!(most_public, Some(zed_channel))
 }
 
+test_both_dbs!(
+    test_guest_access,
+    test_guest_access_postgres,
+    test_guest_access_sqlite
+);
+
+async fn test_guest_access(db: &Arc<Database>) {
+    let server = db.create_server("test").await.unwrap();
+
+    let admin = new_test_user(db, "admin@example.com").await;
+    let guest = new_test_user(db, "guest@example.com").await;
+    let guest_connection = new_test_connection(server);
+
+    let zed_channel = db.create_root_channel("zed", admin).await.unwrap();
+    db.set_channel_visibility(zed_channel, crate::db::ChannelVisibility::Public, admin)
+        .await
+        .unwrap();
+
+    assert!(db
+        .join_channel_chat(zed_channel, guest_connection, guest)
+        .await
+        .is_err());
+
+    db.join_channel(zed_channel, guest, guest_connection, TEST_RELEASE_CHANNEL)
+        .await
+        .unwrap();
+
+    assert!(db
+        .join_channel_chat(zed_channel, guest_connection, guest)
+        .await
+        .is_ok())
+}
+
 #[track_caller]
 fn assert_dag(actual: ChannelGraph, expected: &[(ChannelId, Option<ChannelId>)]) {
     let mut actual_map: HashMap<ChannelId, HashSet<ChannelId>> = HashMap::default();
@@ -1130,3 +1126,11 @@ async fn new_test_user(db: &Arc<Database>, email: &str) -> UserId {
     .unwrap()
     .user_id
 }
+
+static TEST_CONNECTION_ID: AtomicU32 = AtomicU32::new(1);
+fn new_test_connection(server: ServerId) -> ConnectionId {
+    ConnectionId {
+        id: TEST_CONNECTION_ID.fetch_add(1, Ordering::SeqCst),
+        owner_id: server.0 as u32,
+    }
+}

crates/collab/src/rpc.rs 🔗

@@ -3,8 +3,8 @@ mod connection_pool;
 use crate::{
     auth,
     db::{
-        self, BufferId, ChannelId, ChannelVisibility, ChannelsForUser, Database, MessageId,
-        ProjectId, RoomId, ServerId, User, UserId,
+        self, BufferId, ChannelId, ChannelsForUser, Database, MessageId, ProjectId, RoomId,
+        ServerId, User, UserId,
     },
     executor::Executor,
     AppState, Result,
@@ -2206,14 +2206,13 @@ async fn create_channel(
         .create_channel(&request.name, parent_id, session.user_id)
         .await?;
 
-    let channel = proto::Channel {
-        id: id.to_proto(),
-        name: request.name,
-        visibility: proto::ChannelVisibility::Members as i32,
-    };
-
     response.send(proto::CreateChannelResponse {
-        channel: Some(channel.clone()),
+        channel: Some(proto::Channel {
+            id: id.to_proto(),
+            name: request.name,
+            visibility: proto::ChannelVisibility::Members as i32,
+            role: proto::ChannelRole::Admin.into(),
+        }),
         parent_id: request.parent_id,
     })?;
 
@@ -2221,19 +2220,26 @@ async fn create_channel(
         return Ok(());
     };
 
-    let update = proto::UpdateChannels {
-        channels: vec![channel],
-        insert_edge: vec![ChannelEdge {
-            parent_id: parent_id.to_proto(),
-            channel_id: id.to_proto(),
-        }],
-        ..Default::default()
-    };
+    let members: Vec<proto::ChannelMember> = db
+        .get_channel_participant_details(parent_id, session.user_id)
+        .await?
+        .into_iter()
+        .filter(|member| {
+            member.role() == proto::ChannelRole::Admin
+                || member.role() == proto::ChannelRole::Member
+        })
+        .collect();
 
-    let user_ids_to_notify = db.get_channel_members(parent_id).await?;
+    let mut updates: HashMap<UserId, proto::UpdateChannels> = HashMap::default();
+
+    for member in members {
+        let user_id = UserId::from_proto(member.user_id);
+        let channels = db.get_channel_for_user(parent_id, user_id).await?;
+        updates.insert(user_id, build_initial_channels_update(channels, vec![]));
+    }
 
     let connection_pool = session.connection_pool().await;
-    for user_id in user_ids_to_notify {
+    for (user_id, update) in updates {
         for connection_id in connection_pool.user_connection_ids(user_id) {
             if user_id == session.user_id {
                 continue;
@@ -2297,6 +2303,7 @@ async fn invite_channel_member(
         id: channel.id.to_proto(),
         visibility: channel.visibility.into(),
         name: channel.name,
+        role: request.role().into(),
     });
     for connection_id in session
         .connection_pool()
@@ -2350,18 +2357,23 @@ async fn set_channel_visibility(
         .set_channel_visibility(channel_id, visibility, session.user_id)
         .await?;
 
-    let mut update = proto::UpdateChannels::default();
-    update.channels.push(proto::Channel {
-        id: channel.id.to_proto(),
-        name: channel.name,
-        visibility: channel.visibility.into(),
-    });
-
-    let member_ids = db.get_channel_members(channel_id).await?;
+    let members = db
+        .get_channel_participant_details(channel_id, session.user_id)
+        .await?;
 
     let connection_pool = session.connection_pool().await;
-    for member_id in member_ids {
-        for connection_id in connection_pool.user_connection_ids(member_id) {
+    // TODO: notify people who were guests and are now not allowed.
+    for member in members {
+        for connection_id in connection_pool.user_connection_ids(UserId::from_proto(member.user_id))
+        {
+            let mut update = proto::UpdateChannels::default();
+            update.channels.push(proto::Channel {
+                id: channel.id.to_proto(),
+                name: channel.name.clone(),
+                visibility: channel.visibility.into(),
+                role: member.role.into(),
+            });
+
             session.peer.send(connection_id, update.clone())?;
         }
     }
@@ -2387,13 +2399,17 @@ async fn set_channel_member_role(
         )
         .await?;
 
-    let channel = db.get_channel(channel_id, session.user_id).await?;
-
     let mut update = proto::UpdateChannels::default();
     if channel_member.accepted {
-        update.channel_permissions.push(proto::ChannelPermission {
-            channel_id: channel.id.to_proto(),
-            role: request.role,
+        let channels = db.get_channel_for_user(channel_id, member_id).await?;
+        update = build_initial_channels_update(channels, vec![]);
+    } else {
+        let channel = db.get_channel(channel_id, session.user_id).await?;
+        update.channel_invitations.push(proto::Channel {
+            id: channel_id.to_proto(),
+            visibility: channel.visibility.into(),
+            name: channel.name,
+            role: request.role().into(),
         });
     }
 
@@ -2420,22 +2436,31 @@ async fn rename_channel(
         .rename_channel(channel_id, session.user_id, &request.name)
         .await?;
 
-    let channel = proto::Channel {
-        id: channel.id.to_proto(),
-        name: channel.name,
-        visibility: channel.visibility.into(),
-    };
     response.send(proto::RenameChannelResponse {
-        channel: Some(channel.clone()),
+        channel: Some(proto::Channel {
+            id: channel.id.to_proto(),
+            name: channel.name.clone(),
+            visibility: channel.visibility.into(),
+            role: proto::ChannelRole::Admin.into(),
+        }),
     })?;
-    let mut update = proto::UpdateChannels::default();
-    update.channels.push(channel);
 
-    let member_ids = db.get_channel_members(channel_id).await?;
+    let members = db
+        .get_channel_participant_details(channel_id, session.user_id)
+        .await?;
 
     let connection_pool = session.connection_pool().await;
-    for member_id in member_ids {
-        for connection_id in connection_pool.user_connection_ids(member_id) {
+    for member in members {
+        for connection_id in connection_pool.user_connection_ids(UserId::from_proto(member.user_id))
+        {
+            let mut update = proto::UpdateChannels::default();
+            update.channels.push(proto::Channel {
+                id: channel.id.to_proto(),
+                name: channel.name.clone(),
+                visibility: channel.visibility.into(),
+                role: member.role.into(),
+            });
+
             session.peer.send(connection_id, update.clone())?;
         }
     }
@@ -2463,6 +2488,10 @@ async fn link_channel(
                 id: channel.id.to_proto(),
                 visibility: channel.visibility.into(),
                 name: channel.name,
+                // TODO: not all these members should be able to see all those channels
+                // the channels in channels_to_send are from the admin point of view,
+                // but any public guests should only get updates about public channels.
+                role: todo!(),
             })
             .collect(),
         insert_edge: channels_to_send.edges,
@@ -2521,6 +2550,12 @@ async fn move_channel(
     let from_parent = ChannelId::from_proto(request.from);
     let to = ChannelId::from_proto(request.to);
 
+    let from_public_parent = db
+        .public_path_to_channel(from_parent)
+        .await?
+        .last()
+        .cloned();
+
     let channels_to_send = db
         .move_channel(session.user_id, channel_id, from_parent, to)
         .await?;
@@ -2530,38 +2565,68 @@ async fn move_channel(
         return Ok(());
     }
 
-    let members_from = db.get_channel_members(from_parent).await?;
-    let members_to = db.get_channel_members(to).await?;
+    let to_public_parent = db.public_path_to_channel(to).await?.last().cloned();
 
-    let update = proto::UpdateChannels {
-        delete_edge: vec![proto::ChannelEdge {
+    let members_from = db
+        .get_channel_participant_details(from_parent, session.user_id)
+        .await?
+        .into_iter()
+        .filter(|member| {
+            member.role() == proto::ChannelRole::Admin || member.role() == proto::ChannelRole::Guest
+        });
+    let members_to = db
+        .get_channel_participant_details(to, session.user_id)
+        .await?
+        .into_iter()
+        .filter(|member| {
+            member.role() == proto::ChannelRole::Admin || member.role() == proto::ChannelRole::Guest
+        });
+
+    let mut updates: HashMap<UserId, proto::UpdateChannels> = HashMap::default();
+
+    for member in members_to {
+        let user_id = UserId::from_proto(member.user_id);
+        let channels = db.get_channel_for_user(to, user_id).await?;
+        updates.insert(user_id, build_initial_channels_update(channels, vec![]));
+    }
+
+    if let Some(to_public_parent) = to_public_parent {
+        // only notify guests of public channels (admins/members are notified by members_to above, and banned users don't care)
+        let public_members_to = db
+            .get_channel_participant_details(to, session.user_id)
+            .await?
+            .into_iter()
+            .filter(|member| member.role() == proto::ChannelRole::Guest);
+
+        for member in public_members_to {
+            let user_id = UserId::from_proto(member.user_id);
+            if updates.contains_key(&user_id) {
+                continue;
+            }
+            let channels = db.get_channel_for_user(to_public_parent, user_id).await?;
+            updates.insert(user_id, build_initial_channels_update(channels, vec![]));
+        }
+    }
+
+    for member in members_from {
+        let user_id = UserId::from_proto(member.user_id);
+        let update = updates
+            .entry(user_id)
+            .or_insert(proto::UpdateChannels::default());
+        update.delete_edge.push(proto::ChannelEdge {
             channel_id: channel_id.to_proto(),
             parent_id: from_parent.to_proto(),
-        }],
-        ..Default::default()
-    };
-    let connection_pool = session.connection_pool().await;
-    for member_id in members_from {
-        for connection_id in connection_pool.user_connection_ids(member_id) {
-            session.peer.send(connection_id, update.clone())?;
-        }
+        })
     }
 
-    let update = proto::UpdateChannels {
-        channels: channels_to_send
-            .channels
-            .into_iter()
-            .map(|channel| proto::Channel {
-                id: channel.id.to_proto(),
-                visibility: channel.visibility.into(),
-                name: channel.name,
-            })
-            .collect(),
-        insert_edge: channels_to_send.edges,
-        ..Default::default()
-    };
-    for member_id in members_to {
-        for connection_id in connection_pool.user_connection_ids(member_id) {
+    if let Some(_from_public_parent) = from_public_parent {
+        // TODO: for each guest member of the old public parent
+        // delete the edge that they could see (from the from_public_parent down)
+    }
+
+    let connection_pool = session.connection_pool().await;
+    for (user_id, update) in updates {
+        for connection_id in connection_pool.user_connection_ids(user_id) {
             session.peer.send(connection_id, update.clone())?;
         }
     }
@@ -2628,6 +2693,7 @@ async fn channel_membership_updated(
             .map(|channel| proto::Channel {
                 id: channel.id.to_proto(),
                 visibility: channel.visibility.into(),
+                role: channel.role.into(),
                 name: channel.name,
             }),
     );
@@ -2645,17 +2711,6 @@ async fn channel_membership_updated(
                     participant_user_ids: user_ids.into_iter().map(UserId::to_proto).collect(),
                 }),
         );
-    update
-        .channel_permissions
-        .extend(
-            result
-                .channels_with_admin_privileges
-                .into_iter()
-                .map(|channel_id| proto::ChannelPermission {
-                    channel_id: channel_id.to_proto(),
-                    role: proto::ChannelRole::Admin.into(),
-                }),
-        );
     session.peer.send(session.connection_id, update)?;
     Ok(())
 }
@@ -3149,6 +3204,7 @@ fn build_initial_channels_update(
             id: channel.id.to_proto(),
             name: channel.name,
             visibility: channel.visibility.into(),
+            role: channel.role.into(),
         });
     }
 
@@ -3165,24 +3221,12 @@ fn build_initial_channels_update(
             });
     }
 
-    update
-        .channel_permissions
-        .extend(
-            channels
-                .channels_with_admin_privileges
-                .into_iter()
-                .map(|id| proto::ChannelPermission {
-                    channel_id: id.to_proto(),
-                    role: proto::ChannelRole::Admin.into(),
-                }),
-        );
-
     for channel in channel_invites {
         update.channel_invitations.push(proto::Channel {
             id: channel.id.to_proto(),
             name: channel.name,
-            // TODO: Visibility
-            visibility: ChannelVisibility::Public.into(),
+            visibility: channel.visibility.into(),
+            role: channel.role.into(),
         });
     }
 

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

@@ -413,7 +413,7 @@ async fn test_channel_buffer_disconnect(
     channel_buffer_a.update(cx_a, |buffer, _| {
         assert_eq!(
             buffer.channel().as_ref(),
-            &channel(channel_id, "the-channel")
+            &channel(channel_id, "the-channel", proto::ChannelRole::Admin)
         );
         assert!(!buffer.is_connected());
     });
@@ -438,15 +438,16 @@ async fn test_channel_buffer_disconnect(
     channel_buffer_b.update(cx_b, |buffer, _| {
         assert_eq!(
             buffer.channel().as_ref(),
-            &channel(channel_id, "the-channel")
+            &channel(channel_id, "the-channel", proto::ChannelRole::Member)
         );
         assert!(!buffer.is_connected());
     });
 }
 
-fn channel(id: u64, name: &'static str) -> Channel {
+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,

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

@@ -152,6 +152,7 @@ async fn test_core_channels(
             },
         ],
     );
+    dbg!("-------");
 
     let channel_c_id = client_a
         .channel_store()
@@ -1295,7 +1296,7 @@ fn assert_channel_invitations(
                 depth: 0,
                 name: channel.name.clone(),
                 id: channel.id,
-                user_is_admin: store.is_user_admin(channel.id),
+                user_is_admin: store.is_channel_admin(channel.id),
             })
             .collect::<Vec<_>>()
     });
@@ -1315,7 +1316,7 @@ fn assert_channels(
                 depth,
                 name: channel.name.clone(),
                 id: channel.id,
-                user_is_admin: store.is_user_admin(channel.id),
+                user_is_admin: store.is_channel_admin(channel.id),
             })
             .collect::<Vec<_>>()
     });

crates/collab_ui/src/chat_panel.rs 🔗

@@ -360,7 +360,7 @@ impl ChatPanel {
             let is_admin = self
                 .channel_store
                 .read(cx)
-                .is_user_admin(active_chat.channel().id);
+                .is_channel_admin(active_chat.channel().id);
             let last_message = active_chat.message(ix.saturating_sub(1));
             let this_message = active_chat.message(ix);
             let is_continuation = last_message.id != this_message.id

crates/collab_ui/src/collab_panel.rs 🔗

@@ -2721,7 +2721,11 @@ impl CollabPanel {
                 },
             ));
 
-            if self.channel_store.read(cx).is_user_admin(path.channel_id()) {
+            if self
+                .channel_store
+                .read(cx)
+                .is_channel_admin(path.channel_id())
+            {
                 let parent_id = path.parent_id();
 
                 items.extend([
@@ -3160,7 +3164,7 @@ 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_user_admin(action.location.channel_id()) {
+        if !channel_store.is_channel_admin(action.location.channel_id()) {
             return;
         }
         if let Some(channel) = channel_store

crates/rpc/proto/zed.proto 🔗

@@ -970,7 +970,6 @@ message UpdateChannels {
     repeated Channel channel_invitations = 5;
     repeated uint64 remove_channel_invitations = 6;
     repeated ChannelParticipants channel_participants = 7;
-    repeated ChannelPermission channel_permissions = 8;
     repeated UnseenChannelMessage unseen_channel_messages = 9;
     repeated UnseenChannelBufferChange unseen_channel_buffer_changes = 10;
 }
@@ -1568,6 +1567,7 @@ message Channel {
     uint64 id = 1;
     string name = 2;
     ChannelVisibility visibility = 3;
+    ChannelRole role = 4;
 }
 
 message Contact {