Start work on read-only project access for channel guests

Max Brunsfeld , Conrad , and Mikayla created

Co-authored-by: Conrad <conrad@zed.dev>
Co-authored-by: Mikayla <mikayla@zed.dev>

Change summary

crates/call/src/room.rs                                       |  2 
crates/collab/src/db/queries/channels.rs                      | 52 +-
crates/collab/src/tests.rs                                    |  1 
crates/collab/src/tests/channel_guest_tests.rs                | 88 +++++
crates/collab/src/tests/integration_tests.rs                  | 24 
crates/collab/src/tests/random_project_collaboration_tests.rs |  4 
crates/collab/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(-)

Detailed changes

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
                     }

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) {

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;

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::<Workspace>()
+        .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()))
+}

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());
     });
 }
 

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 {

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

@@ -518,7 +518,7 @@ impl<T: RandomizedTest> TestPlan<T> {
                 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()
                         )

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<T, R>(
         self,
         origin: Point<Pixels>,

crates/gpui/src/platform/test/platform.rs 🔗

@@ -19,7 +19,7 @@ pub struct TestPlatform {
     background_executor: BackgroundExecutor,
     foreground_executor: ForegroundExecutor,
 
-    active_window: Arc<Mutex<Option<AnyWindowHandle>>>,
+    pub(crate) active_window: Arc<Mutex<Option<AnyWindowHandle>>>,
     active_display: Rc<dyn PlatformDisplay>,
     active_cursor: Mutex<CursorStyle>,
     current_clipboard_item: Mutex<Option<ClipboardItem>>,
@@ -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(),
         ))

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<dyn PlatformDisplay>,
     pub(crate) title: Option<String>,
     pub(crate) edited: bool,
@@ -32,6 +33,7 @@ pub struct TestWindow {
 impl TestWindow {
     pub fn new(
         options: WindowOptions,
+        handle: AnyWindowHandle,
         platform: Weak<TestPlatform>,
         display: Rc<dyn PlatformDisplay>,
     ) -> 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) {

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));
                 }
 

crates/workspace/src/workspace.rs 🔗

@@ -1184,7 +1184,7 @@ impl Workspace {
         mut save_intent: SaveIntent,
         cx: &mut ViewContext<Self>,
     ) -> Task<Result<bool>> {
-        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<Self>) {
-        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