From 28c39aae17c7e027e07b2198aa4c786f5ae64ea6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 2 Jan 2024 11:44:51 -0800 Subject: [PATCH] Start work on read-only project access for channel guests Co-authored-by: Conrad Co-authored-by: Mikayla --- crates/call/src/room.rs | 2 +- crates/collab/src/db/queries/channels.rs | 52 +++++------ crates/collab/src/tests.rs | 1 + .../collab/src/tests/channel_guest_tests.rs | 88 +++++++++++++++++++ crates/collab/src/tests/integration_tests.rs | 24 ++--- .../random_project_collaboration_tests.rs | 4 +- .../src/tests/randomized_test_helpers.rs | 2 +- crates/gpui/src/element.rs | 5 +- crates/gpui/src/platform/test/platform.rs | 5 +- crates/gpui/src/platform/test/window.rs | 16 +++- crates/project/src/project.rs | 10 ++- crates/workspace/src/workspace.rs | 6 +- 12 files changed, 159 insertions(+), 56 deletions(-) create mode 100644 crates/collab/src/tests/channel_guest_tests.rs diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index 0dfdb50fc7201e1cfd9e03b68741d99b3b14bdc7..14d9a55ef03c92f13e1ecfb398963cf57db7710c 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -1099,7 +1099,7 @@ impl Room { this.update(&mut cx, |this, cx| { this.joined_projects.retain(|project| { if let Some(project) = project.upgrade() { - !project.read(cx).is_read_only() + !project.read(cx).is_disconnected() } else { false } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 780fb783bc84c9b0f741117122fc737689e98ab2..9a14aabfda3ba5d8449fea47397d92be5f217690 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -132,34 +132,34 @@ impl Database { debug_assert!( self.channel_role_for_user(&channel, user_id, &*tx).await? == role ); - } - } - - 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), - user_id: ActiveValue::Set(user_id), - accepted: ActiveValue::Set(true), - role: ActiveValue::Set(ChannelRole::Guest), - }) - .exec(&*tx) - .await?; + } 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), + user_id: ActiveValue::Set(user_id), + accepted: ActiveValue::Set(true), + role: ActiveValue::Set(ChannelRole::Guest), + }) + .exec(&*tx) + .await?; - accept_invite_result = Some( - self.calculate_membership_updated(&channel_to_join, user_id, &*tx) - .await?, - ); + accept_invite_result = Some( + self.calculate_membership_updated(&channel_to_join, user_id, &*tx) + .await?, + ); - debug_assert!(self.channel_role_for_user(&channel, user_id, &*tx).await? == role); + debug_assert!( + self.channel_role_for_user(&channel, user_id, &*tx).await? == role + ); + } } if role.is_none() || role == Some(ChannelRole::Banned) { diff --git a/crates/collab/src/tests.rs b/crates/collab/src/tests.rs index 53d42505bdc3babe023c4a8feb4dbbf5c5e24ab6..aca9329d5ad87a477614803de5143b86f1fe169b 100644 --- a/crates/collab/src/tests.rs +++ b/crates/collab/src/tests.rs @@ -2,6 +2,7 @@ use call::Room; use gpui::{Model, TestAppContext}; mod channel_buffer_tests; +mod channel_guest_tests; mod channel_message_tests; mod channel_tests; mod editor_tests; diff --git a/crates/collab/src/tests/channel_guest_tests.rs b/crates/collab/src/tests/channel_guest_tests.rs new file mode 100644 index 0000000000000000000000000000000000000000..78dd57be026913b3299efe2e1c35a94a5ea798ad --- /dev/null +++ b/crates/collab/src/tests/channel_guest_tests.rs @@ -0,0 +1,88 @@ +use crate::tests::TestServer; +use call::ActiveCall; +use gpui::{BackgroundExecutor, TestAppContext, VisualTestContext}; +use rpc::proto; +use workspace::Workspace; + +#[gpui::test] +async fn test_channel_guests( + executor: BackgroundExecutor, + mut cx_a: &mut TestAppContext, + mut cx_b: &mut TestAppContext, +) { + let mut server = TestServer::start(executor.clone()).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + + let channel_id = server + .make_channel("the-channel", None, (&client_a, cx_a), &mut []) + .await; + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.set_channel_visibility(channel_id, proto::ChannelVisibility::Public, cx) + }) + .await + .unwrap(); + + client_a + .fs() + .insert_tree( + "/a", + serde_json::json!({ + "a.txt": "a-contents", + }), + ) + .await; + + let active_call_a = cx_a.read(ActiveCall::global); + let active_call_b = cx_b.read(ActiveCall::global); + + // Client A shares a project in the channel + active_call_a + .update(cx_a, |call, cx| call.join_channel(channel_id, cx)) + .await + .unwrap(); + let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await; + let worktree_a = project_a.read_with(cx_a, |project, _| project.worktrees().next().unwrap()); + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + cx_a.executor().run_until_parked(); + + // Client B joins channel A as a guest + cx_b.update(|cx| workspace::join_channel(channel_id, client_b.app_state.clone(), None, cx)) + .await + .unwrap(); + + // b should be following a in the shared project. + // B is a guest, + cx_a.executor().run_until_parked(); + + // todo!() the test window does not call activation handlers + // correctly yet, so this API does not work. + // let project_b = active_call_b.read_with(cx_b, |call, _| { + // call.location() + // .unwrap() + // .upgrade() + // .expect("should not be weak") + // }); + + let window_b = cx_b.update(|cx| cx.active_window().unwrap()); + let cx_b = &mut VisualTestContext::from_window(window_b, cx_b); + + let workspace_b = window_b + .downcast::() + .unwrap() + .root_view(cx_b) + .unwrap(); + let project_b = workspace_b.update(cx_b, |workspace, _| workspace.project().clone()); + + assert_eq!( + project_b.read_with(cx_b, |project, _| project.remote_id()), + Some(project_id), + ); + assert!(project_b.read_with(cx_b, |project, _| project.is_read_only())) +} diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 201ba07dbb319663cb2c6f1810c50faa87a077ab..e64fdbec832a07d1dc3753c059653f6ea954fb00 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -1380,7 +1380,7 @@ async fn test_unshare_project( .unwrap(); executor.run_until_parked(); - assert!(project_b.read_with(cx_b, |project, _| project.is_read_only())); + assert!(project_b.read_with(cx_b, |project, _| project.is_disconnected())); // Client C opens the project. let project_c = client_c.build_remote_project(project_id, cx_c).await; @@ -1393,7 +1393,7 @@ async fn test_unshare_project( assert!(worktree_a.read_with(cx_a, |tree, _| !tree.as_local().unwrap().is_shared())); - assert!(project_c.read_with(cx_c, |project, _| project.is_read_only())); + assert!(project_c.read_with(cx_c, |project, _| project.is_disconnected())); // Client C can open the project again after client A re-shares. let project_id = active_call_a @@ -1419,7 +1419,7 @@ async fn test_unshare_project( project_a.read_with(cx_a, |project, _| assert!(!project.is_shared())); project_c2.read_with(cx_c, |project, _| { - assert!(project.is_read_only()); + assert!(project.is_disconnected()); assert!(project.collaborators().is_empty()); }); } @@ -1551,7 +1551,7 @@ async fn test_project_reconnect( }); project_b1.read_with(cx_b, |project, _| { - assert!(!project.is_read_only()); + assert!(!project.is_disconnected()); assert_eq!(project.collaborators().len(), 1); }); @@ -1653,7 +1653,7 @@ async fn test_project_reconnect( }); project_b1.read_with(cx_b, |project, cx| { - assert!(!project.is_read_only()); + assert!(!project.is_disconnected()); assert_eq!( project .worktree_for_id(worktree1_id, cx) @@ -1687,9 +1687,9 @@ async fn test_project_reconnect( ); }); - project_b2.read_with(cx_b, |project, _| assert!(project.is_read_only())); + project_b2.read_with(cx_b, |project, _| assert!(project.is_disconnected())); - project_b3.read_with(cx_b, |project, _| assert!(!project.is_read_only())); + project_b3.read_with(cx_b, |project, _| assert!(!project.is_disconnected())); buffer_a1.read_with(cx_a, |buffer, _| assert_eq!(buffer.text(), "WaZ")); @@ -1746,7 +1746,7 @@ async fn test_project_reconnect( executor.run_until_parked(); project_b1.read_with(cx_b, |project, cx| { - assert!(!project.is_read_only()); + assert!(!project.is_disconnected()); assert_eq!( project .worktree_for_id(worktree1_id, cx) @@ -1780,7 +1780,7 @@ async fn test_project_reconnect( ); }); - project_b3.read_with(cx_b, |project, _| assert!(project.is_read_only())); + project_b3.read_with(cx_b, |project, _| assert!(project.is_disconnected())); buffer_a1.read_with(cx_a, |buffer, _| assert_eq!(buffer.text(), "WXaYZ")); @@ -3535,7 +3535,7 @@ async fn test_leaving_project( }); project_b2.read_with(cx_b, |project, _| { - assert!(project.is_read_only()); + assert!(project.is_disconnected()); }); project_c.read_with(cx_c, |project, _| { @@ -3568,11 +3568,11 @@ async fn test_leaving_project( }); project_b2.read_with(cx_b, |project, _| { - assert!(project.is_read_only()); + assert!(project.is_disconnected()); }); project_c.read_with(cx_c, |project, _| { - assert!(project.is_read_only()); + assert!(project.is_disconnected()); }); } diff --git a/crates/collab/src/tests/random_project_collaboration_tests.rs b/crates/collab/src/tests/random_project_collaboration_tests.rs index f4194b98e8adbf41742a5aa279d766cf09c2477d..53d47eb6b5b44e9a5e34518bcc305c9e27ed399f 100644 --- a/crates/collab/src/tests/random_project_collaboration_tests.rs +++ b/crates/collab/src/tests/random_project_collaboration_tests.rs @@ -1149,7 +1149,7 @@ impl RandomizedTest for ProjectCollaborationTest { Some((project, cx)) }); - if !guest_project.is_read_only() { + if !guest_project.is_disconnected() { if let Some((host_project, host_cx)) = host_project { let host_worktree_snapshots = host_project.read_with(host_cx, |host_project, cx| { @@ -1236,7 +1236,7 @@ impl RandomizedTest for ProjectCollaborationTest { let buffers = client.buffers().clone(); for (guest_project, guest_buffers) in &buffers { let project_id = if guest_project.read_with(client_cx, |project, _| { - project.is_local() || project.is_read_only() + project.is_local() || project.is_disconnected() }) { continue; } else { diff --git a/crates/collab/src/tests/randomized_test_helpers.rs b/crates/collab/src/tests/randomized_test_helpers.rs index 91bd9cf6f698b3a8c6d436b99e35eaefc771e89e..69bec62460bbd2469fe497ce9418442b0f58ba92 100644 --- a/crates/collab/src/tests/randomized_test_helpers.rs +++ b/crates/collab/src/tests/randomized_test_helpers.rs @@ -518,7 +518,7 @@ impl TestPlan { for project in client.remote_projects().iter() { project.read_with(&client_cx, |project, _| { assert!( - project.is_read_only(), + project.is_disconnected(), "project {:?} should be read only", project.remote_id() ) diff --git a/crates/gpui/src/element.rs b/crates/gpui/src/element.rs index 30456f14a7ffeb35df54442e15304dcc68ed1958..987b91b791a93b9fb72672a4608c1b6665fc20e2 100644 --- a/crates/gpui/src/element.rs +++ b/crates/gpui/src/element.rs @@ -44,8 +44,9 @@ pub trait IntoElement: Sized { } /// Convert into an element, then draw in the current window at the given origin. - /// The provided available space is provided to the layout engine to determine the size of the root element. - /// Once the element is drawn, its associated element staet is yielded to the given callback. + /// The available space argument is provided to the layout engine to determine the size of the + // root element. Once the element is drawn, its associated element state is yielded to the + // given callback. fn draw_and_update_state( self, origin: Point, diff --git a/crates/gpui/src/platform/test/platform.rs b/crates/gpui/src/platform/test/platform.rs index cc683cacb68f0e471ef9b1fa5596581d81d3d0f3..142498ce4be88f05595d52109c12238eb8ef021f 100644 --- a/crates/gpui/src/platform/test/platform.rs +++ b/crates/gpui/src/platform/test/platform.rs @@ -19,7 +19,7 @@ pub struct TestPlatform { background_executor: BackgroundExecutor, foreground_executor: ForegroundExecutor, - active_window: Arc>>, + pub(crate) active_window: Arc>>, active_display: Rc, active_cursor: Mutex, current_clipboard_item: Mutex>, @@ -106,7 +106,7 @@ impl Platform for TestPlatform { } fn activate(&self, _ignoring_other_apps: bool) { - unimplemented!() + // } fn hide(&self) { @@ -142,6 +142,7 @@ impl Platform for TestPlatform { *self.active_window.lock() = Some(handle); Box::new(TestWindow::new( options, + handle, self.weak.clone(), self.active_display.clone(), )) diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs index 9df513d1f7ea46b0dffb452d27dbd82f5aba2176..dab53e46d9f10593104a08a0ea0d9517e7d2b911 100644 --- a/crates/gpui/src/platform/test/window.rs +++ b/crates/gpui/src/platform/test/window.rs @@ -1,7 +1,7 @@ use crate::{ - px, AtlasKey, AtlasTextureId, AtlasTile, Pixels, PlatformAtlas, PlatformDisplay, - PlatformInputHandler, PlatformWindow, Point, Size, TestPlatform, TileId, WindowAppearance, - WindowBounds, WindowOptions, + px, AnyWindowHandle, AtlasKey, AtlasTextureId, AtlasTile, Pixels, PlatformAtlas, + PlatformDisplay, PlatformInputHandler, PlatformWindow, Point, Size, TestPlatform, TileId, + WindowAppearance, WindowBounds, WindowOptions, }; use collections::HashMap; use parking_lot::Mutex; @@ -20,6 +20,7 @@ pub(crate) struct TestWindowHandlers { pub struct TestWindow { pub(crate) bounds: WindowBounds, + pub(crate) handle: AnyWindowHandle, display: Rc, pub(crate) title: Option, pub(crate) edited: bool, @@ -32,6 +33,7 @@ pub struct TestWindow { impl TestWindow { pub fn new( options: WindowOptions, + handle: AnyWindowHandle, platform: Weak, display: Rc, ) -> Self { @@ -39,6 +41,7 @@ impl TestWindow { bounds: options.bounds, display, platform, + handle, input_handler: None, sprite_atlas: Arc::new(TestAtlas::new()), handlers: Default::default(), @@ -107,7 +110,12 @@ impl PlatformWindow for TestWindow { } fn activate(&self) { - unimplemented!() + *self + .platform + .upgrade() + .expect("platform dropped") + .active_window + .lock() = Some(self.handle); } fn set_title(&mut self, title: &str) { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index b9c73ae67785d48d7912414113329bb4a6d2e0da..a58a7b804f1120c07c4cabc5c8a5b7e3126acbf9 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1659,7 +1659,7 @@ impl Project { cx.emit(Event::Closed); } - pub fn is_read_only(&self) -> bool { + pub fn is_disconnected(&self) -> bool { match &self.client_state { Some(ProjectClientState::Remote { sharing_has_stopped, @@ -1669,6 +1669,10 @@ impl Project { } } + pub fn is_read_only(&self) -> bool { + self.is_disconnected() + } + pub fn is_local(&self) -> bool { match &self.client_state { Some(ProjectClientState::Remote { .. }) => false, @@ -6015,7 +6019,7 @@ impl Project { this.upgrade().context("project dropped")?; let response = rpc.request(message).await?; let this = this.upgrade().context("project dropped")?; - if this.update(&mut cx, |this, _| this.is_read_only())? { + if this.update(&mut cx, |this, _| this.is_disconnected())? { Err(anyhow!("disconnected before completing request")) } else { request @@ -7942,7 +7946,7 @@ impl Project { if let Some(buffer) = buffer { break buffer; - } else if this.update(&mut cx, |this, _| this.is_read_only())? { + } else if this.update(&mut cx, |this, _| this.is_disconnected())? { return Err(anyhow!("disconnected before buffer {} could be opened", id)); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 76715f69bef9ffb5e7f4ced25d00372df897b7e5..7c2ca8a1f79c857270a30c7f98f3ada4507d32e8 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1184,7 +1184,7 @@ impl Workspace { mut save_intent: SaveIntent, cx: &mut ViewContext, ) -> Task> { - if self.project.read(cx).is_read_only() { + if self.project.read(cx).is_disconnected() { return Task::ready(Ok(true)); } let dirty_items = self @@ -2508,7 +2508,7 @@ impl Workspace { } fn update_window_edited(&mut self, cx: &mut ViewContext) { - let is_edited = !self.project.read(cx).is_read_only() + let is_edited = !self.project.read(cx).is_disconnected() && self .items(cx) .any(|item| item.has_conflict(cx) || item.is_dirty(cx)); @@ -3632,7 +3632,7 @@ impl Render for Workspace { })), ) .child(self.status_bar.clone()) - .children(if self.project.read(cx).is_read_only() { + .children(if self.project.read(cx).is_disconnected() { Some(DisconnectedOverlay) } else { None