Fix bug where Contacts included projects for which the use was a guest

Max Brunsfeld created

Change summary

crates/collab/src/rpc.rs       | 74 ++++++++++++++-------------------
crates/collab/src/rpc/store.rs | 79 ++++++++---------------------------
2 files changed, 49 insertions(+), 104 deletions(-)

Detailed changes

crates/collab/src/rpc.rs 🔗

@@ -4995,13 +4995,11 @@ mod tests {
         cx_c: &mut TestAppContext,
     ) {
         cx_a.foreground().forbid_parking();
-        let lang_registry = Arc::new(LanguageRegistry::test());
-        let fs = FakeFs::new(cx_a.background());
 
         // Connect to a server as 3 clients.
         let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await;
-        let client_a = server.create_client(cx_a, "user_a").await;
-        let client_b = server.create_client(cx_b, "user_b").await;
+        let mut client_a = server.create_client(cx_a, "user_a").await;
+        let mut client_b = server.create_client(cx_b, "user_b").await;
         let client_c = server.create_client(cx_c, "user_c").await;
         server
             .make_contacts(vec![
@@ -5025,27 +5023,10 @@ mod tests {
             });
         }
 
-        // Share a worktree as client A.
+        // Share a project as client A.
+        let fs = FakeFs::new(cx_a.background());
         fs.create_dir(Path::new("/a")).await.unwrap();
-
-        let project_a = cx_a.update(|cx| {
-            Project::local(
-                client_a.clone(),
-                client_a.user_store.clone(),
-                lang_registry.clone(),
-                fs.clone(),
-                cx,
-            )
-        });
-        let (worktree_a, _) = project_a
-            .update(cx_a, |p, cx| {
-                p.find_or_create_local_worktree("/a", true, cx)
-            })
-            .await
-            .unwrap();
-        worktree_a
-            .read_with(cx_a, |tree, _| tree.as_local().unwrap().scan_complete())
-            .await;
+        let (project_a, _) = client_a.build_local_project(fs, "/a", cx_a).await;
 
         deterministic.run_until_parked();
         for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] {
@@ -5083,16 +5064,7 @@ mod tests {
             });
         }
 
-        let _project_b = Project::remote(
-            project_id,
-            client_b.clone(),
-            client_b.user_store.clone(),
-            lang_registry.clone(),
-            fs.clone(),
-            &mut cx_b.to_async(),
-        )
-        .await
-        .unwrap();
+        let _project_b = client_b.build_remote_project(project_id, cx_b).await;
 
         deterministic.run_until_parked();
         for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] {
@@ -5108,12 +5080,32 @@ mod tests {
             });
         }
 
+        // Add a local project as client B
+        let fs = FakeFs::new(cx_b.background());
+        fs.create_dir(Path::new("/b")).await.unwrap();
+        let (_project_b, _) = client_b.build_local_project(fs, "/b", cx_a).await;
+
+        deterministic.run_until_parked();
+        for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] {
+            client.user_store.read_with(*cx, |store, _| {
+                assert_eq!(
+                    contacts(store),
+                    [
+                        ("user_a", true, vec![("a", true, vec!["user_b"])]),
+                        ("user_b", true, vec![("b", false, vec![])]),
+                        ("user_c", true, vec![])
+                    ]
+                )
+            });
+        }
+
         project_a
             .condition(&cx_a, |project, _| {
                 project.collaborators().contains_key(&client_b.peer_id)
             })
             .await;
 
