From a7e883d956852ef761edcb64096490c95ba0f4a3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 3 Aug 2023 14:49:01 -0700 Subject: [PATCH] Implement basic channel member management UI Co-authored-by: Mikayla --- crates/client/src/channel_store.rs | 80 +++++--- crates/collab/src/db.rs | 80 ++++++-- crates/collab/src/db/tests.rs | 45 ++++- crates/collab/src/rpc.rs | 13 ++ crates/collab/src/tests.rs | 9 +- crates/collab/src/tests/channel_tests.rs | 45 ++++- crates/collab_ui/src/collab_panel.rs | 10 +- .../src/collab_panel/channel_modal.rs | 172 +++++++++++++----- crates/rpc/proto/zed.proto | 14 +- 9 files changed, 368 insertions(+), 100 deletions(-) diff --git a/crates/client/src/channel_store.rs b/crates/client/src/channel_store.rs index a1ee7ad6bc..8568317355 100644 --- a/crates/client/src/channel_store.rs +++ b/crates/client/src/channel_store.rs @@ -1,6 +1,8 @@ use crate::{Client, Subscription, User, UserStore}; +use anyhow::anyhow; use anyhow::Result; use collections::HashMap; +use collections::HashSet; use futures::Future; use gpui::{AsyncAppContext, Entity, ModelContext, ModelHandle, Task}; use rpc::{proto, TypedEnvelope}; @@ -13,6 +15,7 @@ pub struct ChannelStore { channels: Vec>, channel_invitations: Vec>, channel_participants: HashMap>>, + outgoing_invites: HashSet<(ChannelId, UserId)>, client: Arc, user_store: ModelHandle, _rpc_subscription: Subscription, @@ -33,6 +36,7 @@ impl Entity for ChannelStore { pub enum ChannelMemberStatus { Invited, Member, + NotMember, } impl ChannelStore { @@ -48,6 +52,7 @@ impl ChannelStore { channels: vec![], channel_invitations: vec![], channel_participants: Default::default(), + outgoing_invites: Default::default(), client, user_store, _rpc_subscription: rpc_subscription, @@ -88,13 +93,19 @@ impl ChannelStore { } pub fn invite_member( - &self, + &mut self, channel_id: ChannelId, user_id: UserId, admin: bool, - ) -> impl Future> { + cx: &mut ModelContext, + ) -> Task> { + if !self.outgoing_invites.insert((channel_id, user_id)) { + return Task::ready(Err(anyhow!("invite request already in progress"))); + } + + cx.notify(); let client = self.client.clone(); - async move { + cx.spawn(|this, mut cx| async move { client .request(proto::InviteChannelMember { channel_id, @@ -102,8 +113,12 @@ impl ChannelStore { admin, }) .await?; + this.update(&mut cx, |this, cx| { + this.outgoing_invites.remove(&(channel_id, user_id)); + cx.notify(); + }); Ok(()) - } + }) } pub fn respond_to_channel_invite( @@ -120,24 +135,34 @@ impl ChannelStore { } } - pub fn get_channel_members( + pub fn get_channel_member_details( &self, channel_id: ChannelId, - ) -> impl 'static + Future>> { + cx: &mut ModelContext, + ) -> Task, proto::channel_member::Kind)>>> { let client = self.client.clone(); - async move { + let user_store = self.user_store.downgrade(); + cx.spawn(|_, mut cx| async move { let response = client .request(proto::GetChannelMembers { channel_id }) .await?; - let mut result = HashMap::default(); - for member_id in response.members { - result.insert(member_id, ChannelMemberStatus::Member); - } - for invitee_id in response.invited_members { - result.insert(invitee_id, ChannelMemberStatus::Invited); - } - Ok(result) - } + + let user_ids = response.members.iter().map(|m| m.user_id).collect(); + let user_store = user_store + .upgrade(&cx) + .ok_or_else(|| anyhow!("user store dropped"))?; + let users = user_store + .update(&mut cx, |user_store, cx| user_store.get_users(user_ids, cx)) + .await?; + + Ok(users + .into_iter() + .zip(response.members) + .filter_map(|(user, member)| { + Some((user, proto::channel_member::Kind::from_i32(member.kind)?)) + }) + .collect()) + }) } pub fn remove_channel(&self, channel_id: ChannelId) -> impl Future> { @@ -148,25 +173,22 @@ impl ChannelStore { } } - pub fn is_channel_invite_pending(&self, _: &Arc) -> bool { + pub fn has_pending_channel_invite_response(&self, _: &Arc) -> bool { false } + pub fn has_pending_channel_invite(&self, channel_id: ChannelId, user_id: UserId) -> bool { + self.outgoing_invites.contains(&(channel_id, user_id)) + } + pub fn remove_member( &self, - channel_id: ChannelId, - user_id: u64, - cx: &mut ModelContext, + _channel_id: ChannelId, + _user_id: u64, + _cx: &mut ModelContext, ) -> Task> { - todo!() - } - - pub fn channel_members( - &self, - channel_id: ChannelId, - cx: &mut ModelContext, - ) -> Task>>> { - todo!() + dbg!("TODO"); + Task::Ready(Some(Ok(()))) } async fn handle_update_channels( diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 36b226b97b..d942b8cab9 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -213,20 +213,21 @@ impl Database { ); let (channel_id, room) = self.get_channel_room(room_id, &tx).await?; - let channel_members = if let Some(channel_id) = channel_id { - self.get_channel_members_internal(channel_id, &tx).await? + let channel_members; + if let Some(channel_id) = channel_id { + channel_members = self.get_channel_members_internal(channel_id, &tx).await?; } else { - Vec::new() - }; + channel_members = Vec::new(); - // Delete the room if it becomes empty. - if room.participants.is_empty() { - project::Entity::delete_many() - .filter(project::Column::RoomId.eq(room_id)) - .exec(&*tx) - .await?; - room::Entity::delete_by_id(room_id).exec(&*tx).await?; - } + // Delete the room if it becomes empty. + if room.participants.is_empty() { + project::Entity::delete_many() + .filter(project::Column::RoomId.eq(room_id)) + .exec(&*tx) + .await?; + room::Entity::delete_by_id(room_id).exec(&*tx).await?; + } + }; Ok(RefreshedRoom { room, @@ -3475,10 +3476,61 @@ impl Database { } pub async fn get_channel_members(&self, id: ChannelId) -> Result> { + self.transaction(|tx| async move { self.get_channel_members_internal(id, &*tx).await }) + .await + } + + // TODO: Add a chekc whether this user is allowed to read this channel + pub async fn get_channel_member_details( + &self, + id: ChannelId, + ) -> Result> { self.transaction(|tx| async move { + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] + enum QueryMemberDetails { + UserId, + IsDirectMember, + Accepted, + } + let tx = tx; - let user_ids = self.get_channel_members_internal(id, &*tx).await?; - Ok(user_ids) + let ancestor_ids = self.get_channel_ancestors(id, &*tx).await?; + let mut stream = channel_member::Entity::find() + .distinct() + .filter(channel_member::Column::ChannelId.is_in(ancestor_ids.iter().copied())) + .select_only() + .column(channel_member::Column::UserId) + .column_as( + channel_member::Column::ChannelId.eq(id), + QueryMemberDetails::IsDirectMember, + ) + .column(channel_member::Column::Accepted) + .order_by_asc(channel_member::Column::UserId) + .into_values::<_, QueryMemberDetails>() + .stream(&*tx) + .await?; + + let mut rows = Vec::::new(); + while let Some(row) = stream.next().await { + let (user_id, is_direct_member, is_invite_accepted): (UserId, bool, bool) = row?; + let kind = match (is_direct_member, is_invite_accepted) { + (true, true) => proto::channel_member::Kind::Member, + (true, false) => proto::channel_member::Kind::Invitee, + (false, true) => proto::channel_member::Kind::AncestorMember, + (false, false) => continue, + }; + let user_id = user_id.to_proto(); + let kind = kind.into(); + if let Some(last_row) = rows.last_mut() { + if last_row.user_id == user_id { + last_row.kind = last_row.kind.min(kind); + continue; + } + } + rows.push(proto::ChannelMember { user_id, kind }); + } + + Ok(rows) }) .await } diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index a1d1a23dc9..e4161d3b55 100644 --- a/crates/collab/src/db/tests.rs +++ b/crates/collab/src/db/tests.rs @@ -1161,7 +1161,50 @@ test_both_dbs!( .map(|channel| channel.id) .collect::>(); - assert_eq!(user_3_invites, &[channel_1_1]) + assert_eq!(user_3_invites, &[channel_1_1]); + + let members = db.get_channel_member_details(channel_1_1).await.unwrap(); + assert_eq!( + members, + &[ + proto::ChannelMember { + user_id: user_1.to_proto(), + kind: proto::channel_member::Kind::Member.into(), + }, + proto::ChannelMember { + user_id: user_2.to_proto(), + kind: proto::channel_member::Kind::Invitee.into(), + }, + proto::ChannelMember { + user_id: user_3.to_proto(), + kind: proto::channel_member::Kind::Invitee.into(), + }, + ] + ); + + db.respond_to_channel_invite(channel_1_1, user_2, true) + .await + .unwrap(); + + let channel_1_3 = db + .create_channel("channel_3", Some(channel_1_1), "1", user_1) + .await + .unwrap(); + + let members = db.get_channel_member_details(channel_1_3).await.unwrap(); + assert_eq!( + members, + &[ + proto::ChannelMember { + user_id: user_1.to_proto(), + kind: proto::channel_member::Kind::Member.into(), + }, + proto::ChannelMember { + user_id: user_2.to_proto(), + kind: proto::channel_member::Kind::AncestorMember.into(), + }, + ] + ); } ); diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 7ee2a2ba83..fdfccea98f 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -246,6 +246,7 @@ impl Server { .add_request_handler(remove_channel) .add_request_handler(invite_channel_member) .add_request_handler(remove_channel_member) + .add_request_handler(get_channel_members) .add_request_handler(respond_to_channel_invite) .add_request_handler(join_channel) .add_request_handler(follow) @@ -2236,6 +2237,18 @@ async fn remove_channel_member( Ok(()) } +async fn get_channel_members( + request: proto::GetChannelMembers, + response: Response, + session: Session, +) -> Result<()> { + let db = session.db().await; + let channel_id = ChannelId::from_proto(request.channel_id); + let members = db.get_channel_member_details(channel_id).await?; + response.send(proto::GetChannelMembersResponse { members })?; + Ok(()) +} + async fn respond_to_channel_invite( request: proto::RespondToChannelInvite, response: Response, diff --git a/crates/collab/src/tests.rs b/crates/collab/src/tests.rs index 26ca5a008e..a8e2a12962 100644 --- a/crates/collab/src/tests.rs +++ b/crates/collab/src/tests.rs @@ -291,8 +291,13 @@ impl TestServer { admin_client .app_state .channel_store - .update(admin_cx, |channel_store, _| { - channel_store.invite_member(channel_id, member_client.user_id().unwrap(), false) + .update(admin_cx, |channel_store, cx| { + channel_store.invite_member( + channel_id, + member_client.user_id().unwrap(), + false, + cx, + ) }) .await .unwrap(); diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index 3999740557..b4f8477a2d 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -1,6 +1,7 @@ use call::ActiveCall; use client::{Channel, User}; use gpui::{executor::Deterministic, TestAppContext}; +use rpc::proto; use std::sync::Arc; use crate::tests::{room_participants, RoomParticipants}; @@ -46,8 +47,14 @@ async fn test_basic_channels( // Invite client B to channel A as client A. client_a .channel_store() - .update(cx_a, |channel_store, _| { - channel_store.invite_member(channel_a_id, client_b.user_id().unwrap(), false) + .update(cx_a, |store, cx| { + assert!(!store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap())); + + let invite = store.invite_member(channel_a_id, client_b.user_id().unwrap(), false, cx); + + // Make sure we're synchronously storing the pending invite + assert!(store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap())); + invite }) .await .unwrap(); @@ -66,6 +73,27 @@ async fn test_basic_channels( })] ) }); + let members = client_a + .channel_store() + .update(cx_a, |store, cx| { + assert!(!store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap())); + store.get_channel_member_details(channel_a_id, cx) + }) + .await + .unwrap(); + assert_members_eq( + &members, + &[ + ( + client_a.user_id().unwrap(), + proto::channel_member::Kind::Member, + ), + ( + client_b.user_id().unwrap(), + proto::channel_member::Kind::Invitee, + ), + ], + ); // Client B now sees that they are a member channel A. client_b @@ -113,6 +141,19 @@ fn assert_participants_eq(participants: &[Arc], expected_partitipants: &[u ); } +fn assert_members_eq( + members: &[(Arc, proto::channel_member::Kind)], + expected_members: &[(u64, proto::channel_member::Kind)], +) { + assert_eq!( + members + .iter() + .map(|(user, status)| (user.id, *status)) + .collect::>(), + expected_members + ); +} + #[gpui::test] async fn test_channel_room( deterministic: Arc, diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 34cb4f3e91..771927c8ac 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -1333,7 +1333,9 @@ impl CollabPanel { enum Accept {} let channel_id = channel.id; - let is_invite_pending = channel_store.read(cx).is_channel_invite_pending(&channel); + let is_invite_pending = channel_store + .read(cx) + .has_pending_channel_invite_response(&channel); let button_spacing = theme.contact_button_spacing; Flex::row() @@ -1682,7 +1684,10 @@ impl CollabPanel { let workspace = self.workspace.clone(); let user_store = self.user_store.clone(); let channel_store = self.channel_store.clone(); - let members = self.channel_store.read(cx).get_channel_members(channel_id); + let members = self.channel_store.update(cx, |channel_store, cx| { + channel_store.get_channel_member_details(channel_id, cx) + }); + cx.spawn(|_, mut cx| async move { let members = members.await?; workspace.update(&mut cx, |workspace, cx| { @@ -1692,6 +1697,7 @@ impl CollabPanel { user_store.clone(), channel_store.clone(), channel_id, + channel_modal::Mode::InviteMembers, members, cx, ) diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index 164759587d..e6a3ba9288 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -1,7 +1,5 @@ -use client::{ - ChannelId, ChannelMemberStatus, ChannelStore, ContactRequestStatus, User, UserId, UserStore, -}; -use collections::HashMap; +use client::{proto, ChannelId, ChannelStore, User, UserId, UserStore}; +use fuzzy::{match_strings, StringMatchCandidate}; use gpui::{elements::*, AppContext, ModelHandle, MouseState, Task, ViewContext}; use picker::{Picker, PickerDelegate, PickerEvent}; use std::sync::Arc; @@ -17,30 +15,48 @@ pub fn build_channel_modal( user_store: ModelHandle, channel_store: ModelHandle, channel: ChannelId, - members: HashMap, + mode: Mode, + members: Vec<(Arc, proto::channel_member::Kind)>, cx: &mut ViewContext, ) -> ChannelModal { Picker::new( ChannelModalDelegate { - potential_contacts: Arc::from([]), + matches: Vec::new(), selected_index: 0, user_store, channel_store, channel_id: channel, - member_statuses: members, + match_candidates: members + .iter() + .enumerate() + .map(|(id, member)| StringMatchCandidate { + id, + string: member.0.github_login.clone(), + char_bag: member.0.github_login.chars().collect(), + }) + .collect(), + members, + mode, }, cx, ) .with_theme(|theme| theme.picker.clone()) } +pub enum Mode { + ManageMembers, + InviteMembers, +} + pub struct ChannelModalDelegate { - potential_contacts: Arc<[Arc]>, + matches: Vec<(Arc, Option)>, user_store: ModelHandle, channel_store: ModelHandle, channel_id: ChannelId, selected_index: usize, - member_statuses: HashMap, + mode: Mode, + match_candidates: Arc<[StringMatchCandidate]>, + members: Vec<(Arc, proto::channel_member::Kind)>, } impl PickerDelegate for ChannelModalDelegate { @@ -49,7 +65,7 @@ impl PickerDelegate for ChannelModalDelegate { } fn match_count(&self) -> usize { - self.potential_contacts.len() + self.matches.len() } fn selected_index(&self) -> usize { @@ -61,39 +77,80 @@ impl PickerDelegate for ChannelModalDelegate { } fn update_matches(&mut self, query: String, cx: &mut ViewContext>) -> Task<()> { - let search_users = self - .user_store - .update(cx, |store, cx| store.fuzzy_search_users(query, cx)); - - cx.spawn(|picker, mut cx| async move { - async { - let potential_contacts = search_users.await?; - picker.update(&mut cx, |picker, cx| { - picker.delegate_mut().potential_contacts = potential_contacts.into(); - cx.notify(); - })?; - anyhow::Ok(()) + match self.mode { + Mode::ManageMembers => { + let match_candidates = self.match_candidates.clone(); + cx.spawn(|picker, mut cx| async move { + async move { + let matches = match_strings( + &match_candidates, + &query, + true, + usize::MAX, + &Default::default(), + cx.background().clone(), + ) + .await; + picker.update(&mut cx, |picker, cx| { + let delegate = picker.delegate_mut(); + delegate.matches.clear(); + delegate.matches.extend(matches.into_iter().map(|m| { + let member = &delegate.members[m.candidate_id]; + (member.0.clone(), Some(member.1)) + })); + cx.notify(); + })?; + anyhow::Ok(()) + } + .log_err() + .await; + }) } - .log_err() - .await; - }) + Mode::InviteMembers => { + let search_users = self + .user_store + .update(cx, |store, cx| store.fuzzy_search_users(query, cx)); + cx.spawn(|picker, mut cx| async move { + async { + let users = search_users.await?; + picker.update(&mut cx, |picker, cx| { + let delegate = picker.delegate_mut(); + delegate.matches.clear(); + delegate + .matches + .extend(users.into_iter().map(|user| (user, None))); + cx.notify(); + })?; + anyhow::Ok(()) + } + .log_err() + .await; + }) + } + } } fn confirm(&mut self, _: bool, cx: &mut ViewContext>) { - if let Some(user) = self.potential_contacts.get(self.selected_index) { - let user_store = self.user_store.read(cx); - match user_store.contact_request_status(user) { - ContactRequestStatus::None | ContactRequestStatus::RequestReceived => { - self.user_store - .update(cx, |store, cx| store.request_contact(user.id, cx)) - .detach(); + if let Some((user, _)) = self.matches.get(self.selected_index) { + match self.mode { + Mode::ManageMembers => { + // } - ContactRequestStatus::RequestSent => { - self.user_store - .update(cx, |store, cx| store.remove_contact(user.id, cx)) - .detach(); - } - _ => {} + Mode::InviteMembers => match self.member_status(user.id, cx) { + Some(proto::channel_member::Kind::Member) => {} + Some(proto::channel_member::Kind::Invitee) => self + .channel_store + .update(cx, |store, cx| { + store.remove_member(self.channel_id, user.id, cx) + }) + .detach(), + Some(proto::channel_member::Kind::AncestorMember) | None => self + .channel_store + .update(cx, |store, cx| { + store.invite_member(self.channel_id, user.id, false, cx) + }) + .detach(), + }, } } } @@ -108,12 +165,16 @@ impl PickerDelegate for ChannelModalDelegate { ) -> Option>> { let theme = &theme::current(cx).collab_panel.channel_modal; + let operation = match self.mode { + Mode::ManageMembers => "Manage", + Mode::InviteMembers => "Add", + }; self.channel_store .read(cx) .channel_for_id(self.channel_id) .map(|channel| { Label::new( - format!("Add members for #{}", channel.name), + format!("{} members for #{}", operation, channel.name), theme.picker.item.default_style().label.clone(), ) .into_any() @@ -128,19 +189,17 @@ impl PickerDelegate for ChannelModalDelegate { cx: &gpui::AppContext, ) -> AnyElement> { let theme = &theme::current(cx).collab_panel.channel_modal; - let user = &self.potential_contacts[ix]; - let request_status = self.member_statuses.get(&user.id); + let (user, _) = &self.matches[ix]; + let request_status = self.member_status(user.id, cx); let icon_path = match request_status { - Some(ChannelMemberStatus::Member) => Some("icons/check_8.svg"), - Some(ChannelMemberStatus::Invited) => Some("icons/x_mark_8.svg"), + Some(proto::channel_member::Kind::AncestorMember) => Some("icons/check_8.svg"), + Some(proto::channel_member::Kind::Member) => Some("icons/check_8.svg"), + Some(proto::channel_member::Kind::Invitee) => Some("icons/x_mark_8.svg"), None => None, }; - let button_style = if self.user_store.read(cx).is_contact_request_pending(user) { - &theme.disabled_contact_button - } else { - &theme.contact_button - }; + let button_style = &theme.contact_button; + let style = theme.picker.item.in_state(selected).style_for(mouse_state); Flex::row() .with_children(user.avatar.clone().map(|avatar| { @@ -177,3 +236,20 @@ impl PickerDelegate for ChannelModalDelegate { .into_any() } } + +impl ChannelModalDelegate { + fn member_status( + &self, + user_id: UserId, + cx: &AppContext, + ) -> Option { + self.members + .iter() + .find_map(|(user, status)| (user.id == user_id).then_some(*status)) + .or(self + .channel_store + .read(cx) + .has_pending_channel_invite(self.channel_id, user_id) + .then_some(proto::channel_member::Kind::Invitee)) + } +} diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 1fdeef98f0..602b34529e 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -893,8 +893,18 @@ message GetChannelMembers { } message GetChannelMembersResponse { - repeated uint64 members = 1; - repeated uint64 invited_members = 2; + repeated ChannelMember members = 1; +} + +message ChannelMember { + uint64 user_id = 1; + Kind kind = 2; + + enum Kind { + Member = 0; + Invitee = 1; + AncestorMember = 2; + } } message CreateChannel {