Merge pull request #2251 from zed-industries/clear-follow-state-on-project-close

Update db followers table when user leaves a project

crates/collab/src/db.rs

@@ -1757,17 +1757,14 @@ impl Database {
                         .add(follower::Column::ProjectId.eq(project_id))
                         .add(
                             follower::Column::LeaderConnectionServerId
-                                .eq(leader_connection.owner_id)
-                                .and(follower::Column::LeaderConnectionId.eq(leader_connection.id)),
+                                .eq(leader_connection.owner_id),
                         )
+                        .add(follower::Column::LeaderConnectionId.eq(leader_connection.id))
                         .add(
                             follower::Column::FollowerConnectionServerId
-                                .eq(follower_connection.owner_id)
-                                .and(
-                                    follower::Column::FollowerConnectionId
-                                        .eq(follower_connection.id),
-                                ),
-                        ),
+                                .eq(follower_connection.owner_id),
+                        )
+                        .add(follower::Column::FollowerConnectionId.eq(follower_connection.id)),
                 )
                 .exec(&*tx)
                 .await?;
@@ -2560,7 +2557,7 @@ impl Database {
         &self,
         project_id: ProjectId,
         connection: ConnectionId,
-    ) -> Result<RoomGuard<LeftProject>> {
+    ) -> Result<RoomGuard<(proto::Room, LeftProject)>> {
         let room_id = self.room_id_for_project(project_id).await?;
         self.room_transaction(room_id, |tx| async move {
             let result = project_collaborator::Entity::delete_many()
@@ -2592,13 +2589,39 @@ impl Database {
                 .map(|collaborator| collaborator.connection())
                 .collect();
 
+            follower::Entity::delete_many()
+                .filter(
+                    Condition::any()
+                        .add(
+                            Condition::all()
+                                .add(follower::Column::ProjectId.eq(project_id))
+                                .add(
+                                    follower::Column::LeaderConnectionServerId
+                                        .eq(connection.owner_id),
+                                )
+                                .add(follower::Column::LeaderConnectionId.eq(connection.id)),
+                        )
+                        .add(
+                            Condition::all()
+                                .add(follower::Column::ProjectId.eq(project_id))
+                                .add(
+                                    follower::Column::FollowerConnectionServerId
+                                        .eq(connection.owner_id),
+                                )
+                                .add(follower::Column::FollowerConnectionId.eq(connection.id)),
+                        ),
+                )
+                .exec(&*tx)
+                .await?;
+
+            let room = self.get_room(project.room_id, &tx).await?;
             let left_project = LeftProject {
                 id: project_id,
                 host_user_id: project.host_user_id,
                 host_connection_id: project.host_connection()?,
                 connection_ids,
             };
-            Ok(left_project)
+            Ok((room, left_project))
         })
         .await
     }

crates/collab/src/rpc.rs

@@ -1408,7 +1408,7 @@ async fn leave_project(request: proto::LeaveProject, session: Session) -> Result
     let sender_id = session.connection_id;
     let project_id = ProjectId::from_proto(request.project_id);
 
-    let project = session
+    let (room, project) = &*session
         .db()
         .await
         .leave_project(project_id, sender_id)
@@ -1419,7 +1419,9 @@ async fn leave_project(request: proto::LeaveProject, session: Session) -> Result
         host_connection_id = %project.host_connection_id,
         "leave project"
     );
+
     project_left(&project, &session);
+    room_updated(&room, &session.peer);
 
     Ok(())
 }

crates/collab/src/tests/integration_tests.rs

@@ -5792,11 +5792,12 @@ async fn test_contact_requests(
 }
 
 #[gpui::test(iterations = 10)]