+        client_a.project.take();
         cx_a.update(move |_| drop(project_a));
         deterministic.run_until_parked();
         for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] {
@@ -5122,7 +5114,7 @@ mod tests {
                     contacts(store),
                     [
                         ("user_a", true, vec![]),
-                        ("user_b", true, vec![]),
+                        ("user_b", true, vec![("b", false, vec![])]),
                         ("user_c", true, vec![])
                     ]
                 )
@@ -5138,7 +5130,7 @@ mod tests {
                     contacts(store),
                     [
                         ("user_a", true, vec![]),
-                        ("user_b", true, vec![]),
+                        ("user_b", true, vec![("b", false, vec![])]),
                         ("user_c", false, vec![])
                     ]
                 )
@@ -5161,7 +5153,7 @@ mod tests {
                     contacts(store),
                     [
                         ("user_a", true, vec![]),
-                        ("user_b", true, vec![]),
+                        ("user_b", true, vec![("b", false, vec![])]),
                         ("user_c", true, vec![])
                     ]
                 )
@@ -5173,7 +5165,7 @@ mod tests {
                 .contacts()
                 .iter()
                 .map(|contact| {
-                    let worktrees = contact
+                    let projects = contact
                         .projects
                         .iter()
                         .map(|p| {
@@ -5184,11 +5176,7 @@ mod tests {
                             )
                         })
                         .collect();
-                    (
-                        contact.user.github_login.as_str(),
-                        contact.online,
-                        worktrees,
-                    )
+                    (contact.user.github_login.as_str(), contact.online, projects)
                 })
                 .collect()
         }

crates/collab/src/rpc/store.rs 🔗

@@ -259,73 +259,30 @@ impl Store {
         let mut metadata = Vec::new();
         for project_id in project_ids {
             if let Some(project) = self.projects.get(&project_id) {
-                metadata.push(proto::ProjectMetadata {
-                    id: project_id,
-                    is_shared: project.share.is_some(),
-                    worktree_root_names: project
-                        .worktrees
-                        .values()
-                        .map(|worktree| worktree.root_name.clone())
-                        .collect(),
-                    guests: project
-                        .share
-                        .iter()
-                        .flat_map(|share| {
-                            share.guests.values().map(|(_, user_id)| user_id.to_proto())
-                        })
-                        .collect(),
-                });
+                if project.host_user_id == user_id {
+                    metadata.push(proto::ProjectMetadata {
+                        id: project_id,
+                        is_shared: project.share.is_some(),
+                        worktree_root_names: project
+                            .worktrees
+                            .values()
+                            .map(|worktree| worktree.root_name.clone())
+                            .collect(),
+                        guests: project
+                            .share
+                            .iter()
+                            .flat_map(|share| {
+                                share.guests.values().map(|(_, user_id)| user_id.to_proto())
+                            })
+                            .collect(),
+                    });
+                }
             }
         }
 
         metadata
     }
 
-    // pub fn contacts_for_user(&self, user_id: UserId) -> Vec<proto::Contact> {
-    //     let mut contacts = HashMap::default();
-    //     for project_id in self
-    //         .visible_projects_by_user_id
-    //         .get(&user_id)
-    //         .unwrap_or(&HashSet::default())
-    //     {
-    //         let project = &self.projects[project_id];
-
-    //         let mut guests = HashSet::default();
-    //         if let Ok(share) = project.share() {
-    //             for guest_connection_id in share.guests.keys() {
-    //                 if let Ok(user_id) = self.user_id_for_connection(*guest_connection_id) {
-    //                     guests.insert(user_id.to_proto());
-    //                 }
-    //             }
-    //         }
-
-    //         if let Ok(host_user_id) = self.user_id_for_connection(project.host_connection_id) {
-    //             let mut worktree_root_names = project
-    //                 .worktrees
-    //                 .values()
-    //                 .filter(|worktree| worktree.visible)
-    //                 .map(|worktree| worktree.root_name.clone())
-    //                 .collect::<Vec<_>>();
-    //             worktree_root_names.sort_unstable();
-    //             contacts
-    //                 .entry(host_user_id)
-    //                 .or_insert_with(|| proto::Contact {
-    //                     user_id: host_user_id.to_proto(),
-    //                     projects: Vec::new(),
-    //                 })
-    //                 .projects
-    //                 .push(proto::ProjectMetadata {
-    //                     id: *project_id,
-    //                     worktree_root_names,
-    //                     is_shared: project.share.is_some(),
-    //                     guests: guests.into_iter().collect(),
-    //                 });
-    //         }
-    //     }
-
-    //     contacts.into_values().collect()
-    // }
-
     pub fn register_project(
         &mut self,
         host_connection_id: ConnectionId,