From 6c69f4e5c688fe7d06e5e70a57999a28d06a7f18 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 26 Jan 2024 11:17:16 -0700 Subject: [PATCH] Simplify Membership Management (#6747) Simplify Zed's collaboration system by: - Only allowing member management on root channels. - Disallowing moving sub-channels between different roots. - Disallowing public channels nested under private channels. This should make the mental model easier to understand, and makes it clearer who has what access. It is also significantly simpler to implement, and so hopefully more performant and less buggy. Still TODO: - [x] Update collab_ui to match. - [x] Fix channel buffer tests. Release Notes: - Simplified channel membership management. --- crates/channel/src/channel_buffer.rs | 5 +- crates/channel/src/channel_store.rs | 281 ++++++-- .../src/channel_store/channel_index.rs | 83 +-- crates/channel/src/channel_store_tests.rs | 42 +- crates/collab/src/db.rs | 41 +- crates/collab/src/db/ids.rs | 9 + crates/collab/src/db/queries/buffers.rs | 61 +- crates/collab/src/db/queries/channels.rs | 632 +++++------------- crates/collab/src/db/queries/messages.rs | 44 +- crates/collab/src/db/tables/channel.rs | 8 + crates/collab/src/db/tests.rs | 5 +- crates/collab/src/db/tests/buffer_tests.rs | 101 +-- crates/collab/src/db/tests/channel_tests.rs | 231 +++---- crates/collab/src/db/tests/message_tests.rs | 94 +-- crates/collab/src/rpc.rs | 196 +++--- .../collab/src/tests/channel_buffer_tests.rs | 5 - .../collab/src/tests/channel_guest_tests.rs | 7 + .../collab/src/tests/channel_message_tests.rs | 5 - crates/collab/src/tests/channel_tests.rs | 172 +---- crates/collab/src/tests/integration_tests.rs | 1 + crates/collab/src/tests/test_server.rs | 1 + crates/collab_ui/src/channel_view.rs | 2 +- crates/collab_ui/src/chat_panel.rs | 2 +- crates/collab_ui/src/collab_panel.rs | 186 ++++-- .../src/collab_panel/channel_modal.rs | 16 +- crates/gpui/src/elements/div.rs | 27 +- crates/project_panel/src/project_panel.rs | 2 +- crates/rpc/proto/zed.proto | 31 +- crates/rpc/src/proto.rs | 1 + crates/rpc/src/rpc.rs | 2 +- crates/workspace/src/pane.rs | 12 +- 31 files changed, 882 insertions(+), 1423 deletions(-) diff --git a/crates/channel/src/channel_buffer.rs b/crates/channel/src/channel_buffer.rs index b5f4a06b972d8925735d0e692d4e682a32a19b5d..e87954edf6c1b664e28768dbff9e0596595881f6 100644 --- a/crates/channel/src/channel_buffer.rs +++ b/crates/channel/src/channel_buffer.rs @@ -61,11 +61,12 @@ impl ChannelBuffer { .map(language::proto::deserialize_operation) .collect::, _>>()?; - let buffer = cx.new_model(|_| { + let buffer = cx.new_model(|cx| { + let capability = channel_store.read(cx).channel_capability(channel.id); language::Buffer::remote( response.buffer_id, response.replica_id as u16, - channel.channel_buffer_capability(), + capability, base_text, ) })?; diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 59b69405a5c8dd5160e9d61d6fd8d64fb1370a94..20a0955678901680334bcd5ab175d2e45a191303 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -13,11 +13,11 @@ use gpui::{ }; use language::Capability; use rpc::{ - proto::{self, ChannelVisibility}, + proto::{self, ChannelRole, ChannelVisibility}, TypedEnvelope, }; use std::{mem, sync::Arc, time::Duration}; -use util::{async_maybe, ResultExt}; +use util::{async_maybe, maybe, ResultExt}; pub fn init(client: &Arc, user_store: Model, cx: &mut AppContext) { let channel_store = @@ -29,33 +29,47 @@ pub const RECONNECT_TIMEOUT: Duration = Duration::from_secs(30); pub type ChannelId = u64; +#[derive(Debug, Clone, Default)] +struct NotesVersion { + epoch: u64, + version: clock::Global, +} + pub struct ChannelStore { pub channel_index: ChannelIndex, channel_invitations: Vec>, channel_participants: HashMap>>, + channel_states: HashMap, + outgoing_invites: HashSet<(ChannelId, UserId)>, update_channels_tx: mpsc::UnboundedSender, opened_buffers: HashMap>, opened_chats: HashMap>, client: Arc, user_store: Model, - _rpc_subscription: Subscription, + _rpc_subscriptions: [Subscription; 2], _watch_connection_status: Task>, disconnect_channel_buffers_task: Option>, _update_channels: Task<()>, } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug)] pub struct Channel { pub id: ChannelId, pub name: SharedString, pub visibility: proto::ChannelVisibility, - pub role: proto::ChannelRole, - pub unseen_note_version: Option<(u64, clock::Global)>, - pub unseen_message_id: Option, pub parent_path: Vec, } +#[derive(Default)] +pub struct ChannelState { + latest_chat_message: Option, + latest_notes_versions: Option, + observed_chat_message: Option, + observed_notes_versions: Option, + role: Option, +} + impl Channel { pub fn link(&self) -> String { RELEASE_CHANNEL.link_prefix().to_owned() @@ -65,6 +79,17 @@ impl Channel { + &self.id.to_string() } + pub fn is_root_channel(&self) -> bool { + self.parent_path.is_empty() + } + + pub fn root_id(&self) -> ChannelId { + self.parent_path + .first() + .map(|id| *id as ChannelId) + .unwrap_or(self.id) + } + pub fn slug(&self) -> String { let slug: String = self .name @@ -74,14 +99,6 @@ impl Channel { slug.trim_matches(|c| c == '-').to_string() } - - pub fn channel_buffer_capability(&self) -> Capability { - if self.role == proto::ChannelRole::Member || self.role == proto::ChannelRole::Admin { - Capability::ReadWrite - } else { - Capability::ReadOnly - } - } } pub struct ChannelMembership { @@ -100,8 +117,7 @@ impl ChannelMembership { }, kind_order: match self.kind { proto::channel_member::Kind::Member => 0, - proto::channel_member::Kind::AncestorMember => 1, - proto::channel_member::Kind::Invitee => 2, + proto::channel_member::Kind::Invitee => 1, }, username_order: self.user.github_login.as_str(), } @@ -137,8 +153,10 @@ impl ChannelStore { user_store: Model, cx: &mut ModelContext, ) -> Self { - let rpc_subscription = - client.add_message_handler(cx.weak_model(), Self::handle_update_channels); + let rpc_subscriptions = [ + client.add_message_handler(cx.weak_model(), Self::handle_update_channels), + client.add_message_handler(cx.weak_model(), Self::handle_update_user_channels), + ]; let mut connection_status = client.status(); let (update_channels_tx, mut update_channels_rx) = mpsc::unbounded(); @@ -175,7 +193,7 @@ impl ChannelStore { update_channels_tx, client, user_store, - _rpc_subscription: rpc_subscription, + _rpc_subscriptions: rpc_subscriptions, _watch_connection_status: watch_connection_status, disconnect_channel_buffers_task: None, _update_channels: cx.spawn(|this, mut cx| async move { @@ -195,6 +213,7 @@ impl ChannelStore { .await .log_err(); }), + channel_states: Default::default(), } } @@ -306,62 +325,70 @@ impl ChannelStore { }) } - pub fn has_channel_buffer_changed(&self, channel_id: ChannelId) -> Option { - self.channel_index - .by_id() + pub fn has_channel_buffer_changed(&self, channel_id: ChannelId) -> bool { + self.channel_states .get(&channel_id) - .map(|channel| channel.unseen_note_version.is_some()) + .is_some_and(|state| state.has_channel_buffer_changed()) } - pub fn has_new_messages(&self, channel_id: ChannelId) -> Option { - self.channel_index - .by_id() + pub fn has_new_messages(&self, channel_id: ChannelId) -> bool { + self.channel_states .get(&channel_id) - .map(|channel| channel.unseen_message_id.is_some()) + .is_some_and(|state| state.has_new_messages()) } - pub fn notes_changed( + pub fn acknowledge_message_id( &mut self, channel_id: ChannelId, - epoch: u64, - version: &clock::Global, + message_id: u64, cx: &mut ModelContext, ) { - self.channel_index.note_changed(channel_id, epoch, version); + self.channel_states + .entry(channel_id) + .or_insert_with(|| Default::default()) + .acknowledge_message_id(message_id); cx.notify(); } - pub fn new_message( + pub fn update_latest_message_id( &mut self, channel_id: ChannelId, message_id: u64, cx: &mut ModelContext, ) { - self.channel_index.new_message(channel_id, message_id); + self.channel_states + .entry(channel_id) + .or_insert_with(|| Default::default()) + .update_latest_message_id(message_id); cx.notify(); } - pub fn acknowledge_message_id( + pub fn acknowledge_notes_version( &mut self, channel_id: ChannelId, - message_id: u64, + epoch: u64, + version: &clock::Global, cx: &mut ModelContext, ) { - self.channel_index - .acknowledge_message_id(channel_id, message_id); - cx.notify(); + self.channel_states + .entry(channel_id) + .or_insert_with(|| Default::default()) + .acknowledge_notes_version(epoch, version); + cx.notify() } - pub fn acknowledge_notes_version( + pub fn update_latest_notes_version( &mut self, channel_id: ChannelId, epoch: u64, version: &clock::Global, cx: &mut ModelContext, ) { - self.channel_index - .acknowledge_note_version(channel_id, epoch, version); - cx.notify(); + self.channel_states + .entry(channel_id) + .or_insert_with(|| Default::default()) + .update_latest_notes_version(epoch, version); + cx.notify() } pub fn open_channel_chat( @@ -454,10 +481,42 @@ impl ChannelStore { } 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 + self.channel_role(channel_id) == proto::ChannelRole::Admin + } + + pub fn is_root_channel(&self, channel_id: ChannelId) -> bool { + self.channel_index + .by_id() + .get(&channel_id) + .map_or(false, |channel| channel.is_root_channel()) + } + + pub fn is_public_channel(&self, channel_id: ChannelId) -> bool { + self.channel_index + .by_id() + .get(&channel_id) + .map_or(false, |channel| { + channel.visibility == ChannelVisibility::Public + }) + } + + pub fn channel_capability(&self, channel_id: ChannelId) -> Capability { + match self.channel_role(channel_id) { + ChannelRole::Admin | ChannelRole::Member => Capability::ReadWrite, + _ => Capability::ReadOnly, + } + } + + pub fn channel_role(&self, channel_id: ChannelId) -> proto::ChannelRole { + maybe!({ + let mut channel = self.channel_for_id(channel_id)?; + if !channel.is_root_channel() { + channel = self.channel_for_id(channel.root_id())?; + } + let root_channel_state = self.channel_states.get(&channel.id); + root_channel_state?.role + }) + .unwrap_or(proto::ChannelRole::Guest) } pub fn channel_participants(&self, channel_id: ChannelId) -> &[Arc] { @@ -508,7 +567,7 @@ impl ChannelStore { pub fn move_channel( &mut self, channel_id: ChannelId, - to: Option, + to: ChannelId, cx: &mut ModelContext, ) -> Task> { let client = self.client.clone(); @@ -747,6 +806,36 @@ impl ChannelStore { Ok(()) } + async fn handle_update_user_channels( + this: Model, + message: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result<()> { + this.update(&mut cx, |this, cx| { + for buffer_version in message.payload.observed_channel_buffer_version { + let version = language::proto::deserialize_version(&buffer_version.version); + this.acknowledge_notes_version( + buffer_version.channel_id, + buffer_version.epoch, + &version, + cx, + ); + } + for message_id in message.payload.observed_channel_message_id { + this.acknowledge_message_id(message_id.channel_id, message_id.message_id, cx); + } + for membership in message.payload.channel_memberships { + if let Some(role) = ChannelRole::from_i32(membership.role) { + this.channel_states + .entry(membership.channel_id) + .or_insert_with(|| ChannelState::default()) + .set_role(role) + } + } + }) + } + fn handle_connect(&mut self, cx: &mut ModelContext) -> Task> { self.channel_index.clear(); self.channel_invitations.clear(); @@ -909,10 +998,7 @@ impl ChannelStore { Arc::new(Channel { id: channel.id, visibility: channel.visibility(), - role: channel.role(), name: channel.name.into(), - unseen_note_version: None, - unseen_message_id: None, parent_path: channel.parent_path, }), ), @@ -921,8 +1007,8 @@ impl ChannelStore { let channels_changed = !payload.channels.is_empty() || !payload.delete_channels.is_empty() - || !payload.unseen_channel_messages.is_empty() - || !payload.unseen_channel_buffer_changes.is_empty(); + || !payload.latest_channel_message_ids.is_empty() + || !payload.latest_channel_buffer_versions.is_empty(); if channels_changed { if !payload.delete_channels.is_empty() { @@ -963,20 +1049,19 @@ impl ChannelStore { } } - for unseen_buffer_change in payload.unseen_channel_buffer_changes { - let version = language::proto::deserialize_version(&unseen_buffer_change.version); - index.note_changed( - unseen_buffer_change.channel_id, - unseen_buffer_change.epoch, - &version, - ); + for latest_buffer_version in payload.latest_channel_buffer_versions { + let version = language::proto::deserialize_version(&latest_buffer_version.version); + self.channel_states + .entry(latest_buffer_version.channel_id) + .or_default() + .update_latest_notes_version(latest_buffer_version.epoch, &version) } - for unseen_channel_message in payload.unseen_channel_messages { - index.new_messages( - unseen_channel_message.channel_id, - unseen_channel_message.message_id, - ); + for latest_channel_message in payload.latest_channel_message_ids { + self.channel_states + .entry(latest_channel_message.channel_id) + .or_default() + .update_latest_message_id(latest_channel_message.message_id); } } @@ -1025,3 +1110,69 @@ impl ChannelStore { })) } } + +impl ChannelState { + fn set_role(&mut self, role: ChannelRole) { + self.role = Some(role); + } + + fn has_channel_buffer_changed(&self) -> bool { + if let Some(latest_version) = &self.latest_notes_versions { + if let Some(observed_version) = &self.observed_notes_versions { + latest_version.epoch > observed_version.epoch + || latest_version + .version + .changed_since(&observed_version.version) + } else { + true + } + } else { + false + } + } + + fn has_new_messages(&self) -> bool { + let latest_message_id = self.latest_chat_message; + let observed_message_id = self.observed_chat_message; + + latest_message_id.is_some_and(|latest_message_id| { + latest_message_id > observed_message_id.unwrap_or_default() + }) + } + + fn acknowledge_message_id(&mut self, message_id: u64) { + let observed = self.observed_chat_message.get_or_insert(message_id); + *observed = (*observed).max(message_id); + } + + fn update_latest_message_id(&mut self, message_id: u64) { + self.latest_chat_message = + Some(message_id.max(self.latest_chat_message.unwrap_or_default())); + } + + fn acknowledge_notes_version(&mut self, epoch: u64, version: &clock::Global) { + if let Some(existing) = &mut self.observed_notes_versions { + if existing.epoch == epoch { + existing.version.join(version); + return; + } + } + self.observed_notes_versions = Some(NotesVersion { + epoch, + version: version.clone(), + }); + } + + fn update_latest_notes_version(&mut self, epoch: u64, version: &clock::Global) { + if let Some(existing) = &mut self.latest_notes_versions { + if existing.epoch == epoch { + existing.version.join(version); + return; + } + } + self.latest_notes_versions = Some(NotesVersion { + epoch, + version: version.clone(), + }); + } +} diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index 88012a57c2374a32d8d52beef0f2e28949315cbb..e70b3e4c46352b039f3b0bb8c58d7dcdcc1a275d 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -37,43 +37,6 @@ impl ChannelIndex { channels_by_id: &mut self.channels_by_id, } } - - pub fn acknowledge_note_version( - &mut self, - channel_id: ChannelId, - epoch: u64, - version: &clock::Global, - ) { - if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - let channel = Arc::make_mut(channel); - if let Some((unseen_epoch, unseen_version)) = &channel.unseen_note_version { - if epoch > *unseen_epoch - || epoch == *unseen_epoch && version.observed_all(unseen_version) - { - channel.unseen_note_version = None; - } - } - } - } - - pub fn acknowledge_message_id(&mut self, channel_id: ChannelId, message_id: u64) { - if let Some(channel) = self.channels_by_id.get_mut(&channel_id) { - let channel = Arc::make_mut(channel); - if let Some(unseen_message_id) = channel.unseen_message_id { - if message_id >= unseen_message_id { - channel.unseen_message_id = None; - } - } - } - } - - pub fn note_changed(&mut self, channel_id: ChannelId, epoch: u64, version: &clock::Global) { - insert_note_changed(&mut self.channels_by_id, channel_id, epoch, version); - } - - pub fn new_message(&mut self, channel_id: ChannelId, message_id: u64) { - insert_new_message(&mut self.channels_by_id, channel_id, message_id) - } } /// A guard for ensuring that the paths index maintains its sort and uniqueness @@ -85,36 +48,25 @@ pub struct ChannelPathsInsertGuard<'a> { } impl<'a> ChannelPathsInsertGuard<'a> { - pub fn note_changed(&mut self, channel_id: ChannelId, epoch: u64, version: &clock::Global) { - insert_note_changed(self.channels_by_id, channel_id, epoch, version); - } - - pub fn new_messages(&mut self, channel_id: ChannelId, message_id: u64) { - insert_new_message(self.channels_by_id, channel_id, message_id) - } - 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.name != channel_proto.name + || existing_channel.parent_path != channel_proto.parent_path; existing_channel.visibility = channel_proto.visibility(); - existing_channel.role = channel_proto.role(); existing_channel.name = channel_proto.name.into(); + existing_channel.parent_path = channel_proto.parent_path.into(); } else { self.channels_by_id.insert( channel_proto.id, Arc::new(Channel { id: channel_proto.id, visibility: channel_proto.visibility(), - role: channel_proto.role(), name: channel_proto.name.into(), - unseen_note_version: None, - unseen_message_id: None, parent_path: channel_proto.parent_path, }), ); @@ -153,32 +105,3 @@ fn channel_path_sorting_key<'a>( .filter_map(|id| Some(channels_by_id.get(id)?.name.as_ref())) .chain(name) } - -fn insert_note_changed( - channels_by_id: &mut BTreeMap>, - channel_id: u64, - epoch: u64, - version: &clock::Global, -) { - if let Some(channel) = channels_by_id.get_mut(&channel_id) { - let unseen_version = Arc::make_mut(channel) - .unseen_note_version - .get_or_insert((0, clock::Global::new())); - if epoch > unseen_version.0 { - *unseen_version = (epoch, version.clone()); - } else { - unseen_version.1.join(version); - } - } -} - -fn insert_new_message( - channels_by_id: &mut BTreeMap>, - channel_id: u64, - message_id: u64, -) { - if let Some(channel) = channels_by_id.get_mut(&channel_id) { - let unseen_message_id = Arc::make_mut(channel).unseen_message_id.get_or_insert(0); - *unseen_message_id = message_id.max(*unseen_message_id); - } -} diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index 0b07918acfba7b9fe3ad87ef001f5fb7c5eafb30..ad2098766d619ec931788ad27f6a99d244817b46 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -19,14 +19,12 @@ fn test_update_channels(cx: &mut AppContext) { id: 1, name: "b".to_string(), visibility: proto::ChannelVisibility::Members as i32, - role: proto::ChannelRole::Admin.into(), parent_path: Vec::new(), }, proto::Channel { id: 2, name: "a".to_string(), visibility: proto::ChannelVisibility::Members as i32, - role: proto::ChannelRole::Member.into(), parent_path: Vec::new(), }, ], @@ -38,8 +36,8 @@ fn test_update_channels(cx: &mut AppContext) { &channel_store, &[ // - (0, "a".to_string(), proto::ChannelRole::Member), - (0, "b".to_string(), proto::ChannelRole::Admin), + (0, "a".to_string()), + (0, "b".to_string()), ], cx, ); @@ -52,14 +50,12 @@ fn test_update_channels(cx: &mut AppContext) { id: 3, name: "x".to_string(), visibility: proto::ChannelVisibility::Members as i32, - role: proto::ChannelRole::Admin.into(), parent_path: vec![1], }, proto::Channel { id: 4, name: "y".to_string(), visibility: proto::ChannelVisibility::Members as i32, - role: proto::ChannelRole::Member.into(), parent_path: vec![2], }, ], @@ -70,10 +66,10 @@ fn test_update_channels(cx: &mut AppContext) { assert_channels( &channel_store, &[ - (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), + (0, "a".to_string()), + (1, "y".to_string()), + (0, "b".to_string()), + (1, "x".to_string()), ], cx, ); @@ -91,21 +87,18 @@ 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(), parent_path: vec![], }, proto::Channel { id: 1, name: "b".to_string(), visibility: proto::ChannelVisibility::Members as i32, - role: proto::ChannelRole::Admin.into(), parent_path: vec![0], }, proto::Channel { id: 2, name: "c".to_string(), visibility: proto::ChannelVisibility::Members as i32, - role: proto::ChannelRole::Admin.into(), parent_path: vec![0, 1], }, ], @@ -118,9 +111,9 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { &channel_store, &[ // - (0, "a".to_string(), proto::ChannelRole::Admin), - (1, "b".to_string(), proto::ChannelRole::Admin), - (2, "c".to_string(), proto::ChannelRole::Admin), + (0, "a".to_string()), + (1, "b".to_string()), + (2, "c".to_string()), ], cx, ); @@ -135,11 +128,7 @@ 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(), proto::ChannelRole::Admin)], - cx, - ); + assert_channels(&channel_store, &[(0, "a".to_string())], cx); } #[gpui::test] @@ -156,18 +145,13 @@ 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(), parent_path: vec![], }], ..Default::default() }); cx.executor().run_until_parked(); cx.update(|cx| { - assert_channels( - &channel_store, - &[(0, "the-channel".to_string(), proto::ChannelRole::Member)], - cx, - ); + assert_channels(&channel_store, &[(0, "the-channel".to_string())], cx); }); let get_users = server.receive::().await.unwrap(); @@ -368,13 +352,13 @@ fn update_channels( #[track_caller] fn assert_channels( channel_store: &Model, - expected_channels: &[(usize, String, proto::ChannelRole)], + expected_channels: &[(usize, String)], cx: &mut AppContext, ) { let actual = channel_store.update(cx, |store, _| { store .ordered_channels() - .map(|(depth, channel)| (depth, channel.name.to_string(), channel.role)) + .map(|(depth, channel)| (depth, channel.name.to_string())) .collect::>() }); assert_eq!(actual, expected_channels); diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index d1a717e66e2cc031afab4f529e14f67891464a63..2e8540641bc4b5a5f5a3a08c1c7cbd6b7efe57b8 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -40,7 +40,7 @@ use std::{ sync::Arc, time::Duration, }; -use tables::*; +pub use tables::*; use tokio::sync::{Mutex, OwnedMutexGuard}; pub use ids::*; @@ -502,35 +502,6 @@ pub struct NewUserResult { pub signup_device_id: Option, } -/// The result of moving a channel. -#[derive(Debug)] -pub struct MoveChannelResult { - pub previous_participants: Vec, - pub descendent_ids: Vec, -} - -/// The result of renaming a channel. -#[derive(Debug)] -pub struct RenameChannelResult { - pub channel: Channel, - pub participants_to_update: HashMap, -} - -/// The result of creating a channel. -#[derive(Debug)] -pub struct CreateChannelResult { - pub channel: Channel, - pub participants_to_update: Vec<(UserId, ChannelsForUser)>, -} - -/// The result of setting a channel's visibility. -#[derive(Debug)] -pub struct SetChannelVisibilityResult { - pub participants_to_update: HashMap, - pub participants_to_remove: HashSet, - pub channels_to_remove: Vec, -} - /// The result of updating a channel membership. #[derive(Debug)] pub struct MembershipUpdated { @@ -570,18 +541,16 @@ pub struct Channel { pub id: ChannelId, pub name: String, pub visibility: ChannelVisibility, - pub role: ChannelRole, /// parent_path is the channel ids from the root to this one (not including this one) pub parent_path: Vec, } impl Channel { - fn from_model(value: channel::Model, role: ChannelRole) -> Self { + fn from_model(value: channel::Model) -> Self { Channel { id: value.id, visibility: value.visibility, name: value.clone().name, - role, parent_path: value.ancestors().collect(), } } @@ -591,7 +560,6 @@ impl Channel { id: self.id.to_proto(), name: self.name.clone(), visibility: self.visibility.into(), - role: self.role.into(), parent_path: self.parent_path.iter().map(|c| c.to_proto()).collect(), } } @@ -617,9 +585,10 @@ impl ChannelMember { #[derive(Debug, PartialEq)] pub struct ChannelsForUser { pub channels: Vec, + pub channel_memberships: Vec, pub channel_participants: HashMap>, - pub unseen_buffer_changes: Vec, - pub channel_messages: Vec, + pub latest_buffer_versions: Vec, + pub latest_channel_messages: Vec, } #[derive(Debug)] diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 9a6a1e78f3cf03660697acdd463d803825e84e36..d69e19643a502a826e4c04b1716ac9865198d045 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -129,6 +129,15 @@ impl ChannelRole { } } + pub fn can_see_channel(&self, visibility: ChannelVisibility) -> bool { + use ChannelRole::*; + match self { + Admin | Member => true, + Guest => visibility == ChannelVisibility::Public, + Banned => false, + } + } + /// True if the role allows access to all descendant channels pub fn can_see_all_descendants(&self) -> bool { use ChannelRole::*; diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index c19cd530a017fe93c89c68b4f5b814836cd1ef77..59b8f8d01fe117300b741890715087a42c8dcc6b 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -748,18 +748,11 @@ impl Database { .await } - pub async fn unseen_channel_buffer_changes( + pub async fn latest_channel_buffer_changes( &self, - user_id: UserId, channel_ids: &[ChannelId], tx: &DatabaseTransaction, - ) -> Result> { - #[derive(Debug, Clone, Copy, EnumIter, DeriveColumn)] - enum QueryIds { - ChannelId, - Id, - } - + ) -> Result> { let mut channel_ids_by_buffer_id = HashMap::default(); let mut rows = buffer::Entity::find() .filter(buffer::Column::ChannelId.is_in(channel_ids.iter().copied())) @@ -771,51 +764,23 @@ impl Database { } drop(rows); - let mut observed_edits_by_buffer_id = HashMap::default(); - let mut rows = observed_buffer_edits::Entity::find() - .filter(observed_buffer_edits::Column::UserId.eq(user_id)) - .filter( - observed_buffer_edits::Column::BufferId - .is_in(channel_ids_by_buffer_id.keys().copied()), - ) - .stream(&*tx) - .await?; - while let Some(row) = rows.next().await { - let row = row?; - observed_edits_by_buffer_id.insert(row.buffer_id, row); - } - drop(rows); - let latest_operations = self .get_latest_operations_for_buffers(channel_ids_by_buffer_id.keys().copied(), &*tx) .await?; - let mut changes = Vec::default(); - for latest in latest_operations { - if let Some(observed) = observed_edits_by_buffer_id.get(&latest.buffer_id) { - if ( - observed.epoch, - observed.lamport_timestamp, - observed.replica_id, - ) >= (latest.epoch, latest.lamport_timestamp, latest.replica_id) - { - continue; - } - } - - if let Some(channel_id) = channel_ids_by_buffer_id.get(&latest.buffer_id) { - changes.push(proto::UnseenChannelBufferChange { - channel_id: channel_id.to_proto(), - epoch: latest.epoch as u64, + Ok(latest_operations + .iter() + .flat_map(|op| { + Some(proto::ChannelBufferVersion { + channel_id: channel_ids_by_buffer_id.get(&op.buffer_id)?.to_proto(), + epoch: op.epoch as u64, version: vec![proto::VectorClockEntry { - replica_id: latest.replica_id as u32, - timestamp: latest.lamport_timestamp as u32, + replica_id: op.replica_id as u32, + timestamp: op.lamport_timestamp as u32, }], - }); - } - } - - Ok(changes) + }) + }) + .collect()) } /// Returns the latest operations for the buffers with the specified IDs. diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 10b73a7094150ece81ed33310788f01da1738142..b7785b0f7f1b934a2e5e62e172f62f5ee8a8940a 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -19,7 +19,7 @@ impl Database { #[cfg(test)] pub async fn create_root_channel(&self, name: &str, creator_id: UserId) -> Result { - Ok(self.create_channel(name, None, creator_id).await?.id) + Ok(self.create_channel(name, None, creator_id).await?.0.id) } #[cfg(test)] @@ -32,6 +32,7 @@ impl Database { Ok(self .create_channel(name, Some(parent), creator_id) .await? + .0 .id) } @@ -41,10 +42,15 @@ impl Database { name: &str, parent_channel_id: Option, admin_id: UserId, - ) -> Result { + ) -> Result<( + Channel, + Option, + Vec, + )> { let name = Self::sanitize_channel_name(name)?; self.transaction(move |tx| async move { let mut parent = None; + let mut membership = None; if let Some(parent_channel_id) = parent_channel_id { let parent_channel = self.get_channel_internal(parent_channel_id, &*tx).await?; @@ -68,18 +74,25 @@ impl Database { .await?; 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?; + membership = Some( + 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?, + ); } - Ok(Channel::from_model(channel, ChannelRole::Admin)) + let channel_members = channel_member::Entity::find() + .filter(channel_member::Column::ChannelId.eq(channel.root_id())) + .all(&*tx) + .await?; + + Ok((Channel::from_model(channel), membership, channel_members)) }) .await } @@ -122,16 +135,9 @@ impl Database { ); } else if channel.visibility == ChannelVisibility::Public { role = Some(ChannelRole::Guest); - let channel_to_join = self - .public_ancestors_including_self(&channel, &*tx) - .await? - .first() - .cloned() - .unwrap_or(channel.clone()); - channel_member::Entity::insert(channel_member::ActiveModel { id: ActiveValue::NotSet, - channel_id: ActiveValue::Set(channel_to_join.id), + channel_id: ActiveValue::Set(channel.root_id()), user_id: ActiveValue::Set(user_id), accepted: ActiveValue::Set(true), role: ActiveValue::Set(ChannelRole::Guest), @@ -140,7 +146,7 @@ impl Database { .await?; accept_invite_result = Some( - self.calculate_membership_updated(&channel_to_join, user_id, &*tx) + self.calculate_membership_updated(&channel, user_id, &*tx) .await?, ); @@ -173,76 +179,47 @@ impl Database { channel_id: ChannelId, visibility: ChannelVisibility, admin_id: UserId, - ) -> Result { + ) -> Result<(Channel, Vec)> { self.transaction(move |tx| async move { let channel = self.get_channel_internal(channel_id, &*tx).await?; - self.check_user_is_channel_admin(&channel, admin_id, &*tx) .await?; - let previous_members = self - .get_channel_participant_details_internal(&channel, &*tx) - .await?; - - let mut model = channel.into_active_model(); - model.visibility = ActiveValue::Set(visibility); - let channel = model.update(&*tx).await?; - - let mut participants_to_update: HashMap = self - .participants_to_notify_for_channel_change(&channel, &*tx) - .await? - .into_iter() - .collect(); + if visibility == ChannelVisibility::Public { + if let Some(parent_id) = channel.parent_id() { + let parent = self.get_channel_internal(parent_id, &*tx).await?; - 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_including_self(vec![channel_id], &*tx) - .await? - .into_iter() - .map(|channel| 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); - } + if parent.visibility != ChannelVisibility::Public { + Err(ErrorCode::BadPublicNesting + .with_tag("direction", "parent") + .anyhow())?; } } - ChannelVisibility::Public => { - if let Some(public_parent) = self.public_parent_channel(&channel, &*tx).await? { - let parent_updates = self - .participants_to_notify_for_channel_change(&public_parent, &*tx) - .await?; - - for (user_id, channels) in parent_updates { - participants_to_update.insert(user_id, channels); - } - } + } else if visibility == ChannelVisibility::Members { + if self + .get_channel_descendants_including_self(vec![channel_id], &*tx) + .await? + .into_iter() + .any(|channel| { + channel.id != channel_id && channel.visibility == ChannelVisibility::Public + }) + { + Err(ErrorCode::BadPublicNesting + .with_tag("direction", "children") + .anyhow())?; } } - Ok(SetChannelVisibilityResult { - participants_to_update, - participants_to_remove, - channels_to_remove, - }) + let mut model = channel.into_active_model(); + model.visibility = ActiveValue::Set(visibility); + let channel = model.update(&*tx).await?; + + let channel_members = channel_member::Entity::find() + .filter(channel_member::Column::ChannelId.eq(channel.root_id())) + .all(&*tx) + .await?; + + Ok((Channel::from_model(channel), channel_members)) }) .await } @@ -275,7 +252,7 @@ impl Database { .await?; let members_to_notify: Vec = channel_member::Entity::find() - .filter(channel_member::Column::ChannelId.is_in(channel.ancestors_including_self())) + .filter(channel_member::Column::ChannelId.eq(channel.root_id())) .select_only() .column(channel_member::Column::UserId) .distinct() @@ -312,6 +289,9 @@ impl Database { let channel = self.get_channel_internal(channel_id, &*tx).await?; self.check_user_is_channel_admin(&channel, inviter_id, &*tx) .await?; + if !channel.is_root() { + Err(ErrorCode::NotARootChannel.anyhow())? + } channel_member::ActiveModel { id: ActiveValue::NotSet, @@ -323,7 +303,7 @@ impl Database { .insert(&*tx) .await?; - let channel = Channel::from_model(channel, role); + let channel = Channel::from_model(channel); let notifications = self .create_notification( @@ -362,35 +342,24 @@ impl Database { channel_id: ChannelId, admin_id: UserId, new_name: &str, - ) -> Result { + ) -> Result<(Channel, Vec)> { self.transaction(move |tx| async move { let new_name = Self::sanitize_channel_name(new_name)?.to_string(); let channel = self.get_channel_internal(channel_id, &*tx).await?; - let role = self - .check_user_is_channel_admin(&channel, admin_id, &*tx) + self.check_user_is_channel_admin(&channel, admin_id, &*tx) .await?; let mut model = channel.into_active_model(); model.name = ActiveValue::Set(new_name.clone()); let channel = model.update(&*tx).await?; - let participants = self - .get_channel_participant_details_internal(&channel, &*tx) + let channel_members = channel_member::Entity::find() + .filter(channel_member::Column::ChannelId.eq(channel.root_id())) + .all(&*tx) .await?; - Ok(RenameChannelResult { - channel: Channel::from_model(channel.clone(), role), - participants_to_update: participants - .iter() - .map(|participant| { - ( - participant.user_id, - Channel::from_model(channel.clone(), participant.role), - ) - }) - .collect(), - }) + Ok((Channel::from_model(channel), channel_members)) }) .await } @@ -565,10 +534,7 @@ impl Database { let channels = channels .into_iter() - .filter_map(|channel| { - let role = *role_for_channel.get(&channel.id)?; - Some(Channel::from_model(channel, role)) - }) + .filter_map(|channel| Some(Channel::from_model(channel))) .collect(); Ok(channels) @@ -576,6 +542,26 @@ impl Database { .await } + pub async fn get_channel_memberships( + &self, + user_id: UserId, + ) -> Result<(Vec, Vec)> { + self.transaction(|tx| async move { + let memberships = channel_member::Entity::find() + .filter(channel_member::Column::UserId.eq(user_id)) + .all(&*tx) + .await?; + let channels = self + .get_channel_descendants_including_self( + memberships.iter().map(|m| m.channel_id), + &*tx, + ) + .await?; + Ok((memberships, channels)) + }) + .await + } + /// Returns all channels for the user with the given ID. pub async fn get_channels_for_user(&self, user_id: UserId) -> Result { self.transaction(|tx| async move { @@ -594,12 +580,16 @@ impl Database { ancestor_channel: Option<&channel::Model>, tx: &DatabaseTransaction, ) -> Result { + let mut filter = channel_member::Column::UserId + .eq(user_id) + .and(channel_member::Column::Accepted.eq(true)); + + if let Some(ancestor) = ancestor_channel { + filter = filter.and(channel_member::Column::ChannelId.eq(ancestor.root_id())); + } + let channel_memberships = channel_member::Entity::find() - .filter( - channel_member::Column::UserId - .eq(user_id) - .and(channel_member::Column::Accepted.eq(true)), - ) + .filter(filter) .all(&*tx) .await?; @@ -610,56 +600,20 @@ impl Database { ) .await?; - let mut roles_by_channel_id: HashMap = HashMap::default(); - for membership in channel_memberships.iter() { - roles_by_channel_id.insert(membership.channel_id, membership.role); - } - - let mut visible_channel_ids: HashSet = HashSet::default(); + let roles_by_channel_id = channel_memberships + .iter() + .map(|membership| (membership.channel_id, membership.role)) + .collect::>(); let channels: Vec = descendants .into_iter() .filter_map(|channel| { - let parent_role = channel - .parent_id() - .and_then(|parent_id| roles_by_channel_id.get(&parent_id)); - - let role = if let Some(parent_role) = parent_role { - let role = if let Some(existing_role) = roles_by_channel_id.get(&channel.id) { - existing_role.max(*parent_role) - } else { - *parent_role - }; - roles_by_channel_id.insert(channel.id, role); - role + let parent_role = roles_by_channel_id.get(&channel.root_id())?; + if parent_role.can_see_channel(channel.visibility) { + Some(Channel::from_model(channel)) } else { - *roles_by_channel_id.get(&channel.id)? - }; - - let can_see_parent_paths = role.can_see_all_descendants() - || role.can_only_see_public_descendants() - && channel.visibility == ChannelVisibility::Public; - if !can_see_parent_paths { - return None; - } - - visible_channel_ids.insert(channel.id); - - if let Some(ancestor) = ancestor_channel { - if !channel - .ancestors_including_self() - .any(|id| id == ancestor.id) - { - return None; - } + None } - - let mut channel = Channel::from_model(channel, role); - channel - .parent_path - .retain(|id| visible_channel_ids.contains(&id)); - - Some(channel) }) .collect(); @@ -687,89 +641,21 @@ impl Database { } let channel_ids = channels.iter().map(|c| c.id).collect::>(); - let channel_buffer_changes = self - .unseen_channel_buffer_changes(user_id, &channel_ids, &*tx) + let latest_buffer_versions = self + .latest_channel_buffer_changes(&channel_ids, &*tx) .await?; - let unseen_messages = self - .unseen_channel_messages(user_id, &channel_ids, &*tx) - .await?; + let latest_messages = self.latest_channel_messages(&channel_ids, &*tx).await?; Ok(ChannelsForUser { + channel_memberships, channels, channel_participants, - unseen_buffer_changes: channel_buffer_changes, - channel_messages: unseen_messages, + latest_buffer_versions, + latest_channel_messages: latest_messages, }) } - pub async fn new_participants_to_notify( - &self, - parent_channel_id: ChannelId, - ) -> Result> { - self.weak_transaction(|tx| async move { - let parent_channel = self.get_channel_internal(parent_channel_id, &*tx).await?; - self.participants_to_notify_for_channel_change(&parent_channel, &*tx) - .await - }) - .await - } - - // TODO: this is very expensive, and we should rethink - async fn participants_to_notify_for_channel_change( - &self, - new_parent: &channel::Model, - 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_parents = self - .public_ancestors_including_self(new_parent, &*tx) - .await?; - let public_parent = public_parents.last(); - - let Some(public_parent) = public_parent else { - return Ok(results); - }; - - // 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) - } - /// Sets the role for the specified channel member. pub async fn set_channel_member_role( &self, @@ -807,7 +693,7 @@ impl Database { )) } else { Ok(SetMemberRoleResult::InviteUpdated(Channel::from_model( - channel, role, + channel, ))) } }) @@ -837,22 +723,30 @@ impl Database { if role == ChannelRole::Admin { Ok(members .into_iter() - .map(|channel_member| channel_member.to_proto()) + .map(|channel_member| proto::ChannelMember { + role: channel_member.role.into(), + user_id: channel_member.user_id.to_proto(), + kind: if channel_member.accepted { + Kind::Member + } else { + Kind::Invitee + } + .into(), + }) .collect()) } else { return Ok(members .into_iter() .filter_map(|member| { - if member.kind == proto::channel_member::Kind::Invitee { + if !member.accepted { return None; } - Some(ChannelMember { - role: member.role, - user_id: member.user_id, - kind: proto::channel_member::Kind::Member, + Some(proto::ChannelMember { + role: member.role.into(), + user_id: member.user_id.to_proto(), + kind: Kind::Member.into(), }) }) - .map(|channel_member| channel_member.to_proto()) .collect()); } } @@ -861,83 +755,11 @@ impl Database { &self, channel: &channel::Model, tx: &DatabaseTransaction, - ) -> Result> { - #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] - enum QueryMemberDetails { - UserId, - Role, - IsDirectMember, - Accepted, - Visibility, - } - - let mut stream = channel_member::Entity::find() - .left_join(channel::Entity) - .filter(channel_member::Column::ChannelId.is_in(channel.ancestors_including_self())) - .select_only() - .column(channel_member::Column::UserId) - .column(channel_member::Column::Role) - .column_as( - channel_member::Column::ChannelId.eq(channel.id), - 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; - } - - 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() - .map(|(_, details)| details) - .collect()) + ) -> Result> { + Ok(channel_member::Entity::find() + .filter(channel_member::Column::ChannelId.eq(channel.root_id())) + .all(tx) + .await?) } /// Returns the participants in the given channel. @@ -1016,7 +838,7 @@ impl Database { tx: &DatabaseTransaction, ) -> Result> { let row = channel_member::Entity::find() - .filter(channel_member::Column::ChannelId.is_in(channel.ancestors_including_self())) + .filter(channel_member::Column::ChannelId.eq(channel.root_id())) .filter(channel_member::Column::UserId.eq(user_id)) .filter(channel_member::Column::Accepted.eq(false)) .one(&*tx) @@ -1025,33 +847,6 @@ impl Database { Ok(row) } - async fn public_parent_channel( - &self, - channel: &channel::Model, - tx: &DatabaseTransaction, - ) -> Result> { - let mut path = self.public_ancestors_including_self(channel, &*tx).await?; - if path.last().unwrap().id == channel.id { - path.pop(); - } - Ok(path.pop()) - } - - pub(crate) async fn public_ancestors_including_self( - &self, - channel: &channel::Model, - tx: &DatabaseTransaction, - ) -> Result> { - let visible_channels = channel::Entity::find() - .filter(channel::Column::Id.is_in(channel.ancestors_including_self())) - .filter(channel::Column::Visibility.eq(ChannelVisibility::Public)) - .order_by_asc(channel::Column::ParentPath) - .all(&*tx) - .await?; - - Ok(visible_channels) - } - /// Returns the role for a user in the given channel. pub async fn channel_role_for_user( &self, @@ -1059,77 +854,25 @@ impl Database { user_id: UserId, tx: &DatabaseTransaction, ) -> Result> { - #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] - enum QueryChannelMembership { - ChannelId, - Role, - Visibility, - } - - let mut rows = channel_member::Entity::find() - .left_join(channel::Entity) + let membership = channel_member::Entity::find() .filter( channel_member::Column::ChannelId - .is_in(channel.ancestors_including_self()) + .eq(channel.root_id()) .and(channel_member::Column::UserId.eq(user_id)) .and(channel_member::Column::Accepted.eq(true)), ) - .select_only() - .column(channel_member::Column::ChannelId) - .column(channel_member::Column::Role) - .column(channel::Column::Visibility) - .into_values::<_, QueryChannelMembership>() - .stream(&*tx) + .one(&*tx) .await?; - let mut user_role: Option = None; - - let mut is_participant = false; - let mut current_channel_visibility = None; - - // note these channels are not iterated in any particular order, - // our current logic takes the highest permission available. - while let Some(row) = rows.next().await { - let (membership_channel, role, visibility): ( - ChannelId, - ChannelRole, - ChannelVisibility, - ) = row?; + let Some(membership) = membership else { + return Ok(None); + }; - match role { - ChannelRole::Admin | ChannelRole::Member | ChannelRole::Banned => { - if let Some(users_role) = user_role { - user_role = Some(users_role.max(role)); - } else { - user_role = Some(role) - } - } - ChannelRole::Guest if visibility == ChannelVisibility::Public => { - is_participant = true - } - ChannelRole::Guest => {} - } - if channel.id == membership_channel { - current_channel_visibility = Some(visibility); - } - } - // free up database connection - drop(rows); - - if is_participant && user_role.is_none() { - if current_channel_visibility.is_none() { - current_channel_visibility = channel::Entity::find() - .filter(channel::Column::Id.eq(channel.id)) - .one(&*tx) - .await? - .map(|channel| channel.visibility); - } - if current_channel_visibility == Some(ChannelVisibility::Public) { - user_role = Some(ChannelRole::Guest); - } + if !membership.role.can_see_channel(channel.visibility) { + return Ok(None); } - Ok(user_role) + Ok(Some(membership.role)) } // Get the descendants of the given set if channels, ordered by their @@ -1182,11 +925,10 @@ impl Database { pub async fn get_channel(&self, channel_id: ChannelId, user_id: UserId) -> Result { self.transaction(|tx| async move { let channel = self.get_channel_internal(channel_id, &*tx).await?; - let role = self - .check_user_is_channel_participant(&channel, user_id, &*tx) + self.check_user_is_channel_participant(&channel, user_id, &*tx) .await?; - Ok(Channel::from_model(channel, role)) + Ok(Channel::from_model(channel)) }) .await } @@ -1243,61 +985,39 @@ impl Database { pub async fn move_channel( &self, channel_id: ChannelId, - new_parent_id: Option, + new_parent_id: ChannelId, admin_id: UserId, - ) -> Result> { + ) -> Result<(Vec, Vec)> { self.transaction(|tx| async move { let channel = self.get_channel_internal(channel_id, &*tx).await?; self.check_user_is_channel_admin(&channel, admin_id, &*tx) .await?; + let new_parent = self.get_channel_internal(new_parent_id, &*tx).await?; - let new_parent_path; - let new_parent_channel; - if let Some(new_parent_id) = new_parent_id { - let new_parent = self.get_channel_internal(new_parent_id, &*tx).await?; - self.check_user_is_channel_admin(&new_parent, admin_id, &*tx) - .await?; - - if new_parent - .ancestors_including_self() - .any(|id| id == channel.id) - { - Err(anyhow!("cannot move a channel into one of its descendants"))?; - } + if new_parent.root_id() != channel.root_id() { + Err(anyhow!(ErrorCode::WrongMoveTarget))?; + } - new_parent_path = new_parent.path(); - new_parent_channel = Some(new_parent); - } else { - new_parent_path = String::new(); - new_parent_channel = None; - }; + if new_parent + .ancestors_including_self() + .any(|id| id == channel.id) + { + Err(anyhow!(ErrorCode::CircularNesting))?; + } - let previous_participants = self - .get_channel_participant_details_internal(&channel, &*tx) - .await?; + if channel.visibility == ChannelVisibility::Public + && new_parent.visibility != ChannelVisibility::Public + { + Err(anyhow!(ErrorCode::BadPublicNesting))?; + } + let root_id = channel.root_id(); let old_path = format!("{}{}/", channel.parent_path, channel.id); - let new_path = format!("{}{}/", new_parent_path, channel.id); - - if old_path == new_path { - return Ok(None); - } + let new_path = format!("{}{}/", new_parent.path(), channel.id); let mut model = channel.into_active_model(); - model.parent_path = ActiveValue::Set(new_parent_path); - model.update(&*tx).await?; - - if new_parent_channel.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?; - } + model.parent_path = ActiveValue::Set(new_parent.path()); + let channel = model.update(&*tx).await?; let descendent_ids = ChannelId::find_by_statement::(Statement::from_sql_and_values( @@ -1312,10 +1032,22 @@ impl Database { .all(&*tx) .await?; - Ok(Some(MoveChannelResult { - previous_participants, - descendent_ids, - })) + let all_moved_ids = Some(channel.id).into_iter().chain(descendent_ids); + + let channels = channel::Entity::find() + .filter(channel::Column::Id.is_in(all_moved_ids)) + .all(&*tx) + .await? + .into_iter() + .map(|c| Channel::from_model(c)) + .collect::>(); + + let channel_members = channel_member::Entity::find() + .filter(channel_member::Column::ChannelId.eq(root_id)) + .all(&*tx) + .await?; + + Ok((channels, channel_members)) }) .await } diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index 9ee313d91b4f916c69789b2048c1867be207f676..9baa7162c50d497fab4cc49864b0c899360cb040 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -385,25 +385,11 @@ impl Database { Ok(()) } - /// Returns the unseen messages for the given user in the specified channels. - pub async fn unseen_channel_messages( + pub async fn latest_channel_messages( &self, - user_id: UserId, channel_ids: &[ChannelId], tx: &DatabaseTransaction, - ) -> Result> { - let mut observed_messages_by_channel_id = HashMap::default(); - let mut rows = observed_channel_messages::Entity::find() - .filter(observed_channel_messages::Column::UserId.eq(user_id)) - .filter(observed_channel_messages::Column::ChannelId.is_in(channel_ids.iter().copied())) - .stream(&*tx) - .await?; - - while let Some(row) = rows.next().await { - let row = row?; - observed_messages_by_channel_id.insert(row.channel_id, row); - } - drop(rows); + ) -> Result> { let mut values = String::new(); for id in channel_ids { if !values.is_empty() { @@ -413,7 +399,7 @@ impl Database { } if values.is_empty() { - return Ok(Default::default()); + return Ok(Vec::default()); } let sql = format!( @@ -437,26 +423,20 @@ impl Database { ); let stmt = Statement::from_string(self.pool.get_database_backend(), sql); - let last_messages = channel_message::Model::find_by_statement(stmt) - .all(&*tx) + let mut last_messages = channel_message::Model::find_by_statement(stmt) + .stream(&*tx) .await?; - let mut changes = Vec::new(); - for last_message in last_messages { - if let Some(observed_message) = - observed_messages_by_channel_id.get(&last_message.channel_id) - { - if observed_message.channel_message_id == last_message.id { - continue; - } - } - changes.push(proto::UnseenChannelMessage { - channel_id: last_message.channel_id.to_proto(), - message_id: last_message.id.to_proto(), + let mut results = Vec::new(); + while let Some(result) = last_messages.next().await { + let message = result?; + results.push(proto::ChannelMessageId { + channel_id: message.channel_id.to_proto(), + message_id: message.id.to_proto(), }); } - Ok(changes) + Ok(results) } /// Removes the channel message with the given ID. diff --git a/crates/collab/src/db/tables/channel.rs b/crates/collab/src/db/tables/channel.rs index a35913a705bd8bd63bcc0ec2bfe3cae7901a10d9..7b38218d67f77bea45c9710b92ce9040826b3a67 100644 --- a/crates/collab/src/db/tables/channel.rs +++ b/crates/collab/src/db/tables/channel.rs @@ -17,6 +17,14 @@ impl Model { self.ancestors().last() } + pub fn is_root(&self) -> bool { + self.parent_path.is_empty() + } + + pub fn root_id(&self) -> ChannelId { + self.ancestors().next().unwrap_or(self.id) + } + pub fn ancestors(&self) -> impl Iterator + '_ { self.parent_path .trim_end_matches('/') diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index 4a9c98f02240964fc27b462cb6764fdbedc9c567..a47e6330d3916b9939459e132490764650934a35 100644 --- a/crates/collab/src/db/tests.rs +++ b/crates/collab/src/db/tests.rs @@ -150,14 +150,13 @@ impl Drop for TestDb { } } -fn channel_tree(channels: &[(ChannelId, &[ChannelId], &'static str, ChannelRole)]) -> Vec { +fn channel_tree(channels: &[(ChannelId, &[ChannelId], &'static str)]) -> Vec { channels .iter() - .map(|(id, parent_path, name, role)| Channel { + .map(|(id, parent_path, name)| Channel { id: *id, name: name.to_string(), visibility: ChannelVisibility::Members, - role: *role, parent_path: parent_path.to_vec(), }) .collect() diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 222514da0b32d9b8bd551bcfa60901b67bb590a0..2eb8e15301e5701c57547120466003d5dd3972e7 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -330,8 +330,7 @@ async fn test_channel_buffers_last_operations(db: &Database) { .transaction(|tx| { let buffers = &buffers; async move { - db.unseen_channel_buffer_changes( - observer_id, + db.latest_channel_buffer_changes( &[ buffers[0].channel_id, buffers[1].channel_id, @@ -348,12 +347,12 @@ async fn test_channel_buffers_last_operations(db: &Database) { pretty_assertions::assert_eq!( buffer_changes, [ - rpc::proto::UnseenChannelBufferChange { + rpc::proto::ChannelBufferVersion { channel_id: buffers[0].channel_id.to_proto(), epoch: 0, version: serialize_version(&text_buffers[0].version()), }, - rpc::proto::UnseenChannelBufferChange { + rpc::proto::ChannelBufferVersion { channel_id: buffers[1].channel_id.to_proto(), epoch: 1, version: serialize_version(&text_buffers[1].version()) @@ -362,99 +361,7 @@ async fn test_channel_buffers_last_operations(db: &Database) { == buffer_changes[1].version.first().unwrap().replica_id) .collect::>(), }, - rpc::proto::UnseenChannelBufferChange { - channel_id: buffers[2].channel_id.to_proto(), - epoch: 0, - version: serialize_version(&text_buffers[2].version()), - }, - ] - ); - - db.observe_buffer_version( - buffers[1].id, - observer_id, - 1, - serialize_version(&text_buffers[1].version()).as_slice(), - ) - .await - .unwrap(); - - let buffer_changes = db - .transaction(|tx| { - let buffers = &buffers; - async move { - db.unseen_channel_buffer_changes( - observer_id, - &[ - buffers[0].channel_id, - buffers[1].channel_id, - buffers[2].channel_id, - ], - &*tx, - ) - .await - } - }) - .await - .unwrap(); - - assert_eq!( - buffer_changes, - [ - rpc::proto::UnseenChannelBufferChange { - channel_id: buffers[0].channel_id.to_proto(), - epoch: 0, - version: serialize_version(&text_buffers[0].version()), - }, - rpc::proto::UnseenChannelBufferChange { - channel_id: buffers[2].channel_id.to_proto(), - epoch: 0, - version: serialize_version(&text_buffers[2].version()), - }, - ] - ); - - // Observe an earlier version of the buffer. - db.observe_buffer_version( - buffers[1].id, - observer_id, - 1, - &[rpc::proto::VectorClockEntry { - replica_id: 0, - timestamp: 0, - }], - ) - .await - .unwrap(); - - let buffer_changes = db - .transaction(|tx| { - let buffers = &buffers; - async move { - db.unseen_channel_buffer_changes( - observer_id, - &[ - buffers[0].channel_id, - buffers[1].channel_id, - buffers[2].channel_id, - ], - &*tx, - ) - .await - } - }) - .await - .unwrap(); - - assert_eq!( - buffer_changes, - [ - rpc::proto::UnseenChannelBufferChange { - channel_id: buffers[0].channel_id.to_proto(), - epoch: 0, - version: serialize_version(&text_buffers[0].version()), - }, - rpc::proto::UnseenChannelBufferChange { + rpc::proto::ChannelBufferVersion { channel_id: buffers[2].channel_id.to_proto(), epoch: 0, version: serialize_version(&text_buffers[2].version()), diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 8a7a19ed3aacbfd6c0859b122dd470535458a150..a5e083f9356c5e144cc5239b9552ac3c8e58e675 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -62,23 +62,13 @@ async fn test_channels(db: &Arc) { assert_eq!( result.channels, channel_tree(&[ - (zed_id, &[], "zed", ChannelRole::Admin), - (crdb_id, &[zed_id], "crdb", ChannelRole::Admin), - ( - livestreaming_id, - &[zed_id], - "livestreaming", - ChannelRole::Admin - ), - (replace_id, &[zed_id], "replace", ChannelRole::Admin), - (rust_id, &[], "rust", ChannelRole::Admin), - (cargo_id, &[rust_id], "cargo", ChannelRole::Admin), - ( - cargo_ra_id, - &[rust_id, cargo_id], - "cargo-ra", - ChannelRole::Admin - ) + (zed_id, &[], "zed"), + (crdb_id, &[zed_id], "crdb"), + (livestreaming_id, &[zed_id], "livestreaming",), + (replace_id, &[zed_id], "replace"), + (rust_id, &[], "rust"), + (cargo_id, &[rust_id], "cargo"), + (cargo_ra_id, &[rust_id, cargo_id], "cargo-ra",) ],) ); @@ -86,15 +76,10 @@ async fn test_channels(db: &Arc) { assert_eq!( result.channels, channel_tree(&[ - (zed_id, &[], "zed", ChannelRole::Member), - (crdb_id, &[zed_id], "crdb", ChannelRole::Member), - ( - livestreaming_id, - &[zed_id], - "livestreaming", - ChannelRole::Member - ), - (replace_id, &[zed_id], "replace", ChannelRole::Member) + (zed_id, &[], "zed"), + (crdb_id, &[zed_id], "crdb"), + (livestreaming_id, &[zed_id], "livestreaming",), + (replace_id, &[zed_id], "replace") ],) ); @@ -112,15 +97,10 @@ async fn test_channels(db: &Arc) { assert_eq!( result.channels, channel_tree(&[ - (zed_id, &[], "zed", ChannelRole::Admin), - (crdb_id, &[zed_id], "crdb", ChannelRole::Admin), - ( - livestreaming_id, - &[zed_id], - "livestreaming", - ChannelRole::Admin - ), - (replace_id, &[zed_id], "replace", ChannelRole::Admin) + (zed_id, &[], "zed"), + (crdb_id, &[zed_id], "crdb"), + (livestreaming_id, &[zed_id], "livestreaming",), + (replace_id, &[zed_id], "replace") ],) ); @@ -271,14 +251,19 @@ async fn test_channel_invites(db: &Arc) { &[ proto::ChannelMember { user_id: user_1.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { user_id: user_2.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Member.into(), }, + proto::ChannelMember { + user_id: user_3.to_proto(), + kind: proto::channel_member::Kind::Invitee.into(), + role: proto::ChannelRole::Admin.into(), + }, ] ); } @@ -420,13 +405,6 @@ async fn test_db_channel_moving_bugs(db: &Arc) { .await .unwrap(); - // Move to same parent should be a no-op - assert!(db - .move_channel(projects_id, Some(zed_id), user_id) - .await - .unwrap() - .is_none()); - let result = db.get_channels_for_user(user_id).await.unwrap(); assert_channel_tree( result.channels, @@ -437,20 +415,8 @@ async fn test_db_channel_moving_bugs(db: &Arc) { ], ); - // Move the project channel to the root - db.move_channel(projects_id, None, user_id).await.unwrap(); - let result = db.get_channels_for_user(user_id).await.unwrap(); - assert_channel_tree( - result.channels, - &[ - (zed_id, &[]), - (projects_id, &[]), - (livestreaming_id, &[projects_id]), - ], - ); - // Can't move a channel into its ancestor - db.move_channel(projects_id, Some(livestreaming_id), user_id) + db.move_channel(projects_id, livestreaming_id, user_id) .await .unwrap_err(); let result = db.get_channels_for_user(user_id).await.unwrap(); @@ -458,8 +424,8 @@ async fn test_db_channel_moving_bugs(db: &Arc) { result.channels, &[ (zed_id, &[]), - (projects_id, &[]), - (livestreaming_id, &[projects_id]), + (projects_id, &[zed_id]), + (livestreaming_id, &[zed_id, projects_id]), ], ); } @@ -476,32 +442,39 @@ async fn test_user_is_channel_participant(db: &Arc) { let guest = new_test_user(db, "guest@example.com").await; let zed_channel = db.create_root_channel("zed", admin).await.unwrap(); - let active_channel_id = db + let internal_channel_id = db .create_sub_channel("active", zed_channel, admin) .await .unwrap(); - let vim_channel_id = db - .create_sub_channel("vim", active_channel_id, admin) + let public_channel_id = db + .create_sub_channel("vim", zed_channel, admin) .await .unwrap(); - db.set_channel_visibility(vim_channel_id, crate::db::ChannelVisibility::Public, admin) + db.set_channel_visibility(zed_channel, crate::db::ChannelVisibility::Public, admin) .await .unwrap(); - db.invite_channel_member(active_channel_id, member, admin, ChannelRole::Member) + db.set_channel_visibility( + public_channel_id, + crate::db::ChannelVisibility::Public, + admin, + ) + .await + .unwrap(); + db.invite_channel_member(zed_channel, member, admin, ChannelRole::Member) .await .unwrap(); - db.invite_channel_member(vim_channel_id, guest, admin, ChannelRole::Guest) + db.invite_channel_member(zed_channel, guest, admin, ChannelRole::Guest) .await .unwrap(); - db.respond_to_channel_invite(active_channel_id, member, true) + db.respond_to_channel_invite(zed_channel, member, true) .await .unwrap(); db.transaction(|tx| async move { db.check_user_is_channel_participant( - &db.get_channel_internal(vim_channel_id, &*tx).await?, + &db.get_channel_internal(public_channel_id, &*tx).await?, admin, &*tx, ) @@ -511,7 +484,7 @@ async fn test_user_is_channel_participant(db: &Arc) { .unwrap(); db.transaction(|tx| async move { db.check_user_is_channel_participant( - &db.get_channel_internal(vim_channel_id, &*tx).await?, + &db.get_channel_internal(public_channel_id, &*tx).await?, member, &*tx, ) @@ -521,7 +494,7 @@ async fn test_user_is_channel_participant(db: &Arc) { .unwrap(); let mut members = db - .get_channel_participant_details(vim_channel_id, admin) + .get_channel_participant_details(public_channel_id, admin) .await .unwrap(); @@ -532,12 +505,12 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { user_id: member.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Member.into(), }, proto::ChannelMember { @@ -548,13 +521,13 @@ async fn test_user_is_channel_participant(db: &Arc) { ] ); - db.respond_to_channel_invite(vim_channel_id, guest, true) + db.respond_to_channel_invite(zed_channel, guest, true) .await .unwrap(); db.transaction(|tx| async move { db.check_user_is_channel_participant( - &db.get_channel_internal(vim_channel_id, &*tx).await?, + &db.get_channel_internal(public_channel_id, &*tx).await?, guest, &*tx, ) @@ -564,23 +537,29 @@ async fn test_user_is_channel_participant(db: &Arc) { .unwrap(); let channels = db.get_channels_for_user(guest).await.unwrap().channels; - assert_channel_tree(channels, &[(vim_channel_id, &[])]); + assert_channel_tree( + channels, + &[(zed_channel, &[]), (public_channel_id, &[zed_channel])], + ); let channels = db.get_channels_for_user(member).await.unwrap().channels; assert_channel_tree( channels, &[ - (active_channel_id, &[]), - (vim_channel_id, &[active_channel_id]), + (zed_channel, &[]), + (internal_channel_id, &[zed_channel]), + (public_channel_id, &[zed_channel]), ], ); - db.set_channel_member_role(vim_channel_id, admin, guest, ChannelRole::Banned) + db.set_channel_member_role(zed_channel, admin, guest, ChannelRole::Banned) .await .unwrap(); assert!(db .transaction(|tx| async move { db.check_user_is_channel_participant( - &db.get_channel_internal(vim_channel_id, &*tx).await.unwrap(), + &db.get_channel_internal(public_channel_id, &*tx) + .await + .unwrap(), guest, &*tx, ) @@ -590,7 +569,7 @@ async fn test_user_is_channel_participant(db: &Arc) { .is_err()); let mut members = db - .get_channel_participant_details(vim_channel_id, admin) + .get_channel_participant_details(public_channel_id, admin) .await .unwrap(); @@ -601,12 +580,12 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { user_id: member.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Member.into(), }, proto::ChannelMember { @@ -617,11 +596,7 @@ async fn test_user_is_channel_participant(db: &Arc) { ] ); - db.remove_channel_member(vim_channel_id, guest, admin) - .await - .unwrap(); - - db.set_channel_visibility(zed_channel, crate::db::ChannelVisibility::Public, admin) + db.remove_channel_member(zed_channel, guest, admin) .await .unwrap(); @@ -631,7 +606,7 @@ async fn test_user_is_channel_participant(db: &Arc) { // currently people invited to parent channels are not shown here let mut members = db - .get_channel_participant_details(vim_channel_id, admin) + .get_channel_participant_details(public_channel_id, admin) .await .unwrap(); @@ -642,14 +617,19 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { user_id: member.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Member.into(), }, + proto::ChannelMember { + user_id: guest.to_proto(), + kind: proto::channel_member::Kind::Invitee.into(), + role: proto::ChannelRole::Guest.into(), + }, ] ); @@ -670,7 +650,7 @@ async fn test_user_is_channel_participant(db: &Arc) { assert!(db .transaction(|tx| async move { db.check_user_is_channel_participant( - &db.get_channel_internal(active_channel_id, &*tx) + &db.get_channel_internal(internal_channel_id, &*tx) .await .unwrap(), guest, @@ -683,7 +663,9 @@ async fn test_user_is_channel_participant(db: &Arc) { db.transaction(|tx| async move { db.check_user_is_channel_participant( - &db.get_channel_internal(vim_channel_id, &*tx).await.unwrap(), + &db.get_channel_internal(public_channel_id, &*tx) + .await + .unwrap(), guest, &*tx, ) @@ -693,7 +675,7 @@ async fn test_user_is_channel_participant(db: &Arc) { .unwrap(); let mut members = db - .get_channel_participant_details(vim_channel_id, admin) + .get_channel_participant_details(public_channel_id, admin) .await .unwrap(); @@ -704,17 +686,17 @@ async fn test_user_is_channel_participant(db: &Arc) { &[ proto::ChannelMember { user_id: admin.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { user_id: member.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Member.into(), }, proto::ChannelMember { user_id: guest.to_proto(), - kind: proto::channel_member::Kind::AncestorMember.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Guest.into(), }, ] @@ -723,67 +705,10 @@ async fn test_user_is_channel_participant(db: &Arc) { let channels = db.get_channels_for_user(guest).await.unwrap().channels; assert_channel_tree( channels, - &[(zed_channel, &[]), (vim_channel_id, &[zed_channel])], + &[(zed_channel, &[]), (public_channel_id, &[zed_channel])], ) } -test_both_dbs!( - test_user_joins_correct_channel, - test_user_joins_correct_channel_postgres, - test_user_joins_correct_channel_sqlite -); - -async fn test_user_joins_correct_channel(db: &Arc) { - let admin = new_test_user(db, "admin@example.com").await; - - let zed_channel = db.create_root_channel("zed", admin).await.unwrap(); - - let active_channel = db - .create_sub_channel("active", zed_channel, admin) - .await - .unwrap(); - - let vim_channel = db - .create_sub_channel("vim", active_channel, admin) - .await - .unwrap(); - - let vim2_channel = db - .create_sub_channel("vim2", vim_channel, admin) - .await - .unwrap(); - - db.set_channel_visibility(zed_channel, crate::db::ChannelVisibility::Public, admin) - .await - .unwrap(); - - db.set_channel_visibility(vim_channel, crate::db::ChannelVisibility::Public, admin) - .await - .unwrap(); - - db.set_channel_visibility(vim2_channel, crate::db::ChannelVisibility::Public, admin) - .await - .unwrap(); - - let most_public = db - .transaction(|tx| async move { - Ok(db - .public_ancestors_including_self( - &db.get_channel_internal(vim_channel, &*tx).await.unwrap(), - &tx, - ) - .await? - .first() - .cloned()) - }) - .await - .unwrap() - .unwrap() - .id; - - assert_eq!(most_public, zed_channel) -} - test_both_dbs!( test_guest_access, test_guest_access_postgres, diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index 23c4da3a81f7dec6221a010689010b9ef1cc7726..fea3ae980532bd68418d7bd26bb48e72c876964d 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -15,7 +15,7 @@ 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 channel = db.create_channel("channel", None, user).await.unwrap().0; let owner_id = db.create_server("test").await.unwrap().0 as u32; db.join_channel_chat(channel.id, rpc::ConnectionId { owner_id, id: 0 }, user) @@ -235,11 +235,10 @@ async fn test_unseen_channel_messages(db: &Arc) { .await .unwrap(); - let second_message = db + let _ = db .create_channel_message(channel_1, user, "1_2", &[], OffsetDateTime::now_utc(), 2) .await - .unwrap() - .message_id; + .unwrap(); let third_message = db .create_channel_message(channel_1, user, "1_3", &[], OffsetDateTime::now_utc(), 3) @@ -258,97 +257,27 @@ async fn test_unseen_channel_messages(db: &Arc) { .message_id; // Check that observer has new messages - let unseen_messages = db + let latest_messages = db .transaction(|tx| async move { - db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) + db.latest_channel_messages(&[channel_1, channel_2], &*tx) .await }) .await .unwrap(); assert_eq!( - unseen_messages, - [ - rpc::proto::UnseenChannelMessage { - channel_id: channel_1.to_proto(), - message_id: third_message.to_proto(), - }, - rpc::proto::UnseenChannelMessage { - channel_id: channel_2.to_proto(), - message_id: fourth_message.to_proto(), - }, - ] - ); - - // Observe the second message - db.observe_channel_message(channel_1, observer, second_message) - .await - .unwrap(); - - // Make sure the observer still has a new message - let unseen_messages = db - .transaction(|tx| async move { - db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) - .await - }) - .await - .unwrap(); - assert_eq!( - unseen_messages, + latest_messages, [ - rpc::proto::UnseenChannelMessage { + rpc::proto::ChannelMessageId { channel_id: channel_1.to_proto(), message_id: third_message.to_proto(), }, - rpc::proto::UnseenChannelMessage { + rpc::proto::ChannelMessageId { channel_id: channel_2.to_proto(), message_id: fourth_message.to_proto(), }, ] ); - - // Observe the third message, - db.observe_channel_message(channel_1, observer, third_message) - .await - .unwrap(); - - // Make sure the observer does not have a new method - let unseen_messages = db - .transaction(|tx| async move { - db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) - .await - }) - .await - .unwrap(); - - assert_eq!( - unseen_messages, - [rpc::proto::UnseenChannelMessage { - channel_id: channel_2.to_proto(), - message_id: fourth_message.to_proto(), - }] - ); - - // Observe the second message again, should not regress our observed state - db.observe_channel_message(channel_1, observer, second_message) - .await - .unwrap(); - - // Make sure the observer does not have a new message - let unseen_messages = db - .transaction(|tx| async move { - db.unseen_channel_messages(observer, &[channel_1, channel_2], &*tx) - .await - }) - .await - .unwrap(); - assert_eq!( - unseen_messages, - [rpc::proto::UnseenChannelMessage { - channel_id: channel_2.to_proto(), - message_id: fourth_message.to_proto(), - }] - ); } test_both_dbs!( @@ -362,7 +291,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().id; + let channel = db + .create_channel("channel", None, user_a) + .await + .unwrap() + .0 + .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 bb173784d3c7ec5b5aceb7a9f1b1e25a064d6fb8..71f0510a69734200fb4650703d9dbb422d35c4bd 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -5,8 +5,7 @@ use crate::{ db::{ self, BufferId, ChannelId, ChannelRole, ChannelsForUser, CreatedChannelMessage, Database, InviteMemberResult, MembershipUpdated, MessageId, NotificationId, ProjectId, - RemoveChannelMemberResult, RenameChannelResult, RespondToChannelInvite, RoomId, ServerId, - SetChannelVisibilityResult, User, UserId, + RemoveChannelMemberResult, RespondToChannelInvite, RoomId, ServerId, User, UserId, }, executor::Executor, AppState, Error, Result, @@ -602,6 +601,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_update_user_channels(&channels_for_user.channel_memberships))?; this.peer.send(connection_id, build_channels_update( channels_for_user, channel_invites @@ -2300,7 +2300,7 @@ async fn create_channel( let db = session.db().await; let parent_id = request.parent_id.map(|id| ChannelId::from_proto(id)); - let channel = db + let (channel, owner, channel_members) = db .create_channel(&request.name, parent_id, session.user_id) .await?; @@ -2309,20 +2309,30 @@ async fn create_channel( parent_id: request.parent_id, })?; - let participants_to_update; - if let Some(parent) = parent_id { - participants_to_update = db.new_participants_to_notify(parent).await?; - } else { - participants_to_update = vec![]; + let connection_pool = session.connection_pool().await; + if let Some(owner) = owner { + let update = proto::UpdateUserChannels { + channel_memberships: vec![proto::ChannelMembership { + channel_id: owner.channel_id.to_proto(), + role: owner.role.into(), + }], + ..Default::default() + }; + for connection_id in connection_pool.user_connection_ids(owner.user_id) { + session.peer.send(connection_id, update.clone())?; + } } - let connection_pool = session.connection_pool().await; - 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; - } + for channel_member in channel_members { + if !channel_member.role.can_see_channel(channel.visibility) { + continue; + } + + let update = proto::UpdateChannels { + channels: vec![channel.to_proto()], + ..Default::default() + }; + for connection_id in connection_pool.user_connection_ids(channel_member.user_id) { session.peer.send(connection_id, update.clone())?; } } @@ -2439,7 +2449,9 @@ async fn remove_channel_member( Ok(()) } -/// Toggle the channel between public and private +/// Toggle the channel between public and private. +/// Care is taken to maintain the invariant that public channels only descend from public channels, +/// (though members-only channels can appear at any point in the hierarchy). async fn set_channel_visibility( request: proto::SetChannelVisibility, response: Response, @@ -2449,27 +2461,25 @@ async fn set_channel_visibility( let channel_id = ChannelId::from_proto(request.channel_id); let visibility = request.visibility().into(); - let SetChannelVisibilityResult { - participants_to_update, - participants_to_remove, - channels_to_remove, - } = db + let (channel, channel_members) = db .set_channel_visibility(channel_id, visibility, session.user_id) .await?; let connection_pool = session.connection_pool().await; - 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 member in channel_members { + let update = if member.role.can_see_channel(channel.visibility) { + proto::UpdateChannels { + channels: vec![channel.to_proto()], + ..Default::default() + } + } else { + proto::UpdateChannels { + delete_channels: vec![channel.id.to_proto()], + ..Default::default() + } }; - for connection_id in connection_pool.user_connection_ids(user_id) { + + for connection_id in connection_pool.user_connection_ids(member.user_id) { session.peer.send(connection_id, update.clone())?; } } @@ -2478,7 +2488,7 @@ async fn set_channel_visibility( Ok(()) } -/// Alter the role for a user in the channel +/// Alter the role for a user in the channel. async fn set_channel_member_role( request: proto::SetChannelMemberRole, response: Response, @@ -2534,10 +2544,7 @@ async fn rename_channel( ) -> Result<()> { let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); - let RenameChannelResult { - channel, - participants_to_update, - } = db + let (channel, channel_members) = db .rename_channel(channel_id, session.user_id, &request.name) .await?; @@ -2546,13 +2553,15 @@ async fn rename_channel( })?; let connection_pool = session.connection_pool().await; - 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() - }; - + for channel_member in channel_members { + if !channel_member.role.can_see_channel(channel.visibility) { + continue; + } + let update = proto::UpdateChannels { + channels: vec![channel.to_proto()], + ..Default::default() + }; + for connection_id in connection_pool.user_connection_ids(channel_member.user_id) { session.peer.send(connection_id, update.clone())?; } } @@ -2567,57 +2576,37 @@ async fn move_channel( session: Session, ) -> Result<()> { let channel_id = ChannelId::from_proto(request.channel_id); - let to = request.to.map(ChannelId::from_proto); + let to = ChannelId::from_proto(request.to); - let result = session + let (channels, channel_members) = session .db() .await .move_channel(channel_id, to, session.user_id) .await?; - if let Some(result) = result { - let participants_to_update: HashMap<_, _> = session - .db() - .await - .new_participants_to_notify(to.unwrap_or(channel_id)) - .await? - .into_iter() - .collect(); - - let mut moved_channels: HashSet = HashSet::default(); - for id in result.descendent_ids { - moved_channels.insert(id); - } - moved_channels.insert(channel_id); - - let mut participants_to_remove: HashSet = HashSet::default(); - for participant in result.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); + let connection_pool = session.connection_pool().await; + for member in channel_members { + let channels = channels + .iter() + .filter_map(|channel| { + if member.role.can_see_channel(channel.visibility) { + Some(channel.to_proto()) + } else { + None } - } + }) + .collect::>(); + if channels.is_empty() { + continue; } - let moved_channels: Vec = moved_channels.iter().map(|id| id.to_proto()).collect(); - - let connection_pool = session.connection_pool().await; - 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, + ..Default::default() + }; - 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())?; - } + for connection_id in connection_pool.user_connection_ids(member.user_id) { + session.peer.send(connection_id, update.clone())?; } } @@ -2851,7 +2840,7 @@ async fn update_channel_buffer( session.peer.send( peer_id.into(), proto::UpdateChannels { - unseen_channel_buffer_changes: vec![proto::UnseenChannelBufferChange { + latest_channel_buffer_versions: vec![proto::ChannelBufferVersion { channel_id: channel_id.to_proto(), epoch: epoch as u64, version: version.clone(), @@ -3037,7 +3026,7 @@ async fn send_channel_message( session.peer.send( peer_id.into(), proto::UpdateChannels { - unseen_channel_messages: vec![proto::UnseenChannelMessage { + latest_channel_message_ids: vec![proto::ChannelMessageId { channel_id: channel_id.to_proto(), message_id: message_id.to_proto(), }], @@ -3292,6 +3281,18 @@ fn notify_membership_updated( user_id: UserId, peer: &Peer, ) { + let user_channels_update = proto::UpdateUserChannels { + channel_memberships: result + .new_channels + .channel_memberships + .iter() + .map(|cm| proto::ChannelMembership { + channel_id: cm.channel_id.to_proto(), + role: cm.role.into(), + }) + .collect(), + ..Default::default() + }; let mut update = build_channels_update(result.new_channels, vec![]); update.delete_channels = result .removed_channels @@ -3301,10 +3302,27 @@ fn notify_membership_updated( 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, user_channels_update.clone()) + .trace_err(); peer.send(connection_id, update.clone()).trace_err(); } } +fn build_update_user_channels( + memberships: &Vec, +) -> proto::UpdateUserChannels { + proto::UpdateUserChannels { + channel_memberships: memberships + .iter() + .map(|m| proto::ChannelMembership { + channel_id: m.channel_id.to_proto(), + role: m.role.into(), + }) + .collect(), + ..Default::default() + } +} + fn build_channels_update( channels: ChannelsForUser, channel_invites: Vec, @@ -3315,8 +3333,8 @@ fn build_channels_update( update.channels.push(channel.to_proto()); } - update.unseen_channel_buffer_changes = channels.unseen_buffer_changes; - update.unseen_channel_messages = channels.channel_messages; + update.latest_channel_buffer_versions = channels.latest_buffer_versions; + update.latest_channel_message_ids = channels.latest_channel_messages; for (channel_id, participants) in channels.channel_participants { update diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 76cc8cb9e1793f8b40f184a80b8d9efaaeb6e6d3..cf5b999ef65bdfba61d5cc558a62c50de08f58de 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -637,7 +637,6 @@ async fn test_channel_buffer_changes( .channel_store() .read(cx) .has_channel_buffer_changed(channel_id) - .unwrap() }); assert!(has_buffer_changed); @@ -655,7 +654,6 @@ async fn test_channel_buffer_changes( .channel_store() .read(cx) .has_channel_buffer_changed(channel_id) - .unwrap() }); assert!(!has_buffer_changed); @@ -672,7 +670,6 @@ async fn test_channel_buffer_changes( .channel_store() .read(cx) .has_channel_buffer_changed(channel_id) - .unwrap() }); assert!(!has_buffer_changed); @@ -687,7 +684,6 @@ async fn test_channel_buffer_changes( .channel_store() .read(cx) .has_channel_buffer_changed(channel_id) - .unwrap() }); assert!(!has_buffer_changed); @@ -714,7 +710,6 @@ async fn test_channel_buffer_changes( .channel_store() .read(cx) .has_channel_buffer_changed(channel_id) - .unwrap() }); assert!(has_buffer_changed); } diff --git a/crates/collab/src/tests/channel_guest_tests.rs b/crates/collab/src/tests/channel_guest_tests.rs index 26e9c56a4be806da69bbbbb895a1ed583c703a1f..bb1f493f0c4034c38e00807e479cc8ba8347138f 100644 --- a/crates/collab/src/tests/channel_guest_tests.rs +++ b/crates/collab/src/tests/channel_guest_tests.rs @@ -195,6 +195,13 @@ async fn test_channel_requires_zed_cla(cx_a: &mut TestAppContext, cx_b: &mut Tes }) .await .unwrap(); + client_a + .channel_store() + .update(cx_a, |store, cx| { + store.set_channel_visibility(parent_channel_id, proto::ChannelVisibility::Public, cx) + }) + .await + .unwrap(); client_a .channel_store() .update(cx_a, |store, cx| { diff --git a/crates/collab/src/tests/channel_message_tests.rs b/crates/collab/src/tests/channel_message_tests.rs index e59aa3c705fd7b19c77b0a3c231fed5486a72786..270ba04f7ece0144c82075a1e40a1c087895a460 100644 --- a/crates/collab/src/tests/channel_message_tests.rs +++ b/crates/collab/src/tests/channel_message_tests.rs @@ -313,7 +313,6 @@ async fn test_channel_message_changes( .channel_store() .read(cx) .has_new_messages(channel_id) - .unwrap() }); assert!(b_has_messages); @@ -341,7 +340,6 @@ async fn test_channel_message_changes( .channel_store() .read(cx) .has_new_messages(channel_id) - .unwrap() }); assert!(!b_has_messages); @@ -359,7 +357,6 @@ async fn test_channel_message_changes( .channel_store() .read(cx) .has_new_messages(channel_id) - .unwrap() }); assert!(!b_has_messages); @@ -382,7 +379,6 @@ async fn test_channel_message_changes( .channel_store() .read(cx) .has_new_messages(channel_id) - .unwrap() }); assert!(b_has_messages); @@ -402,7 +398,6 @@ async fn test_channel_message_changes( .channel_store() .read(cx) .has_new_messages(channel_id) - .unwrap() }); assert!(b_has_messages); diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index 7fbdf8ba7fc09a404f9824205c1d06d94c58f190..950d096df2614fd2a8152bad0172c21e5631a6fa 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -48,13 +48,11 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".into(), depth: 0, - role: ChannelRole::Admin, }, ExpectedChannel { id: channel_b_id, name: "channel-b".into(), depth: 1, - role: ChannelRole::Admin, }, ], ); @@ -94,7 +92,6 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".into(), depth: 0, - role: ChannelRole::Member, }], ); @@ -141,13 +138,11 @@ async fn test_core_channels( ExpectedChannel { id: channel_a_id, name: "channel-a".into(), - role: ChannelRole::Member, depth: 0, }, ExpectedChannel { id: channel_b_id, name: "channel-b".into(), - role: ChannelRole::Member, depth: 1, }, ], @@ -169,19 +164,16 @@ async fn test_core_channels( ExpectedChannel { id: channel_a_id, name: "channel-a".into(), - role: ChannelRole::Member, depth: 0, }, ExpectedChannel { id: channel_b_id, name: "channel-b".into(), - role: ChannelRole::Member, depth: 1, }, ExpectedChannel { id: channel_c_id, name: "channel-c".into(), - role: ChannelRole::Member, depth: 2, }, ], @@ -213,19 +205,16 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".into(), depth: 0, - role: ChannelRole::Admin, }, ExpectedChannel { id: channel_b_id, name: "channel-b".into(), depth: 1, - role: ChannelRole::Admin, }, ExpectedChannel { id: channel_c_id, name: "channel-c".into(), depth: 2, - role: ChannelRole::Admin, }, ], ); @@ -247,7 +236,6 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".into(), depth: 0, - role: ChannelRole::Admin, }], ); assert_channels( @@ -257,7 +245,6 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".into(), depth: 0, - role: ChannelRole::Admin, }], ); @@ -280,7 +267,6 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a".into(), depth: 0, - role: ChannelRole::Admin, }], ); @@ -311,7 +297,6 @@ async fn test_core_channels( id: channel_a_id, name: "channel-a-renamed".into(), depth: 0, - role: ChannelRole::Admin, }], ); } @@ -420,7 +405,6 @@ async fn test_channel_room( id: zed_id, name: "zed".into(), depth: 0, - role: ChannelRole::Member, }], ); cx_b.read(|cx| { @@ -681,7 +665,6 @@ async fn test_permissions_update_while_invited( depth: 0, id: rust_id, name: "rust".into(), - role: ChannelRole::Member, }], ); assert_channels(client_b.channel_store(), cx_b, &[]); @@ -709,7 +692,6 @@ async fn test_permissions_update_while_invited( depth: 0, id: rust_id, name: "rust".into(), - role: ChannelRole::Member, }], ); assert_channels(client_b.channel_store(), cx_b, &[]); @@ -748,7 +730,6 @@ async fn test_channel_rename( depth: 0, id: rust_id, name: "rust-archive".into(), - role: ChannelRole::Admin, }], ); @@ -760,7 +741,6 @@ async fn test_channel_rename( depth: 0, id: rust_id, name: "rust-archive".into(), - role: ChannelRole::Member, }], ); } @@ -889,7 +869,6 @@ async fn test_lost_channel_creation( depth: 0, id: channel_id, name: "x".into(), - role: ChannelRole::Member, }], ); @@ -913,13 +892,11 @@ async fn test_lost_channel_creation( depth: 0, id: channel_id, name: "x".into(), - role: ChannelRole::Admin, }, ExpectedChannel { depth: 1, id: subchannel_id, name: "subchannel".into(), - role: ChannelRole::Admin, }, ], ); @@ -944,13 +921,11 @@ async fn test_lost_channel_creation( depth: 0, id: channel_id, name: "x".into(), - role: ChannelRole::Member, }, ExpectedChannel { depth: 1, id: subchannel_id, name: "subchannel".into(), - role: ChannelRole::Member, }, ], ); @@ -1035,7 +1010,7 @@ async fn test_channel_link_notifications( let vim_channel = client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.create_channel("vim", None, cx) + channel_store.create_channel("vim", Some(zed_channel), cx) }) .await .unwrap(); @@ -1048,26 +1023,16 @@ async fn test_channel_link_notifications( .await .unwrap(); - client_a - .channel_store() - .update(cx_a, |channel_store, cx| { - channel_store.move_channel(vim_channel, Some(active_channel), cx) - }) - .await - .unwrap(); - - executor.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)], + &[(zed_channel, 0), (active_channel, 1), (vim_channel, 1)], ); assert_channels_list_shape( client_b.channel_store(), cx_b, - &[(zed_channel, 0), (active_channel, 1), (vim_channel, 2)], + &[(zed_channel, 0), (active_channel, 1), (vim_channel, 1)], ); assert_channels_list_shape( client_c.channel_store(), @@ -1078,7 +1043,7 @@ async fn test_channel_link_notifications( let helix_channel = client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.create_channel("helix", None, cx) + channel_store.create_channel("helix", Some(zed_channel), cx) }) .await .unwrap(); @@ -1086,7 +1051,7 @@ async fn test_channel_link_notifications( client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.move_channel(helix_channel, Some(vim_channel), cx) + channel_store.move_channel(helix_channel, vim_channel, cx) }) .await .unwrap(); @@ -1102,6 +1067,7 @@ async fn test_channel_link_notifications( }) .await .unwrap(); + cx_a.run_until_parked(); // the new channel shows for b and c assert_channels_list_shape( @@ -1110,8 +1076,8 @@ async fn test_channel_link_notifications( &[ (zed_channel, 0), (active_channel, 1), - (vim_channel, 2), - (helix_channel, 3), + (vim_channel, 1), + (helix_channel, 2), ], ); assert_channels_list_shape( @@ -1119,41 +1085,6 @@ async fn test_channel_link_notifications( 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(); - - executor.run_until_parked(); - - // 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), - ], - ); - cx_b.read(|cx| { - client_b.channel_store().read_with(cx, |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] @@ -1170,24 +1101,20 @@ async fn test_channel_membership_notifications( let channels = server .make_channel_tree( - &[ - ("zed", None), - ("active", Some("zed")), - ("vim", Some("active")), - ], + &[("zed", None), ("vim", Some("zed")), ("opensource", None)], (&client_a, cx_a), ) .await; let zed_channel = channels[0]; - let _active_channel = channels[1]; - let vim_channel = channels[2]; + let vim_channel = channels[1]; + let opensource_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), + channel_store.invite_member(zed_channel, user_b, proto::ChannelRole::Admin, cx), + channel_store.invite_member(opensource_channel, user_b, proto::ChannelRole::Member, cx), ] })) .await @@ -1203,14 +1130,6 @@ async fn test_channel_membership_notifications( .await .unwrap(); - client_b - .channel_store() - .update(cx_b, |channel_store, cx| { - channel_store.respond_to_channel_invite(vim_channel, true, cx) - }) - .await - .unwrap(); - executor.run_until_parked(); // we have an admin (a), and a guest (b) with access to all of zed, and membership in vim. @@ -1222,45 +1141,42 @@ async fn test_channel_membership_notifications( depth: 0, id: zed_channel, name: "zed".into(), - role: ChannelRole::Guest, }, ExpectedChannel { depth: 1, id: vim_channel, name: "vim".into(), - role: ChannelRole::Member, }, ], ); + client_b.channel_store().update(cx_b, |channel_store, _| { + channel_store.is_channel_admin(zed_channel) + }); + + client_b + .channel_store() + .update(cx_b, |channel_store, cx| { + channel_store.respond_to_channel_invite(opensource_channel, true, cx) + }) + .await + .unwrap(); + + cx_a.run_until_parked(); + client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.remove_member(vim_channel, user_b, cx) + channel_store.set_member_role(opensource_channel, user_b, ChannelRole::Admin, cx) }) .await .unwrap(); - executor.run_until_parked(); + cx_a.run_until_parked(); - assert_channels( - client_b.channel_store(), - cx_b, - &[ - ExpectedChannel { - depth: 0, - id: zed_channel, - name: "zed".into(), - role: ChannelRole::Guest, - }, - ExpectedChannel { - depth: 1, - id: vim_channel, - name: "vim".into(), - role: ChannelRole::Guest, - }, - ], - ) + client_b.channel_store().update(cx_b, |channel_store, _| { + channel_store.is_channel_admin(opensource_channel) + }); } #[gpui::test] @@ -1329,25 +1245,6 @@ async fn test_guest_access( 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(); - executor.run_until_parked(); - - 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(); - - executor.run_until_parked(); - assert_channels_list_shape(client_b.channel_store(), cx_b, &[(channel_b, 0)]); } #[gpui::test] @@ -1451,7 +1348,7 @@ async fn test_channel_moving( client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.move_channel(channel_d_id, Some(channel_b_id), cx) + channel_store.move_channel(channel_d_id, channel_b_id, cx) }) .await .unwrap(); @@ -1476,7 +1373,6 @@ struct ExpectedChannel { depth: usize, id: ChannelId, name: SharedString, - role: ChannelRole, } #[track_caller] @@ -1494,7 +1390,6 @@ fn assert_channel_invitations( depth: 0, name: channel.name.clone(), id: channel.id, - role: channel.role, }) .collect::>() }) @@ -1516,7 +1411,6 @@ fn assert_channels( depth, name: channel.name.clone().into(), id: channel.id, - role: channel.role, }) .collect::>() }) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 90fdc64e260cadd38e1148c66670059f70a68be4..625544e4a1572b67ccb09cb52a256631cb5ec88b 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -3884,6 +3884,7 @@ async fn test_collaborating_with_diagnostics( // Join project as client C and observe the diagnostics. let project_c = client_c.build_remote_project(project_id, cx_c).await; + executor.run_until_parked(); let project_c_diagnostic_summaries = Rc::new(RefCell::new(project_c.read_with(cx_c, |project, cx| { project.diagnostic_summaries(false, cx).collect::>() diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index c0386f47852f5ffb0b5f444f89bd645dbdf22db4..009561d9ae245aa77c140965d61c623ba849e6c7 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -125,6 +125,7 @@ impl TestServer { let channel_id = server .make_channel("a", None, (&client_a, cx_a), &mut [(&client_b, cx_b)]) .await; + cx_a.run_until_parked(); (client_a, client_b, channel_id) } diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index b2c243dc898b529b42b63876882361a8901b8dbf..2c0ff77459531c440f9f160ce3e7383db923218c 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/crates/collab_ui/src/channel_view.rs @@ -183,7 +183,7 @@ impl ChannelView { } else { self.channel_store.update(cx, |store, cx| { let channel_buffer = self.channel_buffer.read(cx); - store.notes_changed( + store.update_latest_notes_version( channel_buffer.channel_id, channel_buffer.epoch(), &channel_buffer.buffer().read(cx).version(), diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index aee181e1dfeeea2518496e18495961985b4b60be..7acf72c2842a332660a0652e54025b0d3498fa65 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -274,7 +274,7 @@ impl ChatPanel { } => { if !self.active { self.channel_store.update(cx, |store, cx| { - store.new_message(*channel_id, *message_id, cx) + store.update_latest_message_id(*channel_id, *message_id, cx) }) } } diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index c42a4e35cd37bffd62f617481a64361520a5d775..8d205e6164390b4c4856686554b040898437a083 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -23,7 +23,7 @@ use gpui::{ use menu::{Cancel, Confirm, SecondaryConfirm, SelectNext, SelectPrev}; use project::{Fs, Project}; use rpc::{ - proto::{self, PeerId}, + proto::{self, ChannelVisibility, PeerId}, ErrorCode, ErrorExt, }; use serde_derive::{Deserialize, Serialize}; @@ -1134,13 +1134,6 @@ impl CollabPanel { "Rename", Some(Box::new(SecondaryConfirm)), cx.handler_for(&this, move |this, cx| this.rename_channel(channel_id, cx)), - ) - .entry( - "Move this channel", - None, - cx.handler_for(&this, move |this, cx| { - this.start_move_channel(channel_id, cx) - }), ); if let Some(channel_name) = clipboard_channel_name { @@ -1153,23 +1146,52 @@ impl CollabPanel { ); } - context_menu = context_menu - .separator() - .entry( - "Invite Members", - None, - cx.handler_for(&this, move |this, cx| this.invite_members(channel_id, cx)), - ) - .entry( + if self.channel_store.read(cx).is_root_channel(channel_id) { + context_menu = context_menu.separator().entry( "Manage Members", None, cx.handler_for(&this, move |this, cx| this.manage_members(channel_id, cx)), ) - .entry( - "Delete", + } else { + context_menu = context_menu.entry( + "Move this channel", None, - cx.handler_for(&this, move |this, cx| this.remove_channel(channel_id, cx)), + cx.handler_for(&this, move |this, cx| { + this.start_move_channel(channel_id, cx) + }), ); + if self.channel_store.read(cx).is_public_channel(channel_id) { + context_menu = context_menu.separator().entry( + "Make Channel Private", + None, + cx.handler_for(&this, move |this, cx| { + this.set_channel_visibility( + channel_id, + ChannelVisibility::Members, + cx, + ) + }), + ) + } else { + context_menu = context_menu.separator().entry( + "Make Channel Public", + None, + cx.handler_for(&this, move |this, cx| { + this.set_channel_visibility( + channel_id, + ChannelVisibility::Public, + cx, + ) + }), + ) + } + } + + context_menu = context_menu.entry( + "Delete", + None, + cx.handler_for(&this, move |this, cx| this.remove_channel(channel_id, cx)), + ); } context_menu @@ -1490,10 +1512,6 @@ impl CollabPanel { cx.notify(); } - fn invite_members(&mut self, channel_id: ChannelId, cx: &mut ViewContext) { - self.show_channel_modal(channel_id, channel_modal::Mode::InviteMembers, cx); - } - fn manage_members(&mut self, channel_id: ChannelId, cx: &mut ViewContext) { self.show_channel_modal(channel_id, channel_modal::Mode::ManageMembers, cx); } @@ -1530,6 +1548,27 @@ impl CollabPanel { } } + fn set_channel_visibility( + &mut self, + channel_id: ChannelId, + visibility: ChannelVisibility, + cx: &mut ViewContext, + ) { + self.channel_store + .update(cx, |channel_store, cx| { + channel_store.set_channel_visibility(channel_id, visibility, cx) + }) + .detach_and_prompt_err("Failed to set channel visibility", cx, |e, _| match e.error_code() { + ErrorCode::BadPublicNesting => + if e.error_tag("direction") == Some("parent") { + Some("To make a channel public, its parent channel must be public.".to_string()) + } else { + Some("To make a channel private, all of its subchannels must be private.".to_string()) + }, + _ => None + }); + } + fn start_move_channel(&mut self, channel_id: ChannelId, _cx: &mut ViewContext) { self.channel_clipboard = Some(ChannelMoveClipboard { channel_id }); } @@ -1546,14 +1585,27 @@ impl CollabPanel { cx: &mut ViewContext, ) { if let Some(clipboard) = self.channel_clipboard.take() { - self.channel_store - .update(cx, |channel_store, cx| { - channel_store.move_channel(clipboard.channel_id, Some(to_channel_id), cx) - }) - .detach_and_prompt_err("Failed to move channel", cx, |_, _| None) + self.move_channel(clipboard.channel_id, to_channel_id, cx) } } + fn move_channel(&self, channel_id: ChannelId, to: ChannelId, cx: &mut ViewContext) { + self.channel_store + .update(cx, |channel_store, cx| { + channel_store.move_channel(channel_id, to, cx) + }) + .detach_and_prompt_err("Failed to move channel", cx, |e, _| match e.error_code() { + ErrorCode::BadPublicNesting => { + Some("Public channels must have public parents".into()) + } + ErrorCode::CircularNesting => Some("You cannot move a channel into itself".into()), + ErrorCode::WrongMoveTarget => { + Some("You cannot move a channel into a different root channel".into()) + } + _ => None, + }) + } + fn open_channel_notes(&mut self, channel_id: ChannelId, cx: &mut ViewContext) { if let Some(workspace) = self.workspace.upgrade() { ChannelView::open(channel_id, workspace, cx).detach(); @@ -1980,32 +2032,19 @@ impl CollabPanel { | Section::Offline => true, }; - h_flex() - .w_full() - .group("section-header") - .child( - ListHeader::new(text) - .when(can_collapse, |header| { - header.toggle(Some(!is_collapsed)).on_toggle(cx.listener( - move |this, _, cx| { - this.toggle_section_expanded(section, cx); - }, - )) - }) - .inset(true) - .end_slot::(button) - .selected(is_selected), - ) - .when(section == Section::Channels, |el| { - el.drag_over::(|style| style.bg(cx.theme().colors().ghost_element_hover)) - .on_drop(cx.listener(move |this, dragged_channel: &Channel, cx| { - this.channel_store - .update(cx, |channel_store, cx| { - channel_store.move_channel(dragged_channel.id, None, cx) - }) - .detach_and_prompt_err("Failed to move channel", cx, |_, _| None) - })) - }) + h_flex().w_full().group("section-header").child( + ListHeader::new(text) + .when(can_collapse, |header| { + header + .toggle(Some(!is_collapsed)) + .on_toggle(cx.listener(move |this, _, cx| { + this.toggle_section_expanded(section, cx); + })) + }) + .inset(true) + .end_slot::(button) + .selected(is_selected), + ) } fn render_contact( @@ -2219,17 +2258,16 @@ impl CollabPanel { Some(call_channel == channel_id) }) .unwrap_or(false); - let is_public = self - .channel_store - .read(cx) + let channel_store = self.channel_store.read(cx); + let is_public = channel_store .channel_for_id(channel_id) .map(|channel| channel.visibility) == Some(proto::ChannelVisibility::Public); let disclosed = has_children.then(|| !self.collapsed_channels.binary_search(&channel.id).is_ok()); - let has_messages_notification = channel.unseen_message_id.is_some(); - let has_notes_notification = channel.unseen_note_version.is_some(); + let has_messages_notification = channel_store.has_new_messages(channel_id); + let has_notes_notification = channel_store.has_channel_buffer_changed(channel_id); const FACEPILE_LIMIT: usize = 3; let participants = self.channel_store.read(cx).channel_participants(channel_id); @@ -2260,25 +2298,35 @@ impl CollabPanel { }; let width = self.width.unwrap_or(px(240.)); + let root_id = channel.root_id(); div() .id(channel_id as usize) .group("") .flex() .w_full() - .on_drag(channel.clone(), move |channel, cx| { - cx.new_view(|_| DraggedChannelView { - channel: channel.clone(), - width, + .when(!channel.is_root_channel(), |el| { + el.on_drag(channel.clone(), move |channel, cx| { + cx.new_view(|_| DraggedChannelView { + channel: channel.clone(), + width, + }) }) }) - .drag_over::(|style| style.bg(cx.theme().colors().ghost_element_hover)) + .drag_over::({ + move |style, dragged_channel: &Channel, cx| { + if dragged_channel.root_id() == root_id { + style.bg(cx.theme().colors().ghost_element_hover) + } else { + style + } + } + }) .on_drop(cx.listener(move |this, dragged_channel: &Channel, cx| { - this.channel_store - .update(cx, |channel_store, cx| { - channel_store.move_channel(dragged_channel.id, Some(channel_id), cx) - }) - .detach_and_prompt_err("Failed to move channel", cx, |_, _| None) + if dragged_channel.root_id() != root_id { + return; + } + this.move_channel(dragged_channel.id, channel_id, cx); })) .child( ListItem::new(channel_id as usize) diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index 891c609316162f0d722379f4d6a8298c4a37d032..826a61eb486512e1fc0571b893dd57768f0f5b36 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -10,7 +10,6 @@ use gpui::{ WeakView, }; use picker::{Picker, PickerDelegate}; -use rpc::proto::channel_member; use std::sync::Arc; use ui::{prelude::*, Avatar, Checkbox, ContextMenu, ListItem, ListItemSpacing}; use util::TryFutureExt; @@ -359,10 +358,8 @@ impl PickerDelegate for ChannelModalDelegate { Some(proto::channel_member::Kind::Invitee) => { self.remove_member(selected_user.id, cx); } - Some(proto::channel_member::Kind::AncestorMember) | None => { - self.invite_member(selected_user, cx) - } Some(proto::channel_member::Kind::Member) => {} + None => self.invite_member(selected_user, cx), }, } } @@ -402,10 +399,6 @@ impl PickerDelegate for ChannelModalDelegate { .children( if request_status == Some(proto::channel_member::Kind::Invitee) { Some(Label::new("Invited")) - } else if membership.map(|m| m.kind) - == Some(channel_member::Kind::AncestorMember) - { - Some(Label::new("Parent")) } else { None }, @@ -563,16 +556,9 @@ impl ChannelModalDelegate { let Some(membership) = self.member_at_index(ix) else { return; }; - if membership.kind == proto::channel_member::Kind::AncestorMember { - return; - } let user_id = membership.user.id; let picker = cx.view().clone(); let context_menu = ContextMenu::build(cx, |mut menu, _cx| { - if membership.kind == channel_member::Kind::AncestorMember { - return menu.entry("Inherited membership", None, |_| {}); - }; - let role = membership.role; if role == ChannelRole::Admin || role == ChannelRole::Member { diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index a0bbf6fc7948095adcc175f9cfb81ef3c1c88743..6d529213f01673d767dc7fcb4085d697d8e20715 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -782,10 +782,20 @@ pub trait InteractiveElement: Sized { } /// Apply the given style when the given data type is dragged over this element - fn drag_over(mut self, f: impl FnOnce(StyleRefinement) -> StyleRefinement) -> Self { - self.interactivity() - .drag_over_styles - .push((TypeId::of::(), f(StyleRefinement::default()))); + fn drag_over( + mut self, + f: impl 'static + Fn(StyleRefinement, &S, &WindowContext) -> StyleRefinement, + ) -> Self { + self.interactivity().drag_over_styles.push(( + TypeId::of::(), + Box::new(move |currently_dragged: &dyn Any, cx| { + f( + StyleRefinement::default(), + currently_dragged.downcast_ref::().unwrap(), + cx, + ) + }), + )); self } @@ -1174,7 +1184,10 @@ pub struct Interactivity { pub(crate) group_hover_style: Option, pub(crate) active_style: Option>, pub(crate) group_active_style: Option, - pub(crate) drag_over_styles: Vec<(TypeId, StyleRefinement)>, + pub(crate) drag_over_styles: Vec<( + TypeId, + Box StyleRefinement>, + )>, pub(crate) group_drag_over_styles: Vec<(TypeId, GroupStyle)>, pub(crate) mouse_down_listeners: Vec, pub(crate) mouse_up_listeners: Vec, @@ -1980,7 +1993,7 @@ impl Interactivity { } } - for (state_type, drag_over_style) in &self.drag_over_styles { + for (state_type, build_drag_over_style) in &self.drag_over_styles { if *state_type == drag.value.as_ref().type_id() && bounds .intersect(&cx.content_mask().bounds) @@ -1990,7 +2003,7 @@ impl Interactivity { cx.stacking_order(), ) { - style.refine(drag_over_style); + style.refine(&build_drag_over_style(drag.value.as_ref(), cx)); } } } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 63524d682ee1295a5b31a3a32531fb6580fa40ef..7171ac5f1e2750c1fdf426f07fcbf8bdc79dbcb0 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1371,7 +1371,7 @@ impl ProjectPanel { entry_id: *entry_id, }) }) - .drag_over::(|style| { + .drag_over::(|style, _, cx| { style.bg(cx.theme().colors().drop_target_background) }) .on_drop(cx.listener(move |this, dragged_id: &ProjectEntryId, cx| { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 8bea2f24e0d116a0d954708fec15d353c77a616f..18f9b1f1e8b22e1a34cfbe0ce68dbc676dff08d0 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -181,7 +181,9 @@ message Envelope { MarkNotificationRead mark_notification_read = 153; LspExtExpandMacro lsp_ext_expand_macro = 154; LspExtExpandMacroResponse lsp_ext_expand_macro_response = 155; - SetRoomParticipantRole set_room_participant_role = 156; // Current max + SetRoomParticipantRole set_room_participant_role = 156; + + UpdateUserChannels update_user_channels = 157; // current max } } @@ -210,6 +212,10 @@ enum ErrorCode { Forbidden = 5; WrongReleaseChannel = 6; NeedsCla = 7; + NotARootChannel = 8; + BadPublicNesting = 9; + CircularNesting = 10; + WrongMoveTarget = 11; } message Test { @@ -992,19 +998,24 @@ message UpdateChannels { repeated Channel channel_invitations = 5; repeated uint64 remove_channel_invitations = 6; repeated ChannelParticipants channel_participants = 7; - repeated UnseenChannelMessage unseen_channel_messages = 9; - repeated UnseenChannelBufferChange unseen_channel_buffer_changes = 10; + repeated ChannelMessageId latest_channel_message_ids = 8; + repeated ChannelBufferVersion latest_channel_buffer_versions = 9; +} + +message UpdateUserChannels { + repeated ChannelMessageId observed_channel_message_id = 1; + repeated ChannelBufferVersion observed_channel_buffer_version = 2; + repeated ChannelMembership channel_memberships = 3; } -message UnseenChannelMessage { +message ChannelMembership { uint64 channel_id = 1; - uint64 message_id = 2; + ChannelRole role = 2; } -message UnseenChannelBufferChange { +message ChannelMessageId { uint64 channel_id = 1; - uint64 epoch = 2; - repeated VectorClockEntry version = 3; + uint64 message_id = 2; } message ChannelPermission { @@ -1041,7 +1052,6 @@ message ChannelMember { enum Kind { Member = 0; Invitee = 1; - AncestorMember = 2; } } @@ -1148,7 +1158,7 @@ message GetChannelMessagesById { message MoveChannel { uint64 channel_id = 1; - optional uint64 to = 2; + uint64 to = 2; } message JoinChannelBuffer { @@ -1586,7 +1596,6 @@ message Channel { uint64 id = 1; string name = 2; ChannelVisibility visibility = 3; - ChannelRole role = 4; repeated uint64 parent_path = 5; } diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 5d0a99415480c30969a9a072287841ae6dd6c689..9b885d1840f5964c23fa4314165a3069dba7df74 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -269,6 +269,7 @@ messages!( (UpdateChannelBuffer, Foreground), (UpdateChannelBufferCollaborators, Foreground), (UpdateChannels, Foreground), + (UpdateUserChannels, Foreground), (UpdateContacts, Foreground), (UpdateDiagnosticSummary, Foreground), (UpdateDiffBase, Foreground), diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index c22ecb740d4a52c84a7622e151b8e71e6ee2c07a..2beb7614e858ca6ff492ca140e1b86ad7f880fb2 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -11,4 +11,4 @@ pub use notification::*; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 67; +pub const PROTOCOL_VERSION: u32 = 68; diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 2336230c86feec631b535204afda2aff6102e11e..55ae875fefcfd2e4a0746fd81cfba27f7106834d 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1334,8 +1334,12 @@ impl Pane { }, |tab, cx| cx.new_view(|_| tab.clone()), ) - .drag_over::(|tab| tab.bg(cx.theme().colors().drop_target_background)) - .drag_over::(|tab| tab.bg(cx.theme().colors().drop_target_background)) + .drag_over::(|tab, _, cx| { + tab.bg(cx.theme().colors().drop_target_background) + }) + .drag_over::(|tab, _, cx| { + tab.bg(cx.theme().colors().drop_target_background) + }) .when_some(self.can_drop_predicate.clone(), |this, p| { this.can_drop(move |a, cx| p(a, cx)) }) @@ -1505,10 +1509,10 @@ impl Pane { .child("") .h_full() .flex_grow() - .drag_over::(|bar| { + .drag_over::(|bar, _, cx| { bar.bg(cx.theme().colors().drop_target_background) }) - .drag_over::(|bar| { + .drag_over::(|bar, _, cx| { bar.bg(cx.theme().colors().drop_target_background) }) .on_drop(cx.listener(move |this, dragged_tab: &DraggedTab, cx| {