-async fn test_following(
+async fn test_basic_following(
     deterministic: Arc<Deterministic>,
     cx_a: &mut TestAppContext,
     cx_b: &mut TestAppContext,
     cx_c: &mut TestAppContext,
+    cx_d: &mut TestAppContext,
 ) {
     deterministic.forbid_parking();
     cx_a.update(editor::init);
@@ -5806,11 +5807,14 @@ async fn test_following(
     let client_a = server.create_client(cx_a, "user_a").await;
     let client_b = server.create_client(cx_b, "user_b").await;
     let client_c = server.create_client(cx_c, "user_c").await;
+    let client_d = server.create_client(cx_d, "user_d").await;
     server
-        .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
-        .await;
-    server
-        .make_contacts(&mut [(&client_a, cx_a), (&client_c, cx_c)])
+        .create_room(&mut [
+            (&client_a, cx_a),
+            (&client_b, cx_b),
+            (&client_c, cx_c),
+            (&client_d, cx_d),
+        ])
         .await;
     let active_call_a = cx_a.read(ActiveCall::global);
     let active_call_b = cx_b.read(ActiveCall::global);
@@ -5877,6 +5881,7 @@ async fn test_following(
     let peer_id_a = client_a.peer_id().unwrap();
     let peer_id_b = client_b.peer_id().unwrap();
     let peer_id_c = client_c.peer_id().unwrap();
+    let peer_id_d = client_d.peer_id().unwrap();
 
     // Client A updates their selections in those editors
     editor_a1.update(cx_a, |editor, cx| {
@@ -5896,25 +5901,15 @@ async fn test_following(
         .await
         .unwrap();
 
-    // Client A invites client C to the call.
-    active_call_a
-        .update(cx_a, |call, cx| {
-            call.invite(client_c.current_user_id(cx_c).to_proto(), None, cx)
-        })
-        .await
-        .unwrap();
     cx_c.foreground().run_until_parked();
     let active_call_c = cx_c.read(ActiveCall::global);
-    active_call_c
-        .update(cx_c, |call, cx| call.accept_incoming(cx))
-        .await
-        .unwrap();
     let project_c = client_c.build_remote_project(project_id, cx_c).await;
     let workspace_c = client_c.build_workspace(&project_c, cx_c);
     active_call_c
         .update(cx_c, |call, cx| call.set_location(Some(&project_c), cx))
         .await
         .unwrap();
+    drop(project_c);
 
     // Client C also follows client A.
     workspace_c
@@ -5926,12 +5921,23 @@ async fn test_following(
         .await
         .unwrap();
 
+    cx_d.foreground().run_until_parked();
+    let active_call_d = cx_d.read(ActiveCall::global);
+    let project_d = client_d.build_remote_project(project_id, cx_d).await;
+    let workspace_d = client_d.build_workspace(&project_d, cx_d);
+    active_call_d
+        .update(cx_d, |call, cx| call.set_location(Some(&project_d), cx))
+        .await
+        .unwrap();
+    drop(project_d);
+
     // All clients see that clients B and C are following client A.
     cx_c.foreground().run_until_parked();
     for (name, active_call, cx) in [
         ("A", &active_call_a, &cx_a),
         ("B", &active_call_b, &cx_b),
         ("C", &active_call_c, &cx_c),
+        ("D", &active_call_d, &cx_d),
     ] {
         active_call.read_with(*cx, |call, cx| {
             let room = call.room().unwrap().read(cx);
@@ -5954,6 +5960,7 @@ async fn test_following(
         ("A", &active_call_a, &cx_a),
         ("B", &active_call_b, &cx_b),
         ("C", &active_call_c, &cx_c),
+        ("D", &active_call_d, &cx_d),
     ] {
         active_call.read_with(*cx, |call, cx| {
             let room = call.room().unwrap().read(cx);
@@ -5965,6 +5972,90 @@ async fn test_following(
         });
     }
 
+    // Client C re-follows client A.
+    workspace_c.update(cx_c, |workspace, cx| {
+        workspace.toggle_follow(&ToggleFollow(peer_id_a), cx);
+    });
+
+    // All clients see that clients B and C are following client A.
+    cx_c.foreground().run_until_parked();
+    for (name, active_call, cx) in [
+        ("A", &active_call_a, &cx_a),
+        ("B", &active_call_b, &cx_b),
+        ("C", &active_call_c, &cx_c),
+        ("D", &active_call_d, &cx_d),
+    ] {
+        active_call.read_with(*cx, |call, cx| {
+            let room = call.room().unwrap().read(cx);
+            assert_eq!(
+                room.followers_for(peer_id_a, project_id),
+                &[peer_id_b, peer_id_c],
+                "checking followers for A as {name}"
+            );
+        });
+    }
+
+    // Client D follows client C.
+    workspace_d
+        .update(cx_d, |workspace, cx| {
+            workspace
+                .toggle_follow(&ToggleFollow(peer_id_c), cx)
+                .unwrap()
+        })
+        .await
+        .unwrap();
+
+    // All clients see that D is following C
+    cx_d.foreground().run_until_parked();
+    for (name, active_call, cx) in [
+        ("A", &active_call_a, &cx_a),
+        ("B", &active_call_b, &cx_b),
+        ("C", &active_call_c, &cx_c),
+        ("D", &active_call_d, &cx_d),
+    ] {
+        active_call.read_with(*cx, |call, cx| {
+            let room = call.room().unwrap().read(cx);
+            assert_eq!(
+                room.followers_for(peer_id_c, project_id),
+                &[peer_id_d],
+                "checking followers for C as {name}"
+            );
+        });
+    }
+
+    // Client C closes the project.
+    cx_c.drop_last(workspace_c);
+
+    // Clients A and B see that client B is following A, and client C is not present in the followers.
+    cx_c.foreground().run_until_parked();
+    for (name, active_call, cx) in [("A", &active_call_a, &cx_a), ("B", &active_call_b, &cx_b)] {
+        active_call.read_with(*cx, |call, cx| {
+            let room = call.room().unwrap().read(cx);
+            assert_eq!(
+                room.followers_for(peer_id_a, project_id),
+                &[peer_id_b],
+                "checking followers for A as {name}"
+            );
+        });
+    }
+
+    // All clients see that no-one is following C
+    for (name, active_call, cx) in [
+        ("A", &active_call_a, &cx_a),
+        ("B", &active_call_b, &cx_b),
+        ("C", &active_call_c, &cx_c),
+        ("D", &active_call_d, &cx_d),
+    ] {
+        active_call.read_with(*cx, |call, cx| {
+            let room = call.room().unwrap().read(cx);
+            assert_eq!(
+                room.followers_for(peer_id_c, project_id),
+                &[],
+                "checking followers for C as {name}"
+            );
+        });
+    }
+
     let editor_b2 = workspace_b.read_with(cx_b, |workspace, cx| {
         workspace
             .active_item(cx)

crates/gpui/src/app/test_app_context.rs

@@ -18,9 +18,10 @@ use smol::stream::StreamExt;
 
 use crate::{
     executor, geometry::vector::Vector2F, keymap_matcher::Keystroke, platform, Action,
-    AnyViewHandle, AppContext, Appearance, Entity, Event, FontCache, InputHandler, KeyDownEvent,
-    ModelContext, ModelHandle, MutableAppContext, Platform, ReadModelWith, ReadViewWith,
-    RenderContext, Task, UpdateModel, UpdateView, View, ViewContext, ViewHandle, WeakHandle,
+    AnyViewHandle, AppContext, Appearance, Entity, Event, FontCache, Handle, InputHandler,
+    KeyDownEvent, ModelContext, ModelHandle, MutableAppContext, Platform, ReadModelWith,
+    ReadViewWith, RenderContext, Task, UpdateModel, UpdateView, View, ViewContext, ViewHandle,
+    WeakHandle,
 };
 use collections::BTreeMap;
 
@@ -329,6 +330,14 @@ impl TestAppContext {
             .assert_dropped(handle.id())
     }
 
+    /// Drop a handle, assuming it is the last. If it is not the last, panic with debug information about
+    /// where the stray handles were created.
+    pub fn drop_last<T, W: WeakHandle, H: Handle<T, Weak = W>>(&mut self, handle: H) {
+        let weak = handle.downgrade();
+        self.update(|_| drop(handle));
+        self.assert_dropped(weak);
+    }
+
     fn window_mut(&self, window_id: usize) -> std::cell::RefMut<platform::test::Window> {
         std::cell::RefMut::map(self.cx.borrow_mut(), |state| {
             let (_, window) = state

crates/workspace/src/dock/toggle_dock_button.rs

@@ -42,6 +42,7 @@ impl View for ToggleDockButton {
 
         let workspace = workspace.unwrap();
         let dock_position = workspace.read(cx).dock.position;
+        let dock_pane = workspace.read(cx.app).dock_pane().clone();
 
         let theme = cx.global::<Settings>().theme.clone();
 
@@ -67,7 +68,6 @@ impl View for ToggleDockButton {
         })
         .with_cursor_style(CursorStyle::PointingHand)
         .on_up(MouseButton::Left, move |event, cx| {
-            let dock_pane = workspace.read(cx.app).dock_pane();
             let drop_index = dock_pane.read(cx.app).items_len() + 1;
             handle_dropped_item(event, &dock_pane.downgrade(), drop_index, false, None, cx);
         });