From db978fcb6cf08ce3112547c22cf6c5bcd5b15d40 Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 25 Jan 2023 13:00:20 +0200 Subject: [PATCH 1/6] Add an x mark icon to the list of contacts We want to be able to remove contacts from our list. This was not possible. This change add an icon and dispatches the RemoveContact action. Co-Authored-By: Antonio Scandurra --- crates/collab_ui/src/contact_list.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/collab_ui/src/contact_list.rs b/crates/collab_ui/src/contact_list.rs index 743b98adb0..e2f5e50cdb 100644 --- a/crates/collab_ui/src/contact_list.rs +++ b/crates/collab_ui/src/contact_list.rs @@ -1051,7 +1051,7 @@ impl ContactList { let user_id = contact.user.id; let initial_project = project.clone(); let mut element = - MouseEventHandler::::new(contact.user.id as usize, cx, |_, _| { + MouseEventHandler::::new(contact.user.id as usize, cx, |_, cx| { Flex::row() .with_children(contact.user.avatar.clone().map(|avatar| { let status_badge = if contact.online { @@ -1093,6 +1093,27 @@ impl ContactList { .flex(1., true) .boxed(), ) + .with_child( + MouseEventHandler::::new( + contact.user.id as usize, + cx, + |mouse_state, _| { + let button_style = + theme.contact_button.style_for(mouse_state, false); + render_icon_button(button_style, "icons/x_mark_8.svg") + .aligned() + .flex_float() + .boxed() + }, + ) + .with_padding(Padding::uniform(2.)) + .with_cursor_style(CursorStyle::PointingHand) + .on_click(MouseButton::Left, move |_, cx| { + cx.dispatch_action(RemoveContact(user_id)) + }) + .flex_float() + .boxed(), + ) .with_children(if calling { Some( Label::new("Calling".to_string(), theme.calling_indicator.text.clone()) From 5d4eb2b7ae568132fd7f602e67f5208439d864b4 Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 25 Jan 2023 13:04:33 +0200 Subject: [PATCH 2/6] Push responder and requester to remove_contacts When we ask the server to remove a contact we need to push the requester and responder ids to `remove_contacts` so that when the UI updates, the correct contacts will disappear from the list. Co-Authored-By: Antonio Scandurra --- crates/collab/src/rpc.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 92d4935b23..54ffd9a958 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1966,6 +1966,7 @@ async fn remove_contact( let pool = session.connection_pool().await; // Update outgoing contact requests of requester let mut update = proto::UpdateContacts::default(); + update.remove_contacts.push(responder_id.to_proto()); update .remove_outgoing_requests .push(responder_id.to_proto()); @@ -1975,6 +1976,7 @@ async fn remove_contact( // Update incoming contact requests of responder let mut update = proto::UpdateContacts::default(); + update.remove_contacts.push(requester_id.to_proto()); update .remove_incoming_requests .push(requester_id.to_proto()); From e928c1c61e4ac17213a2abdc18fdc3ec01988b09 Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 25 Jan 2023 17:31:42 +0200 Subject: [PATCH 3/6] Test removing a contact Co-Authored-By: Antonio Scandurra --- crates/collab/src/tests/integration_tests.rs | 21 ++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 3f2a777f87..a645d6dc71 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -5291,6 +5291,27 @@ async fn test_contacts( [("user_b".to_string(), "online", "free")] ); + // Test removing a contact + client_b + .user_store + .update(cx_b, |store, cx| { + store.remove_contact(client_c.user_id().unwrap(), cx) + }) + .await + .unwrap(); + deterministic.run_until_parked(); + assert_eq!( + contacts(&client_b, cx_b), + [ + ("user_a".to_string(), "offline", "free"), + ("user_d".to_string(), "online", "free") + ] + ); + assert_eq!( + contacts(&client_c, cx_c), + [("user_a".to_string(), "offline", "free"),] + ); + fn contacts( client: &TestClient, cx: &TestAppContext, From 35524db1361ec7a071f6baf62c9d5fed3dfabe95 Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 25 Jan 2023 18:55:08 +0200 Subject: [PATCH 4/6] Add a confirmation prompt Co-Authored-By: Antonio Scandurra --- crates/collab_ui/src/contact_list.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/collab_ui/src/contact_list.rs b/crates/collab_ui/src/contact_list.rs index e2f5e50cdb..c4250c142b 100644 --- a/crates/collab_ui/src/contact_list.rs +++ b/crates/collab_ui/src/contact_list.rs @@ -1,22 +1,22 @@ -use std::{mem, sync::Arc}; - use crate::contacts_popover; use call::ActiveCall; use client::{proto::PeerId, Contact, User, UserStore}; use editor::{Cancel, Editor}; +use futures::StreamExt; use fuzzy::{match_strings, StringMatchCandidate}; use gpui::{ elements::*, geometry::{rect::RectF, vector::vec2f}, impl_actions, impl_internal_actions, keymap_matcher::KeymapContext, - AppContext, CursorStyle, Entity, ModelHandle, MouseButton, MutableAppContext, RenderContext, - Subscription, View, ViewContext, ViewHandle, + AppContext, CursorStyle, Entity, ModelHandle, MouseButton, MutableAppContext, PromptLevel, + RenderContext, Subscription, View, ViewContext, ViewHandle, }; use menu::{Confirm, SelectNext, SelectPrev}; use project::Project; use serde::Deserialize; use settings::Settings; +use std::{mem, sync::Arc}; use theme::IconButton; use util::ResultExt; use workspace::{JoinProject, OpenSharedScreen}; @@ -299,9 +299,19 @@ impl ContactList { } fn remove_contact(&mut self, request: &RemoveContact, cx: &mut ViewContext) { - self.user_store - .update(cx, |store, cx| store.remove_contact(request.0, cx)) - .detach(); + let user_id = request.0; + let user_store = self.user_store.clone(); + let prompt_message = "Are you sure you want to remove this contact?"; + let mut answer = cx.prompt(PromptLevel::Warning, prompt_message, &["Remove", "Cancel"]); + cx.spawn(|_, mut cx| async move { + if answer.next().await == Some(0) { + user_store + .update(&mut cx, |store, cx| store.remove_contact(user_id, cx)) + .await + .unwrap(); + } + }) + .detach(); } fn respond_to_contact_request( From 160870c9de053d5ccca7ba12bdd467ef8f2eafca Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 25 Jan 2023 19:46:51 +0200 Subject: [PATCH 5/6] Improve user notification The message is not really true. When one declines, the other person can notice that the contact request is not pending any more. They will know. Switching to not alerted is closer to what is really happening. --- crates/collab_ui/src/contact_notification.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/collab_ui/src/contact_notification.rs b/crates/collab_ui/src/contact_notification.rs index 6f0cfc68c7..f05cca00bf 100644 --- a/crates/collab_ui/src/contact_notification.rs +++ b/crates/collab_ui/src/contact_notification.rs @@ -48,7 +48,7 @@ impl View for ContactNotification { ContactEventKind::Requested => render_user_notification( self.user.clone(), "wants to add you as a contact", - Some("They won't know if you decline."), + Some("They won't be alerted if you decline."), Dismiss(self.user.id), vec![ ( From 73af155dd64ce5460ea4fb3cf84be53290d139ab Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Thu, 26 Jan 2023 19:01:51 +0200 Subject: [PATCH 6/6] Refactor Database::remove_contact Refactor it to avoid sending irrelevant messages to update the UI. Co-Authored-By: Antonio Scandurra --- crates/collab/src/db.rs | 25 ++++++++++++++++--------- crates/collab/src/rpc.rs | 24 +++++++++++++++--------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 63ea7fdd9e..0edd79e0e4 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -595,7 +595,16 @@ impl Database { .await } - pub async fn remove_contact(&self, requester_id: UserId, responder_id: UserId) -> Result<()> { + /// Returns a bool indicating whether the removed contact had originally accepted or not + /// + /// Deletes the contact identified by the requester and responder ids, and then returns + /// whether the deleted contact had originally accepted or was a pending contact request. + /// + /// # Arguments + /// + /// * `requester_id` - The user that initiates this request + /// * `responder_id` - The user that will be removed + pub async fn remove_contact(&self, requester_id: UserId, responder_id: UserId) -> Result { self.transaction(|tx| async move { let (id_a, id_b) = if responder_id < requester_id { (responder_id, requester_id) @@ -603,20 +612,18 @@ impl Database { (requester_id, responder_id) }; - let result = contact::Entity::delete_many() + let contact = contact::Entity::find() .filter( contact::Column::UserIdA .eq(id_a) .and(contact::Column::UserIdB.eq(id_b)), ) - .exec(&*tx) - .await?; + .one(&*tx) + .await? + .ok_or_else(|| anyhow!("no such contact"))?; - if result.rows_affected == 1 { - Ok(()) - } else { - Err(anyhow!("no such contact"))? - } + contact::Entity::delete_by_id(contact.id).exec(&*tx).await?; + Ok(contact.accepted) }) .await } diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 54ffd9a958..32cce1e681 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1961,25 +1961,31 @@ async fn remove_contact( let requester_id = session.user_id; let responder_id = UserId::from_proto(request.user_id); let db = session.db().await; - db.remove_contact(requester_id, responder_id).await?; + let contact_accepted = db.remove_contact(requester_id, responder_id).await?; let pool = session.connection_pool().await; // Update outgoing contact requests of requester let mut update = proto::UpdateContacts::default(); - update.remove_contacts.push(responder_id.to_proto()); - update - .remove_outgoing_requests - .push(responder_id.to_proto()); + if contact_accepted { + update.remove_contacts.push(responder_id.to_proto()); + } else { + update + .remove_outgoing_requests + .push(responder_id.to_proto()); + } for connection_id in pool.user_connection_ids(requester_id) { session.peer.send(connection_id, update.clone())?; } // Update incoming contact requests of responder let mut update = proto::UpdateContacts::default(); - update.remove_contacts.push(requester_id.to_proto()); - update - .remove_incoming_requests - .push(requester_id.to_proto()); + if contact_accepted { + update.remove_contacts.push(requester_id.to_proto()); + } else { + update + .remove_incoming_requests + .push(requester_id.to_proto()); + } for connection_id in pool.user_connection_ids(responder_id) { session.peer.send(connection_id, update.clone())?; }