diff --git a/crates/client/src/channel_store.rs b/crates/client/src/channel_store.rs index 8568317355a67bbf767facbb37eccbdfc9796abd..1d3bbd4435e70f159a980cac9525593057cf1020 100644 --- a/crates/client/src/channel_store.rs +++ b/crates/client/src/channel_store.rs @@ -121,6 +121,33 @@ impl ChannelStore { }) } + pub fn remove_member( + &mut self, + channel_id: ChannelId, + user_id: u64, + 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(); + cx.spawn(|this, mut cx| async move { + client + .request(proto::RemoveChannelMember { + channel_id, + user_id, + }) + .await?; + this.update(&mut cx, |this, cx| { + this.outgoing_invites.remove(&(channel_id, user_id)); + cx.notify(); + }); + Ok(()) + }) + } + pub fn respond_to_channel_invite( &mut self, channel_id: ChannelId, @@ -181,16 +208,6 @@ impl ChannelStore { self.outgoing_invites.contains(&(channel_id, user_id)) } - pub fn remove_member( - &self, - _channel_id: ChannelId, - _user_id: u64, - _cx: &mut ModelContext, - ) -> Task> { - dbg!("TODO"); - Task::Ready(Some(Ok(()))) - } - async fn handle_update_channels( this: ModelHandle, message: TypedEnvelope, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index d942b8cab9a3e7427f649792d7e0bca76683af59..5a2ab24b1e22e2661e8c02af5f9a63145dc6fc71 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -3165,30 +3165,17 @@ impl Database { creator_id: UserId, ) -> Result { self.transaction(move |tx| async move { - let tx = tx; - if let Some(parent) = parent { - let channels = self.get_channel_ancestors(parent, &*tx).await?; - channel_member::Entity::find() - .filter(channel_member::Column::ChannelId.is_in(channels.iter().copied())) - .filter( - channel_member::Column::UserId - .eq(creator_id) - .and(channel_member::Column::Accepted.eq(true)), - ) - .one(&*tx) - .await? - .ok_or_else(|| { - anyhow!("User does not have the permissions to create this channel") - })?; + self.check_user_is_channel_admin(parent, creator_id, &*tx) + .await?; } let channel = channel::ActiveModel { name: ActiveValue::Set(name.to_string()), ..Default::default() - }; - - let channel = channel.insert(&*tx).await?; + } + .insert(&*tx) + .await?; if let Some(parent) = parent { channel_parent::ActiveModel { @@ -3228,45 +3215,36 @@ impl Database { user_id: UserId, ) -> Result<(Vec, Vec)> { self.transaction(move |tx| async move { - let tx = tx; - - // Check if user is an admin - channel_member::Entity::find() - .filter( - channel_member::Column::ChannelId - .eq(channel_id) - .and(channel_member::Column::UserId.eq(user_id)) - .and(channel_member::Column::Admin.eq(true)), - ) - .one(&*tx) - .await? - .ok_or_else(|| anyhow!("user is not allowed to remove this channel"))?; - - let mut descendants = self.get_channel_descendants([channel_id], &*tx).await?; - - // Keep channels which have another active - let mut channels_to_keep = channel_parent::Entity::find() - .filter( - channel_parent::Column::ChildId - .is_in(descendants.keys().copied().filter(|&id| id != channel_id)) - .and( - channel_parent::Column::ParentId.is_not_in(descendants.keys().copied()), - ), - ) - .stream(&*tx) + self.check_user_is_channel_admin(channel_id, user_id, &*tx) .await?; - while let Some(row) = channels_to_keep.next().await { - let row = row?; - descendants.remove(&row.child_id); + // Don't remove descendant channels that have additional parents. + let mut channels_to_remove = self.get_channel_descendants([channel_id], &*tx).await?; + { + let mut channels_to_keep = channel_parent::Entity::find() + .filter( + channel_parent::Column::ChildId + .is_in( + channels_to_remove + .keys() + .copied() + .filter(|&id| id != channel_id), + ) + .and( + channel_parent::Column::ParentId + .is_not_in(channels_to_remove.keys().copied()), + ), + ) + .stream(&*tx) + .await?; + while let Some(row) = channels_to_keep.next().await { + let row = row?; + channels_to_remove.remove(&row.child_id); + } } - drop(channels_to_keep); - - let channels_to_remove = descendants.keys().copied().collect::>(); - let members_to_notify: Vec = channel_member::Entity::find() - .filter(channel_member::Column::ChannelId.is_in(channels_to_remove.iter().copied())) + .filter(channel_member::Column::ChannelId.is_in(channels_to_remove.keys().copied())) .select_only() .column(channel_member::Column::UserId) .distinct() @@ -3274,13 +3252,12 @@ impl Database { .all(&*tx) .await?; - // Channel members and parents should delete via cascade channel::Entity::delete_many() - .filter(channel::Column::Id.is_in(channels_to_remove.iter().copied())) + .filter(channel::Column::Id.is_in(channels_to_remove.keys().copied())) .exec(&*tx) .await?; - Ok((channels_to_remove, members_to_notify)) + Ok((channels_to_remove.into_keys().collect(), members_to_notify)) }) .await } @@ -3293,31 +3270,18 @@ impl Database { is_admin: bool, ) -> Result<()> { self.transaction(move |tx| async move { - let tx = tx; - - // Check if inviter is a member - channel_member::Entity::find() - .filter( - channel_member::Column::ChannelId - .eq(channel_id) - .and(channel_member::Column::UserId.eq(inviter_id)) - .and(channel_member::Column::Admin.eq(true)), - ) - .one(&*tx) - .await? - .ok_or_else(|| { - anyhow!("Inviter does not have permissions to invite the invitee") - })?; + self.check_user_is_channel_admin(channel_id, inviter_id, &*tx) + .await?; - let channel_membership = channel_member::ActiveModel { + channel_member::ActiveModel { channel_id: ActiveValue::Set(channel_id), user_id: ActiveValue::Set(invitee_id), accepted: ActiveValue::Set(false), admin: ActiveValue::Set(is_admin), ..Default::default() - }; - - channel_membership.insert(&*tx).await?; + } + .insert(&*tx) + .await?; Ok(()) }) @@ -3331,8 +3295,6 @@ impl Database { accept: bool, ) -> Result<()> { self.transaction(move |tx| async move { - let tx = tx; - let rows_affected = if accept { channel_member::Entity::update_many() .set(channel_member::ActiveModel { @@ -3368,10 +3330,36 @@ impl Database { .await } - pub async fn get_channel_invites(&self, user_id: UserId) -> Result> { + pub async fn remove_channel_member( + &self, + channel_id: ChannelId, + member_id: UserId, + remover_id: UserId, + ) -> Result<()> { self.transaction(|tx| async move { - let tx = tx; + self.check_user_is_channel_admin(channel_id, remover_id, &*tx) + .await?; + + let result = channel_member::Entity::delete_many() + .filter( + channel_member::Column::ChannelId + .eq(channel_id) + .and(channel_member::Column::UserId.eq(member_id)), + ) + .exec(&*tx) + .await?; + + if result.rows_affected == 0 { + Err(anyhow!("no such member"))?; + } + + Ok(()) + }) + .await + } + pub async fn get_channel_invites_for_user(&self, user_id: UserId) -> Result> { + self.transaction(|tx| async move { let channel_invites = channel_member::Entity::find() .filter( channel_member::Column::UserId @@ -3406,7 +3394,7 @@ impl Database { .await } - pub async fn get_channels( + pub async fn get_channels_for_user( &self, user_id: UserId, ) -> Result<(Vec, HashMap>)> { @@ -3430,47 +3418,48 @@ impl Database { .await?; let mut channels = Vec::with_capacity(parents_by_child_id.len()); - let mut rows = channel::Entity::find() - .filter(channel::Column::Id.is_in(parents_by_child_id.keys().copied())) - .stream(&*tx) - .await?; - - while let Some(row) = rows.next().await { - let row = row?; - channels.push(Channel { - id: row.id, - name: row.name, - parent_id: parents_by_child_id.get(&row.id).copied().flatten(), - }); + { + let mut rows = channel::Entity::find() + .filter(channel::Column::Id.is_in(parents_by_child_id.keys().copied())) + .stream(&*tx) + .await?; + while let Some(row) = rows.next().await { + let row = row?; + channels.push(Channel { + id: row.id, + name: row.name, + parent_id: parents_by_child_id.get(&row.id).copied().flatten(), + }); + } } - drop(rows); - #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] enum QueryUserIdsAndChannelIds { ChannelId, UserId, } - let mut participants = room_participant::Entity::find() - .inner_join(room::Entity) - .filter(room::Column::ChannelId.is_in(channels.iter().map(|c| c.id))) - .select_only() - .column(room::Column::ChannelId) - .column(room_participant::Column::UserId) - .into_values::<_, QueryUserIdsAndChannelIds>() - .stream(&*tx) - .await?; - - let mut participant_map: HashMap> = HashMap::default(); - while let Some(row) = participants.next().await { - let row: (ChannelId, UserId) = row?; - participant_map.entry(row.0).or_default().push(row.1) + let mut participants_by_channel: HashMap> = HashMap::default(); + { + let mut rows = room_participant::Entity::find() + .inner_join(room::Entity) + .filter(room::Column::ChannelId.is_in(channels.iter().map(|c| c.id))) + .select_only() + .column(room::Column::ChannelId) + .column(room_participant::Column::UserId) + .into_values::<_, QueryUserIdsAndChannelIds>() + .stream(&*tx) + .await?; + while let Some(row) = rows.next().await { + let row: (ChannelId, UserId) = row?; + participants_by_channel + .entry(row.0) + .or_default() + .push(row.1) + } } - drop(participants); - - Ok((channels, participant_map)) + Ok((channels, participants_by_channel)) }) .await } @@ -3480,12 +3469,15 @@ impl Database { .await } - // TODO: Add a chekc whether this user is allowed to read this channel pub async fn get_channel_member_details( &self, - id: ChannelId, + channel_id: ChannelId, + user_id: UserId, ) -> Result> { self.transaction(|tx| async move { + self.check_user_is_channel_admin(channel_id, user_id, &*tx) + .await?; + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] enum QueryMemberDetails { UserId, @@ -3494,14 +3486,14 @@ impl Database { } let tx = tx; - let ancestor_ids = self.get_channel_ancestors(id, &*tx).await?; + let ancestor_ids = self.get_channel_ancestors(channel_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), + channel_member::Column::ChannelId.eq(channel_id), QueryMemberDetails::IsDirectMember, ) .column(channel_member::Column::Accepted) @@ -3552,9 +3544,29 @@ impl Database { Ok(user_ids) } + async fn check_user_is_channel_admin( + &self, + channel_id: ChannelId, + user_id: UserId, + tx: &DatabaseTransaction, + ) -> Result<()> { + let channel_ids = self.get_channel_ancestors(channel_id, tx).await?; + channel_member::Entity::find() + .filter( + channel_member::Column::ChannelId + .is_in(channel_ids) + .and(channel_member::Column::UserId.eq(user_id)) + .and(channel_member::Column::Admin.eq(true)), + ) + .one(&*tx) + .await? + .ok_or_else(|| anyhow!("user is not allowed to remove this channel"))?; + Ok(()) + } + async fn get_channel_ancestors( &self, - id: ChannelId, + channel_id: ChannelId, tx: &DatabaseTransaction, ) -> Result> { let sql = format!( @@ -3570,7 +3582,7 @@ impl Database { SELECT DISTINCT channel_tree.parent_id FROM channel_tree "#, - id + channel_id ); #[derive(FromQueryResult, Debug, PartialEq)] diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index e4161d3b5555ad2a50540622e836a4b628326f1f..b4c22430e5bdd955989429f2d514da3dd6d0bff0 100644 --- a/crates/collab/src/db/tests.rs +++ b/crates/collab/src/db/tests.rs @@ -951,7 +951,7 @@ test_both_dbs!(test_channels_postgres, test_channels_sqlite, db, { .await .unwrap(); - let (channels, _) = db.get_channels(a_id).await.unwrap(); + let (channels, _) = db.get_channels_for_user(a_id).await.unwrap(); assert_eq!( channels, @@ -1144,7 +1144,7 @@ test_both_dbs!( .unwrap(); let user_2_invites = db - .get_channel_invites(user_2) // -> [channel_1_1, channel_1_2] + .get_channel_invites_for_user(user_2) // -> [channel_1_1, channel_1_2] .await .unwrap() .into_iter() @@ -1154,7 +1154,7 @@ test_both_dbs!( assert_eq!(user_2_invites, &[channel_1_1, channel_1_2]); let user_3_invites = db - .get_channel_invites(user_3) // -> [channel_1_1] + .get_channel_invites_for_user(user_3) // -> [channel_1_1] .await .unwrap() .into_iter() @@ -1163,7 +1163,10 @@ test_both_dbs!( assert_eq!(user_3_invites, &[channel_1_1]); - let members = db.get_channel_member_details(channel_1_1).await.unwrap(); + let members = db + .get_channel_member_details(channel_1_1, user_1) + .await + .unwrap(); assert_eq!( members, &[ @@ -1191,7 +1194,10 @@ test_both_dbs!( .await .unwrap(); - let members = db.get_channel_member_details(channel_1_3).await.unwrap(); + let members = db + .get_channel_member_details(channel_1_3, user_1) + .await + .unwrap(); assert_eq!( members, &[ diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index fdfccea98f95e0d277e3b876c984b588a6a28c25..17f13345444c216f803892bb113ec9944b2de5ee 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -530,8 +530,8 @@ impl Server { let (contacts, invite_code, (channels, channel_participants), channel_invites) = future::try_join4( this.app_state.db.get_contacts(user_id), this.app_state.db.get_invite_code_for_user(user_id), - this.app_state.db.get_channels(user_id), - this.app_state.db.get_channel_invites(user_id) + this.app_state.db.get_channels_for_user(user_id), + this.app_state.db.get_channel_invites_for_user(user_id) ).await?; { @@ -2230,10 +2230,16 @@ async fn invite_channel_member( } async fn remove_channel_member( - _request: proto::RemoveChannelMember, - _response: Response, - _session: Session, + request: proto::RemoveChannelMember, + response: Response, + session: Session, ) -> Result<()> { + let db = session.db().await; + let channel_id = ChannelId::from_proto(request.channel_id); + let member_id = UserId::from_proto(request.user_id); + db.remove_channel_member(channel_id, member_id, session.user_id) + .await?; + response.send(proto::Ack {})?; Ok(()) } @@ -2244,7 +2250,9 @@ async fn get_channel_members( ) -> 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?; + let members = db + .get_channel_member_details(channel_id, session.user_id) + .await?; response.send(proto::GetChannelMembersResponse { members })?; Ok(()) } diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index 9af6099f655dfc9389f516b129736a8545fe5d05..56285400221203c66d399be99ca98c2d8471872c 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -260,9 +260,12 @@ impl PickerDelegate for ChannelModalDelegate { fn confirm(&mut self, _: bool, cx: &mut ViewContext>) { if let Some((user, _)) = self.matches.get(self.selected_index) { match self.mode { - Mode::ManageMembers => { - // - } + Mode::ManageMembers => self + .channel_store + .update(cx, |store, cx| { + store.remove_member(self.channel_id, 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