diff --git a/Cargo.lock b/Cargo.lock index 04b329939d161e751df28281c9c36a1d134a8d3a..c7b2d51b5d7c935bd3df21a1ad74964b20aa67df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1575,6 +1575,7 @@ dependencies = [ "serde", "serde_derive", "settings", + "smallvec", "theme", "theme_selector", "time", diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index 8422d53abc9143fb88ee92d2ad87ec944126e4cd..ef6a655bdcead3cd64f29e9aa25b90d0d4e4d626 100644 --- a/assets/keymaps/default.json +++ b/assets/keymaps/default.json @@ -370,42 +370,15 @@ { "context": "Pane", "bindings": { - "ctrl-1": [ - "pane::ActivateItem", - 0 - ], - "ctrl-2": [ - "pane::ActivateItem", - 1 - ], - "ctrl-3": [ - "pane::ActivateItem", - 2 - ], - "ctrl-4": [ - "pane::ActivateItem", - 3 - ], - "ctrl-5": [ - "pane::ActivateItem", - 4 - ], - "ctrl-6": [ - "pane::ActivateItem", - 5 - ], - "ctrl-7": [ - "pane::ActivateItem", - 6 - ], - "ctrl-8": [ - "pane::ActivateItem", - 7 - ], - "ctrl-9": [ - "pane::ActivateItem", - 8 - ], + "ctrl-1": ["pane::ActivateItem", 0], + "ctrl-2": ["pane::ActivateItem", 1], + "ctrl-3": ["pane::ActivateItem", 2], + "ctrl-4": ["pane::ActivateItem", 3], + "ctrl-5": ["pane::ActivateItem", 4], + "ctrl-6": ["pane::ActivateItem", 5], + "ctrl-7": ["pane::ActivateItem", 6], + "ctrl-8": ["pane::ActivateItem", 7], + "ctrl-9": ["pane::ActivateItem", 8], "ctrl-0": "pane::ActivateLastItem", "ctrl--": "pane::GoBack", "ctrl-_": "pane::GoForward", @@ -416,42 +389,15 @@ { "context": "Workspace", "bindings": { - "cmd-1": [ - "workspace::ActivatePane", - 0 - ], - "cmd-2": [ - "workspace::ActivatePane", - 1 - ], - "cmd-3": [ - "workspace::ActivatePane", - 2 - ], - "cmd-4": [ - "workspace::ActivatePane", - 3 - ], - "cmd-5": [ - "workspace::ActivatePane", - 4 - ], - "cmd-6": [ - "workspace::ActivatePane", - 5 - ], - "cmd-7": [ - "workspace::ActivatePane", - 6 - ], - "cmd-8": [ - "workspace::ActivatePane", - 7 - ], - "cmd-9": [ - "workspace::ActivatePane", - 8 - ], + "cmd-1": ["workspace::ActivatePane", 0], + "cmd-2": ["workspace::ActivatePane", 1], + "cmd-3": ["workspace::ActivatePane", 2], + "cmd-4": ["workspace::ActivatePane", 3], + "cmd-5": ["workspace::ActivatePane", 4], + "cmd-6": ["workspace::ActivatePane", 5], + "cmd-7": ["workspace::ActivatePane", 6], + "cmd-8": ["workspace::ActivatePane", 7], + "cmd-9": ["workspace::ActivatePane", 8], "cmd-b": "workspace::ToggleLeftDock", "cmd-r": "workspace::ToggleRightDock", "cmd-j": "workspace::ToggleBottomDock", @@ -494,38 +440,14 @@ }, { "bindings": { - "cmd-k cmd-left": [ - "workspace::ActivatePaneInDirection", - "Left" - ], - "cmd-k cmd-right": [ - "workspace::ActivatePaneInDirection", - "Right" - ], - "cmd-k cmd-up": [ - "workspace::ActivatePaneInDirection", - "Up" - ], - "cmd-k cmd-down": [ - "workspace::ActivatePaneInDirection", - "Down" - ], - "cmd-k shift-left": [ - "workspace::SwapPaneInDirection", - "Left" - ], - "cmd-k shift-right": [ - "workspace::SwapPaneInDirection", - "Right" - ], - "cmd-k shift-up": [ - "workspace::SwapPaneInDirection", - "Up" - ], - "cmd-k shift-down": [ - "workspace::SwapPaneInDirection", - "Down" - ] + "cmd-k cmd-left": ["workspace::ActivatePaneInDirection", "Left"], + "cmd-k cmd-right": ["workspace::ActivatePaneInDirection", "Right"], + "cmd-k cmd-up": ["workspace::ActivatePaneInDirection", "Up"], + "cmd-k cmd-down": ["workspace::ActivatePaneInDirection", "Down"], + "cmd-k shift-left": ["workspace::SwapPaneInDirection", "Left"], + "cmd-k shift-right": ["workspace::SwapPaneInDirection", "Right"], + "cmd-k shift-up": ["workspace::SwapPaneInDirection", "Up"], + "cmd-k shift-down": ["workspace::SwapPaneInDirection", "Down"] } }, // Bindings from Atom @@ -627,14 +549,6 @@ "space": "collab_panel::InsertSpace" } }, - { - "context": "(CollabPanel && not_editing) > Editor", - "bindings": { - "cmd-c": "collab_panel::StartLinkChannel", - "cmd-x": "collab_panel::StartMoveChannel", - "cmd-v": "collab_panel::MoveOrLinkToSelected" - } - }, { "context": "ChannelModal", "bindings": { @@ -655,57 +569,21 @@ "cmd-v": "terminal::Paste", "cmd-k": "terminal::Clear", // Some nice conveniences - "cmd-backspace": [ - "terminal::SendText", - "\u0015" - ], - "cmd-right": [ - "terminal::SendText", - "\u0005" - ], - "cmd-left": [ - "terminal::SendText", - "\u0001" - ], + "cmd-backspace": ["terminal::SendText", "\u0015"], + "cmd-right": ["terminal::SendText", "\u0005"], + "cmd-left": ["terminal::SendText", "\u0001"], // Terminal.app compatibility - "alt-left": [ - "terminal::SendText", - "\u001bb" - ], - "alt-right": [ - "terminal::SendText", - "\u001bf" - ], + "alt-left": ["terminal::SendText", "\u001bb"], + "alt-right": ["terminal::SendText", "\u001bf"], // There are conflicting bindings for these keys in the global context. // these bindings override them, remove at your own risk: - "up": [ - "terminal::SendKeystroke", - "up" - ], - "pageup": [ - "terminal::SendKeystroke", - "pageup" - ], - "down": [ - "terminal::SendKeystroke", - "down" - ], - "pagedown": [ - "terminal::SendKeystroke", - "pagedown" - ], - "escape": [ - "terminal::SendKeystroke", - "escape" - ], - "enter": [ - "terminal::SendKeystroke", - "enter" - ], - "ctrl-c": [ - "terminal::SendKeystroke", - "ctrl-c" - ] + "up": ["terminal::SendKeystroke", "up"], + "pageup": ["terminal::SendKeystroke", "pageup"], + "down": ["terminal::SendKeystroke", "down"], + "pagedown": ["terminal::SendKeystroke", "pagedown"], + "escape": ["terminal::SendKeystroke", "escape"], + "enter": ["terminal::SendKeystroke", "enter"], + "ctrl-c": ["terminal::SendKeystroke", "ctrl-c"] } } ] diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index 4e52f57f60b36c0f83400279c2feb45ee9e2b713..594d1b7892a72723c652382e1979a8334703d9c9 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -55,7 +55,7 @@ pub enum Event { pub struct Room { id: u64, - channel_id: Option, + pub channel_id: Option, live_kit: Option, status: RoomStatus, shared_projects: HashSet>, @@ -122,6 +122,10 @@ impl Room { } } + pub fn can_publish(&self) -> bool { + self.live_kit.as_ref().is_some_and(|room| room.can_publish) + } + fn new( id: u64, channel_id: Option, @@ -181,20 +185,23 @@ impl Room { }); let connect = room.connect(&connection_info.server_url, &connection_info.token); - cx.spawn(|this, mut cx| async move { - connect.await?; + if connection_info.can_publish { + cx.spawn(|this, mut cx| async move { + connect.await?; - if !cx.read(Self::mute_on_join) { - this.update(&mut cx, |this, cx| this.share_microphone(cx)) - .await?; - } + if !cx.read(Self::mute_on_join) { + this.update(&mut cx, |this, cx| this.share_microphone(cx)) + .await?; + } - anyhow::Ok(()) - }) - .detach_and_log_err(cx); + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } Some(LiveKitRoom { room, + can_publish: connection_info.can_publish, screen_track: LocalTrack::None, microphone_track: LocalTrack::None, next_publish_id: 0, @@ -1498,6 +1505,7 @@ struct LiveKitRoom { deafened: bool, speaking: bool, next_publish_id: usize, + can_publish: bool, _maintain_room: Task<()>, _maintain_tracks: [Task<()>; 2], } diff --git a/crates/channel/src/channel_buffer.rs b/crates/channel/src/channel_buffer.rs index ab7ea78ac1b70e5c5b0c028289f2b9f55fe924f9..9089973d3203696e6646d9249125ce6fbfdd541b 100644 --- a/crates/channel/src/channel_buffer.rs +++ b/crates/channel/src/channel_buffer.rs @@ -1,4 +1,4 @@ -use crate::Channel; +use crate::{Channel, ChannelId, ChannelStore}; use anyhow::Result; use client::{Client, Collaborator, UserStore}; use collections::HashMap; @@ -19,10 +19,11 @@ pub(crate) fn init(client: &Arc) { } pub struct ChannelBuffer { - pub(crate) channel: Arc, + pub channel_id: ChannelId, connected: bool, collaborators: HashMap, user_store: ModelHandle, + channel_store: ModelHandle, buffer: ModelHandle, buffer_epoch: u64, client: Arc, @@ -34,6 +35,7 @@ pub enum ChannelBufferEvent { CollaboratorsChanged, Disconnected, BufferEdited, + ChannelChanged, } impl Entity for ChannelBuffer { @@ -46,7 +48,7 @@ impl Entity for ChannelBuffer { } self.client .send(proto::LeaveChannelBuffer { - channel_id: self.channel.id, + channel_id: self.channel_id, }) .log_err(); } @@ -58,6 +60,7 @@ impl ChannelBuffer { channel: Arc, client: Arc, user_store: ModelHandle, + channel_store: ModelHandle, mut cx: AsyncAppContext, ) -> Result> { let response = client @@ -90,9 +93,10 @@ impl ChannelBuffer { connected: true, collaborators: Default::default(), acknowledge_task: None, - channel, + channel_id: channel.id, subscription: Some(subscription.set_model(&cx.handle(), &mut cx.to_async())), user_store, + channel_store, }; this.replace_collaborators(response.collaborators, cx); this @@ -179,7 +183,7 @@ impl ChannelBuffer { let operation = language::proto::serialize_operation(operation); self.client .send(proto::UpdateChannelBuffer { - channel_id: self.channel.id, + channel_id: self.channel_id, operations: vec![operation], }) .log_err(); @@ -223,12 +227,15 @@ impl ChannelBuffer { &self.collaborators } - pub fn channel(&self) -> Arc { - self.channel.clone() + pub fn channel(&self, cx: &AppContext) -> Option> { + self.channel_store + .read(cx) + .channel_for_id(self.channel_id) + .cloned() } pub(crate) fn disconnect(&mut self, cx: &mut ModelContext) { - log::info!("channel buffer {} disconnected", self.channel.id); + log::info!("channel buffer {} disconnected", self.channel_id); if self.connected { self.connected = false; self.subscription.take(); @@ -237,6 +244,11 @@ impl ChannelBuffer { } } + pub(crate) fn channel_changed(&mut self, cx: &mut ModelContext) { + cx.emit(ChannelBufferEvent::ChannelChanged); + cx.notify() + } + pub fn is_connected(&self) -> bool { self.connected } diff --git a/crates/channel/src/channel_chat.rs b/crates/channel/src/channel_chat.rs index ca344c409f5df1d09c830fbecc5b649fbdd3d844..ef11d964249f35bdab3462c1b128342318c8e418 100644 --- a/crates/channel/src/channel_chat.rs +++ b/crates/channel/src/channel_chat.rs @@ -19,7 +19,7 @@ use time::OffsetDateTime; use util::{post_inc, ResultExt as _, TryFutureExt}; pub struct ChannelChat { - channel: Arc, + pub channel_id: ChannelId, messages: SumTree, acknowledged_message_ids: HashSet, channel_store: ModelHandle, @@ -87,7 +87,7 @@ impl Entity for ChannelChat { fn release(&mut self, _: &mut AppContext) { self.rpc .send(proto::LeaveChannelChat { - channel_id: self.channel.id, + channel_id: self.channel_id, }) .log_err(); } @@ -112,7 +112,7 @@ impl ChannelChat { Ok(cx.add_model(|cx| { let mut this = Self { - channel, + channel_id: channel.id, user_store, channel_store, rpc: client, @@ -130,8 +130,11 @@ impl ChannelChat { })) } - pub fn channel(&self) -> &Arc { - &self.channel + pub fn channel(&self, cx: &AppContext) -> Option> { + self.channel_store + .read(cx) + .channel_for_id(self.channel_id) + .cloned() } pub fn client(&self) -> &Arc { @@ -153,7 +156,7 @@ impl ChannelChat { .current_user() .ok_or_else(|| anyhow!("current_user is not present"))?; - let channel_id = self.channel.id; + let channel_id = self.channel_id; let pending_id = ChannelMessageId::Pending(post_inc(&mut self.next_pending_message_id)); let nonce = self.rng.gen(); self.insert_messages( @@ -195,7 +198,7 @@ impl ChannelChat { pub fn remove_message(&mut self, id: u64, cx: &mut ModelContext) -> Task> { let response = self.rpc.request(proto::RemoveChannelMessage { - channel_id: self.channel.id, + channel_id: self.channel_id, message_id: id, }); cx.spawn(|this, mut cx| async move { @@ -215,7 +218,7 @@ impl ChannelChat { let rpc = self.rpc.clone(); let user_store = self.user_store.clone(); - let channel_id = self.channel.id; + let channel_id = self.channel_id; let before_message_id = self.first_loaded_message_id()?; Some(cx.spawn(|this, mut cx| { async move { @@ -288,13 +291,13 @@ impl ChannelChat { { self.rpc .send(proto::AckChannelMessage { - channel_id: self.channel.id, + channel_id: self.channel_id, message_id: latest_message_id, }) .ok(); self.last_acknowledged_id = Some(latest_message_id); self.channel_store.update(cx, |store, cx| { - store.acknowledge_message_id(self.channel.id, latest_message_id, cx); + store.acknowledge_message_id(self.channel_id, latest_message_id, cx); }); } } @@ -303,7 +306,7 @@ impl ChannelChat { pub fn rejoin(&mut self, cx: &mut ModelContext) { let user_store = self.user_store.clone(); let rpc = self.rpc.clone(); - let channel_id = self.channel.id; + let channel_id = self.channel_id; cx.spawn(|this, mut cx| { async move { let response = rpc.request(proto::JoinChannelChat { channel_id }).await?; @@ -376,7 +379,7 @@ impl ChannelChat { if self.acknowledged_message_ids.insert(id) { self.rpc .send(proto::AckChannelMessage { - channel_id: self.channel.id, + channel_id: self.channel_id, message_id: id, }) .ok(); @@ -412,7 +415,7 @@ impl ChannelChat { this.update(&mut cx, |this, cx| { this.insert_messages(SumTree::from_item(message, &()), cx); cx.emit(ChannelChatEvent::NewMessage { - channel_id: this.channel.id, + channel_id: this.channel_id, message_id, }) }); diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 221b84529706a36aa22371fedf2e8ceb5cc11987..2665c2e1ec0533e0eb4cfa318c391b517b614e72 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, } @@ -72,6 +72,10 @@ impl Channel { slug.trim_matches(|c| c == '-').to_string() } + + pub fn can_edit_notes(&self) -> bool { + self.role == proto::ChannelRole::Member || self.role == proto::ChannelRole::Admin + } } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)] @@ -161,7 +165,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(), @@ -269,10 +272,11 @@ impl ChannelStore { ) -> Task>> { let client = self.client.clone(); let user_store = self.user_store.clone(); + let channel_store = cx.handle(); self.open_channel_resource( channel_id, |this| &mut this.opened_buffers, - |channel, cx| ChannelBuffer::new(channel, client, user_store, cx), + |channel, cx| ChannelBuffer::new(channel, client, user_store, channel_store, cx), cx, ) } @@ -449,16 +453,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] { @@ -499,10 +498,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, @@ -800,6 +795,11 @@ impl ChannelStore { } fn handle_connect(&mut self, cx: &mut ModelContext) -> Task> { + self.channel_index.clear(); + self.channel_invitations.clear(); + self.channel_participants.clear(); + self.channel_index.clear(); + self.outgoing_invites.clear(); self.disconnect_channel_buffers_task.take(); for chat in self.opened_chats.values() { @@ -819,7 +819,7 @@ impl ChannelStore { let channel_buffer = buffer.read(cx); let buffer = channel_buffer.buffer().read(cx); buffer_versions.push(proto::ChannelBufferVersion { - channel_id: channel_buffer.channel().id, + channel_id: channel_buffer.channel_id, epoch: channel_buffer.epoch(), version: language::proto::serialize_version(&buffer.version()), }); @@ -846,13 +846,13 @@ impl ChannelStore { }; channel_buffer.update(cx, |channel_buffer, cx| { - let channel_id = channel_buffer.channel().id; + let channel_id = channel_buffer.channel_id; if let Some(remote_buffer) = response .buffers .iter_mut() .find(|buffer| buffer.channel_id == channel_id) { - let channel_id = channel_buffer.channel().id; + let channel_id = channel_buffer.channel_id; let remote_version = language::proto::deserialize_version(&remote_buffer.version); @@ -909,12 +909,6 @@ impl ChannelStore { } fn handle_disconnect(&mut self, wait_for_reconnect: bool, cx: &mut ModelContext) { - 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(); self.disconnect_channel_buffers_task.get_or_insert_with(|| { @@ -958,6 +952,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, @@ -977,12 +972,17 @@ impl ChannelStore { if !payload.delete_channels.is_empty() { 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)); + .retain(|channel_id, _| !&payload.delete_channels.contains(channel_id)); for channel_id in &payload.delete_channels { let channel_id = *channel_id; + if payload + .channels + .iter() + .any(|channel| channel.id == channel_id) + { + continue; + } if let Some(OpenedModelHandle::Open(buffer)) = self.opened_buffers.remove(&channel_id) { @@ -995,7 +995,16 @@ impl ChannelStore { let mut index = self.channel_index.bulk_insert(); for channel in payload.channels { - index.insert(channel) + let id = channel.id; + let channel_changed = index.insert(channel); + + if channel_changed { + if let Some(OpenedModelHandle::Open(buffer)) = self.opened_buffers.get(&id) { + if let Some(buffer) = buffer.upgrade(cx) { + buffer.update(cx, ChannelBuffer::channel_changed); + } + } + } } for unseen_buffer_change in payload.unseen_channel_buffer_changes { @@ -1023,16 +1032,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 36379a3942be2f2df3a24b70eb8833aabc214af2..b221ce1b020372ef9f6beeed5d0055e8a968c6c8 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -26,10 +26,8 @@ impl ChannelIndex { pub fn delete_channels(&mut self, channels: &[ChannelId]) { self.channels_by_id .retain(|channel_id, _| !channels.contains(channel_id)); - self.paths.retain(|path| { - path.iter() - .all(|channel_id| self.channels_by_id.contains_key(channel_id)) - }); + self.paths + .retain(|path| !path.iter().any(|channel_id| channels.contains(channel_id))); } pub fn bulk_insert(&mut self) -> ChannelPathsInsertGuard { @@ -121,10 +119,17 @@ impl<'a> ChannelPathsInsertGuard<'a> { insert_new_message(&mut self.channels_by_id, channel_id, message_id) } - pub fn insert(&mut self, channel_proto: proto::Channel) { + pub fn insert(&mut self, channel_proto: proto::Channel) -> bool { + let mut ret = false; if let Some(existing_channel) = self.channels_by_id.get_mut(&channel_proto.id) { let existing_channel = Arc::make_mut(existing_channel); + + ret = existing_channel.visibility != channel_proto.visibility() + || existing_channel.role != channel_proto.role() + || existing_channel.name != channel_proto.name; + 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 +137,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, @@ -139,6 +145,7 @@ impl<'a> ChannelPathsInsertGuard<'a> { ); self.insert_root(channel_proto.id); } + ret } pub fn insert_edge(&mut self, channel_id: ChannelId, parent_id: ChannelId) { diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index 8cc9cb73daed6f32280b3e2d0c61b5235173de51..dd4f24e2caf1e72ad442f9eaaa323ab9ac9e65b8 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -19,17 +19,15 @@ fn test_update_channels(cx: &mut AppContext) { id: 1, name: "b".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Admin.into(), }, proto::Channel { id: 2, name: "a".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Member.into(), }, ], - channel_permissions: vec![proto::ChannelPermission { - channel_id: 1, - role: proto::ChannelRole::Admin.into(), - }], ..Default::default() }, cx, @@ -38,8 +36,8 @@ fn test_update_channels(cx: &mut AppContext) { &channel_store, &[ // - (0, "a".to_string(), false), - (0, "b".to_string(), true), + (0, "a".to_string(), proto::ChannelRole::Member), + (0, "b".to_string(), proto::ChannelRole::Admin), ], cx, ); @@ -52,11 +50,13 @@ fn test_update_channels(cx: &mut AppContext) { id: 3, name: "x".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Admin.into(), }, proto::Channel { id: 4, name: "y".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Member.into(), }, ], insert_edge: vec![ @@ -76,10 +76,10 @@ fn test_update_channels(cx: &mut AppContext) { assert_channels( &channel_store, &[ - (0, "a".to_string(), false), - (1, "y".to_string(), false), - (0, "b".to_string(), true), - (1, "x".to_string(), true), + (0, "a".to_string(), proto::ChannelRole::Member), + (1, "y".to_string(), proto::ChannelRole::Member), + (0, "b".to_string(), proto::ChannelRole::Admin), + (1, "x".to_string(), proto::ChannelRole::Admin), ], cx, ); @@ -97,16 +97,19 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { id: 0, name: "a".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Admin.into(), }, proto::Channel { id: 1, name: "b".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Admin.into(), }, proto::Channel { id: 2, name: "c".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Admin.into(), }, ], insert_edge: vec![ @@ -119,10 +122,6 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { channel_id: 2, }, ], - channel_permissions: vec![proto::ChannelPermission { - channel_id: 0, - role: proto::ChannelRole::Admin.into(), - }], ..Default::default() }, cx, @@ -132,9 +131,9 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { &channel_store, &[ // - (0, "a".to_string(), true), - (1, "b".to_string(), true), - (2, "c".to_string(), true), + (0, "a".to_string(), proto::ChannelRole::Admin), + (1, "b".to_string(), proto::ChannelRole::Admin), + (2, "c".to_string(), proto::ChannelRole::Admin), ], cx, ); @@ -149,7 +148,11 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { ); // Make sure that the 1/2/3 path is gone - assert_channels(&channel_store, &[(0, "a".to_string(), true)], cx); + assert_channels( + &channel_store, + &[(0, "a".to_string(), proto::ChannelRole::Admin)], + cx, + ); } #[gpui::test] @@ -166,12 +169,17 @@ async fn test_channel_messages(cx: &mut TestAppContext) { id: channel_id, name: "the-channel".to_string(), visibility: proto::ChannelVisibility::Members as i32, + role: proto::ChannelRole::Member.into(), }], ..Default::default() }); cx.foreground().run_until_parked(); cx.read(|cx| { - assert_channels(&channel_store, &[(0, "the-channel".to_string(), false)], cx); + assert_channels( + &channel_store, + &[(0, "the-channel".to_string(), proto::ChannelRole::Member)], + cx, + ); }); let get_users = server.receive::().await.unwrap(); @@ -371,19 +379,13 @@ fn update_channels( #[track_caller] fn assert_channels( channel_store: &ModelHandle, - expected_channels: &[(usize, String, bool)], + expected_channels: &[(usize, String, proto::ChannelRole)], cx: &AppContext, ) { let actual = channel_store.read_with(cx, |store, _| { store .channel_dag_entries() - .map(|(depth, channel)| { - ( - depth, - channel.name.to_string(), - store.is_user_admin(channel.id), - ) - }) + .map(|(depth, channel)| (depth, channel.name.to_string(), channel.role)) .collect::>() }); assert_eq!(actual, expected_channels); diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 5f3d0fc0c7c7775b2fb2bb69257873cf8c8684e2..f4e2602cc6061a76b7806d12afb5eb6fa2e5eb11 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -435,18 +435,103 @@ pub struct NewUserResult { pub signup_device_id: Option, } +#[derive(Debug)] +pub struct MoveChannelResult { + pub participants_to_update: HashMap, + pub participants_to_remove: HashSet, + pub moved_channels: HashSet, +} + +#[derive(Debug)] +pub struct RenameChannelResult { + pub channel: Channel, + pub participants_to_update: HashMap, +} + +#[derive(Debug)] +pub struct CreateChannelResult { + pub channel: Channel, + pub participants_to_update: Vec<(UserId, ChannelsForUser)>, +} + +#[derive(Debug)] +pub struct SetChannelVisibilityResult { + pub participants_to_update: HashMap, + pub participants_to_remove: HashSet, + pub channels_to_remove: Vec, +} + +#[derive(Debug)] +pub struct MembershipUpdated { + pub channel_id: ChannelId, + pub new_channels: ChannelsForUser, + pub removed_channels: Vec, +} + +#[derive(Debug)] +pub enum SetMemberRoleResult { + InviteUpdated(Channel), + MembershipUpdated(MembershipUpdated), +} + +#[derive(Debug)] +pub struct InviteMemberResult { + pub channel: Channel, + pub notifications: NotificationBatch, +} + +#[derive(Debug)] +pub struct RespondToChannelInvite { + pub membership_update: Option, + pub notifications: NotificationBatch, +} + +#[derive(Debug)] +pub struct RemoveChannelMemberResult { + pub membership_update: MembershipUpdated, + pub notification_id: Option, +} + #[derive(FromQueryResult, Debug, PartialEq, Eq, Hash)] pub struct Channel { pub id: ChannelId, pub name: String, pub visibility: ChannelVisibility, + pub role: ChannelRole, +} + +impl Channel { + pub fn to_proto(&self) -> proto::Channel { + proto::Channel { + id: self.id.to_proto(), + name: self.name.clone(), + visibility: self.visibility.into(), + role: self.role.into(), + } + } +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub struct ChannelMember { + pub role: ChannelRole, + pub user_id: UserId, + pub kind: proto::channel_member::Kind, +} + +impl ChannelMember { + pub fn to_proto(&self) -> proto::ChannelMember { + proto::ChannelMember { + role: self.role.into(), + user_id: self.user_id.to_proto(), + kind: self.kind.into(), + } + } } #[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 433444de67e773a8114109326832e02722f3fe5e..5f0df90811cce78167643d176f41cedc7a2d7d9c 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -84,7 +84,7 @@ id_type!(FlagId); id_type!(NotificationId); id_type!(NotificationKindId); -#[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")] @@ -116,6 +116,22 @@ impl ChannelRole { other } } + + pub fn can_see_all_descendants(&self) -> bool { + use ChannelRole::*; + match self { + Admin | Member => true, + Guest | Banned => false, + } + } + + pub fn can_only_see_public_descendants(&self) -> bool { + use ChannelRole::*; + match self { + Guest => true, + Admin | Member | Banned => false, + } + } } impl From for ChannelRole { diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 69f100e6b8351a5e88d8fbda94aeb325e899634e..3aa9cff17116a378c52d459fab7c37cf3bbb85c3 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() { @@ -482,9 +482,7 @@ impl Database { ) .await?; - channel_members = self - .get_channel_participants_internal(channel_id, &*tx) - .await?; + channel_members = self.get_channel_participants(channel_id, &*tx).await?; let collaborators = self .get_channel_buffer_collaborators_internal(channel_id, &*tx) .await?; diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 4ee7625afdf950a04ffad74516d384a3ee299bd3..9e7b1cabf5623f53051e1c6f482712496262687e 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -16,20 +16,39 @@ impl Database { .await } + #[cfg(test)] pub async fn create_root_channel(&self, name: &str, creator_id: UserId) -> Result { - self.create_channel(name, None, creator_id).await + Ok(self + .create_channel(name, None, creator_id) + .await? + .channel + .id) } - pub async fn create_channel( + #[cfg(test)] + pub async fn create_sub_channel( &self, name: &str, - parent: Option, + parent: ChannelId, creator_id: UserId, ) -> Result { + Ok(self + .create_channel(name, Some(parent), creator_id) + .await? + .channel + .id) + } + + pub async fn create_channel( + &self, + name: &str, + parent: Option, + admin_id: UserId, + ) -> Result { let name = Self::sanitize_channel_name(name)?; self.transaction(move |tx| async move { if let Some(parent) = parent { - self.check_user_is_channel_admin(parent, creator_id, &*tx) + self.check_user_is_channel_admin(parent, admin_id, &*tx) .await?; } @@ -71,17 +90,34 @@ impl Database { .await?; } - channel_member::ActiveModel { - id: ActiveValue::NotSet, - channel_id: ActiveValue::Set(channel.id), - user_id: ActiveValue::Set(creator_id), - accepted: ActiveValue::Set(true), - role: ActiveValue::Set(ChannelRole::Admin), + if parent.is_none() { + channel_member::ActiveModel { + id: ActiveValue::NotSet, + channel_id: ActiveValue::Set(channel.id), + user_id: ActiveValue::Set(admin_id), + accepted: ActiveValue::Set(true), + role: ActiveValue::Set(ChannelRole::Admin), + } + .insert(&*tx) + .await?; } - .insert(&*tx) - .await?; - Ok(channel.id) + let participants_to_update = if let Some(parent) = parent { + self.participants_to_notify_for_channel_change(parent, &*tx) + .await? + } else { + vec![] + }; + + Ok(CreateChannelResult { + channel: Channel { + id: channel.id, + visibility: channel.visibility, + name: channel.name, + role: ChannelRole::Admin, + }, + participants_to_update, + }) }) .await } @@ -92,9 +128,9 @@ impl Database { user_id: UserId, connection: ConnectionId, environment: &str, - ) -> Result<(JoinRoom, Option)> { + ) -> Result<(JoinRoom, Option, ChannelRole)> { self.transaction(move |tx| async move { - let mut joined_channel_id = None; + let mut accept_invite_result = None; let channel = channel::Entity::find() .filter(channel::Column::Id.eq(channel_id)) @@ -111,9 +147,7 @@ impl Database { .await? { // note, this may be a parent channel - joined_channel_id = Some(invitation.channel_id); role = Some(invitation.role); - channel_member::Entity::update(channel_member::ActiveModel { accepted: ActiveValue::Set(true), ..invitation.into_active_model() @@ -121,6 +155,11 @@ impl Database { .exec(&*tx) .await?; + accept_invite_result = Some( + self.calculate_membership_updated(channel_id, user_id, &*tx) + .await?, + ); + debug_assert!( self.channel_role_for_user(channel_id, user_id, &*tx) .await? @@ -131,25 +170,29 @@ impl Database { if role.is_none() && channel.as_ref().map(|c| c.visibility) == Some(ChannelVisibility::Public) { + role = Some(ChannelRole::Guest); let channel_id_to_join = self - .most_public_ancestor_for_channel(channel_id, &*tx) + .public_path_to_channel(channel_id, &*tx) .await? + .first() + .cloned() .unwrap_or(channel_id); - // TODO: change this back to Guest. - role = Some(ChannelRole::Member); - joined_channel_id = Some(channel_id_to_join); channel_member::Entity::insert(channel_member::ActiveModel { id: ActiveValue::NotSet, channel_id: ActiveValue::Set(channel_id_to_join), user_id: ActiveValue::Set(user_id), accepted: ActiveValue::Set(true), - // TODO: change this back to Guest. - role: ActiveValue::Set(ChannelRole::Member), + role: ActiveValue::Set(ChannelRole::Guest), }) .exec(&*tx) .await?; + accept_invite_result = Some( + self.calculate_membership_updated(channel_id, user_id, &*tx) + .await?, + ); + debug_assert!( self.channel_role_for_user(channel_id, user_id, &*tx) .await? @@ -168,7 +211,7 @@ impl Database { self.join_channel_room_internal(channel_id, room_id, user_id, connection, &*tx) .await - .map(|jr| (jr, joined_channel_id)) + .map(|jr| (jr, accept_invite_result, role.unwrap())) }) .await } @@ -177,13 +220,17 @@ impl Database { &self, channel_id: ChannelId, visibility: ChannelVisibility, - user_id: UserId, - ) -> Result { + admin_id: UserId, + ) -> Result { self.transaction(move |tx| async move { - self.check_user_is_channel_admin(channel_id, user_id, &*tx) + self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; - let channel = channel::ActiveModel { + let previous_members = self + .get_channel_participant_details_internal(channel_id, &*tx) + .await?; + + channel::ActiveModel { id: ActiveValue::Unchanged(channel_id), visibility: ActiveValue::Set(visibility), ..Default::default() @@ -191,7 +238,62 @@ impl Database { .update(&*tx) .await?; - Ok(channel) + let mut participants_to_update: HashMap = self + .participants_to_notify_for_channel_change(channel_id, &*tx) + .await? + .into_iter() + .collect(); + + let mut channels_to_remove: Vec = vec![]; + let mut participants_to_remove: HashSet = HashSet::default(); + match visibility { + ChannelVisibility::Members => { + let all_descendents: Vec = self + .get_channel_descendants(vec![channel_id], &*tx) + .await? + .into_iter() + .map(|edge| ChannelId::from_proto(edge.channel_id)) + .collect(); + + channels_to_remove = channel::Entity::find() + .filter( + channel::Column::Id + .is_in(all_descendents) + .and(channel::Column::Visibility.eq(ChannelVisibility::Public)), + ) + .all(&*tx) + .await? + .into_iter() + .map(|channel| channel.id) + .collect(); + + channels_to_remove.push(channel_id); + for member in previous_members { + if member.role.can_only_see_public_descendants() { + participants_to_remove.insert(member.user_id); + } + } + } + ChannelVisibility::Public => { + if let Some(public_parent_id) = + self.public_parent_channel_id(channel_id, &*tx).await? + { + let parent_updates = self + .participants_to_notify_for_channel_change(public_parent_id, &*tx) + .await?; + + for (user_id, channels) in parent_updates { + participants_to_update.insert(user_id, channels); + } + } + } + } + + Ok(SetChannelVisibilityResult { + participants_to_update, + participants_to_remove, + channels_to_remove, + }) }) .await } @@ -271,16 +373,11 @@ impl Database { invitee_id: UserId, inviter_id: UserId, role: ChannelRole, - ) -> Result { + ) -> Result { self.transaction(move |tx| async move { self.check_user_is_channel_admin(channel_id, inviter_id, &*tx) .await?; - let channel = channel::Entity::find_by_id(channel_id) - .one(&*tx) - .await? - .ok_or_else(|| anyhow!("no such channel"))?; - channel_member::ActiveModel { id: ActiveValue::NotSet, channel_id: ActiveValue::Set(channel_id), @@ -291,12 +388,24 @@ impl Database { .insert(&*tx) .await?; - Ok(self + let channel = channel::Entity::find_by_id(channel_id) + .one(&*tx) + .await? + .unwrap(); + + let channel = Channel { + id: channel.id, + visibility: channel.visibility, + name: channel.name, + role, + }; + + let notifications = self .create_notification( invitee_id, rpc::Notification::ChannelInvitation { channel_id: channel_id.to_proto(), - channel_name: channel.name, + channel_name: channel.name.clone(), inviter_id: inviter_id.to_proto(), }, true, @@ -304,7 +413,12 @@ impl Database { ) .await? .into_iter() - .collect()) + .collect(); + + Ok(InviteMemberResult { + channel, + notifications, + }) }) .await } @@ -320,13 +434,14 @@ impl Database { pub async fn rename_channel( &self, channel_id: ChannelId, - user_id: UserId, + admin_id: UserId, new_name: &str, - ) -> Result { + ) -> Result { 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, admin_id, &*tx) .await?; let channel = channel::ActiveModel { @@ -337,10 +452,31 @@ impl Database { .update(&*tx) .await?; - Ok(Channel { - id: channel.id, - name: channel.name, - visibility: channel.visibility, + let participants = self + .get_channel_participant_details_internal(channel_id, &*tx) + .await?; + + Ok(RenameChannelResult { + channel: Channel { + id: channel.id, + name: channel.name, + visibility: channel.visibility, + role, + }, + participants_to_update: participants + .iter() + .map(|participant| { + ( + participant.user_id, + Channel { + id: channel.id, + name: new_name.clone(), + visibility: channel.visibility, + role: participant.role, + }, + ) + }) + .collect(), }) }) .await @@ -351,10 +487,10 @@ impl Database { channel_id: ChannelId, user_id: UserId, accept: bool, - ) -> Result { + ) -> Result { self.transaction(move |tx| async move { - let rows_affected = if accept { - channel_member::Entity::update_many() + let membership_update = if accept { + let rows_affected = channel_member::Entity::update_many() .set(channel_member::ActiveModel { accepted: ActiveValue::Set(accept), ..Default::default() @@ -367,9 +503,18 @@ impl Database { ) .exec(&*tx) .await? - .rows_affected + .rows_affected; + + if rows_affected == 0 { + Err(anyhow!("no such invitation"))?; + } + + Some( + self.calculate_membership_updated(channel_id, user_id, &*tx) + .await?, + ) } else { - channel_member::Entity::delete_many() + let rows_affected = channel_member::Entity::delete_many() .filter( channel_member::Column::ChannelId .eq(channel_id) @@ -378,29 +523,87 @@ impl Database { ) .exec(&*tx) .await? - .rows_affected + .rows_affected; + if rows_affected == 0 { + Err(anyhow!("no such invitation"))?; + } + + None }; - if rows_affected == 0 { - Err(anyhow!("no such invitation"))?; + Ok(RespondToChannelInvite { + membership_update, + notifications: self + .mark_notification_as_read_with_response( + user_id, + &rpc::Notification::ChannelInvitation { + channel_id: channel_id.to_proto(), + channel_name: Default::default(), + inviter_id: Default::default(), + }, + accept, + &*tx, + ) + .await? + .into_iter() + .collect(), + }) + }) + .await + } + + async fn calculate_membership_updated( + &self, + channel_id: ChannelId, + user_id: UserId, + tx: &DatabaseTransaction, + ) -> Result { + let mut channel_to_refresh = channel_id; + let mut removed_channels: Vec = Vec::new(); + + // if the user was previously a guest of a parent public channel they may have seen this + // channel (or its descendants) in the tree already. + // Now they have new permissions, the graph of channels needs updating from that point. + if let Some(public_parent) = self.public_parent_channel_id(channel_id, &*tx).await? { + if self + .channel_role_for_user(public_parent, user_id, &*tx) + .await? + == Some(ChannelRole::Guest) + { + channel_to_refresh = public_parent; } + } - Ok(self - .mark_notification_as_read_with_response( - user_id, - &rpc::Notification::ChannelInvitation { - channel_id: channel_id.to_proto(), - channel_name: Default::default(), - inviter_id: Default::default(), - }, - accept, - &*tx, - ) + // remove all descendant channels from the user's tree + removed_channels.append( + &mut self + .get_channel_descendants(vec![channel_to_refresh], &*tx) .await? .into_iter() - .collect()) + .map(|edge| ChannelId::from_proto(edge.channel_id)) + .collect(), + ); + + let new_channels = self + .get_user_channels(user_id, Some(channel_to_refresh), &*tx) + .await?; + + // We only add the current channel to "moved" if the user has lost access, + // otherwise it would be made a root channel on the client. + if !new_channels + .channels + .channels + .iter() + .any(|c| c.id == channel_id) + { + removed_channels.push(channel_id); + } + + Ok(MembershipUpdated { + channel_id, + new_channels, + removed_channels, }) - .await } pub async fn remove_channel_member( @@ -408,7 +611,7 @@ impl Database { channel_id: ChannelId, member_id: UserId, admin_id: UserId, - ) -> Result> { + ) -> Result { self.transaction(|tx| async move { self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; @@ -426,23 +629,30 @@ impl Database { Err(anyhow!("no such member"))?; } - Ok(self - .remove_notification( - member_id, - rpc::Notification::ChannelInvitation { - channel_id: channel_id.to_proto(), - channel_name: Default::default(), - inviter_id: Default::default(), - }, - &*tx, - ) - .await?) + Ok(RemoveChannelMemberResult { + membership_update: self + .calculate_membership_updated(channel_id, member_id, &*tx) + .await?, + notification_id: self + .remove_notification( + member_id, + rpc::Notification::ChannelInvitation { + channel_id: channel_id.to_proto(), + channel_name: Default::default(), + inviter_id: Default::default(), + }, + &*tx, + ) + .await?, + }) }) .await } 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 @@ -452,14 +662,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().copied())) .all(&*tx) .await?; @@ -469,6 +677,7 @@ impl Database { id: channel.id, name: channel.name, visibility: channel.visibility, + role: role_for_channel[&channel.id], }) .collect(); @@ -481,41 +690,7 @@ impl Database { self.transaction(|tx| async move { let tx = tx; - let channel_memberships = channel_member::Entity::find() - .filter( - channel_member::Column::UserId - .eq(user_id) - .and(channel_member::Column::Accepted.eq(true)), - ) - .all(&*tx) - .await?; - - self.get_user_channels(user_id, channel_memberships, &tx) - .await - }) - .await - } - - pub async fn get_channel_for_user( - &self, - channel_id: ChannelId, - user_id: UserId, - ) -> 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) - .await?; - - self.get_user_channels(user_id, channel_membership, &tx) - .await + self.get_user_channels(user_id, None, &tx).await }) .await } @@ -523,17 +698,34 @@ impl Database { pub async fn get_user_channels( &self, user_id: UserId, - channel_memberships: Vec, + parent_channel_id: Option, tx: &DatabaseTransaction, ) -> Result { + // note: we could (maybe) win some efficiency here when parent_channel_id + // is set by getting just the role for that channel, then getting descendants + // with roles attached; but that's not as straightforward as it sounds + // because we need to calculate the path to the channel to make the query + // efficient, which currently requires an extra round trip to the database. + // Fix this later... + let channel_memberships = channel_member::Entity::find() + .filter( + channel_member::Column::UserId + .eq(user_id) + .and(channel_member::Column::Accepted.eq(true)), + ) + .all(&*tx) + .await?; + let mut edges = self .get_channel_descendants(channel_memberships.iter().map(|m| m.channel_id), &*tx) .await?; - let mut role_for_channel: HashMap = HashMap::default(); + let mut role_for_channel: HashMap = HashMap::default(); for membership in channel_memberships.iter() { - role_for_channel.insert(membership.channel_id, membership.role); + let included = + parent_channel_id.is_none() || membership.channel_id == parent_channel_id.unwrap(); + role_for_channel.insert(membership.channel_id, (membership.role, included)); } for ChannelEdge { @@ -544,17 +736,27 @@ impl Database { let parent_id = ChannelId::from_proto(*parent_id); let channel_id = ChannelId::from_proto(*channel_id); debug_assert!(role_for_channel.get(&parent_id).is_some()); - let parent_role = role_for_channel[&parent_id]; - if let Some(existing_role) = role_for_channel.get(&channel_id) { - if existing_role.should_override(parent_role) { - continue; - } + let (parent_role, parent_included) = role_for_channel[&parent_id]; + + if let Some((existing_role, included)) = role_for_channel.get(&channel_id) { + role_for_channel.insert( + channel_id, + (existing_role.max(parent_role), *included || parent_included), + ); + } else { + role_for_channel.insert( + channel_id, + ( + parent_role, + parent_included + || parent_channel_id.is_none() + || Some(channel_id) == parent_channel_id, + ), + ); } - role_for_channel.insert(channel_id, parent_role); } 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() @@ -564,9 +766,10 @@ impl Database { while let Some(row) = rows.next().await { let channel = row?; - let role = role_for_channel[&channel.id]; + let (role, included) = role_for_channel[&channel.id]; - if role == ChannelRole::Banned + if !included + || role == ChannelRole::Banned || role == ChannelRole::Guest && channel.visibility != ChannelVisibility::Public { channels_to_remove.insert(channel.id.0 as u64); @@ -577,11 +780,8 @@ impl Database { id: channel.id, name: channel.name, visibility: channel.visibility, + role, }); - - if role == ChannelRole::Admin { - channels_with_admin_privileges.insert(channel.id); - } } drop(rows); @@ -663,15 +863,64 @@ impl Database { Ok(ChannelsForUser { channels: ChannelGraph { channels, edges }, channel_participants, - channels_with_admin_privileges, unseen_buffer_changes: channel_buffer_changes, channel_messages: unseen_messages, }) } - pub async fn get_channel_members(&self, id: ChannelId) -> Result> { - self.transaction(|tx| async move { self.get_channel_participants_internal(id, &*tx).await }) - .await + async fn participants_to_notify_for_channel_change( + &self, + new_parent: ChannelId, + tx: &DatabaseTransaction, + ) -> Result> { + let mut results: Vec<(UserId, ChannelsForUser)> = Vec::new(); + + let members = self + .get_channel_participant_details_internal(new_parent, &*tx) + .await?; + + for member in members.iter() { + if !member.role.can_see_all_descendants() { + continue; + } + results.push(( + member.user_id, + self.get_user_channels(member.user_id, Some(new_parent), &*tx) + .await?, + )) + } + + let public_parent = self + .public_path_to_channel(new_parent, &*tx) + .await? + .last() + .copied(); + + let Some(public_parent) = public_parent else { + return Ok(results); + }; + + // could save some time in the common case by skipping this if the + // new channel is not public and has no public descendants. + let public_members = if public_parent == new_parent { + members + } else { + self.get_channel_participant_details_internal(public_parent, &*tx) + .await? + }; + + for member in public_members { + if !member.role.can_only_see_public_descendants() { + continue; + }; + results.push(( + member.user_id, + self.get_user_channels(member.user_id, Some(public_parent), &*tx) + .await?, + )) + } + + Ok(results) } pub async fn set_channel_member_role( @@ -680,7 +929,7 @@ impl Database { admin_id: UserId, for_user: UserId, role: ChannelRole, - ) -> Result { + ) -> Result { self.transaction(|tx| async move { self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; @@ -702,7 +951,24 @@ impl Database { update.role = ActiveValue::Set(role); let updated = channel_member::Entity::update(update).exec(&*tx).await?; - Ok(updated) + if !updated.accepted { + let channel = channel::Entity::find_by_id(channel_id) + .one(&*tx) + .await? + .unwrap(); + + return Ok(SetMemberRoleResult::InviteUpdated(Channel { + id: channel.id, + visibility: channel.visibility, + name: channel.name, + role, + })); + } + + Ok(SetMemberRoleResult::MembershipUpdated( + self.calculate_membership_updated(channel_id, for_user, &*tx) + .await?, + )) }) .await } @@ -712,136 +978,146 @@ impl Database { channel_id: ChannelId, user_id: UserId, ) -> Result> { - self.transaction(|tx| async move { - let user_role = self - .check_user_is_channel_member(channel_id, user_id, &*tx) - .await?; + let (role, members) = self + .transaction(move |tx| async move { + let role = self + .check_user_is_channel_participant(channel_id, user_id, &*tx) + .await?; + Ok(( + role, + self.get_channel_participant_details_internal(channel_id, &*tx) + .await?, + )) + }) + .await?; - let channel_visibility = channel::Entity::find() - .filter(channel::Column::Id.eq(channel_id)) - .one(&*tx) - .await? - .map(|channel| channel.visibility) - .unwrap_or(ChannelVisibility::Members); + if role == ChannelRole::Admin { + Ok(members + .into_iter() + .map(|channel_member| channel_member.to_proto()) + .collect()) + } else { + return Ok(members + .into_iter() + .filter_map(|member| { + if member.kind == proto::channel_member::Kind::Invitee { + return None; + } + Some(ChannelMember { + role: member.role, + user_id: member.user_id, + kind: proto::channel_member::Kind::Member, + }) + }) + .map(|channel_member| channel_member.to_proto()) + .collect()); + } + } - #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] - enum QueryMemberDetails { - UserId, - Role, - IsDirectMember, - Accepted, - Visibility, - } + async fn get_channel_participant_details_internal( + &self, + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result> { + let channel_visibility = channel::Entity::find() + .filter(channel::Column::Id.eq(channel_id)) + .one(&*tx) + .await? + .map(|channel| channel.visibility) + .unwrap_or(ChannelVisibility::Members); - let tx = tx; - let ancestor_ids = self.get_channel_ancestors(channel_id, &*tx).await?; - let mut stream = channel_member::Entity::find() - .left_join(channel::Entity) - .filter(channel_member::Column::ChannelId.is_in(ancestor_ids.iter().copied())) - .select_only() - .column(channel_member::Column::UserId) - .column(channel_member::Column::Role) - .column_as( - channel_member::Column::ChannelId.eq(channel_id), - QueryMemberDetails::IsDirectMember, - ) - .column(channel_member::Column::Accepted) - .column(channel::Column::Visibility) - .into_values::<_, QueryMemberDetails>() - .stream(&*tx) - .await?; + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] + enum QueryMemberDetails { + UserId, + Role, + IsDirectMember, + Accepted, + Visibility, + } - struct UserDetail { - kind: Kind, - channel_role: ChannelRole, + let tx = tx; + let ancestor_ids = self.get_channel_ancestors(channel_id, &*tx).await?; + let mut stream = channel_member::Entity::find() + .left_join(channel::Entity) + .filter(channel_member::Column::ChannelId.is_in(ancestor_ids.iter().copied())) + .select_only() + .column(channel_member::Column::UserId) + .column(channel_member::Column::Role) + .column_as( + channel_member::Column::ChannelId.eq(channel_id), + QueryMemberDetails::IsDirectMember, + ) + .column(channel_member::Column::Accepted) + .column(channel::Column::Visibility) + .into_values::<_, QueryMemberDetails>() + .stream(&*tx) + .await?; + + let mut user_details: HashMap = HashMap::default(); + + while let Some(user_membership) = stream.next().await { + let (user_id, channel_role, is_direct_member, is_invite_accepted, visibility): ( + UserId, + ChannelRole, + bool, + bool, + ChannelVisibility, + ) = user_membership?; + let kind = match (is_direct_member, is_invite_accepted) { + (true, true) => proto::channel_member::Kind::Member, + (true, false) => proto::channel_member::Kind::Invitee, + (false, true) => proto::channel_member::Kind::AncestorMember, + (false, false) => continue, + }; + + if channel_role == ChannelRole::Guest + && visibility != ChannelVisibility::Public + && channel_visibility != ChannelVisibility::Public + { + continue; } - let mut user_details: HashMap = HashMap::default(); - - while let Some(user_membership) = stream.next().await { - let (user_id, channel_role, is_direct_member, is_invite_accepted, visibility): ( - UserId, - ChannelRole, - bool, - bool, - ChannelVisibility, - ) = user_membership?; - let kind = match (is_direct_member, is_invite_accepted) { - (true, true) => proto::channel_member::Kind::Member, - (true, false) => proto::channel_member::Kind::Invitee, - (false, true) => proto::channel_member::Kind::AncestorMember, - (false, false) => continue, - }; - - if channel_role == ChannelRole::Guest - && visibility != ChannelVisibility::Public - && channel_visibility != ChannelVisibility::Public - { - continue; - } - if let Some(details_mut) = user_details.get_mut(&user_id) { - if channel_role.should_override(details_mut.channel_role) { - details_mut.channel_role = channel_role; - } - if kind == Kind::Member { - details_mut.kind = kind; - // the UI is going to be a bit confusing if you already have permissions - // that are greater than or equal to the ones you're being invited to. - } else if kind == Kind::Invitee && details_mut.kind == Kind::AncestorMember { - details_mut.kind = kind; - } - } else { - user_details.insert(user_id, UserDetail { kind, channel_role }); + if let Some(details_mut) = user_details.get_mut(&user_id) { + if channel_role.should_override(details_mut.role) { + details_mut.role = channel_role; } + if kind == Kind::Member { + details_mut.kind = kind; + // the UI is going to be a bit confusing if you already have permissions + // that are greater than or equal to the ones you're being invited to. + } else if kind == Kind::Invitee && details_mut.kind == Kind::AncestorMember { + details_mut.kind = kind; + } + } else { + user_details.insert( + user_id, + ChannelMember { + user_id, + kind, + role: channel_role, + }, + ); } + } - Ok(user_details - .into_iter() - .filter_map(|(user_id, mut details)| { - // If the user is not an admin, don't give them as much - // information about the other members. - if user_role != ChannelRole::Admin { - if details.kind == Kind::Invitee - || details.channel_role == ChannelRole::Banned - { - return None; - } - - if details.channel_role == ChannelRole::Admin { - details.channel_role = ChannelRole::Member; - } - } - - Some(proto::ChannelMember { - user_id: user_id.to_proto(), - kind: details.kind.into(), - role: details.channel_role.into(), - }) - }) - .collect()) - }) - .await + Ok(user_details + .into_iter() + .map(|(_, details)| details) + .collect()) } - pub async fn get_channel_participants_internal( + pub async fn get_channel_participants( &self, - id: ChannelId, + channel_id: ChannelId, tx: &DatabaseTransaction, ) -> Result> { - let ancestor_ids = self.get_channel_ancestors(id, tx).await?; - let user_ids = channel_member::Entity::find() - .distinct() - .filter( - channel_member::Column::ChannelId - .is_in(ancestor_ids.iter().copied()) - .and(channel_member::Column::Accepted.eq(true)), - ) - .select_only() - .column(channel_member::Column::UserId) - .into_values::<_, QueryUserIds>() - .all(&*tx) + let participants = self + .get_channel_participant_details_internal(channel_id, &*tx) .await?; - Ok(user_ids) + Ok(participants + .into_iter() + .map(|member| member.user_id) + .collect()) } pub async fn check_user_is_channel_admin( @@ -849,9 +1125,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) @@ -881,10 +1158,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" @@ -910,12 +1188,37 @@ impl Database { Ok(row) } - pub async fn most_public_ancestor_for_channel( + pub async fn parent_channel_id( + &self, + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result> { + let path = self.path_to_channel(channel_id, &*tx).await?; + if path.len() >= 2 { + Ok(Some(path[path.len() - 2])) + } else { + Ok(None) + } + } + + pub async fn public_parent_channel_id( &self, channel_id: ChannelId, tx: &DatabaseTransaction, ) -> Result> { - // Note: if there are many paths to a channel, this will return just one + let path = self.public_path_to_channel(channel_id, &*tx).await?; + if path.len() >= 2 && path.last().copied() == Some(channel_id) { + Ok(Some(path[path.len() - 2])) + } else { + Ok(path.last().copied()) + } + } + + pub async fn path_to_channel( + &self, + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result> { 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) @@ -923,15 +1226,23 @@ impl Database { .await?; let Some(path) = arbitary_path else { - return Ok(None); + return Ok(vec![]); }; - let ancestor_ids: Vec = path + Ok(path .id_path .trim_matches('/') .split('/') .map(|id| ChannelId::from_proto(id.parse().unwrap())) - .collect(); + .collect()) + } + + pub async fn public_path_to_channel( + &self, + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result> { + let ancestor_ids = self.path_to_channel(channel_id, &*tx).await?; let rows = channel::Entity::find() .filter(channel::Column::Id.is_in(ancestor_ids.iter().copied())) @@ -945,13 +1256,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( @@ -1122,7 +1430,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?; @@ -1133,6 +1442,7 @@ impl Database { Ok(Channel { id: channel.id, visibility: channel.visibility, + role, name: channel.name, }) }) @@ -1182,9 +1492,6 @@ impl Database { to: ChannelId, ) -> Result { self.transaction(|tx| async move { - // Note that even with these maxed permissions, this linking operation - // is still insecure because you can't remove someone's permissions to a - // channel if they've linked the channel to one where they're an admin. self.check_user_is_channel_admin(channel, user, &*tx) .await?; @@ -1249,16 +1556,7 @@ impl Database { .await?; } - let membership = channel_member::Entity::find() - .filter( - channel_member::Column::ChannelId - .eq(channel) - .and(channel_member::Column::UserId.eq(user)), - ) - .all(tx) - .await?; - - let mut channel_info = self.get_user_channels(user, membership, &*tx).await?; + let mut channel_info = self.get_user_channels(user, Some(channel), &*tx).await?; channel_info.channels.edges.push(ChannelEdge { channel_id: channel.to_proto(), @@ -1277,9 +1575,6 @@ impl Database { from: ChannelId, ) -> Result<()> { self.transaction(|tx| async move { - // Note that even with these maxed permissions, this linking operation - // is still insecure because you can't remove someone's permissions to a - // channel if they've linked the channel to one where they're an admin. self.check_user_is_channel_admin(channel, user, &*tx) .await?; @@ -1334,6 +1629,7 @@ impl Database { } }) .collect(); + channel_path::Entity::insert_many(root_paths) .exec(&*tx) .await?; @@ -1342,32 +1638,64 @@ impl Database { Ok(()) } - /// Move a channel from one parent to another, returns the - /// Channels that were moved for notifying clients + /// Move a channel from one parent to another pub async fn move_channel( &self, - user: UserId, - channel: ChannelId, - from: ChannelId, - to: ChannelId, - ) -> Result { - if from == to { - return Ok(ChannelGraph { - channels: vec![], - edges: vec![], - }); - } - + channel_id: ChannelId, + old_parent_id: Option, + new_parent_id: ChannelId, + admin_id: UserId, + ) -> Result> { self.transaction(|tx| async move { - self.check_user_is_channel_admin(channel, user, &*tx) + self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; - let moved_channels = self.link_channel_internal(user, channel, to, &*tx).await?; + debug_assert_eq!( + self.parent_channel_id(channel_id, &*tx).await?, + old_parent_id + ); - self.unlink_channel_internal(user, channel, from, &*tx) + if old_parent_id == Some(new_parent_id) { + return Ok(None); + } + let previous_participants = self + .get_channel_participant_details_internal(channel_id, &*tx) + .await?; + + self.link_channel_internal(admin_id, channel_id, new_parent_id, &*tx) .await?; - Ok(moved_channels) + if let Some(from) = old_parent_id { + self.unlink_channel_internal(admin_id, channel_id, from, &*tx) + .await?; + } + + let participants_to_update: HashMap = self + .participants_to_notify_for_channel_change(new_parent_id, &*tx) + .await? + .into_iter() + .collect(); + + let mut moved_channels: HashSet = HashSet::default(); + moved_channels.insert(channel_id); + for edge in self.get_channel_descendants([channel_id], &*tx).await? { + moved_channels.insert(ChannelId::from_proto(edge.channel_id)); + } + + let mut participants_to_remove: HashSet = HashSet::default(); + for participant in previous_participants { + if participant.kind == proto::channel_member::Kind::AncestorMember { + if !participants_to_update.contains_key(&participant.user_id) { + participants_to_remove.insert(participant.user_id); + } + } + } + + Ok(Some(MoveChannelResult { + participants_to_remove, + participants_to_update, + moved_channels, + })) }) .await } diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index d406cbb091389cdcc6073873752ffa4e14000fad..562c4e45c45eb86ab1e2b0b1bc67d535bfa41eb7 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -12,7 +12,7 @@ impl Database { user_id: UserId, ) -> 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?; channel_chat_participant::ActiveModel { id: ActiveValue::NotSet, @@ -80,7 +80,7 @@ impl Database { before_message_id: Option, ) -> 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 mut condition = @@ -203,6 +203,9 @@ impl Database { nonce: u128, ) -> Result { self.transaction(|tx| async move { + self.check_user_is_channel_participant(channel_id, user_id, &*tx) + .await?; + let mut rows = channel_chat_participant::Entity::find() .filter(channel_chat_participant::Column::ChannelId.eq(channel_id)) .stream(&*tx) @@ -307,9 +310,7 @@ impl Database { } } - let mut channel_members = self - .get_channel_participants_internal(channel_id, &*tx) - .await?; + let mut channel_members = self.get_channel_participants(channel_id, &*tx).await?; channel_members.retain(|member| !participant_user_ids.contains(member)); Ok(CreatedChannelMessage { diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index d2120495b05063550765eeb7fbe151512d524403..630d51cfe693cb8440b1b3507fa30557915073ef 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -53,9 +53,7 @@ impl Database { let (channel_id, room) = self.get_channel_room(room_id, &tx).await?; let channel_members; if let Some(channel_id) = channel_id { - channel_members = self - .get_channel_participants_internal(channel_id, &tx) - .await?; + channel_members = self.get_channel_participants(channel_id, &tx).await?; } else { channel_members = Vec::new(); @@ -423,9 +421,7 @@ impl Database { .await?; let room = self.get_room(room_id, &tx).await?; - let channel_members = self - .get_channel_participants_internal(channel_id, &tx) - .await?; + let channel_members = self.get_channel_participants(channel_id, &tx).await?; Ok(JoinRoom { room, channel_id: Some(channel_id), @@ -724,8 +720,7 @@ impl Database { let (channel_id, room) = self.get_channel_room(room_id, &tx).await?; let channel_members = if let Some(channel_id) = channel_id { - self.get_channel_participants_internal(channel_id, &tx) - .await? + self.get_channel_participants(channel_id, &tx).await? } else { Vec::new() }; @@ -883,8 +878,7 @@ impl Database { }; let channel_members = if let Some(channel_id) = channel_id { - self.get_channel_participants_internal(channel_id, &tx) - .await? + self.get_channel_participants(channel_id, &tx).await? } else { Vec::new() }; diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index 83154b9a0dfbbdb63b13e6791bb8a150fadc434a..8f2939040d5544bfb66aa0c2f09a9b5e991176a7 100644 --- a/crates/collab/src/db/tests.rs +++ b/crates/collab/src/db/tests.rs @@ -11,7 +11,7 @@ use rpc::proto::ChannelEdge; use sea_orm::ConnectionTrait; use sqlx::migrate::MigrateDatabase; use std::sync::{ - atomic::{AtomicI32, Ordering::SeqCst}, + atomic::{AtomicI32, AtomicU32, Ordering::SeqCst}, Arc, }; @@ -154,17 +154,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, }) } @@ -193,3 +197,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, SeqCst), + owner_id: server.0 as u32, + } +} diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 556437e45b6e5471b93619e0d65f1d4430d8dedf..32cba2fdd90ea2f669a675c0f28ccf3ff5c8ce2f 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -1,7 +1,9 @@ +use std::sync::Arc; + use crate::{ db::{ queries::channels::ChannelGraph, - tests::{graph, new_test_user, TEST_RELEASE_CHANNEL}, + tests::{graph, new_test_connection, new_test_user, TEST_RELEASE_CHANNEL}, ChannelId, ChannelRole, Database, NewUserParams, RoomId, }, test_both_dbs, @@ -11,36 +13,12 @@ use rpc::{ proto::{self}, ConnectionId, }; -use std::sync::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, - }, - ) - .await - .unwrap() - .user_id; - - let b_id = db - .create_user( - "user2@example.com", - false, - NewUserParams { - github_login: "user2".into(), - github_user_id: 6, - }, - ) - .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(); @@ -55,28 +33,28 @@ async fn test_channels(db: &Arc) { .await .unwrap(); - let crdb_id = db.create_channel("crdb", Some(zed_id), a_id).await.unwrap(); + let crdb_id = db.create_sub_channel("crdb", zed_id, a_id).await.unwrap(); let livestreaming_id = db - .create_channel("livestreaming", Some(zed_id), a_id) + .create_sub_channel("livestreaming", zed_id, a_id) .await .unwrap(); let replace_id = db - .create_channel("replace", Some(zed_id), a_id) + .create_sub_channel("replace", zed_id, a_id) .await .unwrap(); - let mut members = db.get_channel_members(replace_id).await.unwrap(); + let mut members = db + .transaction(|tx| async move { Ok(db.get_channel_participants(replace_id, &*tx).await?) }) + .await + .unwrap(); members.sort(); assert_eq!(members, &[a_id, b_id]); let rust_id = db.create_root_channel("rust", a_id).await.unwrap(); - let cargo_id = db - .create_channel("cargo", Some(rust_id), a_id) - .await - .unwrap(); + let cargo_id = db.create_sub_channel("cargo", rust_id, a_id).await.unwrap(); let cargo_ra_id = db - .create_channel("cargo-ra", Some(cargo_id), a_id) + .create_sub_channel("cargo-ra", cargo_id, a_id) .await .unwrap(); @@ -85,13 +63,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), @@ -108,10 +86,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), @@ -136,10 +114,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), @@ -173,35 +151,13 @@ 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, - }, - ) - .await - .unwrap() - .user_id; - let user_2 = db - .create_user( - "user2@example.com", - false, - NewUserParams { - github_login: "user2".into(), - github_user_id: 6, - }, - ) - .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(); // can join a room with membership to its channel - let (joined_room, _) = db + let (joined_room, _, _) = db .join_channel( channel_1, user_1, @@ -305,7 +261,7 @@ async fn test_channel_invites(db: &Arc) { .unwrap(); let channel_1_3 = db - .create_channel("channel_3", Some(channel_1_1), user_1) + .create_sub_channel("channel_3", channel_1_1, user_1) .await .unwrap(); @@ -318,7 +274,7 @@ async fn test_channel_invites(db: &Arc) { &[ proto::ChannelMember { user_id: user_1.to_proto(), - kind: proto::channel_member::Kind::Member.into(), + kind: proto::channel_member::Kind::AncestorMember.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { @@ -407,20 +363,17 @@ async fn test_db_channel_moving(db: &Arc) { let zed_id = db.create_root_channel("zed", a_id).await.unwrap(); - let crdb_id = db.create_channel("crdb", Some(zed_id), a_id).await.unwrap(); + let crdb_id = db.create_sub_channel("crdb", zed_id, a_id).await.unwrap(); - let gpui2_id = db - .create_channel("gpui2", Some(zed_id), a_id) - .await - .unwrap(); + let gpui2_id = db.create_sub_channel("gpui2", zed_id, a_id).await.unwrap(); let livestreaming_id = db - .create_channel("livestreaming", Some(crdb_id), a_id) + .create_sub_channel("livestreaming", crdb_id, a_id) .await .unwrap(); let livestreaming_dag_id = db - .create_channel("livestreaming_dag", Some(livestreaming_id), a_id) + .create_sub_channel("livestreaming_dag", livestreaming_id, a_id) .await .unwrap(); @@ -447,299 +400,311 @@ async fn test_db_channel_moving(db: &Arc) { .await .is_err()); - // ======================================================================== - // Make a link - db.link_channel(a_id, livestreaming_id, zed_id) - .await - .unwrap(); - - // DAG is now: - // /- gpui2 - // zed -- crdb - livestreaming - livestreaming_dag - // \---------/ - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (gpui2_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(crdb_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - ], - ); - - // ======================================================================== - // Create a new channel below a channel with multiple parents - let livestreaming_dag_sub_id = db - .create_channel("livestreaming_dag_sub", Some(livestreaming_dag_id), a_id) - .await - .unwrap(); - - // DAG is now: - // /- gpui2 - // zed -- crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id - // \---------/ - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (gpui2_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(crdb_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Test a complex DAG by making another link - let returned_channels = db - .link_channel(a_id, livestreaming_dag_sub_id, livestreaming_id) - .await - .unwrap(); - - // DAG is now: - // /- gpui2 /---------------------\ - // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id - // \--------/ - - // make sure we're getting just the new link - // Not using the assert_dag helper because we want to make sure we're returning the full data - pretty_assertions::assert_eq!( - returned_channels, - graph( - &[(livestreaming_dag_sub_id, "livestreaming_dag_sub")], - &[(livestreaming_dag_sub_id, livestreaming_id)] - ) - ); - - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (gpui2_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(crdb_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Test a complex DAG by making another link - let returned_channels = db - .link_channel(a_id, livestreaming_id, gpui2_id) - .await - .unwrap(); - - // DAG is now: - // /- gpui2 -\ /---------------------\ - // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub_id - // \---------/ - - // Make sure that we're correctly getting the full sub-dag - pretty_assertions::assert_eq!( - returned_channels, - graph( - &[ - (livestreaming_id, "livestreaming"), - (livestreaming_dag_id, "livestreaming_dag"), - (livestreaming_dag_sub_id, "livestreaming_dag_sub"), - ], - &[ - (livestreaming_id, gpui2_id), - (livestreaming_dag_id, livestreaming_id), - (livestreaming_dag_sub_id, livestreaming_id), - (livestreaming_dag_sub_id, livestreaming_dag_id), - ] - ) - ); - - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (gpui2_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(crdb_id)), - (livestreaming_id, Some(gpui2_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Test unlinking in a complex DAG by removing the inner link - db.unlink_channel(a_id, livestreaming_dag_sub_id, livestreaming_id) - .await - .unwrap(); - - // DAG is now: - // /- gpui2 -\ - // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub - // \---------/ - - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (gpui2_id, Some(zed_id)), - (livestreaming_id, Some(gpui2_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(crdb_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Test unlinking in a complex DAG by removing the inner link - db.unlink_channel(a_id, livestreaming_id, gpui2_id) - .await - .unwrap(); - - // DAG is now: - // /- gpui2 - // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub - // \---------/ - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (gpui2_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(crdb_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Test moving DAG nodes by moving livestreaming to be below gpui2 - db.move_channel(a_id, livestreaming_id, crdb_id, gpui2_id) - .await - .unwrap(); - - // DAG is now: - // /- gpui2 -- livestreaming - livestreaming_dag - livestreaming_dag_sub - // zed - crdb / - // \---------/ - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (gpui2_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(gpui2_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Deleting a channel should not delete children that still have other parents - db.delete_channel(gpui2_id, a_id).await.unwrap(); - - // DAG is now: - // zed - crdb - // \- livestreaming - livestreaming_dag - livestreaming_dag_sub - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Unlinking a channel from it's parent should automatically promote it to a root channel - db.unlink_channel(a_id, crdb_id, zed_id).await.unwrap(); - - // DAG is now: - // crdb - // zed - // \- livestreaming - livestreaming_dag - livestreaming_dag_sub - - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, None), - (livestreaming_id, Some(zed_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // You should be able to move a root channel into a non-root channel - db.link_channel(a_id, crdb_id, zed_id).await.unwrap(); - - // DAG is now: - // zed - crdb - // \- livestreaming - livestreaming_dag - livestreaming_dag_sub - - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // ======================================================================== - // Prep for DAG deletion test - db.link_channel(a_id, livestreaming_id, crdb_id) - .await - .unwrap(); - - // DAG is now: - // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub - // \--------/ - - let result = db.get_channels_for_user(a_id).await.unwrap(); - assert_dag( - result.channels, - &[ - (zed_id, None), - (crdb_id, Some(zed_id)), - (livestreaming_id, Some(zed_id)), - (livestreaming_id, Some(crdb_id)), - (livestreaming_dag_id, Some(livestreaming_id)), - (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), - ], - ); - - // Deleting the parent of a DAG should delete the whole DAG: - db.delete_channel(zed_id, a_id).await.unwrap(); - let result = db.get_channels_for_user(a_id).await.unwrap(); - - assert!(result.channels.is_empty()) + // // ======================================================================== + // // Make a link + // db.link_channel(a_id, livestreaming_id, zed_id) + // .await + // .unwrap(); + + // // DAG is now: + // // /- gpui2 + // // zed -- crdb - livestreaming - livestreaming_dag + // // \---------/ + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (gpui2_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(crdb_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // ], + // ); + + // // ======================================================================== + // // Create a new channel below a channel with multiple parents + // let livestreaming_dag_sub_id = db + // .create_channel("livestreaming_dag_sub", Some(livestreaming_dag_id), a_id) + // .await + // .unwrap(); + + // // DAG is now: + // // /- gpui2 + // // zed -- crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id + // // \---------/ + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (gpui2_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(crdb_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Test a complex DAG by making another link + // let returned_channels = db + // .link_channel(a_id, livestreaming_dag_sub_id, livestreaming_id) + // .await + // .unwrap(); + + // // DAG is now: + // // /- gpui2 /---------------------\ + // // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub_id + // // \--------/ + + // // make sure we're getting just the new link + // // Not using the assert_dag helper because we want to make sure we're returning the full data + // pretty_assertions::assert_eq!( + // returned_channels, + // graph( + // &[( + // livestreaming_dag_sub_id, + // "livestreaming_dag_sub", + // ChannelRole::Admin + // )], + // &[(livestreaming_dag_sub_id, livestreaming_id)] + // ) + // ); + + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (gpui2_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(crdb_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Test a complex DAG by making another link + // let returned_channels = db + // .link_channel(a_id, livestreaming_id, gpui2_id) + // .await + // .unwrap(); + + // // DAG is now: + // // /- gpui2 -\ /---------------------\ + // // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub_id + // // \---------/ + + // // Make sure that we're correctly getting the full sub-dag + // pretty_assertions::assert_eq!( + // returned_channels, + // graph( + // &[ + // (livestreaming_id, "livestreaming", ChannelRole::Admin), + // ( + // livestreaming_dag_id, + // "livestreaming_dag", + // ChannelRole::Admin + // ), + // ( + // livestreaming_dag_sub_id, + // "livestreaming_dag_sub", + // ChannelRole::Admin + // ), + // ], + // &[ + // (livestreaming_id, gpui2_id), + // (livestreaming_dag_id, livestreaming_id), + // (livestreaming_dag_sub_id, livestreaming_id), + // (livestreaming_dag_sub_id, livestreaming_dag_id), + // ] + // ) + // ); + + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (gpui2_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(crdb_id)), + // (livestreaming_id, Some(gpui2_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Test unlinking in a complex DAG by removing the inner link + // db.unlink_channel(a_id, livestreaming_dag_sub_id, livestreaming_id) + // .await + // .unwrap(); + + // // DAG is now: + // // /- gpui2 -\ + // // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub + // // \---------/ + + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (gpui2_id, Some(zed_id)), + // (livestreaming_id, Some(gpui2_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(crdb_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Test unlinking in a complex DAG by removing the inner link + // db.unlink_channel(a_id, livestreaming_id, gpui2_id) + // .await + // .unwrap(); + + // // DAG is now: + // // /- gpui2 + // // zed - crdb -- livestreaming - livestreaming_dag - livestreaming_dag_sub + // // \---------/ + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (gpui2_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(crdb_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Test moving DAG nodes by moving livestreaming to be below gpui2 + // db.move_channel(livestreaming_id, Some(crdb_id), gpui2_id, a_id) + // .await + // .unwrap(); + + // // DAG is now: + // // /- gpui2 -- livestreaming - livestreaming_dag - livestreaming_dag_sub + // // zed - crdb / + // // \---------/ + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (gpui2_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(gpui2_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Deleting a channel should not delete children that still have other parents + // db.delete_channel(gpui2_id, a_id).await.unwrap(); + + // // DAG is now: + // // zed - crdb + // // \- livestreaming - livestreaming_dag - livestreaming_dag_sub + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Unlinking a channel from it's parent should automatically promote it to a root channel + // db.unlink_channel(a_id, crdb_id, zed_id).await.unwrap(); + + // // DAG is now: + // // crdb + // // zed + // // \- livestreaming - livestreaming_dag - livestreaming_dag_sub + + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, None), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // You should be able to move a root channel into a non-root channel + // db.link_channel(a_id, crdb_id, zed_id).await.unwrap(); + + // // DAG is now: + // // zed - crdb + // // \- livestreaming - livestreaming_dag - livestreaming_dag_sub + + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // ======================================================================== + // // Prep for DAG deletion test + // db.link_channel(a_id, livestreaming_id, crdb_id) + // .await + // .unwrap(); + + // // DAG is now: + // // zed - crdb - livestreaming - livestreaming_dag - livestreaming_dag_sub + // // \--------/ + + // let result = db.get_channels_for_user(a_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (crdb_id, Some(zed_id)), + // (livestreaming_id, Some(zed_id)), + // (livestreaming_id, Some(crdb_id)), + // (livestreaming_dag_id, Some(livestreaming_id)), + // (livestreaming_dag_sub_id, Some(livestreaming_dag_id)), + // ], + // ); + + // // Deleting the parent of a DAG should delete the whole DAG: + // db.delete_channel(zed_id, a_id).await.unwrap(); + // let result = db.get_channels_for_user(a_id).await.unwrap(); + + // assert!(result.channels.is_empty()) } test_both_dbs!( @@ -765,12 +730,12 @@ async fn test_db_channel_moving_bugs(db: &Arc) { let zed_id = db.create_root_channel("zed", user_id).await.unwrap(); let projects_id = db - .create_channel("projects", Some(zed_id), user_id) + .create_sub_channel("projects", zed_id, user_id) .await .unwrap(); let livestreaming_id = db - .create_channel("livestreaming", Some(projects_id), user_id) + .create_sub_channel("livestreaming", projects_id, user_id) .await .unwrap(); @@ -778,25 +743,37 @@ async fn test_db_channel_moving_bugs(db: &Arc) { // Move to same parent should be a no-op assert!(db - .move_channel(user_id, projects_id, zed_id, zed_id) + .move_channel(projects_id, Some(zed_id), zed_id, user_id) .await .unwrap() - .is_empty()); - - // Stranding a channel should retain it's sub channels - db.unlink_channel(user_id, projects_id, zed_id) - .await - .unwrap(); + .is_none()); let result = db.get_channels_for_user(user_id).await.unwrap(); assert_dag( result.channels, &[ (zed_id, None), - (projects_id, None), + (projects_id, Some(zed_id)), (livestreaming_id, Some(projects_id)), ], ); + + // Stranding a channel should retain it's sub channels + // Commented out as we don't fix permissions when this happens yet. + // + // db.unlink_channel(user_id, projects_id, zed_id) + // .await + // .unwrap(); + + // let result = db.get_channels_for_user(user_id).await.unwrap(); + // assert_dag( + // result.channels, + // &[ + // (zed_id, None), + // (projects_id, None), + // (livestreaming_id, Some(projects_id)), + // ], + // ); } test_both_dbs!( @@ -812,11 +789,11 @@ async fn test_user_is_channel_participant(db: &Arc) { let zed_channel = db.create_root_channel("zed", admin).await.unwrap(); let active_channel = db - .create_channel("active", Some(zed_channel), admin) + .create_sub_channel("active", zed_channel, admin) .await .unwrap(); let vim_channel = db - .create_channel("vim", Some(active_channel), admin) + .create_sub_channel("vim", active_channel, admin) .await .unwrap(); @@ -859,7 +836,7 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::Member.into(), + kind: proto::channel_member::Kind::AncestorMember.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { @@ -917,7 +894,7 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::Member.into(), + kind: proto::channel_member::Kind::AncestorMember.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { @@ -958,7 +935,7 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::Member.into(), + kind: proto::channel_member::Kind::AncestorMember.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { @@ -1006,7 +983,7 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::Member.into(), + kind: proto::channel_member::Kind::AncestorMember.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { @@ -1041,17 +1018,17 @@ async fn test_user_joins_correct_channel(db: &Arc) { let zed_channel = db.create_root_channel("zed", admin).await.unwrap(); let active_channel = db - .create_channel("active", Some(zed_channel), admin) + .create_sub_channel("active", zed_channel, admin) .await .unwrap(); let vim_channel = db - .create_channel("vim", Some(active_channel), admin) + .create_sub_channel("vim", active_channel, admin) .await .unwrap(); let vim2_channel = db - .create_channel("vim2", Some(vim_channel), admin) + .create_sub_channel("vim2", vim_channel, admin) .await .unwrap(); @@ -1068,15 +1045,52 @@ 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 }, - ) + .transaction(|tx| async move { + Ok(db + .public_path_to_channel(vim_channel, &tx) + .await? + .first() + .cloned()) + }) .await .unwrap(); 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!(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)]) { let mut actual_map: HashMap> = HashMap::default(); diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index 97b3142930abc5d5f641825f7844b63e8c79a5dc..10d9778612f4d6a66410ee1e77b4d04ff0b5a771 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -15,18 +15,22 @@ test_both_dbs!( async fn test_channel_message_retrieval(db: &Arc) { let user = new_test_user(db, "user@example.com").await; - let channel = db.create_channel("channel", None, user).await.unwrap(); + let result = db.create_channel("channel", None, user).await.unwrap(); let owner_id = db.create_server("test").await.unwrap().0 as u32; - db.join_channel_chat(channel, rpc::ConnectionId { owner_id, id: 0 }, user) - .await - .unwrap(); + db.join_channel_chat( + result.channel.id, + rpc::ConnectionId { owner_id, id: 0 }, + user, + ) + .await + .unwrap(); let mut all_messages = Vec::new(); for i in 0..10 { all_messages.push( db.create_channel_message( - channel, + result.channel.id, user, &i.to_string(), &[], @@ -41,7 +45,7 @@ async fn test_channel_message_retrieval(db: &Arc) { } let messages = db - .get_channel_messages(channel, user, 3, None) + .get_channel_messages(result.channel.id, user, 3, None) .await .unwrap() .into_iter() @@ -51,7 +55,7 @@ async fn test_channel_message_retrieval(db: &Arc) { let messages = db .get_channel_messages( - channel, + result.channel.id, user, 4, Some(MessageId::from_proto(all_messages[6])), @@ -74,7 +78,7 @@ async fn test_channel_message_nonces(db: &Arc) { let user_a = new_test_user(db, "user_a@example.com").await; let user_b = new_test_user(db, "user_b@example.com").await; let user_c = new_test_user(db, "user_c@example.com").await; - let channel = db.create_channel("channel", None, user_a).await.unwrap(); + let channel = db.create_root_channel("channel", user_a).await.unwrap(); db.invite_channel_member(channel, user_b, user_a, ChannelRole::Member) .await .unwrap(); @@ -206,8 +210,8 @@ async fn test_unseen_channel_messages(db: &Arc) { let user = new_test_user(db, "user_a@example.com").await; let observer = new_test_user(db, "user_b@example.com").await; - let channel_1 = db.create_channel("channel", None, user).await.unwrap(); - let channel_2 = db.create_channel("channel-2", None, user).await.unwrap(); + let channel_1 = db.create_root_channel("channel", user).await.unwrap(); + let channel_2 = db.create_root_channel("channel-2", user).await.unwrap(); db.invite_channel_member(channel_1, observer, user, ChannelRole::Member) .await @@ -362,7 +366,12 @@ async fn test_channel_message_mentions(db: &Arc) { let user_b = new_test_user(db, "user_b@example.com").await; let user_c = new_test_user(db, "user_c@example.com").await; - let channel = db.create_channel("channel", None, user_a).await.unwrap(); + let channel = db + .create_channel("channel", None, user_a) + .await + .unwrap() + .channel + .id; db.invite_channel_member(channel, user_b, user_a, ChannelRole::Member) .await .unwrap(); diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 5a29861351daf1ff91ecb382e8a4ad7614c01a66..dda638e107a069e089124ece025d627604253906 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -3,8 +3,11 @@ mod connection_pool; use crate::{ auth, db::{ - self, BufferId, ChannelId, ChannelVisibility, ChannelsForUser, CreatedChannelMessage, - Database, MessageId, NotificationId, ProjectId, RoomId, ServerId, User, UserId, + self, BufferId, ChannelId, ChannelRole, ChannelsForUser, CreateChannelResult, + CreatedChannelMessage, Database, InviteMemberResult, MembershipUpdated, MessageId, + MoveChannelResult, NotificationId, ProjectId, RemoveChannelMemberResult, + RenameChannelResult, RespondToChannelInvite, RoomId, ServerId, SetChannelVisibilityResult, + User, UserId, }, executor::Executor, AppState, Result, @@ -38,8 +41,8 @@ use lazy_static::lazy_static; use prometheus::{register_int_gauge, IntGauge}; use rpc::{ proto::{ - self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, - LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, + self, Ack, AnyTypedEnvelope, EntityMessage, EnvelopedMessage, LiveKitConnectionInfo, + RequestMessage, UpdateChannelBufferCollaborators, }, Connection, ConnectionId, Peer, Receipt, TypedEnvelope, }; @@ -594,7 +597,7 @@ impl Server { let mut pool = this.connection_pool.lock(); pool.add_connection(connection_id, user_id, user.admin); this.peer.send(connection_id, build_initial_contacts_update(contacts, &pool))?; - this.peer.send(connection_id, build_initial_channels_update( + this.peer.send(connection_id, build_channels_update( channels_for_user, channel_invites ))?; @@ -951,6 +954,7 @@ async fn create_room( Some(proto::LiveKitConnectionInfo { server_url: live_kit.url().into(), token, + can_publish: true, }) }) } @@ -1031,6 +1035,7 @@ async fn join_room( Some(proto::LiveKitConnectionInfo { server_url: live_kit.url().into(), token, + can_publish: true, }) } else { None @@ -2217,38 +2222,21 @@ async fn create_channel( let db = session.db().await; let parent_id = request.parent_id.map(|id| ChannelId::from_proto(id)); - let id = db + let CreateChannelResult { + channel, + participants_to_update, + } = db .create_channel(&request.name, parent_id, session.user_id) .await?; - 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(channel.to_proto()), parent_id: request.parent_id, })?; - let Some(parent_id) = parent_id else { - 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 user_ids_to_notify = db.get_channel_members(parent_id).await?; - let connection_pool = session.connection_pool().await; - for user_id in user_ids_to_notify { + for (user_id, channels) in participants_to_update { + let update = build_channels_update(channels, vec![]); for connection_id in connection_pool.user_connection_ids(user_id) { if user_id == session.user_id { continue; @@ -2297,7 +2285,10 @@ async fn invite_channel_member( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let invitee_id = UserId::from_proto(request.user_id); - let notifications = db + let InviteMemberResult { + channel, + notifications, + } = db .invite_channel_member( channel_id, invitee_id, @@ -2306,21 +2297,17 @@ async fn invite_channel_member( ) .await?; - let channel = db.get_channel(channel_id, session.user_id).await?; - - let mut update = proto::UpdateChannels::default(); - update.channel_invitations.push(proto::Channel { - id: channel.id.to_proto(), - visibility: channel.visibility.into(), - name: channel.name, - }); + let update = proto::UpdateChannels { + channel_invitations: vec![channel.to_proto()], + ..Default::default() + }; - let pool = session.connection_pool().await; - for connection_id in pool.user_connection_ids(invitee_id) { + let connection_pool = session.connection_pool().await; + for connection_id in connection_pool.user_connection_ids(invitee_id) { session.peer.send(connection_id, update.clone())?; } - send_notifications(&*pool, &session.peer, notifications); + send_notifications(&*connection_pool, &session.peer, notifications); response.send(proto::Ack {})?; Ok(()) @@ -2335,20 +2322,22 @@ async fn remove_channel_member( let channel_id = ChannelId::from_proto(request.channel_id); let member_id = UserId::from_proto(request.user_id); - let removed_notification_id = db + let RemoveChannelMemberResult { + membership_update, + notification_id, + } = db .remove_channel_member(channel_id, member_id, session.user_id) .await?; - let mut update = proto::UpdateChannels::default(); - update.delete_channels.push(channel_id.to_proto()); - - for connection_id in session - .connection_pool() - .await - .user_connection_ids(member_id) - { - session.peer.send(connection_id, update.clone()).trace_err(); - if let Some(notification_id) = removed_notification_id { + let connection_pool = &session.connection_pool().await; + notify_membership_updated( + &connection_pool, + membership_update, + member_id, + &session.peer, + ); + for connection_id in connection_pool.user_connection_ids(member_id) { + if let Some(notification_id) = notification_id { session .peer .send( @@ -2374,22 +2363,27 @@ async fn set_channel_visibility( let channel_id = ChannelId::from_proto(request.channel_id); let visibility = request.visibility().into(); - let channel = db + let SetChannelVisibilityResult { + participants_to_update, + participants_to_remove, + channels_to_remove, + } = db .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 connection_pool = session.connection_pool().await; - for member_id in member_ids { - for connection_id in connection_pool.user_connection_ids(member_id) { + for (user_id, channels) in participants_to_update { + let update = build_channels_update(channels, vec![]); + for connection_id in connection_pool.user_connection_ids(user_id) { + session.peer.send(connection_id, update.clone())?; + } + } + for user_id in participants_to_remove { + let update = proto::UpdateChannels { + delete_channels: channels_to_remove.iter().map(|id| id.to_proto()).collect(), + ..Default::default() + }; + for connection_id in connection_pool.user_connection_ids(user_id) { session.peer.send(connection_id, update.clone())?; } } @@ -2406,7 +2400,7 @@ async fn set_channel_member_role( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let member_id = UserId::from_proto(request.user_id); - let channel_member = db + let result = db .set_channel_member_role( channel_id, session.user_id, @@ -2415,22 +2409,30 @@ 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, - }); - } + match result { + db::SetMemberRoleResult::MembershipUpdated(membership_update) => { + let connection_pool = session.connection_pool().await; + notify_membership_updated( + &connection_pool, + membership_update, + member_id, + &session.peer, + ) + } + db::SetMemberRoleResult::InviteUpdated(channel) => { + let update = proto::UpdateChannels { + channel_invitations: vec![channel.to_proto()], + ..Default::default() + }; - for connection_id in session - .connection_pool() - .await - .user_connection_ids(member_id) - { - session.peer.send(connection_id, update.clone())?; + for connection_id in session + .connection_pool() + .await + .user_connection_ids(member_id) + { + session.peer.send(connection_id, update.clone())?; + } + } } response.send(proto::Ack {})?; @@ -2444,26 +2446,25 @@ async fn rename_channel( ) -> Result<()> { let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); - let channel = db + let RenameChannelResult { + channel, + participants_to_update, + } = db .rename_channel(channel_id, session.user_id, &request.name) .await?; - 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(channel.to_proto()), })?; - let mut update = proto::UpdateChannels::default(); - update.channels.push(channel); - - let member_ids = db.get_channel_members(channel_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 (user_id, channel) in participants_to_update { + for connection_id in connection_pool.user_connection_ids(user_id) { + let update = proto::UpdateChannels { + channels: vec![channel.to_proto()], + ..Default::default() + }; + session.peer.send(connection_id, update.clone())?; } } @@ -2471,6 +2472,8 @@ async fn rename_channel( Ok(()) } +// TODO: Implement in terms of symlinks +// Current behavior of this is more like 'Move root channel' async fn link_channel( request: proto::LinkChannel, response: Response, @@ -2479,64 +2482,26 @@ async fn link_channel( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let to = ChannelId::from_proto(request.to); - let channels_to_send = db.link_channel(session.user_id, channel_id, to).await?; - let members = db.get_channel_members(to).await?; - let connection_pool = session.connection_pool().await; - 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 { - for connection_id in connection_pool.user_connection_ids(member_id) { - session.peer.send(connection_id, update.clone())?; - } - } + let result = db + .move_channel(channel_id, None, to, session.user_id) + .await?; + drop(db); + + notify_channel_moved(result, session).await?; response.send(Ack {})?; Ok(()) } +// TODO: Implement in terms of symlinks async fn unlink_channel( - request: proto::UnlinkChannel, - response: Response, - session: Session, + _request: proto::UnlinkChannel, + _response: Response, + _session: Session, ) -> Result<()> { - let db = session.db().await; - let channel_id = ChannelId::from_proto(request.channel_id); - let from = ChannelId::from_proto(request.from); - - db.unlink_channel(session.user_id, channel_id, from).await?; - - let members = db.get_channel_members(from).await?; - - let update = proto::UpdateChannels { - delete_edge: vec![proto::ChannelEdge { - channel_id: channel_id.to_proto(), - parent_id: from.to_proto(), - }], - ..Default::default() - }; - let connection_pool = session.connection_pool().await; - for member_id in members { - for connection_id in connection_pool.user_connection_ids(member_id) { - session.peer.send(connection_id, update.clone())?; - } - } - - response.send(Ack {})?; - - Ok(()) + Err(anyhow!("unimplemented").into()) } async fn move_channel( @@ -2549,53 +2514,46 @@ async fn move_channel( let from_parent = ChannelId::from_proto(request.from); let to = ChannelId::from_proto(request.to); - let channels_to_send = db - .move_channel(session.user_id, channel_id, from_parent, to) + let result = db + .move_channel(channel_id, Some(from_parent), to, session.user_id) .await?; + drop(db); - if channels_to_send.is_empty() { - response.send(Ack {})?; - return Ok(()); - } + notify_channel_moved(result, session).await?; - let members_from = db.get_channel_members(from_parent).await?; - let members_to = db.get_channel_members(to).await?; + response.send(Ack {})?; + Ok(()) +} - let update = proto::UpdateChannels { - delete_edge: vec![proto::ChannelEdge { - channel_id: channel_id.to_proto(), - parent_id: from_parent.to_proto(), - }], - ..Default::default() +async fn notify_channel_moved(result: Option, session: Session) -> Result<()> { + let Some(MoveChannelResult { + participants_to_remove, + participants_to_update, + moved_channels, + }) = result + else { + return Ok(()); }; + let moved_channels: Vec = moved_channels.iter().map(|id| id.to_proto()).collect(); + let connection_pool = session.connection_pool().await; - for member_id in members_from { - for connection_id in connection_pool.user_connection_ids(member_id) { + for (user_id, channels) in participants_to_update { + let mut update = build_channels_update(channels, vec![]); + update.delete_channels = moved_channels.clone(); + for connection_id in connection_pool.user_connection_ids(user_id) { session.peer.send(connection_id, update.clone())?; } } - 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 user_id in participants_to_remove { + let update = proto::UpdateChannels { + delete_channels: moved_channels.clone(), + ..Default::default() + }; + for connection_id in connection_pool.user_connection_ids(user_id) { session.peer.send(connection_id, update.clone())?; } } - - response.send(Ack {})?; - Ok(()) } @@ -2620,78 +2578,36 @@ async fn respond_to_channel_invite( ) -> Result<()> { let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); - let notifications = db + let RespondToChannelInvite { + membership_update, + notifications, + } = db .respond_to_channel_invite(channel_id, session.user_id, request.accept) .await?; - if request.accept { - channel_membership_updated(db, channel_id, &session).await?; + let connection_pool = session.connection_pool().await; + if let Some(membership_update) = membership_update { + notify_membership_updated( + &connection_pool, + membership_update, + session.user_id, + &session.peer, + ); } else { - let mut update = proto::UpdateChannels::default(); - update - .remove_channel_invitations - .push(channel_id.to_proto()); - session.peer.send(session.connection_id, update)?; - } + let update = proto::UpdateChannels { + remove_channel_invitations: vec![channel_id.to_proto()], + ..Default::default() + }; - send_notifications( - &*session.connection_pool().await, - &session.peer, - notifications, - ); - response.send(proto::Ack {})?; + for connection_id in connection_pool.user_connection_ids(session.user_id) { + session.peer.send(connection_id, update.clone())?; + } + }; - Ok(()) -} + send_notifications(&*connection_pool, &session.peer, notifications); + + response.send(proto::Ack {})?; -async fn channel_membership_updated( - db: tokio::sync::MutexGuard<'_, DbHandle>, - channel_id: ChannelId, - session: &Session, -) -> Result<(), crate::Error> { - let mut update = proto::UpdateChannels::default(); - update - .remove_channel_invitations - .push(channel_id.to_proto()); - - let result = db.get_channel_for_user(channel_id, session.user_id).await?; - update.channels.extend( - result - .channels - .channels - .into_iter() - .map(|channel| proto::Channel { - id: channel.id.to_proto(), - visibility: channel.visibility.into(), - name: channel.name, - }), - ); - update.unseen_channel_messages = result.channel_messages; - update.unseen_channel_buffer_changes = result.unseen_buffer_changes; - update.insert_edge = result.channels.edges; - update - .channel_participants - .extend( - result - .channel_participants - .into_iter() - .map(|(channel_id, user_ids)| proto::ChannelParticipants { - channel_id: channel_id.to_proto(), - participant_user_ids: user_ids.into_iter().map(UserId::to_proto).collect(), - }), - ); - 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(()) } @@ -2727,7 +2643,7 @@ async fn join_channel_internal( leave_room_for_session(&session).await?; let db = session.db().await; - let (joined_room, joined_channel) = db + let (joined_room, membership_updated, role) = db .join_channel( channel_id, session.user_id, @@ -2737,16 +2653,32 @@ async fn join_channel_internal( .await?; let live_kit_connection_info = session.live_kit_client.as_ref().and_then(|live_kit| { - let token = live_kit - .room_token( - &joined_room.room.live_kit_room, - &session.user_id.to_string(), + let (can_publish, token) = if role == ChannelRole::Guest { + ( + false, + live_kit + .guest_token( + &joined_room.room.live_kit_room, + &session.user_id.to_string(), + ) + .trace_err()?, ) - .trace_err()?; + } else { + ( + true, + live_kit + .room_token( + &joined_room.room.live_kit_room, + &session.user_id.to_string(), + ) + .trace_err()?, + ) + }; Some(LiveKitConnectionInfo { server_url: live_kit.url().into(), token, + can_publish, }) }); @@ -2756,8 +2688,14 @@ async fn join_channel_internal( live_kit_connection_info, })?; - if let Some(joined_channel) = joined_channel { - channel_membership_updated(db, joined_channel, &session).await? + let connection_pool = session.connection_pool().await; + if let Some(membership_updated) = membership_updated { + notify_membership_updated( + &connection_pool, + membership_updated, + session.user_id, + &session.peer, + ); } room_updated(&joined_room.room, &session.peer); @@ -3281,7 +3219,26 @@ fn to_tungstenite_message(message: AxumMessage) -> TungsteniteMessage { } } -fn build_initial_channels_update( +fn notify_membership_updated( + connection_pool: &ConnectionPool, + result: MembershipUpdated, + user_id: UserId, + peer: &Peer, +) { + let mut update = build_channels_update(result.new_channels, vec![]); + update.delete_channels = result + .removed_channels + .into_iter() + .map(|id| id.to_proto()) + .collect(); + update.remove_channel_invitations = vec![result.channel_id.to_proto()]; + + for connection_id in connection_pool.user_connection_ids(user_id) { + peer.send(connection_id, update.clone()).trace_err(); + } +} + +fn build_channels_update( channels: ChannelsForUser, channel_invites: Vec, ) -> proto::UpdateChannels { @@ -3292,6 +3249,7 @@ fn build_initial_channels_update( id: channel.id.to_proto(), name: channel.name, visibility: channel.visibility.into(), + role: channel.role.into(), }); } @@ -3308,24 +3266,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 14ae159ab80fd5b46359ef1183e7ea18478f3f08..931610f5ffc8fe56010b4cda3614fcff6fa992bc 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -410,10 +410,10 @@ async fn test_channel_buffer_disconnect( server.disconnect_client(client_a.peer_id().unwrap()); deterministic.advance_clock(RECEIVE_TIMEOUT + RECONNECT_TIMEOUT); - channel_buffer_a.update(cx_a, |buffer, _| { + channel_buffer_a.update(cx_a, |buffer, cx| { assert_eq!( - buffer.channel().as_ref(), - &channel(channel_id, "the-channel") + buffer.channel(cx).unwrap().as_ref(), + &channel(channel_id, "the-channel", proto::ChannelRole::Admin) ); assert!(!buffer.is_connected()); }); @@ -435,18 +435,16 @@ async fn test_channel_buffer_disconnect( deterministic.run_until_parked(); // Channel buffer observed the deletion - channel_buffer_b.update(cx_b, |buffer, _| { - assert_eq!( - buffer.channel().as_ref(), - &channel(channel_id, "the-channel") - ); + channel_buffer_b.update(cx_b, |buffer, cx| { + assert!(buffer.channel(cx).is_none()); 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, @@ -698,7 +696,7 @@ async fn test_following_to_channel_notes_without_a_shared_project( .await .unwrap(); channel_view_1_a.update(cx_a, |notes, cx| { - assert_eq!(notes.channel(cx).name, "channel-1"); + assert_eq!(notes.channel(cx).unwrap().name, "channel-1"); notes.editor.update(cx, |editor, cx| { editor.insert("Hello from A.", cx); editor.change_selections(None, cx, |selections| { @@ -730,7 +728,7 @@ async fn test_following_to_channel_notes_without_a_shared_project( .expect("active item is not a channel view") }); channel_view_1_b.read_with(cx_b, |notes, cx| { - assert_eq!(notes.channel(cx).name, "channel-1"); + assert_eq!(notes.channel(cx).unwrap().name, "channel-1"); let editor = notes.editor.read(cx); assert_eq!(editor.text(cx), "Hello from A."); assert_eq!(editor.selections.ranges::(cx), &[3..4]); @@ -742,7 +740,7 @@ async fn test_following_to_channel_notes_without_a_shared_project( .await .unwrap(); channel_view_2_a.read_with(cx_a, |notes, cx| { - assert_eq!(notes.channel(cx).name, "channel-2"); + assert_eq!(notes.channel(cx).unwrap().name, "channel-2"); }); // Client B is taken to the notes for channel 2. @@ -759,7 +757,7 @@ async fn test_following_to_channel_notes_without_a_shared_project( .expect("active item is not a channel view") }); channel_view_2_b.read_with(cx_b, |notes, cx| { - assert_eq!(notes.channel(cx).name, "channel-2"); + assert_eq!(notes.channel(cx).unwrap().name, "channel-2"); }); } diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index 54a958e71c62a5a8f6051b0def9388f070271ca5..5b0bf02f8fb11fc85b9fef07472b3a7e106ab5f9 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -1,10 +1,12 @@ use crate::{ + db::{self, UserId}, rpc::RECONNECT_TIMEOUT, tests::{room_participants, RoomParticipants, TestServer}, }; use call::ActiveCall; use channel::{ChannelId, ChannelMembership, ChannelStore}; use client::User; +use futures::future::try_join_all; use gpui::{executor::Deterministic, ModelHandle, TestAppContext}; use rpc::{ proto::{self, ChannelRole}, @@ -47,13 +49,13 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".to_string(), depth: 0, - user_is_admin: true, + role: ChannelRole::Admin, }, ExpectedChannel { id: channel_b_id, name: "channel-b".to_string(), depth: 1, - user_is_admin: true, + role: ChannelRole::Admin, }, ], ); @@ -94,7 +96,7 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".to_string(), depth: 0, - user_is_admin: false, + role: ChannelRole::Member, }], ); @@ -141,13 +143,13 @@ async fn test_core_channels( ExpectedChannel { id: channel_a_id, name: "channel-a".to_string(), - user_is_admin: false, + role: ChannelRole::Member, depth: 0, }, ExpectedChannel { id: channel_b_id, name: "channel-b".to_string(), - user_is_admin: false, + role: ChannelRole::Member, depth: 1, }, ], @@ -169,19 +171,19 @@ async fn test_core_channels( ExpectedChannel { id: channel_a_id, name: "channel-a".to_string(), - user_is_admin: false, + role: ChannelRole::Member, depth: 0, }, ExpectedChannel { id: channel_b_id, name: "channel-b".to_string(), - user_is_admin: false, + role: ChannelRole::Member, depth: 1, }, ExpectedChannel { id: channel_c_id, name: "channel-c".to_string(), - user_is_admin: false, + role: ChannelRole::Member, depth: 2, }, ], @@ -213,19 +215,19 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".to_string(), depth: 0, - user_is_admin: true, + role: ChannelRole::Admin, }, ExpectedChannel { id: channel_b_id, name: "channel-b".to_string(), depth: 1, - user_is_admin: true, + role: ChannelRole::Admin, }, ExpectedChannel { id: channel_c_id, name: "channel-c".to_string(), depth: 2, - user_is_admin: true, + role: ChannelRole::Admin, }, ], ); @@ -247,7 +249,7 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".to_string(), depth: 0, - user_is_admin: true, + role: ChannelRole::Admin, }], ); assert_channels( @@ -257,7 +259,7 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".to_string(), depth: 0, - user_is_admin: true, + role: ChannelRole::Admin, }], ); @@ -280,18 +282,27 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".to_string(), depth: 0, - user_is_admin: true, + role: ChannelRole::Admin, }], ); // Client B no longer has access to the channel assert_channels(client_b.channel_store(), cx_b, &[]); - // 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); - assert_channels(client_a.channel_store(), cx_a, &[]); + + server + .app_state + .db + .rename_channel( + db::ChannelId::from_proto(channel_a_id), + UserId::from_proto(client_a.id()), + "channel-a-renamed", + ) + .await + .unwrap(); server.allow_connections(); deterministic.advance_clock(RECEIVE_TIMEOUT + RECONNECT_TIMEOUT); @@ -300,9 +311,9 @@ async fn test_core_channels( cx_a, &[ExpectedChannel { id: channel_a_id, - name: "channel-a".to_string(), + name: "channel-a-renamed".to_string(), depth: 0, - user_is_admin: true, + role: ChannelRole::Admin, }], ); } @@ -410,7 +421,7 @@ async fn test_channel_room( id: zed_id, name: "zed".to_string(), depth: 0, - user_is_admin: false, + role: ChannelRole::Member, }], ); client_b.channel_store().read_with(cx_b, |channels, _| { @@ -643,7 +654,7 @@ async fn test_permissions_update_while_invited( depth: 0, id: rust_id, name: "rust".to_string(), - user_is_admin: false, + role: ChannelRole::Member, }], ); assert_channels(client_b.channel_store(), cx_b, &[]); @@ -671,7 +682,7 @@ async fn test_permissions_update_while_invited( depth: 0, id: rust_id, name: "rust".to_string(), - user_is_admin: false, + role: ChannelRole::Member, }], ); assert_channels(client_b.channel_store(), cx_b, &[]); @@ -711,7 +722,7 @@ async fn test_channel_rename( depth: 0, id: rust_id, name: "rust-archive".to_string(), - user_is_admin: true, + role: ChannelRole::Admin, }], ); @@ -723,7 +734,7 @@ async fn test_channel_rename( depth: 0, id: rust_id, name: "rust-archive".to_string(), - user_is_admin: false, + role: ChannelRole::Member, }], ); } @@ -846,7 +857,7 @@ async fn test_lost_channel_creation( depth: 0, id: channel_id, name: "x".to_string(), - user_is_admin: false, + role: ChannelRole::Member, }], ); @@ -870,13 +881,13 @@ async fn test_lost_channel_creation( depth: 0, id: channel_id, name: "x".to_string(), - user_is_admin: true, + role: ChannelRole::Admin, }, ExpectedChannel { depth: 1, id: subchannel_id, name: "subchannel".to_string(), - user_is_admin: true, + role: ChannelRole::Admin, }, ], ); @@ -901,17 +912,327 @@ async fn test_lost_channel_creation( depth: 0, id: channel_id, name: "x".to_string(), - user_is_admin: false, + role: ChannelRole::Member, }, ExpectedChannel { depth: 1, id: subchannel_id, name: "subchannel".to_string(), - user_is_admin: false, + role: ChannelRole::Member, }, ], ); } + +#[gpui::test] +async fn test_channel_link_notifications( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, + cx_c: &mut TestAppContext, +) { + deterministic.forbid_parking(); + + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + let client_c = server.create_client(cx_c, "user_c").await; + + let user_b = client_b.user_id().unwrap(); + let user_c = client_c.user_id().unwrap(); + + let channels = server + .make_channel_tree(&[("zed", None)], (&client_a, cx_a)) + .await; + let zed_channel = channels[0]; + + try_join_all(client_a.channel_store().update(cx_a, |channel_store, cx| { + [ + channel_store.set_channel_visibility(zed_channel, proto::ChannelVisibility::Public, cx), + channel_store.invite_member(zed_channel, user_b, proto::ChannelRole::Member, cx), + channel_store.invite_member(zed_channel, user_c, proto::ChannelRole::Guest, cx), + ] + })) + .await + .unwrap(); + + deterministic.run_until_parked(); + + client_b + .channel_store() + .update(cx_b, |channel_store, cx| { + channel_store.respond_to_channel_invite(zed_channel, true, cx) + }) + .await + .unwrap(); + + client_c + .channel_store() + .update(cx_c, |channel_store, cx| { + channel_store.respond_to_channel_invite(zed_channel, true, cx) + }) + .await + .unwrap(); + + deterministic.run_until_parked(); + + // we have an admin (a), member (b) and guest (c) all part of the zed channel. + + // create a new private channel, make it public, and move it under the previous one, and verify it shows for b and not c + let active_channel = client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.create_channel("active", Some(zed_channel), cx) + }) + .await + .unwrap(); + + // the new channel shows for b and not c + assert_channels_list_shape( + client_a.channel_store(), + cx_a, + &[(zed_channel, 0), (active_channel, 1)], + ); + assert_channels_list_shape( + client_b.channel_store(), + cx_b, + &[(zed_channel, 0), (active_channel, 1)], + ); + assert_channels_list_shape(client_c.channel_store(), cx_c, &[(zed_channel, 0)]); + + let vim_channel = client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.create_channel("vim", None, cx) + }) + .await + .unwrap(); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.set_channel_visibility(vim_channel, proto::ChannelVisibility::Public, cx) + }) + .await + .unwrap(); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.link_channel(vim_channel, active_channel, cx) + }) + .await + .unwrap(); + + deterministic.run_until_parked(); + + // the new channel shows for b and c + assert_channels_list_shape( + client_a.channel_store(), + cx_a, + &[(zed_channel, 0), (active_channel, 1), (vim_channel, 2)], + ); + assert_channels_list_shape( + client_b.channel_store(), + cx_b, + &[(zed_channel, 0), (active_channel, 1), (vim_channel, 2)], + ); + assert_channels_list_shape( + client_c.channel_store(), + cx_c, + &[(zed_channel, 0), (vim_channel, 1)], + ); + + let helix_channel = client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.create_channel("helix", None, cx) + }) + .await + .unwrap(); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.link_channel(helix_channel, vim_channel, cx) + }) + .await + .unwrap(); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.set_channel_visibility( + helix_channel, + proto::ChannelVisibility::Public, + cx, + ) + }) + .await + .unwrap(); + + // the new channel shows for b and c + assert_channels_list_shape( + client_b.channel_store(), + cx_b, + &[ + (zed_channel, 0), + (active_channel, 1), + (vim_channel, 2), + (helix_channel, 3), + ], + ); + assert_channels_list_shape( + client_c.channel_store(), + cx_c, + &[(zed_channel, 0), (vim_channel, 1), (helix_channel, 2)], + ); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.set_channel_visibility(vim_channel, proto::ChannelVisibility::Members, cx) + }) + .await + .unwrap(); + + // the members-only channel is still shown for c, but hidden for b + assert_channels_list_shape( + client_b.channel_store(), + cx_b, + &[ + (zed_channel, 0), + (active_channel, 1), + (vim_channel, 2), + (helix_channel, 3), + ], + ); + client_b + .channel_store() + .read_with(cx_b, |channel_store, _| { + assert_eq!( + channel_store + .channel_for_id(vim_channel) + .unwrap() + .visibility, + proto::ChannelVisibility::Members + ) + }); + + assert_channels_list_shape(client_c.channel_store(), cx_c, &[(zed_channel, 0)]); +} + +#[gpui::test] +async fn test_channel_membership_notifications( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + + deterministic.forbid_parking(); + + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_c").await; + + let user_b = client_b.user_id().unwrap(); + + let channels = server + .make_channel_tree( + &[ + ("zed", None), + ("active", Some("zed")), + ("vim", Some("active")), + ], + (&client_a, cx_a), + ) + .await; + let zed_channel = channels[0]; + let _active_channel = channels[1]; + let vim_channel = channels[2]; + + try_join_all(client_a.channel_store().update(cx_a, |channel_store, cx| { + [ + channel_store.set_channel_visibility(zed_channel, proto::ChannelVisibility::Public, cx), + channel_store.set_channel_visibility(vim_channel, proto::ChannelVisibility::Public, cx), + channel_store.invite_member(vim_channel, user_b, proto::ChannelRole::Member, cx), + channel_store.invite_member(zed_channel, user_b, proto::ChannelRole::Guest, cx), + ] + })) + .await + .unwrap(); + + deterministic.run_until_parked(); + + client_b + .channel_store() + .update(cx_b, |channel_store, cx| { + channel_store.respond_to_channel_invite(zed_channel, true, cx) + }) + .await + .unwrap(); + + client_b + .channel_store() + .update(cx_b, |channel_store, cx| { + channel_store.respond_to_channel_invite(vim_channel, true, cx) + }) + .await + .unwrap(); + + deterministic.run_until_parked(); + + // we have an admin (a), and a guest (b) with access to all of zed, and membership in vim. + assert_channels( + client_b.channel_store(), + cx_b, + &[ + ExpectedChannel { + depth: 0, + id: zed_channel, + name: "zed".to_string(), + role: ChannelRole::Guest, + }, + ExpectedChannel { + depth: 1, + id: vim_channel, + name: "vim".to_string(), + role: ChannelRole::Member, + }, + ], + ); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.remove_member(vim_channel, user_b, cx) + }) + .await + .unwrap(); + + deterministic.run_until_parked(); + + assert_channels( + client_b.channel_store(), + cx_b, + &[ + ExpectedChannel { + depth: 0, + id: zed_channel, + name: "zed".to_string(), + role: ChannelRole::Guest, + }, + ExpectedChannel { + depth: 1, + id: vim_channel, + name: "vim".to_string(), + role: ChannelRole::Guest, + }, + ], + ) +} + #[gpui::test] async fn test_guest_access( deterministic: Arc, @@ -925,44 +1246,79 @@ async fn test_guest_access( let client_b = server.create_client(cx_b, "user_b").await; let channels = server - .make_channel_tree(&[("channel-a", None)], (&client_a, cx_a)) + .make_channel_tree( + &[("channel-a", None), ("channel-b", Some("channel-a"))], + (&client_a, cx_a), + ) .await; - let channel_a_id = channels[0]; + let channel_a = channels[0]; + let channel_b = channels[1]; let active_call_b = cx_b.read(ActiveCall::global); - // should not be allowed to join + // Non-members should not be allowed to join assert!(active_call_b - .update(cx_b, |call, cx| call.join_channel(channel_a_id, cx)) + .update(cx_b, |call, cx| call.join_channel(channel_a, cx)) .await .is_err()); + // Make channels A and B public client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.set_channel_visibility(channel_a_id, proto::ChannelVisibility::Public, cx) + channel_store.set_channel_visibility(channel_a, proto::ChannelVisibility::Public, cx) + }) + .await + .unwrap(); + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.set_channel_visibility(channel_b, proto::ChannelVisibility::Public, cx) }) .await .unwrap(); + // Client B joins channel A as a guest active_call_b - .update(cx_b, |call, cx| call.join_channel(channel_a_id, cx)) + .update(cx_b, |call, cx| call.join_channel(channel_a, cx)) .await .unwrap(); deterministic.run_until_parked(); - - assert!(client_b - .channel_store() - .update(cx_b, |channel_store, _| channel_store - .channel_for_id(channel_a_id) - .is_some())); + assert_channels_list_shape( + client_a.channel_store(), + cx_a, + &[(channel_a, 0), (channel_b, 1)], + ); + assert_channels_list_shape( + client_b.channel_store(), + cx_b, + &[(channel_a, 0), (channel_b, 1)], + ); client_a.channel_store().update(cx_a, |channel_store, _| { - let participants = channel_store.channel_participants(channel_a_id); + let participants = channel_store.channel_participants(channel_a); assert_eq!(participants.len(), 1); assert_eq!(participants[0].id, client_b.user_id().unwrap()); - }) + }); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.set_channel_visibility(channel_a, proto::ChannelVisibility::Members, cx) + }) + .await + .unwrap(); + + assert_channels_list_shape(client_b.channel_store(), cx_b, &[]); + + active_call_b + .update(cx_b, |call, cx| call.join_channel(channel_b, cx)) + .await + .unwrap(); + + deterministic.run_until_parked(); + assert_channels_list_shape(client_b.channel_store(), cx_b, &[(channel_b, 0)]); } #[gpui::test] @@ -1030,14 +1386,14 @@ async fn test_invite_access( async fn test_channel_moving( deterministic: Arc, cx_a: &mut TestAppContext, - cx_b: &mut TestAppContext, - cx_c: &mut TestAppContext, + _cx_b: &mut TestAppContext, + _cx_c: &mut TestAppContext, ) { deterministic.forbid_parking(); let mut server = TestServer::start(&deterministic).await; let client_a = server.create_client(cx_a, "user_a").await; - let client_b = server.create_client(cx_b, "user_b").await; - let client_c = server.create_client(cx_c, "user_c").await; + // let client_b = server.create_client(cx_b, "user_b").await; + // let client_c = server.create_client(cx_c, "user_c").await; let channels = server .make_channel_tree( @@ -1090,187 +1446,188 @@ async fn test_channel_moving( ], ); - client_a - .channel_store() - .update(cx_a, |channel_store, cx| { - channel_store.link_channel(channel_d_id, channel_c_id, cx) - }) - .await - .unwrap(); - - // Current shape for A: - // /------\ - // a - b -- c -- d - assert_channels_list_shape( - client_a.channel_store(), - cx_a, - &[ - (channel_a_id, 0), - (channel_b_id, 1), - (channel_c_id, 2), - (channel_d_id, 3), - (channel_d_id, 2), - ], - ); - - let b_channels = server - .make_channel_tree( - &[ - ("channel-mu", None), - ("channel-gamma", Some("channel-mu")), - ("channel-epsilon", Some("channel-mu")), - ], - (&client_b, cx_b), - ) - .await; - let channel_mu_id = b_channels[0]; - let channel_ga_id = b_channels[1]; - let channel_ep_id = b_channels[2]; - - // Current shape for B: - // /- ep - // mu -- ga - assert_channels_list_shape( - client_b.channel_store(), - cx_b, - &[(channel_mu_id, 0), (channel_ep_id, 1), (channel_ga_id, 1)], - ); - - client_a - .add_admin_to_channel((&client_b, cx_b), channel_b_id, cx_a) - .await; - - // Current shape for B: - // /- ep - // mu -- ga - // /---------\ - // b -- c -- d - assert_channels_list_shape( - client_b.channel_store(), - cx_b, - &[ - // New channels from a - (channel_b_id, 0), - (channel_c_id, 1), - (channel_d_id, 2), - (channel_d_id, 1), - // B's old channels - (channel_mu_id, 0), - (channel_ep_id, 1), - (channel_ga_id, 1), - ], - ); - - client_b - .add_admin_to_channel((&client_c, cx_c), channel_ep_id, cx_b) - .await; - - // Current shape for C: - // - ep - assert_channels_list_shape(client_c.channel_store(), cx_c, &[(channel_ep_id, 0)]); - - client_b - .channel_store() - .update(cx_b, |channel_store, cx| { - channel_store.link_channel(channel_b_id, channel_ep_id, cx) - }) - .await - .unwrap(); - - // Current shape for B: - // /---------\ - // /- ep -- b -- c -- d - // mu -- ga - assert_channels_list_shape( - client_b.channel_store(), - cx_b, - &[ - (channel_mu_id, 0), - (channel_ep_id, 1), - (channel_b_id, 2), - (channel_c_id, 3), - (channel_d_id, 4), - (channel_d_id, 3), - (channel_ga_id, 1), - ], - ); - - // Current shape for C: - // /---------\ - // ep -- b -- c -- d - assert_channels_list_shape( - client_c.channel_store(), - cx_c, - &[ - (channel_ep_id, 0), - (channel_b_id, 1), - (channel_c_id, 2), - (channel_d_id, 3), - (channel_d_id, 2), - ], - ); - - client_b - .channel_store() - .update(cx_b, |channel_store, cx| { - channel_store.link_channel(channel_ga_id, channel_b_id, cx) - }) - .await - .unwrap(); - - // Current shape for B: - // /---------\ - // /- ep -- b -- c -- d - // / \ - // mu ---------- ga - assert_channels_list_shape( - client_b.channel_store(), - cx_b, - &[ - (channel_mu_id, 0), - (channel_ep_id, 1), - (channel_b_id, 2), - (channel_c_id, 3), - (channel_d_id, 4), - (channel_d_id, 3), - (channel_ga_id, 3), - (channel_ga_id, 1), - ], - ); - - // Current shape for A: - // /------\ - // a - b -- c -- d - // \-- ga - assert_channels_list_shape( - client_a.channel_store(), - cx_a, - &[ - (channel_a_id, 0), - (channel_b_id, 1), - (channel_c_id, 2), - (channel_d_id, 3), - (channel_d_id, 2), - (channel_ga_id, 2), - ], - ); - - // Current shape for C: - // /-------\ - // ep -- b -- c -- d - // \-- ga - assert_channels_list_shape( - client_c.channel_store(), - cx_c, - &[ - (channel_ep_id, 0), - (channel_b_id, 1), - (channel_c_id, 2), - (channel_d_id, 3), - (channel_d_id, 2), - (channel_ga_id, 2), - ], - ); + // TODO: Restore this test once we have a way to make channel symlinks + // client_a + // .channel_store() + // .update(cx_a, |channel_store, cx| { + // channel_store.link_channel(channel_d_id, channel_c_id, cx) + // }) + // .await + // .unwrap(); + + // // Current shape for A: + // // /------\ + // // a - b -- c -- d + // assert_channels_list_shape( + // client_a.channel_store(), + // cx_a, + // &[ + // (channel_a_id, 0), + // (channel_b_id, 1), + // (channel_c_id, 2), + // (channel_d_id, 3), + // (channel_d_id, 2), + // ], + // ); + // + // let b_channels = server + // .make_channel_tree( + // &[ + // ("channel-mu", None), + // ("channel-gamma", Some("channel-mu")), + // ("channel-epsilon", Some("channel-mu")), + // ], + // (&client_b, cx_b), + // ) + // .await; + // let channel_mu_id = b_channels[0]; + // let channel_ga_id = b_channels[1]; + // let channel_ep_id = b_channels[2]; + + // // Current shape for B: + // // /- ep + // // mu -- ga + // assert_channels_list_shape( + // client_b.channel_store(), + // cx_b, + // &[(channel_mu_id, 0), (channel_ep_id, 1), (channel_ga_id, 1)], + // ); + + // client_a + // .add_admin_to_channel((&client_b, cx_b), channel_b_id, cx_a) + // .await; + + // // Current shape for B: + // // /- ep + // // mu -- ga + // // /---------\ + // // b -- c -- d + // assert_channels_list_shape( + // client_b.channel_store(), + // cx_b, + // &[ + // // New channels from a + // (channel_b_id, 0), + // (channel_c_id, 1), + // (channel_d_id, 2), + // (channel_d_id, 1), + // // B's old channels + // (channel_mu_id, 0), + // (channel_ep_id, 1), + // (channel_ga_id, 1), + // ], + // ); + + // client_b + // .add_admin_to_channel((&client_c, cx_c), channel_ep_id, cx_b) + // .await; + + // // Current shape for C: + // // - ep + // assert_channels_list_shape(client_c.channel_store(), cx_c, &[(channel_ep_id, 0)]); + + // client_b + // .channel_store() + // .update(cx_b, |channel_store, cx| { + // channel_store.link_channel(channel_b_id, channel_ep_id, cx) + // }) + // .await + // .unwrap(); + + // // Current shape for B: + // // /---------\ + // // /- ep -- b -- c -- d + // // mu -- ga + // assert_channels_list_shape( + // client_b.channel_store(), + // cx_b, + // &[ + // (channel_mu_id, 0), + // (channel_ep_id, 1), + // (channel_b_id, 2), + // (channel_c_id, 3), + // (channel_d_id, 4), + // (channel_d_id, 3), + // (channel_ga_id, 1), + // ], + // ); + + // // Current shape for C: + // // /---------\ + // // ep -- b -- c -- d + // assert_channels_list_shape( + // client_c.channel_store(), + // cx_c, + // &[ + // (channel_ep_id, 0), + // (channel_b_id, 1), + // (channel_c_id, 2), + // (channel_d_id, 3), + // (channel_d_id, 2), + // ], + // ); + + // client_b + // .channel_store() + // .update(cx_b, |channel_store, cx| { + // channel_store.link_channel(channel_ga_id, channel_b_id, cx) + // }) + // .await + // .unwrap(); + + // // Current shape for B: + // // /---------\ + // // /- ep -- b -- c -- d + // // / \ + // // mu ---------- ga + // assert_channels_list_shape( + // client_b.channel_store(), + // cx_b, + // &[ + // (channel_mu_id, 0), + // (channel_ep_id, 1), + // (channel_b_id, 2), + // (channel_c_id, 3), + // (channel_d_id, 4), + // (channel_d_id, 3), + // (channel_ga_id, 3), + // (channel_ga_id, 1), + // ], + // ); + + // // Current shape for A: + // // /------\ + // // a - b -- c -- d + // // \-- ga + // assert_channels_list_shape( + // client_a.channel_store(), + // cx_a, + // &[ + // (channel_a_id, 0), + // (channel_b_id, 1), + // (channel_c_id, 2), + // (channel_d_id, 3), + // (channel_d_id, 2), + // (channel_ga_id, 2), + // ], + // ); + + // // Current shape for C: + // // /-------\ + // // ep -- b -- c -- d + // // \-- ga + // assert_channels_list_shape( + // client_c.channel_store(), + // cx_c, + // &[ + // (channel_ep_id, 0), + // (channel_b_id, 1), + // (channel_c_id, 2), + // (channel_d_id, 3), + // (channel_d_id, 2), + // (channel_ga_id, 2), + // ], + // ); } #[derive(Debug, PartialEq)] @@ -1278,7 +1635,7 @@ struct ExpectedChannel { depth: usize, id: ChannelId, name: String, - user_is_admin: bool, + role: ChannelRole, } #[track_caller] @@ -1295,7 +1652,7 @@ fn assert_channel_invitations( depth: 0, name: channel.name.clone(), id: channel.id, - user_is_admin: store.is_user_admin(channel.id), + role: channel.role, }) .collect::>() }); @@ -1315,7 +1672,7 @@ fn assert_channels( depth, name: channel.name.clone(), id: channel.id, - user_is_admin: store.is_user_admin(channel.id), + role: channel.role, }) .collect::>() }); diff --git a/crates/collab/src/tests/random_channel_buffer_tests.rs b/crates/collab/src/tests/random_channel_buffer_tests.rs index 1b24c7a3d2f45e5e374c5a70cd54a426b4043753..64fecd5e62cd3727339406d9ae9f83462c0b2353 100644 --- a/crates/collab/src/tests/random_channel_buffer_tests.rs +++ b/crates/collab/src/tests/random_channel_buffer_tests.rs @@ -48,7 +48,7 @@ impl RandomizedTest for RandomChannelBufferTest { let db = &server.app_state.db; for ix in 0..CHANNEL_COUNT { let id = db - .create_channel(&format!("channel-{ix}"), None, users[0].user_id) + .create_root_channel(&format!("channel-{ix}"), users[0].user_id) .await .unwrap(); for user in &users[1..] { @@ -98,15 +98,16 @@ impl RandomizedTest for RandomChannelBufferTest { 30..=40 => { if let Some(buffer) = channel_buffers.iter().choose(rng) { - let channel_name = buffer.read_with(cx, |b, _| b.channel().name.clone()); + let channel_name = + buffer.read_with(cx, |b, cx| b.channel(cx).unwrap().name.clone()); break ChannelBufferOperation::LeaveChannelNotes { channel_name }; } } _ => { if let Some(buffer) = channel_buffers.iter().choose(rng) { - break buffer.read_with(cx, |b, _| { - let channel_name = b.channel().name.clone(); + break buffer.read_with(cx, |b, cx| { + let channel_name = b.channel(cx).unwrap().name.clone(); let edits = b .buffer() .read_with(cx, |buffer, _| buffer.get_random_edits(rng, 3)); @@ -153,7 +154,7 @@ impl RandomizedTest for RandomChannelBufferTest { let buffer = cx.update(|cx| { let mut left_buffer = Err(TestError::Inapplicable); client.channel_buffers().retain(|buffer| { - if buffer.read(cx).channel().name == channel_name { + if buffer.read(cx).channel(cx).unwrap().name == channel_name { left_buffer = Ok(buffer.clone()); false } else { @@ -179,7 +180,9 @@ impl RandomizedTest for RandomChannelBufferTest { client .channel_buffers() .iter() - .find(|buffer| buffer.read(cx).channel().name == channel_name) + .find(|buffer| { + buffer.read(cx).channel(cx).unwrap().name == channel_name + }) .cloned() }) .ok_or_else(|| TestError::Inapplicable)?; @@ -250,7 +253,7 @@ impl RandomizedTest for RandomChannelBufferTest { if let Some(channel_buffer) = client .channel_buffers() .iter() - .find(|b| b.read(cx).channel().id == channel_id.to_proto()) + .find(|b| b.read(cx).channel_id == channel_id.to_proto()) { let channel_buffer = channel_buffer.read(cx); diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index f7c4fa4146cd8a6e665048ce9a8f3e1b4c391a25..d6ebe1e84ef8ad72d85d1d204fdbf1346a48f83d 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -611,38 +611,6 @@ impl TestClient { ) -> WindowHandle { cx.add_window(|cx| Workspace::new(0, project.clone(), self.app_state.clone(), cx)) } - - pub async fn add_admin_to_channel( - &self, - user: (&TestClient, &mut TestAppContext), - channel: u64, - cx_self: &mut TestAppContext, - ) { - let (other_client, other_cx) = user; - - cx_self - .read(ChannelStore::global) - .update(cx_self, |channel_store, cx| { - channel_store.invite_member( - channel, - other_client.user_id().unwrap(), - ChannelRole::Admin, - cx, - ) - }) - .await - .unwrap(); - - cx_self.foreground().run_until_parked(); - - other_cx - .read(ChannelStore::global) - .update(other_cx, |channel_store, cx| { - channel_store.respond_to_channel_invite(channel, true, cx) - }) - .await - .unwrap(); - } } impl Drop for TestClient { diff --git a/crates/collab_ui/Cargo.toml b/crates/collab_ui/Cargo.toml index 8aee0da8dd5978f2a97fb6e52f406f3de7cf44ba..791c6b2fa76c1d4a7acc860718c267b8b0c384db 100644 --- a/crates/collab_ui/Cargo.toml +++ b/crates/collab_ui/Cargo.toml @@ -61,6 +61,7 @@ postage.workspace = true serde.workspace = true serde_derive.workspace = true time.workspace = true +smallvec.workspace = true [dev-dependencies] call = { path = "../call", features = ["test-support"] } diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index e62ee8ef4b7b0c091000cdfd5b272e62c6fee7f7..1bdcebd01855e00948505fa48011ccccde7565f7 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/crates/collab_ui/src/channel_view.rs @@ -15,13 +15,14 @@ use gpui::{ ViewContext, ViewHandle, }; use project::Project; +use smallvec::SmallVec; use std::{ any::{Any, TypeId}, sync::Arc, }; use util::ResultExt; use workspace::{ - item::{FollowableItem, Item, ItemHandle}, + item::{FollowableItem, Item, ItemEvent, ItemHandle}, register_followable_item, searchable::SearchableItemHandle, ItemNavHistory, Pane, SaveIntent, ViewId, Workspace, WorkspaceId, @@ -140,6 +141,12 @@ impl ChannelView { editor.set_collaboration_hub(Box::new(ChannelBufferCollaborationHub( channel_buffer.clone(), ))); + editor.set_read_only( + !channel_buffer + .read(cx) + .channel(cx) + .is_some_and(|c| c.can_edit_notes()), + ); editor }); let _editor_event_subscription = cx.subscribe(&editor, |_, _, e, cx| cx.emit(e.clone())); @@ -157,8 +164,8 @@ impl ChannelView { } } - pub fn channel(&self, cx: &AppContext) -> Arc { - self.channel_buffer.read(cx).channel() + pub fn channel(&self, cx: &AppContext) -> Option> { + self.channel_buffer.read(cx).channel(cx) } fn handle_channel_buffer_event( @@ -172,6 +179,13 @@ impl ChannelView { editor.set_read_only(true); cx.notify(); }), + ChannelBufferEvent::ChannelChanged => { + self.editor.update(cx, |editor, cx| { + editor.set_read_only(!self.channel(cx).is_some_and(|c| c.can_edit_notes())); + cx.emit(editor::Event::TitleChanged); + cx.notify() + }); + } ChannelBufferEvent::BufferEdited => { if cx.is_self_focused() || self.editor.is_focused(cx) { self.acknowledge_buffer_version(cx); @@ -179,7 +193,7 @@ impl ChannelView { self.channel_store.update(cx, |store, cx| { let channel_buffer = self.channel_buffer.read(cx); store.notes_changed( - channel_buffer.channel().id, + channel_buffer.channel_id, channel_buffer.epoch(), &channel_buffer.buffer().read(cx).version(), cx, @@ -187,7 +201,7 @@ impl ChannelView { }); } } - _ => {} + ChannelBufferEvent::CollaboratorsChanged => {} } } @@ -195,7 +209,7 @@ impl ChannelView { self.channel_store.update(cx, |store, cx| { let channel_buffer = self.channel_buffer.read(cx); store.acknowledge_notes_version( - channel_buffer.channel().id, + channel_buffer.channel_id, channel_buffer.epoch(), &channel_buffer.buffer().read(cx).version(), cx, @@ -250,11 +264,17 @@ impl Item for ChannelView { style: &theme::Tab, cx: &gpui::AppContext, ) -> AnyElement { - let channel_name = &self.channel_buffer.read(cx).channel().name; - let label = if self.channel_buffer.read(cx).is_connected() { - format!("#{}", channel_name) + let label = if let Some(channel) = self.channel(cx) { + match ( + channel.can_edit_notes(), + self.channel_buffer.read(cx).is_connected(), + ) { + (true, true) => format!("#{}", channel.name), + (false, true) => format!("#{} (read-only)", channel.name), + (_, false) => format!("#{} (disconnected)", channel.name), + } } else { - format!("#{} (disconnected)", channel_name) + format!("channel notes (disconnected)") }; Label::new(label, style.label.to_owned()).into_any() } @@ -298,6 +318,10 @@ impl Item for ChannelView { fn pixel_position_of_cursor(&self, cx: &AppContext) -> Option { self.editor.read(cx).pixel_position_of_cursor(cx) } + + fn to_item_events(event: &Self::Event) -> SmallVec<[ItemEvent; 2]> { + editor::Editor::to_item_events(event) + } } impl FollowableItem for ChannelView { @@ -313,7 +337,7 @@ impl FollowableItem for ChannelView { Some(proto::view::Variant::ChannelView( proto::view::ChannelView { - channel_id: channel_buffer.channel().id, + channel_id: channel_buffer.channel_id, editor: if let Some(proto::view::Variant::Editor(proto)) = self.editor.read(cx).to_state_proto(cx) { diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index 5b922037c51c1e5995d2eff31975915f9cd679ab..5a4dafb6d4179da42ffaa0ef9b2aed0f19144285 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -263,21 +263,22 @@ impl ChatPanel { fn set_active_chat(&mut self, chat: ModelHandle, cx: &mut ViewContext) { if self.active_chat.as_ref().map(|e| &e.0) != Some(&chat) { - self.markdown_data.clear(); - let id = { + let channel_id = chat.read(cx).channel_id; + { + self.markdown_data.clear(); let chat = chat.read(cx); - let channel = chat.channel().clone(); self.message_list.reset(chat.message_count()); + + let channel_name = chat.channel(cx).map(|channel| channel.name.clone()); self.input_editor.update(cx, |editor, cx| { - editor.set_channel(channel.clone(), cx); + editor.set_channel(channel_id, channel_name, cx); }); - channel.id }; let subscription = cx.subscribe(&chat, Self::channel_did_change); self.active_chat = Some((chat, subscription)); self.acknowledge_last_message(cx); self.channel_select.update(cx, |select, cx| { - if let Some(ix) = self.channel_store.read(cx).index_of_channel(id) { + if let Some(ix) = self.channel_store.read(cx).index_of_channel(channel_id) { select.set_selected_index(ix, cx); } }); @@ -361,7 +362,8 @@ 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).clone(); let is_continuation = last_message.id != this_message.id @@ -676,7 +678,7 @@ impl ChatPanel { .active_chat .as_ref() .and_then(|(chat, _)| { - (chat.read(cx).channel().id == selected_channel_id) + (chat.read(cx).channel_id == selected_channel_id) .then(|| Task::ready(anyhow::Ok(chat.clone()))) }) .unwrap_or_else(|| { @@ -714,7 +716,7 @@ impl ChatPanel { fn open_notes(&mut self, _: &OpenChannelNotes, cx: &mut ViewContext) { if let Some((chat, _)) = &self.active_chat { - let channel_id = chat.read(cx).channel().id; + let channel_id = chat.read(cx).channel_id; if let Some(workspace) = self.workspace.upgrade(cx) { ChannelView::open(channel_id, workspace, cx).detach(); } @@ -723,7 +725,7 @@ impl ChatPanel { fn join_call(&mut self, _: &JoinCall, cx: &mut ViewContext) { if let Some((chat, _)) = &self.active_chat { - let channel_id = chat.read(cx).channel().id; + let channel_id = chat.read(cx).channel_id; ActiveCall::global(cx) .update(cx, |call, cx| call.join_channel(channel_id, cx)) .detach_and_log_err(cx); diff --git a/crates/collab_ui/src/chat_panel/message_editor.rs b/crates/collab_ui/src/chat_panel/message_editor.rs index b72af36e632c6d3ff9cb39dcbe8dca0ba54897d8..6dbe3aa204e9edf19d605ef880e52dacf4fe627d 100644 --- a/crates/collab_ui/src/chat_panel/message_editor.rs +++ b/crates/collab_ui/src/chat_panel/message_editor.rs @@ -1,4 +1,4 @@ -use channel::{Channel, ChannelMembership, ChannelStore, MessageParams}; +use channel::{ChannelId, ChannelMembership, ChannelStore, MessageParams}; use client::UserId; use collections::HashMap; use editor::{AnchorRangeExt, Editor}; @@ -30,7 +30,7 @@ pub struct MessageEditor { users: HashMap, mentions: Vec, mentions_task: Option>, - channel: Option>, + channel_id: Option, } impl MessageEditor { @@ -68,24 +68,33 @@ impl MessageEditor { editor, channel_store, users: HashMap::default(), - channel: None, + channel_id: None, mentions: Vec::new(), mentions_task: None, } } - pub fn set_channel(&mut self, channel: Arc, cx: &mut ViewContext) { + pub fn set_channel( + &mut self, + channel_id: u64, + channel_name: Option, + cx: &mut ViewContext, + ) { self.editor.update(cx, |editor, cx| { - editor.set_placeholder_text(format!("Message #{}", channel.name), cx); + if let Some(channel_name) = channel_name { + editor.set_placeholder_text(format!("Message #{}", channel_name), cx); + } else { + editor.set_placeholder_text(format!("Message Channel"), cx); + } }); - self.channel = Some(channel); + self.channel_id = Some(channel_id); self.refresh_users(cx); } pub fn refresh_users(&mut self, cx: &mut ViewContext) { - if let Some(channel) = &self.channel { + if let Some(channel_id) = self.channel_id { let members = self.channel_store.update(cx, |store, cx| { - store.get_channel_member_details(channel.id, cx) + store.get_channel_member_details(channel_id, cx) }); cx.spawn(|this, mut cx| async move { let members = members.await?; diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index e907127ca47912af46ae57fa96935b77319e5b58..8c33727cfba668736777c5696bdc682ef3cd9319 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -120,22 +120,11 @@ struct StartLinkChannelFor { parent_id: Option, } -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct LinkChannel { - to: ChannelId, -} - #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] struct MoveChannel { to: ChannelId, } -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -struct UnlinkChannel { - channel_id: ChannelId, - parent_id: ChannelId, -} - type DraggedChannel = (Channel, Option); actions!( @@ -147,8 +136,7 @@ actions!( CollapseSelectedChannel, ExpandSelectedChannel, StartMoveChannel, - StartLinkChannel, - MoveOrLinkToSelected, + MoveSelected, InsertSpace, ] ); @@ -166,11 +154,8 @@ impl_actions!( JoinChannelCall, JoinChannelChat, CopyChannelLink, - LinkChannel, StartMoveChannelFor, - StartLinkChannelFor, MoveChannel, - UnlinkChannel, ToggleSelectedIx ] ); @@ -185,7 +170,7 @@ struct ChannelMoveClipboard { #[derive(Debug, Copy, Clone, PartialEq, Eq)] enum ClipboardIntent { Move, - Link, + // Link, } const COLLABORATION_PANEL_KEY: &'static str = "CollaborationPanel"; @@ -238,18 +223,6 @@ pub fn init(cx: &mut AppContext) { }, ); - cx.add_action( - |panel: &mut CollabPanel, - action: &StartLinkChannelFor, - _: &mut ViewContext| { - panel.channel_clipboard = Some(ChannelMoveClipboard { - channel_id: action.channel_id, - parent_id: action.parent_id, - intent: ClipboardIntent::Link, - }) - }, - ); - cx.add_action( |panel: &mut CollabPanel, _: &StartMoveChannel, _: &mut ViewContext| { if let Some((_, path)) = panel.selected_channel() { @@ -263,86 +236,51 @@ pub fn init(cx: &mut AppContext) { ); cx.add_action( - |panel: &mut CollabPanel, _: &StartLinkChannel, _: &mut ViewContext| { - if let Some((_, path)) = panel.selected_channel() { - panel.channel_clipboard = Some(ChannelMoveClipboard { - channel_id: path.channel_id(), - parent_id: path.parent_id(), - intent: ClipboardIntent::Link, - }) - } - }, - ); - - cx.add_action( - |panel: &mut CollabPanel, _: &MoveOrLinkToSelected, cx: &mut ViewContext| { + |panel: &mut CollabPanel, _: &MoveSelected, cx: &mut ViewContext| { let clipboard = panel.channel_clipboard.take(); if let Some(((selected_channel, _), clipboard)) = panel.selected_channel().zip(clipboard) { match clipboard.intent { - ClipboardIntent::Move if clipboard.parent_id.is_some() => { - let parent_id = clipboard.parent_id.unwrap(); - panel.channel_store.update(cx, |channel_store, cx| { - channel_store - .move_channel( - clipboard.channel_id, - parent_id, - selected_channel.id, - cx, - ) - .detach_and_log_err(cx) - }) - } - _ => panel.channel_store.update(cx, |channel_store, cx| { - channel_store - .link_channel(clipboard.channel_id, selected_channel.id, cx) - .detach_and_log_err(cx) + ClipboardIntent::Move => panel.channel_store.update(cx, |channel_store, cx| { + match clipboard.parent_id { + Some(parent_id) => channel_store.move_channel( + clipboard.channel_id, + parent_id, + selected_channel.id, + cx, + ), + None => channel_store.link_channel( + clipboard.channel_id, + selected_channel.id, + cx, + ), + } + .detach_and_log_err(cx) }), } } }, ); - cx.add_action( - |panel: &mut CollabPanel, action: &LinkChannel, cx: &mut ViewContext| { - if let Some(clipboard) = panel.channel_clipboard.take() { - panel.channel_store.update(cx, |channel_store, cx| { - channel_store - .link_channel(clipboard.channel_id, action.to, cx) - .detach_and_log_err(cx) - }) - } - }, - ); - cx.add_action( |panel: &mut CollabPanel, action: &MoveChannel, cx: &mut ViewContext| { if let Some(clipboard) = panel.channel_clipboard.take() { panel.channel_store.update(cx, |channel_store, cx| { - if let Some(parent) = clipboard.parent_id { - channel_store - .move_channel(clipboard.channel_id, parent, action.to, cx) - .detach_and_log_err(cx) - } else { - channel_store - .link_channel(clipboard.channel_id, action.to, cx) - .detach_and_log_err(cx) + match clipboard.parent_id { + Some(parent_id) => channel_store.move_channel( + clipboard.channel_id, + parent_id, + action.to, + cx, + ), + None => channel_store.link_channel(clipboard.channel_id, action.to, cx), } + .detach_and_log_err(cx) }) } }, ); - - cx.add_action( - |panel: &mut CollabPanel, action: &UnlinkChannel, cx: &mut ViewContext| { - panel.channel_store.update(cx, |channel_store, cx| { - channel_store - .unlink_channel(action.channel_id, action.parent_id, cx) - .detach_and_log_err(cx) - }) - }, - ); } #[derive(Debug)] @@ -2235,33 +2173,23 @@ impl CollabPanel { this.deploy_channel_context_menu(Some(e.position), &path, ix, cx); } }) - .on_up(MouseButton::Left, move |e, this, cx| { + .on_up(MouseButton::Left, move |_, this, cx| { if let Some((_, dragged_channel)) = cx .global::>() .currently_dragged::(cx.window()) { - if e.modifiers.alt { - this.channel_store.update(cx, |channel_store, cx| { - channel_store - .link_channel(dragged_channel.0.id, channel_id, cx) - .detach_and_log_err(cx) - }) - } else { - this.channel_store.update(cx, |channel_store, cx| { - match dragged_channel.1 { - Some(parent_id) => channel_store.move_channel( - dragged_channel.0.id, - parent_id, - channel_id, - cx, - ), - None => { - channel_store.link_channel(dragged_channel.0.id, channel_id, cx) - } - } - .detach_and_log_err(cx) - }) - } + this.channel_store.update(cx, |channel_store, cx| { + match dragged_channel.1 { + Some(parent_id) => channel_store.move_channel( + dragged_channel.0.id, + parent_id, + channel_id, + cx, + ), + None => channel_store.link_channel(dragged_channel.0.id, channel_id, cx), + } + .detach_and_log_err(cx) + }) } }) .on_move({ @@ -2288,18 +2216,10 @@ impl CollabPanel { }) .as_draggable( (channel.clone(), path.parent_id()), - move |modifiers, (channel, _), cx: &mut ViewContext| { + move |_, (channel, _), cx: &mut ViewContext| { let theme = &theme::current(cx).collab_panel; Flex::::row() - .with_children(modifiers.alt.then(|| { - Svg::new("icons/plus.svg") - .with_color(theme.channel_hash.color) - .constrained() - .with_width(theme.channel_hash.width) - .aligned() - .left() - })) .with_child( Svg::new("icons/hash.svg") .with_color(theme.channel_hash.color) @@ -2721,7 +2641,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([ @@ -2738,20 +2662,6 @@ impl CollabPanel { location: path.clone(), }, ), - ContextMenuItem::Separator, - ]); - - if let Some(parent_id) = parent_id { - items.push(ContextMenuItem::action( - "Unlink from parent", - UnlinkChannel { - channel_id: path.channel_id(), - parent_id, - }, - )); - } - - items.extend([ ContextMenuItem::action( "Move this channel", StartMoveChannelFor { @@ -2759,13 +2669,6 @@ impl CollabPanel { parent_id, }, ), - ContextMenuItem::action( - "Link this channel", - StartLinkChannelFor { - channel_id: path.channel_id(), - parent_id, - }, - ), ]); if let Some(channel_name) = channel_name { @@ -2776,12 +2679,6 @@ impl CollabPanel { to: path.channel_id(), }, )); - items.push(ContextMenuItem::action( - format!("Link '#{}' here", channel_name), - LinkChannel { - to: path.channel_id(), - }, - )); } items.extend([ @@ -3160,7 +3057,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/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index 14d9466e3e8b76282871c56b233863752f9787c3..cef8faf6016d60c2df12e4ff6404dd86d9591480 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -88,8 +88,10 @@ impl View for CollabTitlebarItem { .zip(peer_id) .zip(ActiveCall::global(cx).read(cx).room().cloned()) { - right_container - .add_children(self.render_in_call_share_unshare_button(&workspace, &theme, cx)); + if room.read(cx).can_publish() { + right_container + .add_children(self.render_in_call_share_unshare_button(&workspace, &theme, cx)); + } right_container.add_child(self.render_leave_call(&theme, cx)); let muted = room.read(cx).is_muted(cx); let speaking = room.read(cx).is_speaking(); @@ -97,9 +99,14 @@ impl View for CollabTitlebarItem { self.render_current_user(&workspace, &theme, &user, peer_id, muted, speaking, cx), ); left_container.add_children(self.render_collaborators(&workspace, &theme, &room, cx)); - right_container.add_child(self.render_toggle_mute(&theme, &room, cx)); + if room.read(cx).can_publish() { + right_container.add_child(self.render_toggle_mute(&theme, &room, cx)); + } right_container.add_child(self.render_toggle_deafen(&theme, &room, cx)); - right_container.add_child(self.render_toggle_screen_sharing_button(&theme, &room, cx)); + if room.read(cx).can_publish() { + right_container + .add_child(self.render_toggle_screen_sharing_button(&theme, &room, cx)); + } } let status = workspace.read(cx).client().status(); diff --git a/crates/collab_ui/src/notification_panel.rs b/crates/collab_ui/src/notification_panel.rs index e245a919f3a549ddb264720bfe0abe9103434b9c..1720c8dcdc8fe382c469d6962b75811df80f4c8d 100644 --- a/crates/collab_ui/src/notification_panel.rs +++ b/crates/collab_ui/src/notification_panel.rs @@ -477,7 +477,7 @@ impl NotificationPanel { return panel.read_with(cx, |panel, cx| { panel.is_scrolled_to_bottom() && panel.active_chat().map_or(false, |chat| { - chat.read(cx).channel().id == *channel_id + chat.read(cx).channel_id == *channel_id }) }); } diff --git a/crates/live_kit_client/src/test.rs b/crates/live_kit_client/src/test.rs index 8df8ab4abb6142fea292aec7f3377e86e93cb388..b39ad333d2c2d14cbae35a69ac6072d343b31bbd 100644 --- a/crates/live_kit_client/src/test.rs +++ b/crates/live_kit_client/src/test.rs @@ -306,6 +306,16 @@ impl live_kit_server::api::Client for TestApiClient { token::VideoGrant::to_join(room), ) } + + fn guest_token(&self, room: &str, identity: &str) -> Result { + let server = TestServer::get(&self.url)?; + token::create( + &server.api_key, + &server.secret_key, + Some(identity), + token::VideoGrant::for_guest(room), + ) + } } pub type Sid = String; diff --git a/crates/live_kit_server/src/api.rs b/crates/live_kit_server/src/api.rs index 417a17bdc9859111d57806200ca35595866c79ef..2c1e174fb41551d22b498b75d9ce36011ca5a5a9 100644 --- a/crates/live_kit_server/src/api.rs +++ b/crates/live_kit_server/src/api.rs @@ -12,6 +12,7 @@ pub trait Client: Send + Sync { async fn delete_room(&self, name: String) -> Result<()>; async fn remove_participant(&self, room: String, identity: String) -> Result<()>; fn room_token(&self, room: &str, identity: &str) -> Result; + fn guest_token(&self, room: &str, identity: &str) -> Result; } #[derive(Clone)] @@ -138,4 +139,13 @@ impl Client for LiveKitClient { token::VideoGrant::to_join(room), ) } + + fn guest_token(&self, room: &str, identity: &str) -> Result { + token::create( + &self.key, + &self.secret, + Some(identity), + token::VideoGrant::for_guest(room), + ) + } } diff --git a/crates/live_kit_server/src/token.rs b/crates/live_kit_server/src/token.rs index 072a8be0c9c6fc93f4b320a63650cc3a3f148c9c..b98f5892aea9ead8472b614da8491bdf7f2a38b4 100644 --- a/crates/live_kit_server/src/token.rs +++ b/crates/live_kit_server/src/token.rs @@ -57,6 +57,15 @@ impl<'a> VideoGrant<'a> { ..Default::default() } } + + pub fn for_guest(room: &'a str) -> Self { + Self { + room: Some(Cow::Borrowed(room)), + room_join: Some(true), + can_subscribe: Some(true), + ..Default::default() + } + } } pub fn create( diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index f69d0284830bed4e508b5cccd49f6143ffc3cf74..9ecdbde66b7208937f046e902b746fcbf3e1f488 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -342,6 +342,7 @@ message RoomUpdated { message LiveKitConnectionInfo { string server_url = 1; string token = 2; + bool can_publish = 3; } message ShareProject { @@ -977,7 +978,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; } @@ -1585,6 +1585,7 @@ message Channel { uint64 id = 1; string name = 2; ChannelVisibility visibility = 3; + ChannelRole role = 4; } message Contact {