From 70aed4a60571baf321afc842f845e3843f9eed7b Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Oct 2023 22:48:44 -0600 Subject: [PATCH] Sync Role as part of channels Begin to fix guest notifications --- crates/channel/src/channel_store.rs | 38 +-- .../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 | 240 +++++++++++------- .../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, 323 insertions(+), 266 deletions(-) diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 9c80dcc2b7..de1f35bef9 100644 --- a/crates/channel/src/channel_store.rs +++ b/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>, channel_participants: HashMap>>, - channels_with_admin_privileges: HashSet, outgoing_invites: HashSet<(ChannelId, UserId)>, update_channels_tx: mpsc::UnboundedSender, opened_buffers: HashMap>, @@ -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, } @@ -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] { @@ -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; diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index 36379a3942..54de15974e 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/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, diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index 23f2e11a03..6a9560f608 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -376,7 +376,7 @@ fn assert_channels( ( depth, channel.name.to_string(), - store.is_user_admin(channel.id), + store.is_channel_admin(channel.id), ) }) .collect::>() diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 08f78c685d..32cf5cb20a 100644 --- a/crates/collab/src/db.rs +++ b/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>, - pub channels_with_admin_privileges: HashSet, pub unseen_buffer_changes: Vec, pub channel_messages: Vec, } diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index f0de4c255e..55aae7ed3b 100644 --- a/crates/collab/src/db/ids.rs +++ b/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")] diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 69f100e6b8..1b8467c75a 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -16,7 +16,7 @@ impl Database { connection: ConnectionId, ) -> Result { 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() { diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index d64d97f2ad..65393e07f8 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/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> { self.transaction(|tx| async move { + let mut role_for_channel: HashMap = 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 { 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 = Vec::new(); - let mut channels_with_admin_privileges: HashSet = HashSet::default(); let mut channels_to_remove: HashSet = 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 { + 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 { + 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> { + 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> { - // Note: if there are many paths to a channel, this will return just one + ) -> Result> { 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 = 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 { 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, }) }) diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index 99a605106e..a53509a59f 100644 --- a/crates/collab/src/db/tests.rs +++ b/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, }) } diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 40842aff5c..a323f2919e 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/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) { - 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) { 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) { 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) { 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) { 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) { 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) { 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,13 +1043,46 @@ async fn test_user_joins_correct_channel(db: &Arc) { .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() + .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) { + 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_eq!(most_public, Some(zed_channel)) + 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] @@ -1130,3 +1126,11 @@ async fn new_test_user(db: &Arc, 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, + } +} diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 15ea3b24e1..aa8a4552b3 100644 --- a/crates/collab/src/rpc.rs +++ b/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 = 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 = 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 { - 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 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 = 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![])); } } - 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) { + 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(), + }) + } + + 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(), }); } diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 14ae159ab8..f2ab2a9d09 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/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, diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index 1bb8c92ac8..af4b99c4ef 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/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::>() }); @@ -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::>() }); diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index a8c4006cb8..f0a6c96ced 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/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 diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 2e68a1c939..dcebe35b26 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/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) { 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 diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 0ca10b381a..20a47954a4 100644 --- a/crates/rpc/proto/zed.proto +++ b/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 {