From d173b1d4120cd5313104150e19ca0a9d31f2df35 Mon Sep 17 00:00:00 2001 From: Julia Date: Tue, 7 Mar 2023 18:56:03 -0500 Subject: [PATCH] Update db followers table when user leaves a project Co-Authored-By: Nathan Sobo --- crates/collab/src/db.rs | 43 ++++-- crates/collab/src/rpc.rs | 4 +- crates/collab/src/tests/integration_tests.rs | 123 +++++++++++++++--- crates/gpui/src/app/test_app_context.rs | 15 ++- .../workspace/src/dock/toggle_dock_button.rs | 2 +- 5 files changed, 156 insertions(+), 31 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index c4ff2e3918..1d0fc377ab 100644 --- a/crates/collab/src/db.rs +++ b/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> { + ) -> Result> { 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 } diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 710ddcb890..d13487440f 100644 --- a/crates/collab/src/rpc.rs +++ b/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(()) } diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 28266824e3..4f027af758 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/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, 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) diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 1366e7e6ed..ad78346991 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/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>(&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 { std::cell::RefMut::map(self.cx.borrow_mut(), |state| { let (_, window) = state diff --git a/crates/workspace/src/dock/toggle_dock_button.rs b/crates/workspace/src/dock/toggle_dock_button.rs index cafbea7db3..0b96c3e67b 100644 --- a/crates/workspace/src/dock/toggle_dock_button.rs +++ b/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::().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); });