From 690d9fb971996b17cd58136558118e9e2a02068d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 11 Oct 2023 16:34:11 -0600 Subject: [PATCH 01/25] Add a role column to the database and start using it We cannot yet stop using `admin` because stable will continue writing it. --- .../20221109000000_test_schema.sql | 1 + .../20231011214412_add_guest_role.sql | 4 +++ crates/collab/src/db/ids.rs | 11 ++++++ crates/collab/src/db/queries/channels.rs | 34 +++++++++++++------ crates/collab/src/db/tables/channel_member.rs | 6 ++-- crates/collab/src/db/tests/buffer_tests.rs | 4 +-- crates/collab/src/db/tests/channel_tests.rs | 18 ++++++---- crates/collab/src/db/tests/message_tests.rs | 6 ++-- crates/collab/src/rpc.rs | 18 +++++++--- .../src/tests/random_channel_buffer_tests.rs | 4 ++- 10 files changed, 76 insertions(+), 30 deletions(-) create mode 100644 crates/collab/migrations/20231011214412_add_guest_role.sql diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 5a84bfd796a88e09769004cc40d0cc6a06e3118a..dd6e80150be6034c02368372d2ff6f6936a00ef7 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -226,6 +226,7 @@ CREATE TABLE "channel_members" ( "channel_id" INTEGER NOT NULL REFERENCES channels (id) ON DELETE CASCADE, "user_id" INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, "admin" BOOLEAN NOT NULL DEFAULT false, + "role" VARCHAR, "accepted" BOOLEAN NOT NULL DEFAULT false, "updated_at" TIMESTAMP NOT NULL DEFAULT now ); diff --git a/crates/collab/migrations/20231011214412_add_guest_role.sql b/crates/collab/migrations/20231011214412_add_guest_role.sql new file mode 100644 index 0000000000000000000000000000000000000000..378590a0f9702fd63630a32957780096e3d7ba56 --- /dev/null +++ b/crates/collab/migrations/20231011214412_add_guest_role.sql @@ -0,0 +1,4 @@ +-- Add migration script here + +ALTER TABLE channel_members ADD COLUMN role TEXT; +UPDATE channel_members SET role = CASE WHEN admin THEN 'admin' ELSE 'member' END; diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 23bb9e53bf9803ba64693f94605a8e87f904c571..747e3a7d3b17642191a0e1067c11cf1d1c3e205c 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -80,3 +80,14 @@ id_type!(SignupId); id_type!(UserId); id_type!(ChannelBufferCollaboratorId); id_type!(FlagId); + +#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum)] +#[sea_orm(rs_type = "String", db_type = "String(None)")] +pub enum ChannelRole { + #[sea_orm(string_value = "admin")] + Admin, + #[sea_orm(string_value = "member")] + Member, + #[sea_orm(string_value = "guest")] + Guest, +} diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index c576d2406b81279c38561406c3801c02ddaf4377..0fe78209164be20ba0f2f618a6be7190beed1c26 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -74,11 +74,12 @@ impl Database { } channel_member::ActiveModel { + id: ActiveValue::NotSet, channel_id: ActiveValue::Set(channel.id), user_id: ActiveValue::Set(creator_id), accepted: ActiveValue::Set(true), admin: ActiveValue::Set(true), - ..Default::default() + role: ActiveValue::Set(Some(ChannelRole::Admin)), } .insert(&*tx) .await?; @@ -160,18 +161,19 @@ impl Database { channel_id: ChannelId, invitee_id: UserId, inviter_id: UserId, - is_admin: bool, + role: ChannelRole, ) -> Result<()> { self.transaction(move |tx| async move { self.check_user_is_channel_admin(channel_id, inviter_id, &*tx) .await?; channel_member::ActiveModel { + id: ActiveValue::NotSet, channel_id: ActiveValue::Set(channel_id), user_id: ActiveValue::Set(invitee_id), accepted: ActiveValue::Set(false), - admin: ActiveValue::Set(is_admin), - ..Default::default() + admin: ActiveValue::Set(role == ChannelRole::Admin), + role: ActiveValue::Set(Some(role)), } .insert(&*tx) .await?; @@ -417,7 +419,13 @@ impl Database { let channels_with_admin_privileges = channel_memberships .iter() - .filter_map(|membership| membership.admin.then_some(membership.channel_id)) + .filter_map(|membership| { + if membership.role == Some(ChannelRole::Admin) || membership.admin { + Some(membership.channel_id) + } else { + None + } + }) .collect(); let graph = self @@ -470,12 +478,12 @@ impl Database { .await } - pub async fn set_channel_member_admin( + pub async fn set_channel_member_role( &self, channel_id: ChannelId, from: UserId, for_user: UserId, - admin: bool, + role: ChannelRole, ) -> Result<()> { self.transaction(|tx| async move { self.check_user_is_channel_admin(channel_id, from, &*tx) @@ -488,7 +496,8 @@ impl Database { .and(channel_member::Column::UserId.eq(for_user)), ) .set(channel_member::ActiveModel { - admin: ActiveValue::set(admin), + admin: ActiveValue::set(role == ChannelRole::Admin), + role: ActiveValue::set(Some(role)), ..Default::default() }) .exec(&*tx) @@ -516,6 +525,7 @@ impl Database { enum QueryMemberDetails { UserId, Admin, + Role, IsDirectMember, Accepted, } @@ -528,6 +538,7 @@ impl Database { .select_only() .column(channel_member::Column::UserId) .column(channel_member::Column::Admin) + .column(channel_member::Column::Role) .column_as( channel_member::Column::ChannelId.eq(channel_id), QueryMemberDetails::IsDirectMember, @@ -540,9 +551,10 @@ impl Database { let mut rows = Vec::::new(); while let Some(row) = stream.next().await { - let (user_id, is_admin, is_direct_member, is_invite_accepted): ( + let (user_id, is_admin, channel_role, is_direct_member, is_invite_accepted): ( UserId, bool, + Option, bool, bool, ) = row?; @@ -558,7 +570,7 @@ impl Database { if last_row.user_id == user_id { if is_direct_member { last_row.kind = kind; - last_row.admin = is_admin; + last_row.admin = channel_role == Some(ChannelRole::Admin) || is_admin; } continue; } @@ -566,7 +578,7 @@ impl Database { rows.push(proto::ChannelMember { user_id, kind, - admin: is_admin, + admin: channel_role == Some(ChannelRole::Admin) || is_admin, }); } diff --git a/crates/collab/src/db/tables/channel_member.rs b/crates/collab/src/db/tables/channel_member.rs index ba3db5a15504e60838439cce60e5bef9a02d83bb..e8162bfcbd133fe647dbbfa13cab1136dcbe441e 100644 --- a/crates/collab/src/db/tables/channel_member.rs +++ b/crates/collab/src/db/tables/channel_member.rs @@ -1,7 +1,7 @@ -use crate::db::{channel_member, ChannelId, ChannelMemberId, UserId}; +use crate::db::{channel_member, ChannelId, ChannelMemberId, ChannelRole, UserId}; use sea_orm::entity::prelude::*; -#[derive(Clone, Debug, Default, PartialEq, Eq, DeriveEntityModel)] +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] #[sea_orm(table_name = "channel_members")] pub struct Model { #[sea_orm(primary_key)] @@ -10,6 +10,8 @@ pub struct Model { pub user_id: UserId, pub accepted: bool, pub admin: bool, + // only optional while migrating + pub role: Option, } impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 0ac41a8b0b4267fdd50e8e2c8392319169194888..51ba9bf655221a5c611ad3fa023631f46151f144 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -56,7 +56,7 @@ async fn test_channel_buffers(db: &Arc) { let zed_id = db.create_root_channel("zed", a_id).await.unwrap(); - db.invite_channel_member(zed_id, b_id, a_id, false) + db.invite_channel_member(zed_id, b_id, a_id, ChannelRole::Member) .await .unwrap(); @@ -211,7 +211,7 @@ async fn test_channel_buffers_last_operations(db: &Database) { .await .unwrap(); - db.invite_channel_member(channel, observer_id, user_id, false) + db.invite_channel_member(channel, observer_id, user_id, ChannelRole::Member) .await .unwrap(); db.respond_to_channel_invite(channel, observer_id, true) diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 7d2bc04a35aac3adb30ead310705d2ee192ed54a..ed4b9e061b0ee7ab1181431b24ffdb1cd88ba5c2 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -8,7 +8,7 @@ use crate::{ db::{ queries::channels::ChannelGraph, tests::{graph, TEST_RELEASE_CHANNEL}, - ChannelId, Database, NewUserParams, + ChannelId, ChannelRole, Database, NewUserParams, }, test_both_dbs, }; @@ -50,7 +50,7 @@ async fn test_channels(db: &Arc) { // Make sure that people cannot read channels they haven't been invited to assert!(db.get_channel(zed_id, b_id).await.unwrap().is_none()); - db.invite_channel_member(zed_id, b_id, a_id, false) + db.invite_channel_member(zed_id, b_id, a_id, ChannelRole::Member) .await .unwrap(); @@ -125,9 +125,13 @@ async fn test_channels(db: &Arc) { ); // Update member permissions - let set_subchannel_admin = db.set_channel_member_admin(crdb_id, a_id, b_id, true).await; + let set_subchannel_admin = db + .set_channel_member_role(crdb_id, a_id, b_id, ChannelRole::Admin) + .await; assert!(set_subchannel_admin.is_err()); - let set_channel_admin = db.set_channel_member_admin(zed_id, a_id, b_id, true).await; + let set_channel_admin = db + .set_channel_member_role(zed_id, a_id, b_id, ChannelRole::Admin) + .await; assert!(set_channel_admin.is_ok()); let result = db.get_channels_for_user(b_id).await.unwrap(); @@ -284,13 +288,13 @@ async fn test_channel_invites(db: &Arc) { let channel_1_2 = db.create_root_channel("channel_2", user_1).await.unwrap(); - db.invite_channel_member(channel_1_1, user_2, user_1, false) + db.invite_channel_member(channel_1_1, user_2, user_1, ChannelRole::Member) .await .unwrap(); - db.invite_channel_member(channel_1_2, user_2, user_1, false) + db.invite_channel_member(channel_1_2, user_2, user_1, ChannelRole::Member) .await .unwrap(); - db.invite_channel_member(channel_1_1, user_3, user_1, true) + db.invite_channel_member(channel_1_1, user_3, user_1, ChannelRole::Admin) .await .unwrap(); diff --git a/crates/collab/src/db/tests/message_tests.rs b/crates/collab/src/db/tests/message_tests.rs index e758fcfb5d0104a61ad84dd82ce10fad784fdf5a..272d8e01009ac8758e887d2c0ec4f04570464923 100644 --- a/crates/collab/src/db/tests/message_tests.rs +++ b/crates/collab/src/db/tests/message_tests.rs @@ -1,5 +1,5 @@ use crate::{ - db::{Database, MessageId, NewUserParams}, + db::{ChannelRole, Database, MessageId, NewUserParams}, test_both_dbs, }; use std::sync::Arc; @@ -155,7 +155,7 @@ async fn test_channel_message_new_notification(db: &Arc) { let channel_2 = db.create_channel("channel-2", None, user).await.unwrap(); - db.invite_channel_member(channel_1, observer, user, false) + db.invite_channel_member(channel_1, observer, user, ChannelRole::Member) .await .unwrap(); @@ -163,7 +163,7 @@ async fn test_channel_message_new_notification(db: &Arc) { .await .unwrap(); - db.invite_channel_member(channel_2, observer, user, false) + db.invite_channel_member(channel_2, observer, user, ChannelRole::Member) .await .unwrap(); diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index e5c6d94ce03b8b3b1d64ed58be4da53f9dcca112..f13f482c2b3b156401fa20a3b8a8a8a7f51c2f5f 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -3,8 +3,8 @@ mod connection_pool; use crate::{ auth, db::{ - self, BufferId, ChannelId, ChannelsForUser, Database, MessageId, ProjectId, RoomId, - ServerId, User, UserId, + self, BufferId, ChannelId, ChannelRole, ChannelsForUser, Database, MessageId, ProjectId, + RoomId, ServerId, User, UserId, }, executor::Executor, AppState, Result, @@ -2282,7 +2282,12 @@ async fn invite_channel_member( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let invitee_id = UserId::from_proto(request.user_id); - db.invite_channel_member(channel_id, invitee_id, session.user_id, request.admin) + let role = if request.admin { + ChannelRole::Admin + } else { + ChannelRole::Member + }; + db.invite_channel_member(channel_id, invitee_id, session.user_id, role) .await?; let (channel, _) = db @@ -2342,7 +2347,12 @@ async fn set_channel_member_admin( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let member_id = UserId::from_proto(request.user_id); - db.set_channel_member_admin(channel_id, session.user_id, member_id, request.admin) + let role = if request.admin { + ChannelRole::Admin + } else { + ChannelRole::Member + }; + db.set_channel_member_role(channel_id, session.user_id, member_id, role) .await?; let (channel, has_accepted) = db diff --git a/crates/collab/src/tests/random_channel_buffer_tests.rs b/crates/collab/src/tests/random_channel_buffer_tests.rs index 6e0bef225c9fc2d3bc78faa5ddca6e65e28f5495..1b24c7a3d2f45e5e374c5a70cd54a426b4043753 100644 --- a/crates/collab/src/tests/random_channel_buffer_tests.rs +++ b/crates/collab/src/tests/random_channel_buffer_tests.rs @@ -1,3 +1,5 @@ +use crate::db::ChannelRole; + use super::{run_randomized_test, RandomizedTest, TestClient, TestError, TestServer, UserTestPlan}; use anyhow::Result; use async_trait::async_trait; @@ -50,7 +52,7 @@ impl RandomizedTest for RandomChannelBufferTest { .await .unwrap(); for user in &users[1..] { - db.invite_channel_member(id, user.user_id, users[0].user_id, false) + db.invite_channel_member(id, user.user_id, users[0].user_id, ChannelRole::Member) .await .unwrap(); db.respond_to_channel_invite(id, user.user_id, true) From 540436a1f9892b9f3c6aafbf21d525335880d8bc Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 11 Oct 2023 21:57:46 -0600 Subject: [PATCH 02/25] Push role refactoring through RPC/client --- .cargo/config.toml | 2 +- crates/channel/src/channel_store.rs | 24 ++++---- crates/channel/src/channel_store_tests.rs | 4 +- crates/collab/src/db/ids.rs | 28 +++++++++ crates/collab/src/db/queries/channels.rs | 18 ++++-- crates/collab/src/db/tests/channel_tests.rs | 10 ++-- crates/collab/src/rpc.rs | 46 +++++++-------- crates/collab/src/tests/channel_tests.rs | 43 +++++++++++--- crates/collab/src/tests/test_server.rs | 11 +++- .../src/collab_panel/channel_modal.rs | 57 ++++++++++++------- crates/rpc/proto/zed.proto | 20 ++++--- crates/rpc/src/proto.rs | 4 +- 12 files changed, 178 insertions(+), 89 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 9da6b3be080072d89d16a199e2d60d527eeacd07..e22bdb0f2c70a1ffda714674253cc533e9e7c1d1 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -3,4 +3,4 @@ xtask = "run --package xtask --" [build] # v0 mangling scheme provides more detailed backtraces around closures -rustflags = ["-C", "symbol-mangling-version=v0"] +rustflags = ["-C", "symbol-mangling-version=v0", "-C", "link-arg=-fuse-ld=/opt/homebrew/Cellar/llvm/16.0.6/bin/ld64.lld"] diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 2a2fa454f2b4435a806d90304940a4ce61450d09..64c76a0a390c3dc5ae6ba1ef402b16d8b58efdb8 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -9,7 +9,7 @@ use db::RELEASE_CHANNEL; use futures::{channel::mpsc, future::Shared, Future, FutureExt, StreamExt}; use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Task, WeakModelHandle}; use rpc::{ - proto::{self, ChannelEdge, ChannelPermission}, + proto::{self, ChannelEdge, ChannelPermission, ChannelRole}, TypedEnvelope, }; use serde_derive::{Deserialize, Serialize}; @@ -79,7 +79,7 @@ pub struct ChannelPath(Arc<[ChannelId]>); pub struct ChannelMembership { pub user: Arc, pub kind: proto::channel_member::Kind, - pub admin: bool, + pub role: proto::ChannelRole, } pub enum ChannelEvent { @@ -436,7 +436,7 @@ impl ChannelStore { insert_edge: parent_edge, channel_permissions: vec![ChannelPermission { channel_id, - is_admin: true, + role: ChannelRole::Admin.into(), }], ..Default::default() }, @@ -512,7 +512,7 @@ impl ChannelStore { &mut self, channel_id: ChannelId, user_id: UserId, - admin: bool, + role: proto::ChannelRole, cx: &mut ModelContext, ) -> Task> { if !self.outgoing_invites.insert((channel_id, user_id)) { @@ -526,7 +526,7 @@ impl ChannelStore { .request(proto::InviteChannelMember { channel_id, user_id, - admin, + role: role.into(), }) .await; @@ -570,11 +570,11 @@ impl ChannelStore { }) } - pub fn set_member_admin( + pub fn set_member_role( &mut self, channel_id: ChannelId, user_id: UserId, - admin: bool, + role: proto::ChannelRole, cx: &mut ModelContext, ) -> Task> { if !self.outgoing_invites.insert((channel_id, user_id)) { @@ -585,10 +585,10 @@ impl ChannelStore { let client = self.client.clone(); cx.spawn(|this, mut cx| async move { let result = client - .request(proto::SetChannelMemberAdmin { + .request(proto::SetChannelMemberRole { channel_id, user_id, - admin, + role: role.into(), }) .await; @@ -676,8 +676,8 @@ impl ChannelStore { .filter_map(|(user, member)| { Some(ChannelMembership { user, - admin: member.admin, - kind: proto::channel_member::Kind::from_i32(member.kind)?, + role: member.role(), + kind: member.kind(), }) }) .collect()) @@ -935,7 +935,7 @@ impl ChannelStore { } for permission in payload.channel_permissions { - if permission.is_admin { + if permission.role() == proto::ChannelRole::Admin { self.channels_with_admin_privileges .insert(permission.channel_id); } else { diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index 9303a52092e13a1592e4a5786c4ed636b969cb73..f8828159bdc5049adc5b789383ee72c3eca8629c 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -26,7 +26,7 @@ fn test_update_channels(cx: &mut AppContext) { ], channel_permissions: vec![proto::ChannelPermission { channel_id: 1, - is_admin: true, + role: proto::ChannelRole::Admin.into(), }], ..Default::default() }, @@ -114,7 +114,7 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { ], channel_permissions: vec![proto::ChannelPermission { channel_id: 0, - is_admin: true, + role: proto::ChannelRole::Admin.into(), }], ..Default::default() }, diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 747e3a7d3b17642191a0e1067c11cf1d1c3e205c..946702f36ce5eee521741d4556ea201d0966ab34 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -1,4 +1,5 @@ use crate::Result; +use rpc::proto; use sea_orm::{entity::prelude::*, DbErr}; use serde::{Deserialize, Serialize}; @@ -91,3 +92,30 @@ pub enum ChannelRole { #[sea_orm(string_value = "guest")] Guest, } + +impl From for ChannelRole { + fn from(value: proto::ChannelRole) -> Self { + match value { + proto::ChannelRole::Admin => ChannelRole::Admin, + proto::ChannelRole::Member => ChannelRole::Member, + proto::ChannelRole::Guest => ChannelRole::Guest, + } + } +} + +impl Into for ChannelRole { + fn into(self) -> proto::ChannelRole { + match self { + ChannelRole::Admin => proto::ChannelRole::Admin, + ChannelRole::Member => proto::ChannelRole::Member, + ChannelRole::Guest => proto::ChannelRole::Guest, + } + } +} + +impl Into for ChannelRole { + fn into(self) -> i32 { + let proto: proto::ChannelRole = self.into(); + proto.into() + } +} diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 0fe78209164be20ba0f2f618a6be7190beed1c26..5c96955eba5cb88d55abe35506c6867466029eee 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -564,13 +564,18 @@ impl Database { (false, true) => proto::channel_member::Kind::AncestorMember, (false, false) => continue, }; + let channel_role = channel_role.unwrap_or(if is_admin { + ChannelRole::Admin + } else { + ChannelRole::Member + }); 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 { if is_direct_member { last_row.kind = kind; - last_row.admin = channel_role == Some(ChannelRole::Admin) || is_admin; + last_row.role = channel_role.into() } continue; } @@ -578,7 +583,7 @@ impl Database { rows.push(proto::ChannelMember { user_id, kind, - admin: channel_role == Some(ChannelRole::Admin) || is_admin, + role: channel_role.into(), }); } @@ -851,10 +856,11 @@ impl Database { &self, user: UserId, channel: ChannelId, - to: ChannelId, + new_parent: ChannelId, tx: &DatabaseTransaction, ) -> Result { - self.check_user_is_channel_admin(to, user, &*tx).await?; + self.check_user_is_channel_admin(new_parent, user, &*tx) + .await?; let paths = channel_path::Entity::find() .filter(channel_path::Column::IdPath.like(&format!("%/{}/%", channel))) @@ -872,7 +878,7 @@ impl Database { } let paths_to_new_parent = channel_path::Entity::find() - .filter(channel_path::Column::ChannelId.eq(to)) + .filter(channel_path::Column::ChannelId.eq(new_parent)) .all(tx) .await?; @@ -906,7 +912,7 @@ impl Database { if let Some(channel) = channel_descendants.get_mut(&channel) { // Remove the other parents channel.clear(); - channel.insert(to); + channel.insert(new_parent); } let channels = self diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index ed4b9e061b0ee7ab1181431b24ffdb1cd88ba5c2..90b3a0cd2e6e8f98c1b258cb0d603d7c9cfddd68 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -328,17 +328,17 @@ async fn test_channel_invites(db: &Arc) { proto::ChannelMember { user_id: user_1.to_proto(), kind: proto::channel_member::Kind::Member.into(), - admin: true, + role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { user_id: user_2.to_proto(), kind: proto::channel_member::Kind::Invitee.into(), - admin: false, + role: proto::ChannelRole::Member.into(), }, proto::ChannelMember { user_id: user_3.to_proto(), kind: proto::channel_member::Kind::Invitee.into(), - admin: true, + role: proto::ChannelRole::Admin.into(), }, ] ); @@ -362,12 +362,12 @@ async fn test_channel_invites(db: &Arc) { proto::ChannelMember { user_id: user_1.to_proto(), kind: proto::channel_member::Kind::Member.into(), - admin: true, + role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { user_id: user_2.to_proto(), kind: proto::channel_member::Kind::AncestorMember.into(), - admin: false, + role: proto::ChannelRole::Member.into(), }, ] ); diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index f13f482c2b3b156401fa20a3b8a8a8a7f51c2f5f..b05421e9601f293501448b935db357e94ebc2dee 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -3,8 +3,8 @@ mod connection_pool; use crate::{ auth, db::{ - self, BufferId, ChannelId, ChannelRole, ChannelsForUser, Database, MessageId, ProjectId, - RoomId, ServerId, User, UserId, + self, BufferId, ChannelId, ChannelsForUser, Database, MessageId, ProjectId, RoomId, + ServerId, User, UserId, }, executor::Executor, AppState, Result, @@ -254,7 +254,7 @@ impl Server { .add_request_handler(delete_channel) .add_request_handler(invite_channel_member) .add_request_handler(remove_channel_member) - .add_request_handler(set_channel_member_admin) + .add_request_handler(set_channel_member_role) .add_request_handler(rename_channel) .add_request_handler(join_channel_buffer) .add_request_handler(leave_channel_buffer) @@ -2282,13 +2282,13 @@ async fn invite_channel_member( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let invitee_id = UserId::from_proto(request.user_id); - let role = if request.admin { - ChannelRole::Admin - } else { - ChannelRole::Member - }; - db.invite_channel_member(channel_id, invitee_id, session.user_id, role) - .await?; + db.invite_channel_member( + channel_id, + invitee_id, + session.user_id, + request.role().into(), + ) + .await?; let (channel, _) = db .get_channel(channel_id, session.user_id) @@ -2339,21 +2339,21 @@ async fn remove_channel_member( Ok(()) } -async fn set_channel_member_admin( - request: proto::SetChannelMemberAdmin, - response: Response, +async fn set_channel_member_role( + request: proto::SetChannelMemberRole, + 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); - let role = if request.admin { - ChannelRole::Admin - } else { - ChannelRole::Member - }; - db.set_channel_member_role(channel_id, session.user_id, member_id, role) - .await?; + db.set_channel_member_role( + channel_id, + session.user_id, + member_id, + request.role().into(), + ) + .await?; let (channel, has_accepted) = db .get_channel(channel_id, member_id) @@ -2364,7 +2364,7 @@ async fn set_channel_member_admin( if has_accepted { update.channel_permissions.push(proto::ChannelPermission { channel_id: channel.id.to_proto(), - is_admin: request.admin, + role: request.role, }); } @@ -2603,7 +2603,7 @@ async fn respond_to_channel_invite( .into_iter() .map(|channel_id| proto::ChannelPermission { channel_id: channel_id.to_proto(), - is_admin: true, + role: proto::ChannelRole::Admin.into(), }), ); } @@ -3106,7 +3106,7 @@ fn build_initial_channels_update( .into_iter() .map(|id| proto::ChannelPermission { channel_id: id.to_proto(), - is_admin: true, + role: proto::ChannelRole::Admin.into(), }), ); diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index 7cfcce832b4cd4e05a953156828e517ef85af9fe..bc814d06a26721963fd82b345cb7dc85aee6771a 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -68,7 +68,12 @@ async fn test_core_channels( .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); + let invite = store.invite_member( + channel_a_id, + client_b.user_id().unwrap(), + proto::ChannelRole::Member, + cx, + ); // Make sure we're synchronously storing the pending invite assert!(store.has_pending_channel_invite(channel_a_id, client_b.user_id().unwrap())); @@ -103,12 +108,12 @@ async fn test_core_channels( &[ ( client_a.user_id().unwrap(), - true, + proto::ChannelRole::Admin, proto::channel_member::Kind::Member, ), ( client_b.user_id().unwrap(), - false, + proto::ChannelRole::Member, proto::channel_member::Kind::Invitee, ), ], @@ -183,7 +188,12 @@ async fn test_core_channels( client_a .channel_store() .update(cx_a, |store, cx| { - store.set_member_admin(channel_a_id, client_b.user_id().unwrap(), true, cx) + store.set_member_role( + channel_a_id, + client_b.user_id().unwrap(), + proto::ChannelRole::Admin, + cx, + ) }) .await .unwrap(); @@ -305,12 +315,12 @@ fn assert_participants_eq(participants: &[Arc], expected_partitipants: &[u #[track_caller] fn assert_members_eq( members: &[ChannelMembership], - expected_members: &[(u64, bool, proto::channel_member::Kind)], + expected_members: &[(u64, proto::ChannelRole, proto::channel_member::Kind)], ) { assert_eq!( members .iter() - .map(|member| (member.user.id, member.admin, member.kind)) + .map(|member| (member.user.id, member.role, member.kind)) .collect::>(), expected_members ); @@ -611,7 +621,12 @@ async fn test_permissions_update_while_invited( client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.invite_member(rust_id, client_b.user_id().unwrap(), false, cx) + channel_store.invite_member( + rust_id, + client_b.user_id().unwrap(), + proto::ChannelRole::Member, + cx, + ) }) .await .unwrap(); @@ -634,7 +649,12 @@ async fn test_permissions_update_while_invited( client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.set_member_admin(rust_id, client_b.user_id().unwrap(), true, cx) + channel_store.set_member_role( + rust_id, + client_b.user_id().unwrap(), + proto::ChannelRole::Admin, + cx, + ) }) .await .unwrap(); @@ -803,7 +823,12 @@ async fn test_lost_channel_creation( client_a .channel_store() .update(cx_a, |channel_store, cx| { - channel_store.invite_member(channel_id, client_b.user_id().unwrap(), false, cx) + channel_store.invite_member( + channel_id, + client_b.user_id().unwrap(), + proto::ChannelRole::Member, + cx, + ) }) .await .unwrap(); diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index 2e13874125472cd53b68d4d688c90ca02569615a..54a59c0c000011a329f64ed5fab36a1a091d309c 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -17,7 +17,7 @@ use gpui::{executor::Deterministic, ModelHandle, Task, TestAppContext, WindowHan use language::LanguageRegistry; use parking_lot::Mutex; use project::{Project, WorktreeId}; -use rpc::RECEIVE_TIMEOUT; +use rpc::{proto::ChannelRole, RECEIVE_TIMEOUT}; use settings::SettingsStore; use std::{ cell::{Ref, RefCell, RefMut}, @@ -325,7 +325,7 @@ impl TestServer { channel_store.invite_member( channel_id, member_client.user_id().unwrap(), - false, + ChannelRole::Member, cx, ) }) @@ -613,7 +613,12 @@ impl TestClient { cx_self .read(ChannelStore::global) .update(cx_self, |channel_store, cx| { - channel_store.invite_member(channel, other_client.user_id().unwrap(), true, cx) + channel_store.invite_member( + channel, + other_client.user_id().unwrap(), + ChannelRole::Admin, + cx, + ) }) .await .unwrap(); diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index 4c811a2df547dc78e0a602ae2002a4e9dbeb4e46..16d5e48f452b7acd4b220076ccb73ac7c8308854 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -1,5 +1,8 @@ use channel::{ChannelId, ChannelMembership, ChannelStore}; -use client::{proto, User, UserId, UserStore}; +use client::{ + proto::{self, ChannelRole}, + User, UserId, UserStore, +}; use context_menu::{ContextMenu, ContextMenuItem}; use fuzzy::{match_strings, StringMatchCandidate}; use gpui::{ @@ -343,9 +346,11 @@ impl PickerDelegate for ChannelModalDelegate { } fn confirm(&mut self, _: bool, cx: &mut ViewContext>) { - if let Some((selected_user, admin)) = self.user_at_index(self.selected_index) { + if let Some((selected_user, role)) = self.user_at_index(self.selected_index) { match self.mode { - Mode::ManageMembers => self.show_context_menu(admin.unwrap_or(false), cx), + Mode::ManageMembers => { + self.show_context_menu(role.unwrap_or(ChannelRole::Member), cx) + } Mode::InviteMembers => match self.member_status(selected_user.id, cx) { Some(proto::channel_member::Kind::Invitee) => { self.remove_selected_member(cx); @@ -373,7 +378,7 @@ impl PickerDelegate for ChannelModalDelegate { let full_theme = &theme::current(cx); let theme = &full_theme.collab_panel.channel_modal; let tabbed_modal = &full_theme.collab_panel.tabbed_modal; - let (user, admin) = self.user_at_index(ix).unwrap(); + let (user, role) = self.user_at_index(ix).unwrap(); let request_status = self.member_status(user.id, cx); let style = tabbed_modal @@ -409,15 +414,25 @@ impl PickerDelegate for ChannelModalDelegate { }, ) }) - .with_children(admin.and_then(|admin| { - (in_manage && admin).then(|| { + .with_children(if in_manage && role == Some(ChannelRole::Admin) { + Some( Label::new("Admin", theme.member_tag.text.clone()) .contained() .with_style(theme.member_tag.container) .aligned() - .left() - }) - })) + .left(), + ) + } else if in_manage && role == Some(ChannelRole::Guest) { + Some( + Label::new("Guest", theme.member_tag.text.clone()) + .contained() + .with_style(theme.member_tag.container) + .aligned() + .left(), + ) + } else { + None + }) .with_children({ let svg = match self.mode { Mode::ManageMembers => Some( @@ -502,13 +517,13 @@ impl ChannelModalDelegate { }) } - fn user_at_index(&self, ix: usize) -> Option<(Arc, Option)> { + fn user_at_index(&self, ix: usize) -> Option<(Arc, Option)> { match self.mode { Mode::ManageMembers => self.matching_member_indices.get(ix).and_then(|ix| { let channel_membership = self.members.get(*ix)?; Some(( channel_membership.user.clone(), - Some(channel_membership.admin), + Some(channel_membership.role), )) }), Mode::InviteMembers => Some((self.matching_users.get(ix).cloned()?, None)), @@ -516,17 +531,21 @@ impl ChannelModalDelegate { } fn toggle_selected_member_admin(&mut self, cx: &mut ViewContext>) -> Option<()> { - let (user, admin) = self.user_at_index(self.selected_index)?; - let admin = !admin.unwrap_or(false); + let (user, role) = self.user_at_index(self.selected_index)?; + let new_role = if role == Some(ChannelRole::Admin) { + ChannelRole::Member + } else { + ChannelRole::Admin + }; let update = self.channel_store.update(cx, |store, cx| { - store.set_member_admin(self.channel_id, user.id, admin, cx) + store.set_member_role(self.channel_id, user.id, new_role, cx) }); cx.spawn(|picker, mut cx| async move { update.await?; picker.update(&mut cx, |picker, cx| { let this = picker.delegate_mut(); if let Some(member) = this.members.iter_mut().find(|m| m.user.id == user.id) { - member.admin = admin; + member.role = new_role; } cx.focus_self(); cx.notify(); @@ -572,7 +591,7 @@ impl ChannelModalDelegate { fn invite_member(&mut self, user: Arc, cx: &mut ViewContext>) { let invite_member = self.channel_store.update(cx, |store, cx| { - store.invite_member(self.channel_id, user.id, false, cx) + store.invite_member(self.channel_id, user.id, ChannelRole::Member, cx) }); cx.spawn(|this, mut cx| async move { @@ -582,7 +601,7 @@ impl ChannelModalDelegate { this.delegate_mut().members.push(ChannelMembership { user, kind: proto::channel_member::Kind::Invitee, - admin: false, + role: ChannelRole::Member, }); cx.notify(); }) @@ -590,7 +609,7 @@ impl ChannelModalDelegate { .detach_and_log_err(cx); } - fn show_context_menu(&mut self, user_is_admin: bool, cx: &mut ViewContext>) { + fn show_context_menu(&mut self, role: ChannelRole, cx: &mut ViewContext>) { self.context_menu.update(cx, |context_menu, cx| { context_menu.show( Default::default(), @@ -598,7 +617,7 @@ impl ChannelModalDelegate { vec![ ContextMenuItem::action("Remove", RemoveMember), ContextMenuItem::action( - if user_is_admin { + if role == ChannelRole::Admin { "Make non-admin" } else { "Make admin" diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 3501e70e6ac4191c5fe3141620e18cd8f7b8c32d..dbd28bcf5de29b39a5d9eb2b94f97109ef200c76 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -144,7 +144,7 @@ message Envelope { DeleteChannel delete_channel = 118; GetChannelMembers get_channel_members = 119; GetChannelMembersResponse get_channel_members_response = 120; - SetChannelMemberAdmin set_channel_member_admin = 121; + SetChannelMemberRole set_channel_member_role = 145; RenameChannel rename_channel = 122; RenameChannelResponse rename_channel_response = 123; @@ -170,7 +170,7 @@ message Envelope { LinkChannel link_channel = 140; UnlinkChannel unlink_channel = 141; - MoveChannel move_channel = 142; // current max: 144 + MoveChannel move_channel = 142; // current max: 145 } } @@ -979,7 +979,7 @@ message ChannelEdge { message ChannelPermission { uint64 channel_id = 1; - bool is_admin = 2; + ChannelRole role = 3; } message ChannelParticipants { @@ -1005,8 +1005,8 @@ message GetChannelMembersResponse { message ChannelMember { uint64 user_id = 1; - bool admin = 2; Kind kind = 3; + ChannelRole role = 4; enum Kind { Member = 0; @@ -1028,7 +1028,7 @@ message CreateChannelResponse { message InviteChannelMember { uint64 channel_id = 1; uint64 user_id = 2; - bool admin = 3; + ChannelRole role = 4; } message RemoveChannelMember { @@ -1036,10 +1036,16 @@ message RemoveChannelMember { uint64 user_id = 2; } -message SetChannelMemberAdmin { +enum ChannelRole { + Admin = 0; + Member = 1; + Guest = 2; +} + +message SetChannelMemberRole { uint64 channel_id = 1; uint64 user_id = 2; - bool admin = 3; + ChannelRole role = 3; } message RenameChannel { diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index f0d7937f6f9592b2574bff7b93d3097ceba24d94..57292a52ca21db846ca426945b40b2dd08738370 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -230,7 +230,7 @@ messages!( (SaveBuffer, Foreground), (RenameChannel, Foreground), (RenameChannelResponse, Foreground), - (SetChannelMemberAdmin, Foreground), + (SetChannelMemberRole, Foreground), (SearchProject, Background), (SearchProjectResponse, Background), (ShareProject, Foreground), @@ -326,7 +326,7 @@ request_messages!( (RemoveContact, Ack), (RespondToContactRequest, Ack), (RespondToChannelInvite, Ack), - (SetChannelMemberAdmin, Ack), + (SetChannelMemberRole, Ack), (SendChannelMessage, SendChannelMessageResponse), (GetChannelMessages, GetChannelMessagesResponse), (GetChannelMembers, GetChannelMembersResponse), From 78432d08ca7c120e246ec854ca34ff224374dab8 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Thu, 12 Oct 2023 12:21:09 -0700 Subject: [PATCH 03/25] Add channel visibility columns and protos --- crates/channel/src/channel_store_tests.rs | 10 +++++- .../20231011214412_add_guest_role.sql | 4 +-- crates/collab/src/db/ids.rs | 35 +++++++++++++++++++ crates/collab/src/db/tables/channel.rs | 3 +- crates/collab/src/rpc.rs | 20 +++++++++-- crates/rpc/proto/zed.proto | 6 ++++ 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index f8828159bdc5049adc5b789383ee72c3eca8629c..faa0ade51d215f3ef5a78d001fd5779405a52c48 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -3,7 +3,7 @@ use crate::channel_chat::ChannelChatEvent; use super::*; use client::{test::FakeServer, Client, UserStore}; use gpui::{AppContext, ModelHandle, TestAppContext}; -use rpc::proto; +use rpc::proto::{self, ChannelRole}; use settings::SettingsStore; use util::http::FakeHttpClient; @@ -18,10 +18,12 @@ fn test_update_channels(cx: &mut AppContext) { proto::Channel { id: 1, name: "b".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }, proto::Channel { id: 2, name: "a".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }, ], channel_permissions: vec![proto::ChannelPermission { @@ -49,10 +51,12 @@ fn test_update_channels(cx: &mut AppContext) { proto::Channel { id: 3, name: "x".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }, proto::Channel { id: 4, name: "y".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }, ], insert_edge: vec![ @@ -92,14 +96,17 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { proto::Channel { id: 0, name: "a".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }, proto::Channel { id: 1, name: "b".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }, proto::Channel { id: 2, name: "c".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }, ], insert_edge: vec![ @@ -158,6 +165,7 @@ async fn test_channel_messages(cx: &mut TestAppContext) { channels: vec![proto::Channel { id: channel_id, name: "the-channel".to_string(), + visibility: proto::ChannelVisibility::ChannelMembers as i32, }], ..Default::default() }); diff --git a/crates/collab/migrations/20231011214412_add_guest_role.sql b/crates/collab/migrations/20231011214412_add_guest_role.sql index 378590a0f9702fd63630a32957780096e3d7ba56..bd178ec63d4723a2f16bec51bdc71ab22fa13a3a 100644 --- a/crates/collab/migrations/20231011214412_add_guest_role.sql +++ b/crates/collab/migrations/20231011214412_add_guest_role.sql @@ -1,4 +1,4 @@ --- Add migration script here - ALTER TABLE channel_members ADD COLUMN role TEXT; UPDATE channel_members SET role = CASE WHEN admin THEN 'admin' ELSE 'member' END; + +ALTER TABLE channels ADD COLUMN visibility TEXT NOT NULL DEFAULT 'channel_members'; diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 946702f36ce5eee521741d4556ea201d0966ab34..d2e990a6409429307d03130e4bf1cc19093b0e57 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -119,3 +119,38 @@ impl Into for ChannelRole { proto.into() } } + +#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default)] +#[sea_orm(rs_type = "String", db_type = "String(None)")] +pub enum ChannelVisibility { + #[sea_orm(string_value = "public")] + Public, + #[sea_orm(string_value = "channel_members")] + #[default] + ChannelMembers, +} + +impl From for ChannelVisibility { + fn from(value: proto::ChannelVisibility) -> Self { + match value { + proto::ChannelVisibility::Public => ChannelVisibility::Public, + proto::ChannelVisibility::ChannelMembers => ChannelVisibility::ChannelMembers, + } + } +} + +impl Into for ChannelVisibility { + fn into(self) -> proto::ChannelVisibility { + match self { + ChannelVisibility::Public => proto::ChannelVisibility::Public, + ChannelVisibility::ChannelMembers => proto::ChannelVisibility::ChannelMembers, + } + } +} + +impl Into for ChannelVisibility { + fn into(self) -> i32 { + let proto: proto::ChannelVisibility = self.into(); + proto.into() + } +} diff --git a/crates/collab/src/db/tables/channel.rs b/crates/collab/src/db/tables/channel.rs index 54f12defc1b56570a0629e2e92a896ad167aa6d6..efda02ec43e18fe582637c60e3c362077b4dd9af 100644 --- a/crates/collab/src/db/tables/channel.rs +++ b/crates/collab/src/db/tables/channel.rs @@ -1,4 +1,4 @@ -use crate::db::ChannelId; +use crate::db::{ChannelId, ChannelVisibility}; use sea_orm::entity::prelude::*; #[derive(Clone, Debug, Default, PartialEq, Eq, DeriveEntityModel)] @@ -7,6 +7,7 @@ pub struct Model { #[sea_orm(primary_key)] pub id: ChannelId, pub name: String, + pub visbility: ChannelVisibility, } impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index b05421e9601f293501448b935db357e94ebc2dee..962a032ece1a8cdb33fe1a0ecf391c7514a88baa 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -38,8 +38,8 @@ use lazy_static::lazy_static; use prometheus::{register_int_gauge, IntGauge}; use rpc::{ proto::{ - self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, - LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, + self, Ack, AnyTypedEnvelope, ChannelEdge, ChannelVisibility, EntityMessage, + EnvelopedMessage, LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, }, Connection, ConnectionId, Peer, Receipt, TypedEnvelope, }; @@ -2210,6 +2210,8 @@ async fn create_channel( let channel = proto::Channel { id: id.to_proto(), name: request.name, + // TODO: Visibility + visibility: proto::ChannelVisibility::ChannelMembers as i32, }; response.send(proto::CreateChannelResponse { @@ -2299,6 +2301,8 @@ async fn invite_channel_member( update.channel_invitations.push(proto::Channel { id: channel.id.to_proto(), name: channel.name, + // TODO: Visibility + visibility: proto::ChannelVisibility::ChannelMembers as i32, }); for connection_id in session .connection_pool() @@ -2394,6 +2398,8 @@ async fn rename_channel( let channel = proto::Channel { id: request.channel_id, name: new_name, + // TODO: Visibility + visibility: proto::ChannelVisibility::ChannelMembers as i32, }; response.send(proto::RenameChannelResponse { channel: Some(channel.clone()), @@ -2432,6 +2438,8 @@ async fn link_channel( .map(|channel| proto::Channel { id: channel.id.to_proto(), name: channel.name, + // TODO: Visibility + visibility: proto::ChannelVisibility::ChannelMembers as i32, }) .collect(), insert_edge: channels_to_send.edges, @@ -2523,6 +2531,8 @@ async fn move_channel( .map(|channel| proto::Channel { id: channel.id.to_proto(), name: channel.name, + // TODO: Visibility + visibility: proto::ChannelVisibility::ChannelMembers as i32, }) .collect(), insert_edge: channels_to_send.edges, @@ -2579,6 +2589,8 @@ async fn respond_to_channel_invite( .map(|channel| proto::Channel { id: channel.id.to_proto(), name: channel.name, + // TODO: Visibility + visibility: ChannelVisibility::ChannelMembers.into(), }), ); update.unseen_channel_messages = result.channel_messages; @@ -3082,6 +3094,8 @@ fn build_initial_channels_update( update.channels.push(proto::Channel { id: channel.id.to_proto(), name: channel.name, + // TODO: Visibility + visibility: ChannelVisibility::Public.into(), }); } @@ -3114,6 +3128,8 @@ fn build_initial_channels_update( update.channel_invitations.push(proto::Channel { id: channel.id.to_proto(), name: channel.name, + // TODO: Visibility + visibility: ChannelVisibility::Public.into(), }); } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index dbd28bcf5de29b39a5d9eb2b94f97109ef200c76..fec56ad9dc6377006efb84869d4d08a2114afe09 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -1539,9 +1539,15 @@ message Nonce { uint64 lower_half = 2; } +enum ChannelVisibility { + Public = 0; + ChannelMembers = 1; +} + message Channel { uint64 id = 1; string name = 2; + ChannelVisibility visibility = 3; } message Contact { From a7db2aa39dfd5293c0569db22fa132887f53c63c Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Thu, 12 Oct 2023 19:59:50 -0600 Subject: [PATCH 04/25] Add check_is_channel_participant Refactor permission checks to load ancestor permissions into memory for all checks to make the different logics more explicit. --- .../20221109000000_test_schema.sql | 3 +- crates/collab/src/db/ids.rs | 4 + crates/collab/src/db/queries/channels.rs | 180 +++++++++++++++--- crates/collab/src/db/tables/channel.rs | 2 +- crates/collab/src/db/tests/channel_tests.rs | 121 +++++++++++- crates/collab/src/tests/channel_tests.rs | 5 +- crates/rpc/proto/zed.proto | 1 + 7 files changed, 285 insertions(+), 31 deletions(-) diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index dd6e80150be6034c02368372d2ff6f6936a00ef7..dcb793aa513d0bde1a5e6f5787dc0d7f95fead99 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -192,7 +192,8 @@ CREATE INDEX "index_followers_on_room_id" ON "followers" ("room_id"); CREATE TABLE "channels" ( "id" INTEGER PRIMARY KEY AUTOINCREMENT, "name" VARCHAR NOT NULL, - "created_at" TIMESTAMP NOT NULL DEFAULT now + "created_at" TIMESTAMP NOT NULL DEFAULT now, + "visibility" VARCHAR NOT NULL ); CREATE TABLE IF NOT EXISTS "channel_chat_participants" ( diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index d2e990a6409429307d03130e4bf1cc19093b0e57..5ba724dd1262fee425af8c6dd395416cd96aae50 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -91,6 +91,8 @@ pub enum ChannelRole { Member, #[sea_orm(string_value = "guest")] Guest, + #[sea_orm(string_value = "banned")] + Banned, } impl From for ChannelRole { @@ -99,6 +101,7 @@ impl From for ChannelRole { proto::ChannelRole::Admin => ChannelRole::Admin, proto::ChannelRole::Member => ChannelRole::Member, proto::ChannelRole::Guest => ChannelRole::Guest, + proto::ChannelRole::Banned => ChannelRole::Banned, } } } @@ -109,6 +112,7 @@ impl Into for ChannelRole { ChannelRole::Admin => proto::ChannelRole::Admin, ChannelRole::Member => proto::ChannelRole::Member, ChannelRole::Guest => proto::ChannelRole::Guest, + ChannelRole::Banned => proto::ChannelRole::Banned, } } } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 5c96955eba5cb88d55abe35506c6867466029eee..7ce20e1a20a8eb8a5cd2a3777c20a65fc7bdb6af 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -37,8 +37,9 @@ impl Database { } let channel = channel::ActiveModel { + id: ActiveValue::NotSet, name: ActiveValue::Set(name.to_string()), - ..Default::default() + visibility: ActiveValue::Set(ChannelVisibility::ChannelMembers), } .insert(&*tx) .await?; @@ -89,6 +90,29 @@ impl Database { .await } + pub async fn set_channel_visibility( + &self, + channel_id: ChannelId, + visibility: ChannelVisibility, + user_id: UserId, + ) -> Result<()> { + self.transaction(move |tx| async move { + self.check_user_is_channel_admin(channel_id, user_id, &*tx) + .await?; + + channel::ActiveModel { + id: ActiveValue::Unchanged(channel_id), + visibility: ActiveValue::Set(visibility), + ..Default::default() + } + .update(&*tx) + .await?; + + Ok(()) + }) + .await + } + pub async fn delete_channel( &self, channel_id: ChannelId, @@ -160,11 +184,11 @@ impl Database { &self, channel_id: ChannelId, invitee_id: UserId, - inviter_id: UserId, + admin_id: UserId, role: ChannelRole, ) -> Result<()> { self.transaction(move |tx| async move { - self.check_user_is_channel_admin(channel_id, inviter_id, &*tx) + self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; channel_member::ActiveModel { @@ -262,10 +286,10 @@ impl Database { &self, channel_id: ChannelId, member_id: UserId, - remover_id: UserId, + admin_id: UserId, ) -> Result<()> { self.transaction(|tx| async move { - self.check_user_is_channel_admin(channel_id, remover_id, &*tx) + self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; let result = channel_member::Entity::delete_many() @@ -481,12 +505,12 @@ impl Database { pub async fn set_channel_member_role( &self, channel_id: ChannelId, - from: UserId, + admin_id: UserId, for_user: UserId, role: ChannelRole, ) -> Result<()> { self.transaction(|tx| async move { - self.check_user_is_channel_admin(channel_id, from, &*tx) + self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; let result = channel_member::Entity::update_many() @@ -613,43 +637,147 @@ impl Database { Ok(user_ids) } + pub async fn check_user_is_channel_admin( + &self, + channel_id: ChannelId, + user_id: UserId, + tx: &DatabaseTransaction, + ) -> Result<()> { + match self.channel_role_for_user(channel_id, user_id, tx).await? { + Some(ChannelRole::Admin) => Ok(()), + Some(ChannelRole::Member) + | Some(ChannelRole::Banned) + | Some(ChannelRole::Guest) + | None => Err(anyhow!( + "user is not a channel admin or channel does not exist" + ))?, + } + } + pub async fn check_user_is_channel_member( &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)), - ) - .one(&*tx) - .await? - .ok_or_else(|| anyhow!("user is not a channel member or channel does not exist"))?; - Ok(()) + match self.channel_role_for_user(channel_id, user_id, tx).await? { + Some(ChannelRole::Admin) | Some(ChannelRole::Member) => Ok(()), + Some(ChannelRole::Banned) | Some(ChannelRole::Guest) | None => Err(anyhow!( + "user is not a channel member or channel does not exist" + ))?, + } } - pub async fn check_user_is_channel_admin( + pub async fn check_user_is_channel_participant( &self, channel_id: ChannelId, user_id: UserId, tx: &DatabaseTransaction, ) -> Result<()> { + match self.channel_role_for_user(channel_id, user_id, tx).await? { + Some(ChannelRole::Admin) | Some(ChannelRole::Member) | Some(ChannelRole::Guest) => { + Ok(()) + } + Some(ChannelRole::Banned) | None => Err(anyhow!( + "user is not a channel participant or channel does not exist" + ))?, + } + } + + pub async fn channel_role_for_user( + &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() + + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] + enum QueryChannelMembership { + ChannelId, + Role, + Admin, + Visibility, + } + + let mut rows = channel_member::Entity::find() + .left_join(channel::Entity) .filter( channel_member::Column::ChannelId .is_in(channel_ids) - .and(channel_member::Column::UserId.eq(user_id)) - .and(channel_member::Column::Admin.eq(true)), + .and(channel_member::Column::UserId.eq(user_id)), ) - .one(&*tx) - .await? - .ok_or_else(|| anyhow!("user is not a channel admin or channel does not exist"))?; - Ok(()) + .select_only() + .column(channel_member::Column::ChannelId) + .column(channel_member::Column::Role) + .column(channel_member::Column::Admin) + .column(channel::Column::Visibility) + .into_values::<_, QueryChannelMembership>() + .stream(&*tx) + .await?; + + let mut is_admin = false; + let mut is_member = false; + let mut is_participant = false; + let mut is_banned = false; + let mut current_channel_visibility = None; + + // note these channels are not iterated in any particular order, + // our current logic takes the highest permission available. + while let Some(row) = rows.next().await { + let (ch_id, role, admin, visibility): ( + ChannelId, + Option, + bool, + ChannelVisibility, + ) = row?; + match role { + Some(ChannelRole::Admin) => is_admin = true, + Some(ChannelRole::Member) => is_member = true, + Some(ChannelRole::Guest) => { + if visibility == ChannelVisibility::Public { + is_participant = true + } + } + Some(ChannelRole::Banned) => is_banned = true, + None => { + // rows created from pre-role collab server. + if admin { + is_admin = true + } else { + is_member = true + } + } + } + if channel_id == ch_id { + current_channel_visibility = Some(visibility); + } + } + // free up database connection + drop(rows); + + Ok(if is_admin { + Some(ChannelRole::Admin) + } else if is_member { + Some(ChannelRole::Member) + } else if is_banned { + Some(ChannelRole::Banned) + } else if is_participant { + if current_channel_visibility.is_none() { + current_channel_visibility = channel::Entity::find() + .filter(channel::Column::Id.eq(channel_id)) + .one(&*tx) + .await? + .map(|channel| channel.visibility); + } + if current_channel_visibility == Some(ChannelVisibility::Public) { + Some(ChannelRole::Guest) + } else { + None + } + } else { + None + }) } /// Returns the channel ancestors, deepest first diff --git a/crates/collab/src/db/tables/channel.rs b/crates/collab/src/db/tables/channel.rs index efda02ec43e18fe582637c60e3c362077b4dd9af..0975a8cc30c7109f4ad34a1039780b0b33471e77 100644 --- a/crates/collab/src/db/tables/channel.rs +++ b/crates/collab/src/db/tables/channel.rs @@ -7,7 +7,7 @@ pub struct Model { #[sea_orm(primary_key)] pub id: ChannelId, pub name: String, - pub visbility: ChannelVisibility, + pub visibility: ChannelVisibility, } impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 90b3a0cd2e6e8f98c1b258cb0d603d7c9cfddd68..2263920955e256a1dfda138433dc3ddc837a61b4 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -8,11 +8,14 @@ use crate::{ db::{ queries::channels::ChannelGraph, tests::{graph, TEST_RELEASE_CHANNEL}, - ChannelId, ChannelRole, Database, NewUserParams, + ChannelId, ChannelRole, Database, NewUserParams, UserId, }, test_both_dbs, }; -use std::sync::Arc; +use std::sync::{ + atomic::{AtomicI32, Ordering}, + Arc, +}; test_both_dbs!(test_channels, test_channels_postgres, test_channels_sqlite); @@ -850,6 +853,101 @@ async fn test_db_channel_moving_bugs(db: &Arc) { ); } +test_both_dbs!( + test_user_is_channel_participant, + test_user_is_channel_participant_postgres, + test_user_is_channel_participant_sqlite +); + +async fn test_user_is_channel_participant(db: &Arc) { + let admin_id = new_test_user(db, "admin@example.com").await; + let member_id = new_test_user(db, "member@example.com").await; + let guest_id = new_test_user(db, "guest@example.com").await; + + let zed_id = db.create_root_channel("zed", admin_id).await.unwrap(); + let intermediate_id = db + .create_channel("active", Some(zed_id), admin_id) + .await + .unwrap(); + let public_id = db + .create_channel("active", Some(intermediate_id), admin_id) + .await + .unwrap(); + + db.set_channel_visibility(public_id, crate::db::ChannelVisibility::Public, admin_id) + .await + .unwrap(); + db.invite_channel_member(intermediate_id, member_id, admin_id, ChannelRole::Member) + .await + .unwrap(); + db.invite_channel_member(public_id, guest_id, admin_id, ChannelRole::Guest) + .await + .unwrap(); + + db.transaction(|tx| async move { + db.check_user_is_channel_participant(public_id, admin_id, &*tx) + .await + }) + .await + .unwrap(); + db.transaction(|tx| async move { + db.check_user_is_channel_participant(public_id, member_id, &*tx) + .await + }) + .await + .unwrap(); + db.transaction(|tx| async move { + db.check_user_is_channel_participant(public_id, guest_id, &*tx) + .await + }) + .await + .unwrap(); + + db.set_channel_member_role(public_id, admin_id, guest_id, ChannelRole::Banned) + .await + .unwrap(); + assert!(db + .transaction(|tx| async move { + db.check_user_is_channel_participant(public_id, guest_id, &*tx) + .await + }) + .await + .is_err()); + + db.remove_channel_member(public_id, guest_id, admin_id) + .await + .unwrap(); + + db.set_channel_visibility(zed_id, crate::db::ChannelVisibility::Public, admin_id) + .await + .unwrap(); + + db.invite_channel_member(zed_id, guest_id, admin_id, ChannelRole::Guest) + .await + .unwrap(); + + db.transaction(|tx| async move { + db.check_user_is_channel_participant(zed_id, guest_id, &*tx) + .await + }) + .await + .unwrap(); + assert!(db + .transaction(|tx| async move { + db.check_user_is_channel_participant(intermediate_id, guest_id, &*tx) + .await + }) + .await + .is_err(),); + + db.transaction(|tx| async move { + db.check_user_is_channel_participant(public_id, guest_id, &*tx) + .await + }) + .await + .unwrap(); +} + #[track_caller] fn assert_dag(actual: ChannelGraph, expected: &[(ChannelId, Option)]) { let mut actual_map: HashMap> = HashMap::default(); @@ -874,3 +972,22 @@ fn assert_dag(actual: ChannelGraph, expected: &[(ChannelId, Option)]) pretty_assertions::assert_eq!(actual_map, expected_map) } + +static GITHUB_USER_ID: AtomicI32 = AtomicI32::new(5); + +async fn new_test_user(db: &Arc, email: &str) -> UserId { + let gid = GITHUB_USER_ID.fetch_add(1, Ordering::SeqCst); + + db.create_user( + email, + false, + NewUserParams { + github_login: email[0..email.find("@").unwrap()].to_string(), + github_user_id: GITHUB_USER_ID.fetch_add(1, Ordering::SeqCst), + invite_count: 0, + }, + ) + .await + .unwrap() + .user_id +} diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index bc814d06a26721963fd82b345cb7dc85aee6771a..95a672e76c7b70e18c7deadbc1a19193655a8dcd 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -6,7 +6,10 @@ use call::ActiveCall; use channel::{ChannelId, ChannelMembership, ChannelStore}; use client::User; use gpui::{executor::Deterministic, ModelHandle, TestAppContext}; -use rpc::{proto, RECEIVE_TIMEOUT}; +use rpc::{ + proto::{self}, + RECEIVE_TIMEOUT, +}; use std::sync::Arc; #[gpui::test] diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index fec56ad9dc6377006efb84869d4d08a2114afe09..90e425a39ffef4ab04b05f65e94951296768124e 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -1040,6 +1040,7 @@ enum ChannelRole { Admin = 0; Member = 1; Guest = 2; + Banned = 3; } message SetChannelMemberRole { From da2b8082b36d704131e6ce9f2555b8a17ca6ca35 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Thu, 12 Oct 2023 20:42:42 -0600 Subject: [PATCH 05/25] Rename members to participants in db crate --- crates/collab/src/db/queries/buffers.rs | 4 +++- crates/collab/src/db/queries/channels.rs | 6 +++--- crates/collab/src/db/queries/messages.rs | 4 +++- crates/collab/src/db/queries/rooms.rs | 13 +++++++++---- crates/collab/src/db/tests/channel_tests.rs | 4 ++-- crates/collab/src/rpc.rs | 2 +- 6 files changed, 21 insertions(+), 12 deletions(-) diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index c85432f2bba1b62a1e346626171fb59bfb8ef7ac..69f100e6b8351a5e88d8fbda94aeb325e899634e 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -482,7 +482,9 @@ impl Database { ) .await?; - channel_members = self.get_channel_members_internal(channel_id, &*tx).await?; + channel_members = self + .get_channel_participants_internal(channel_id, &*tx) + .await?; let collaborators = self .get_channel_buffer_collaborators_internal(channel_id, &*tx) .await?; diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 7ce20e1a20a8eb8a5cd2a3777c20a65fc7bdb6af..a9601d54c827a378b3bc0d149f5567596d575778 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -498,7 +498,7 @@ 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 }) + self.transaction(|tx| async move { self.get_channel_participants_internal(id, &*tx).await }) .await } @@ -536,7 +536,7 @@ impl Database { .await } - pub async fn get_channel_member_details( + pub async fn get_channel_participant_details( &self, channel_id: ChannelId, user_id: UserId, @@ -616,7 +616,7 @@ impl Database { .await } - pub async fn get_channel_members_internal( + pub async fn get_channel_participants_internal( &self, id: ChannelId, tx: &DatabaseTransaction, diff --git a/crates/collab/src/db/queries/messages.rs b/crates/collab/src/db/queries/messages.rs index a48d425d90ee6021dd13f3c06febba15beec041f..7b389197750f0a2423a714e85fc389c35f9d2dc6 100644 --- a/crates/collab/src/db/queries/messages.rs +++ b/crates/collab/src/db/queries/messages.rs @@ -180,7 +180,9 @@ impl Database { ) .await?; - let mut channel_members = self.get_channel_members_internal(channel_id, &*tx).await?; + let mut channel_members = self + .get_channel_participants_internal(channel_id, &*tx) + .await?; channel_members.retain(|member| !participant_user_ids.contains(member)); Ok(( diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index a38c77dc0fc67eab6ea18da14dad43dd612bf69d..625615db5f2b5316dc9f2fbb7a9ba2ac0e98d242 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -53,7 +53,9 @@ impl Database { let (channel_id, room) = self.get_channel_room(room_id, &tx).await?; let channel_members; if let Some(channel_id) = channel_id { - channel_members = self.get_channel_members_internal(channel_id, &tx).await?; + channel_members = self + .get_channel_participants_internal(channel_id, &tx) + .await?; } else { channel_members = Vec::new(); @@ -377,7 +379,8 @@ impl Database { let room = self.get_room(room_id, &tx).await?; let channel_members = if let Some(channel_id) = channel_id { - self.get_channel_members_internal(channel_id, &tx).await? + self.get_channel_participants_internal(channel_id, &tx) + .await? } else { Vec::new() }; @@ -681,7 +684,8 @@ 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? + self.get_channel_participants_internal(channel_id, &tx) + .await? } else { Vec::new() }; @@ -839,7 +843,8 @@ impl Database { }; let channel_members = if let Some(channel_id) = channel_id { - self.get_channel_members_internal(channel_id, &tx).await? + self.get_channel_participants_internal(channel_id, &tx) + .await? } else { Vec::new() }; diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 2263920955e256a1dfda138433dc3ddc837a61b4..846af94a523ed5be7637afc2f0e6ad5213eb9a8c 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -322,7 +322,7 @@ async fn test_channel_invites(db: &Arc) { assert_eq!(user_3_invites, &[channel_1_1]); let members = db - .get_channel_member_details(channel_1_1, user_1) + .get_channel_participant_details(channel_1_1, user_1) .await .unwrap(); assert_eq!( @@ -356,7 +356,7 @@ async fn test_channel_invites(db: &Arc) { .unwrap(); let members = db - .get_channel_member_details(channel_1_3, user_1) + .get_channel_participant_details(channel_1_3, user_1) .await .unwrap(); assert_eq!( diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 962a032ece1a8cdb33fe1a0ecf391c7514a88baa..f8ac77325c951633df3227eab72c92f08081893b 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -2557,7 +2557,7 @@ async fn get_channel_members( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let members = db - .get_channel_member_details(channel_id, session.user_id) + .get_channel_participant_details(channel_id, session.user_id) .await?; response.send(proto::GetChannelMembersResponse { members })?; Ok(()) From 65a0ebf97598134e19a55d89e324e6655f208d28 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Thu, 12 Oct 2023 21:36:21 -0600 Subject: [PATCH 06/25] Update get_channel_participant_details to include guests --- crates/collab/src/db/ids.rs | 12 ++ crates/collab/src/db/queries/channels.rs | 106 +++++++--- crates/collab/src/db/tests/channel_tests.rs | 203 +++++++++++++------- 3 files changed, 231 insertions(+), 90 deletions(-) diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 5ba724dd1262fee425af8c6dd395416cd96aae50..ee8c879ed3da3fc3ec68a1cece455095380f7768 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -95,6 +95,18 @@ pub enum ChannelRole { Banned, } +impl ChannelRole { + pub fn should_override(&self, other: Self) -> bool { + use ChannelRole::*; + match self { + Admin => matches!(other, Member | Banned | Guest), + Member => matches!(other, Banned | Guest), + Banned => matches!(other, Guest), + Guest => false, + } + } +} + impl From for ChannelRole { fn from(value: proto::ChannelRole) -> Self { match value { diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index a9601d54c827a378b3bc0d149f5567596d575778..4cb3d00b167fa3d9d857f6dea8dc43ddbd7e343b 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -1,5 +1,7 @@ +use std::cmp::Ordering; + use super::*; -use rpc::proto::ChannelEdge; +use rpc::proto::{channel_member::Kind, ChannelEdge}; use smallvec::SmallVec; type ChannelDescendants = HashMap>; @@ -539,12 +541,19 @@ impl Database { pub async fn get_channel_participant_details( &self, channel_id: ChannelId, - user_id: UserId, + admin_id: UserId, ) -> Result> { self.transaction(|tx| async move { - self.check_user_is_channel_admin(channel_id, user_id, &*tx) + self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; + let channel_visibility = channel::Entity::find() + .filter(channel::Column::Id.eq(channel_id)) + .one(&*tx) + .await? + .map(|channel| channel.visibility) + .unwrap_or(ChannelVisibility::ChannelMembers); + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] enum QueryMemberDetails { UserId, @@ -552,12 +561,13 @@ impl Database { Role, IsDirectMember, Accepted, + Visibility, } let tx = tx; let ancestor_ids = self.get_channel_ancestors(channel_id, &*tx).await?; let mut stream = channel_member::Entity::find() - .distinct() + .left_join(channel::Entity) .filter(channel_member::Column::ChannelId.is_in(ancestor_ids.iter().copied())) .select_only() .column(channel_member::Column::UserId) @@ -568,19 +578,32 @@ impl Database { QueryMemberDetails::IsDirectMember, ) .column(channel_member::Column::Accepted) - .order_by_asc(channel_member::Column::UserId) + .column(channel::Column::Visibility) .into_values::<_, QueryMemberDetails>() .stream(&*tx) .await?; - let mut rows = Vec::::new(); + struct UserDetail { + kind: Kind, + channel_role: ChannelRole, + } + let mut user_details: HashMap = HashMap::default(); + while let Some(row) = stream.next().await { - let (user_id, is_admin, channel_role, is_direct_member, is_invite_accepted): ( + let ( + user_id, + is_admin, + channel_role, + is_direct_member, + is_invite_accepted, + visibility, + ): ( UserId, bool, Option, bool, bool, + ChannelVisibility, ) = row?; let kind = match (is_direct_member, is_invite_accepted) { (true, true) => proto::channel_member::Kind::Member, @@ -593,25 +616,64 @@ impl Database { } else { ChannelRole::Member }); - 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 { - if is_direct_member { - last_row.kind = kind; - last_row.role = channel_role.into() - } - continue; + + if channel_role == ChannelRole::Guest + && visibility != ChannelVisibility::Public + && channel_visibility != ChannelVisibility::Public + { + continue; + } + + if let Some(details_mut) = user_details.get_mut(&user_id) { + if channel_role.should_override(details_mut.channel_role) { + details_mut.channel_role = channel_role; + } + if kind == Kind::Member { + details_mut.kind = kind; + // the UI is going to be a bit confusing if you already have permissions + // that are greater than or equal to the ones you're being invited to. + } else if kind == Kind::Invitee && details_mut.kind == Kind::AncestorMember { + details_mut.kind = kind; } + } else { + user_details.insert(user_id, UserDetail { kind, channel_role }); } - rows.push(proto::ChannelMember { - user_id, - kind, - role: channel_role.into(), - }); } - Ok(rows) + // sort by permissions descending, within each section, show members, then ancestor members, then invitees. + let mut results: Vec<(UserId, UserDetail)> = user_details.into_iter().collect(); + results.sort_by(|a, b| { + if a.1.channel_role.should_override(b.1.channel_role) { + return Ordering::Less; + } else if b.1.channel_role.should_override(a.1.channel_role) { + return Ordering::Greater; + } + + if a.1.kind == Kind::Member && b.1.kind != Kind::Member { + return Ordering::Less; + } else if b.1.kind == Kind::Member && a.1.kind != Kind::Member { + return Ordering::Greater; + } + + if a.1.kind == Kind::AncestorMember && b.1.kind != Kind::AncestorMember { + return Ordering::Less; + } else if b.1.kind == Kind::AncestorMember && a.1.kind != Kind::AncestorMember { + return Ordering::Greater; + } + + // would be nice to sort alphabetically instead of by user id. + // (or defer all sorting to the UI, but we need something to help the tests) + return a.0.cmp(&b.0); + }); + + Ok(results + .into_iter() + .map(|(user_id, details)| proto::ChannelMember { + user_id: user_id.to_proto(), + kind: details.kind.into(), + role: details.channel_role.into(), + }) + .collect()) }) .await } diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 846af94a523ed5be7637afc2f0e6ad5213eb9a8c..2044310d8e025119de85dbb49cde3149ced93c6c 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -246,46 +246,9 @@ test_both_dbs!( async fn test_channel_invites(db: &Arc) { db.create_server("test").await.unwrap(); - let user_1 = db - .create_user( - "user1@example.com", - false, - NewUserParams { - github_login: "user1".into(), - github_user_id: 5, - invite_count: 0, - }, - ) - .await - .unwrap() - .user_id; - let user_2 = db - .create_user( - "user2@example.com", - false, - NewUserParams { - github_login: "user2".into(), - github_user_id: 6, - invite_count: 0, - }, - ) - .await - .unwrap() - .user_id; - - let user_3 = db - .create_user( - "user3@example.com", - false, - NewUserParams { - github_login: "user3".into(), - github_user_id: 7, - invite_count: 0, - }, - ) - .await - .unwrap() - .user_id; + let user_1 = new_test_user(db, "user1@example.com").await; + let user_2 = new_test_user(db, "user2@example.com").await; + let user_3 = new_test_user(db, "user3@example.com").await; let channel_1_1 = db.create_root_channel("channel_1", user_1).await.unwrap(); @@ -334,14 +297,14 @@ async fn test_channel_invites(db: &Arc) { role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { - user_id: user_2.to_proto(), + user_id: user_3.to_proto(), kind: proto::channel_member::Kind::Invitee.into(), - role: proto::ChannelRole::Member.into(), + role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { - user_id: user_3.to_proto(), + user_id: user_2.to_proto(), kind: proto::channel_member::Kind::Invitee.into(), - role: proto::ChannelRole::Admin.into(), + role: proto::ChannelRole::Member.into(), }, ] ); @@ -860,92 +823,198 @@ test_both_dbs!( ); async fn test_user_is_channel_participant(db: &Arc) { - let admin_id = new_test_user(db, "admin@example.com").await; - let member_id = new_test_user(db, "member@example.com").await; - let guest_id = new_test_user(db, "guest@example.com").await; + let admin = new_test_user(db, "admin@example.com").await; + let member = new_test_user(db, "member@example.com").await; + let guest = new_test_user(db, "guest@example.com").await; - let zed_id = db.create_root_channel("zed", admin_id).await.unwrap(); - let intermediate_id = db - .create_channel("active", Some(zed_id), admin_id) + let zed_channel = db.create_root_channel("zed", admin).await.unwrap(); + let active_channel = db + .create_channel("active", Some(zed_channel), admin) .await .unwrap(); - let public_id = db - .create_channel("active", Some(intermediate_id), admin_id) + let vim_channel = db + .create_channel("vim", Some(active_channel), admin) .await .unwrap(); - db.set_channel_visibility(public_id, crate::db::ChannelVisibility::Public, admin_id) + db.set_channel_visibility(vim_channel, crate::db::ChannelVisibility::Public, admin) + .await + .unwrap(); + db.invite_channel_member(active_channel, member, admin, ChannelRole::Member) .await .unwrap(); - db.invite_channel_member(intermediate_id, member_id, admin_id, ChannelRole::Member) + db.invite_channel_member(vim_channel, guest, admin, ChannelRole::Guest) .await .unwrap(); - db.invite_channel_member(public_id, guest_id, admin_id, ChannelRole::Guest) + + db.respond_to_channel_invite(active_channel, member, true) .await .unwrap(); db.transaction(|tx| async move { - db.check_user_is_channel_participant(public_id, admin_id, &*tx) + db.check_user_is_channel_participant(vim_channel, admin, &*tx) .await }) .await .unwrap(); db.transaction(|tx| async move { - db.check_user_is_channel_participant(public_id, member_id, &*tx) + db.check_user_is_channel_participant(vim_channel, member, &*tx) .await }) .await .unwrap(); db.transaction(|tx| async move { - db.check_user_is_channel_participant(public_id, guest_id, &*tx) + db.check_user_is_channel_participant(vim_channel, guest, &*tx) .await }) .await .unwrap(); - db.set_channel_member_role(public_id, admin_id, guest_id, ChannelRole::Banned) + let members = db + .get_channel_participant_details(vim_channel, admin) + .await + .unwrap(); + assert_eq!( + members, + &[ + proto::ChannelMember { + user_id: admin.to_proto(), + kind: proto::channel_member::Kind::Member.into(), + role: proto::ChannelRole::Admin.into(), + }, + proto::ChannelMember { + user_id: member.to_proto(), + kind: proto::channel_member::Kind::AncestorMember.into(), + role: proto::ChannelRole::Member.into(), + }, + proto::ChannelMember { + user_id: guest.to_proto(), + kind: proto::channel_member::Kind::Invitee.into(), + role: proto::ChannelRole::Guest.into(), + }, + ] + ); + + db.set_channel_member_role(vim_channel, admin, guest, ChannelRole::Banned) .await .unwrap(); assert!(db .transaction(|tx| async move { - db.check_user_is_channel_participant(public_id, guest_id, &*tx) + db.check_user_is_channel_participant(vim_channel, guest, &*tx) .await }) .await .is_err()); - db.remove_channel_member(public_id, guest_id, admin_id) + let members = db + .get_channel_participant_details(vim_channel, admin) + .await + .unwrap(); + + assert_eq!( + members, + &[ + proto::ChannelMember { + user_id: admin.to_proto(), + kind: proto::channel_member::Kind::Member.into(), + role: proto::ChannelRole::Admin.into(), + }, + proto::ChannelMember { + user_id: member.to_proto(), + kind: proto::channel_member::Kind::AncestorMember.into(), + role: proto::ChannelRole::Member.into(), + }, + proto::ChannelMember { + user_id: guest.to_proto(), + kind: proto::channel_member::Kind::Invitee.into(), + role: proto::ChannelRole::Banned.into(), + }, + ] + ); + + db.remove_channel_member(vim_channel, guest, admin) .await .unwrap(); - db.set_channel_visibility(zed_id, crate::db::ChannelVisibility::Public, admin_id) + db.set_channel_visibility(zed_channel, crate::db::ChannelVisibility::Public, admin) .await .unwrap(); - db.invite_channel_member(zed_id, guest_id, admin_id, ChannelRole::Guest) + db.invite_channel_member(zed_channel, guest, admin, ChannelRole::Guest) .await .unwrap(); db.transaction(|tx| async move { - db.check_user_is_channel_participant(zed_id, guest_id, &*tx) + db.check_user_is_channel_participant(zed_channel, guest, &*tx) .await }) .await .unwrap(); assert!(db .transaction(|tx| async move { - db.check_user_is_channel_participant(intermediate_id, guest_id, &*tx) + db.check_user_is_channel_participant(active_channel, guest, &*tx) .await }) .await .is_err(),); db.transaction(|tx| async move { - db.check_user_is_channel_participant(public_id, guest_id, &*tx) + db.check_user_is_channel_participant(vim_channel, guest, &*tx) .await }) .await .unwrap(); + + // currently people invited to parent channels are not shown here + // (though they *do* have permissions!) + let members = db + .get_channel_participant_details(vim_channel, admin) + .await + .unwrap(); + assert_eq!( + members, + &[ + proto::ChannelMember { + user_id: admin.to_proto(), + kind: proto::channel_member::Kind::Member.into(), + role: proto::ChannelRole::Admin.into(), + }, + proto::ChannelMember { + user_id: member.to_proto(), + kind: proto::channel_member::Kind::AncestorMember.into(), + role: proto::ChannelRole::Member.into(), + }, + ] + ); + + db.respond_to_channel_invite(zed_channel, guest, true) + .await + .unwrap(); + + let members = db + .get_channel_participant_details(vim_channel, admin) + .await + .unwrap(); + assert_eq!( + members, + &[ + proto::ChannelMember { + user_id: admin.to_proto(), + kind: proto::channel_member::Kind::Member.into(), + role: proto::ChannelRole::Admin.into(), + }, + proto::ChannelMember { + user_id: member.to_proto(), + kind: proto::channel_member::Kind::AncestorMember.into(), + role: proto::ChannelRole::Member.into(), + }, + proto::ChannelMember { + user_id: guest.to_proto(), + kind: proto::channel_member::Kind::AncestorMember.into(), + role: proto::ChannelRole::Guest.into(), + }, + ] + ); } #[track_caller] @@ -976,8 +1045,6 @@ fn assert_dag(actual: ChannelGraph, expected: &[(ChannelId, Option)]) static GITHUB_USER_ID: AtomicI32 = AtomicI32::new(5); async fn new_test_user(db: &Arc, email: &str) -> UserId { - let gid = GITHUB_USER_ID.fetch_add(1, Ordering::SeqCst); - db.create_user( email, false, From a8e352a473eac6d3581ba3ee39bd0ee6da814752 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 11:34:13 -0600 Subject: [PATCH 07/25] Rewrite get_user_channels with new permissions --- crates/channel/src/channel_store_tests.rs | 2 +- crates/collab/src/db/queries/channels.rs | 174 ++++++++++++++++++++-- 2 files changed, 159 insertions(+), 17 deletions(-) diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index faa0ade51d215f3ef5a78d001fd5779405a52c48..ea47c7c7b7fe0f5db2c8776cebc7791d9b306a91 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -3,7 +3,7 @@ use crate::channel_chat::ChannelChatEvent; use super::*; use client::{test::FakeServer, Client, UserStore}; use gpui::{AppContext, ModelHandle, TestAppContext}; -use rpc::proto::{self, ChannelRole}; +use rpc::proto::{self}; use settings::SettingsStore; use util::http::FakeHttpClient; diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 4cb3d00b167fa3d9d857f6dea8dc43ddbd7e343b..625655f27735f707d4f05e8b28041406b3f89be9 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -439,25 +439,108 @@ impl Database { channel_memberships: Vec, tx: &DatabaseTransaction, ) -> Result { - let parents_by_child_id = self - .get_channel_descendants(channel_memberships.iter().map(|m| m.channel_id), &*tx) + let mut edges = self + .get_channel_descendants_2(channel_memberships.iter().map(|m| m.channel_id), &*tx) .await?; - let channels_with_admin_privileges = channel_memberships - .iter() - .filter_map(|membership| { - if membership.role == Some(ChannelRole::Admin) || membership.admin { - Some(membership.channel_id) + let mut role_for_channel: HashMap = HashMap::default(); + + for membership in channel_memberships.iter() { + role_for_channel.insert( + membership.channel_id, + membership.role.unwrap_or(if membership.admin { + ChannelRole::Admin } else { - None + ChannelRole::Member + }), + ); + } + + for ChannelEdge { + parent_id, + channel_id, + } in edges.iter() + { + let parent_id = ChannelId::from_proto(*parent_id); + let channel_id = ChannelId::from_proto(*channel_id); + debug_assert!(role_for_channel.get(&parent_id).is_some()); + let parent_role = role_for_channel[&parent_id]; + if let Some(existing_role) = role_for_channel.get(&channel_id) { + if existing_role.should_override(parent_role) { + continue; } - }) - .collect(); + } + role_for_channel.insert(channel_id, parent_role); + } + + let mut channels: Vec = Vec::new(); + let mut channels_with_admin_privileges: HashSet = HashSet::default(); + let mut channels_to_remove: HashSet = HashSet::default(); - let graph = self - .get_channel_graph(parents_by_child_id, true, &tx) + let mut rows = channel::Entity::find() + .filter(channel::Column::Id.is_in(role_for_channel.keys().cloned())) + .stream(&*tx) .await?; + while let Some(row) = rows.next().await { + let channel = row?; + let role = role_for_channel[&channel.id]; + + if role == ChannelRole::Banned + || role == ChannelRole::Guest && channel.visibility != ChannelVisibility::Public + { + channels_to_remove.insert(channel.id.0 as u64); + continue; + } + + channels.push(Channel { + id: channel.id, + name: channel.name, + }); + + if role == ChannelRole::Admin { + channels_with_admin_privileges.insert(channel.id); + } + } + drop(rows); + + if !channels_to_remove.is_empty() { + // Note: this code assumes each channel has one parent. + let mut replacement_parent: HashMap = HashMap::default(); + for ChannelEdge { + parent_id, + channel_id, + } in edges.iter() + { + if channels_to_remove.contains(channel_id) { + replacement_parent.insert(*channel_id, *parent_id); + } + } + + let mut new_edges: Vec = Vec::new(); + 'outer: for ChannelEdge { + mut parent_id, + channel_id, + } in edges.iter() + { + if channels_to_remove.contains(channel_id) { + continue; + } + while channels_to_remove.contains(&parent_id) { + if let Some(new_parent_id) = replacement_parent.get(&parent_id) { + parent_id = *new_parent_id; + } else { + continue 'outer; + } + } + new_edges.push(ChannelEdge { + parent_id, + channel_id: *channel_id, + }) + } + edges = new_edges; + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] enum QueryUserIdsAndChannelIds { ChannelId, @@ -468,7 +551,7 @@ impl Database { { let mut rows = room_participant::Entity::find() .inner_join(room::Entity) - .filter(room::Column::ChannelId.is_in(graph.channels.iter().map(|c| c.id))) + .filter(room::Column::ChannelId.is_in(channels.iter().map(|c| c.id))) .select_only() .column(room::Column::ChannelId) .column(room_participant::Column::UserId) @@ -481,7 +564,7 @@ impl Database { } } - let channel_ids = graph.channels.iter().map(|c| c.id).collect::>(); + let channel_ids = channels.iter().map(|c| c.id).collect::>(); let channel_buffer_changes = self .unseen_channel_buffer_changes(user_id, &channel_ids, &*tx) .await?; @@ -491,7 +574,7 @@ impl Database { .await?; Ok(ChannelsForUser { - channels: graph, + channels: ChannelGraph { channels, edges }, channel_participants, channels_with_admin_privileges, unseen_buffer_changes: channel_buffer_changes, @@ -842,7 +925,7 @@ impl Database { }) } - /// Returns the channel ancestors, deepest first + /// Returns the channel ancestors, include itself, deepest first pub async fn get_channel_ancestors( &self, channel_id: ChannelId, @@ -867,6 +950,65 @@ impl Database { Ok(channel_ids) } + // Returns the channel desendants as a sorted list of edges for further processing. + // The edges are sorted such that you will see unknown channel ids as children + // before you see them as parents. + async fn get_channel_descendants_2( + &self, + channel_ids: impl IntoIterator, + tx: &DatabaseTransaction, + ) -> Result> { + let mut values = String::new(); + for id in channel_ids { + if !values.is_empty() { + values.push_str(", "); + } + write!(&mut values, "({})", id).unwrap(); + } + + if values.is_empty() { + return Ok(vec![]); + } + + let sql = format!( + r#" + SELECT + descendant_paths.* + FROM + channel_paths parent_paths, channel_paths descendant_paths + WHERE + parent_paths.channel_id IN ({values}) AND + descendant_paths.id_path != parent_paths.id_path AND + descendant_paths.id_path LIKE (parent_paths.id_path || '%') + ORDER BY + descendant_paths.id_path + "# + ); + + let stmt = Statement::from_string(self.pool.get_database_backend(), sql); + + let mut paths = channel_path::Entity::find() + .from_raw_sql(stmt) + .stream(tx) + .await?; + + let mut results: Vec = Vec::new(); + while let Some(path) = paths.next().await { + let path = path?; + let ids: Vec<&str> = path.id_path.trim_matches('/').split('/').collect(); + + debug_assert!(ids.len() >= 2); + debug_assert!(ids[ids.len() - 1] == path.channel_id.to_string()); + + results.push(ChannelEdge { + parent_id: ids[ids.len() - 2].parse().unwrap(), + channel_id: ids[ids.len() - 1].parse().unwrap(), + }) + } + + Ok(results) + } + /// Returns the channel descendants, /// Structured as a map from child ids to their parent ids /// For example, the descendants of 'a' in this DAG: From 9c6f5de551ca9cf81ef08428d2ee5b24fe8e05a3 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 13:17:19 -0600 Subject: [PATCH 08/25] Use new get_channel_descendants for delete --- crates/collab/src/db/queries/channels.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 625655f27735f707d4f05e8b28041406b3f89be9..0b97569ec44086093e1e168c544c54af25c458ac 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -125,17 +125,19 @@ impl Database { .await?; // 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_remove: HashSet = HashSet::default(); + channels_to_remove.insert(channel_id); + + let graph = self.get_channel_descendants_2([channel_id], &*tx).await?; + for edge in graph.iter() { + channels_to_remove.insert(ChannelId::from_proto(edge.channel_id)); + } + { let mut channels_to_keep = channel_path::Entity::find() .filter( channel_path::Column::ChannelId - .is_in( - channels_to_remove - .keys() - .copied() - .filter(|&id| id != channel_id), - ) + .is_in(channels_to_remove.clone()) .and( channel_path::Column::IdPath .not_like(&format!("%/{}/%", channel_id)), @@ -160,7 +162,7 @@ impl Database { .await?; channel::Entity::delete_many() - .filter(channel::Column::Id.is_in(channels_to_remove.keys().copied())) + .filter(channel::Column::Id.is_in(channels_to_remove.clone())) .exec(&*tx) .await?; @@ -177,7 +179,7 @@ impl Database { ); tx.execute(channel_paths_stmt).await?; - Ok((channels_to_remove.into_keys().collect(), members_to_notify)) + Ok((channels_to_remove.into_iter().collect(), members_to_notify)) }) .await } From e050d168a726742dfe4e836c1bcbd758d4916ea0 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 13:39:46 -0600 Subject: [PATCH 09/25] Delete some old code, reame ChannelMembers -> Members --- crates/collab/src/db/ids.rs | 8 +- crates/collab/src/db/queries/channels.rs | 183 +++-------------------- 2 files changed, 21 insertions(+), 170 deletions(-) diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index ee8c879ed3da3fc3ec68a1cece455095380f7768..b935c658dd1546ad94508360e4a494482aaef041 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -141,16 +141,16 @@ impl Into for ChannelRole { pub enum ChannelVisibility { #[sea_orm(string_value = "public")] Public, - #[sea_orm(string_value = "channel_members")] + #[sea_orm(string_value = "members")] #[default] - ChannelMembers, + Members, } impl From for ChannelVisibility { fn from(value: proto::ChannelVisibility) -> Self { match value { proto::ChannelVisibility::Public => ChannelVisibility::Public, - proto::ChannelVisibility::ChannelMembers => ChannelVisibility::ChannelMembers, + proto::ChannelVisibility::ChannelMembers => ChannelVisibility::Members, } } } @@ -159,7 +159,7 @@ impl Into for ChannelVisibility { fn into(self) -> proto::ChannelVisibility { match self { ChannelVisibility::Public => proto::ChannelVisibility::Public, - ChannelVisibility::ChannelMembers => proto::ChannelVisibility::ChannelMembers, + ChannelVisibility::Members => proto::ChannelVisibility::ChannelMembers, } } } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 0b97569ec44086093e1e168c544c54af25c458ac..74d5b797b83bf851937b8fbdf82654409f8cb165 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -2,9 +2,6 @@ use std::cmp::Ordering; use super::*; use rpc::proto::{channel_member::Kind, ChannelEdge}; -use smallvec::SmallVec; - -type ChannelDescendants = HashMap>; impl Database { #[cfg(test)] @@ -41,7 +38,7 @@ impl Database { let channel = channel::ActiveModel { id: ActiveValue::NotSet, name: ActiveValue::Set(name.to_string()), - visibility: ActiveValue::Set(ChannelVisibility::ChannelMembers), + visibility: ActiveValue::Set(ChannelVisibility::Members), } .insert(&*tx) .await?; @@ -349,49 +346,6 @@ impl Database { .await } - async fn get_channel_graph( - &self, - parents_by_child_id: ChannelDescendants, - trim_dangling_parents: bool, - tx: &DatabaseTransaction, - ) -> Result { - 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, - }) - } - } - - let mut edges = Vec::with_capacity(parents_by_child_id.len()); - for (channel, parents) in parents_by_child_id.iter() { - for parent in parents.into_iter() { - if trim_dangling_parents { - if parents_by_child_id.contains_key(parent) { - edges.push(ChannelEdge { - channel_id: channel.to_proto(), - parent_id: parent.to_proto(), - }); - } - } else { - edges.push(ChannelEdge { - channel_id: channel.to_proto(), - parent_id: parent.to_proto(), - }); - } - } - } - - Ok(ChannelGraph { channels, edges }) - } - pub async fn get_channels_for_user(&self, user_id: UserId) -> Result { self.transaction(|tx| async move { let tx = tx; @@ -637,7 +591,7 @@ impl Database { .one(&*tx) .await? .map(|channel| channel.visibility) - .unwrap_or(ChannelVisibility::ChannelMembers); + .unwrap_or(ChannelVisibility::Members); #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] enum QueryMemberDetails { @@ -1011,79 +965,6 @@ impl Database { Ok(results) } - /// Returns the channel descendants, - /// Structured as a map from child ids to their parent ids - /// For example, the descendants of 'a' in this DAG: - /// - /// /- b -\ - /// a -- c -- d - /// - /// would be: - /// { - /// a: [], - /// b: [a], - /// c: [a], - /// d: [a, c], - /// } - async fn get_channel_descendants( - &self, - channel_ids: impl IntoIterator, - tx: &DatabaseTransaction, - ) -> Result { - let mut values = String::new(); - for id in channel_ids { - if !values.is_empty() { - values.push_str(", "); - } - write!(&mut values, "({})", id).unwrap(); - } - - if values.is_empty() { - return Ok(HashMap::default()); - } - - let sql = format!( - r#" - SELECT - descendant_paths.* - FROM - channel_paths parent_paths, channel_paths descendant_paths - WHERE - parent_paths.channel_id IN ({values}) AND - descendant_paths.id_path LIKE (parent_paths.id_path || '%') - "# - ); - - let stmt = Statement::from_string(self.pool.get_database_backend(), sql); - - let mut parents_by_child_id: ChannelDescendants = HashMap::default(); - let mut paths = channel_path::Entity::find() - .from_raw_sql(stmt) - .stream(tx) - .await?; - - while let Some(path) = paths.next().await { - let path = path?; - let ids = path.id_path.trim_matches('/').split('/'); - let mut parent_id = None; - for id in ids { - if let Ok(id) = id.parse() { - let id = ChannelId::from_proto(id); - if id == path.channel_id { - break; - } - parent_id = Some(id); - } - } - let entry = parents_by_child_id.entry(path.channel_id).or_default(); - if let Some(parent_id) = parent_id { - entry.insert(parent_id); - } - } - - Ok(parents_by_child_id) - } - /// Returns the channel with the given ID and: /// - true if the user is a member /// - false if the user hasn't accepted the invitation yet @@ -1242,18 +1123,23 @@ impl Database { .await?; } - let mut channel_descendants = self.get_channel_descendants([channel], &*tx).await?; - if let Some(channel) = channel_descendants.get_mut(&channel) { - // Remove the other parents - channel.clear(); - channel.insert(new_parent); - } - - let channels = self - .get_channel_graph(channel_descendants, false, &*tx) + let membership = channel_member::Entity::find() + .filter( + channel_member::Column::ChannelId + .eq(channel) + .and(channel_member::Column::UserId.eq(user)), + ) + .all(tx) .await?; - Ok(channels) + let mut channel_info = self.get_user_channels(user, membership, &*tx).await?; + + channel_info.channels.edges.push(ChannelEdge { + channel_id: channel.to_proto(), + parent_id: new_parent.to_proto(), + }); + + Ok(channel_info.channels) } /// Unlink a channel from a given parent. This will add in a root edge if @@ -1405,38 +1291,3 @@ impl PartialEq for ChannelGraph { self.channels == other.channels && self.edges == other.edges } } - -struct SmallSet(SmallVec<[T; 1]>); - -impl Deref for SmallSet { - type Target = [T]; - - fn deref(&self) -> &Self::Target { - self.0.deref() - } -} - -impl Default for SmallSet { - fn default() -> Self { - Self(SmallVec::new()) - } -} - -impl SmallSet { - fn insert(&mut self, value: T) -> bool - where - T: Ord, - { - match self.binary_search(&value) { - Ok(_) => false, - Err(ix) => { - self.0.insert(ix, value); - true - } - } - } - - fn clear(&mut self) { - self.0.clear(); - } -} From bb408936e9aed78c73bf9345746439327e876b24 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 14:08:40 -0600 Subject: [PATCH 10/25] Ignore old admin column --- crates/collab/src/db/ids.rs | 3 +- crates/collab/src/db/queries/channels.rs | 62 ++++--------------- crates/collab/src/db/tables/channel_member.rs | 4 +- 3 files changed, 14 insertions(+), 55 deletions(-) diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index b935c658dd1546ad94508360e4a494482aaef041..6dd1f2f596087d1db6b2180c11cd03acc1576ac1 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -82,12 +82,13 @@ id_type!(UserId); id_type!(ChannelBufferCollaboratorId); id_type!(FlagId); -#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum)] +#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default)] #[sea_orm(rs_type = "String", db_type = "String(None)")] pub enum ChannelRole { #[sea_orm(string_value = "admin")] Admin, #[sea_orm(string_value = "member")] + #[default] Member, #[sea_orm(string_value = "guest")] Guest, diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 74d5b797b83bf851937b8fbdf82654409f8cb165..e7db0d4cfc32197034c7c03ccd464d8a239b2fa0 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -78,8 +78,7 @@ impl Database { channel_id: ActiveValue::Set(channel.id), user_id: ActiveValue::Set(creator_id), accepted: ActiveValue::Set(true), - admin: ActiveValue::Set(true), - role: ActiveValue::Set(Some(ChannelRole::Admin)), + role: ActiveValue::Set(ChannelRole::Admin), } .insert(&*tx) .await?; @@ -197,8 +196,7 @@ impl Database { channel_id: ActiveValue::Set(channel_id), user_id: ActiveValue::Set(invitee_id), accepted: ActiveValue::Set(false), - admin: ActiveValue::Set(role == ChannelRole::Admin), - role: ActiveValue::Set(Some(role)), + role: ActiveValue::Set(role), } .insert(&*tx) .await?; @@ -402,14 +400,7 @@ impl Database { let mut role_for_channel: HashMap = HashMap::default(); for membership in channel_memberships.iter() { - role_for_channel.insert( - membership.channel_id, - membership.role.unwrap_or(if membership.admin { - ChannelRole::Admin - } else { - ChannelRole::Member - }), - ); + role_for_channel.insert(membership.channel_id, membership.role); } for ChannelEdge { @@ -561,8 +552,7 @@ impl Database { .and(channel_member::Column::UserId.eq(for_user)), ) .set(channel_member::ActiveModel { - admin: ActiveValue::set(role == ChannelRole::Admin), - role: ActiveValue::set(Some(role)), + role: ActiveValue::set(role), ..Default::default() }) .exec(&*tx) @@ -596,7 +586,6 @@ impl Database { #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] enum QueryMemberDetails { UserId, - Admin, Role, IsDirectMember, Accepted, @@ -610,7 +599,6 @@ impl Database { .filter(channel_member::Column::ChannelId.is_in(ancestor_ids.iter().copied())) .select_only() .column(channel_member::Column::UserId) - .column(channel_member::Column::Admin) .column(channel_member::Column::Role) .column_as( channel_member::Column::ChannelId.eq(channel_id), @@ -629,17 +617,9 @@ impl Database { let mut user_details: HashMap = HashMap::default(); while let Some(row) = stream.next().await { - let ( - user_id, - is_admin, - channel_role, - is_direct_member, - is_invite_accepted, - visibility, - ): ( + let (user_id, channel_role, is_direct_member, is_invite_accepted, visibility): ( UserId, - bool, - Option, + ChannelRole, bool, bool, ChannelVisibility, @@ -650,11 +630,6 @@ impl Database { (false, true) => proto::channel_member::Kind::AncestorMember, (false, false) => continue, }; - let channel_role = channel_role.unwrap_or(if is_admin { - ChannelRole::Admin - } else { - ChannelRole::Member - }); if channel_role == ChannelRole::Guest && visibility != ChannelVisibility::Public @@ -797,7 +772,6 @@ impl Database { enum QueryChannelMembership { ChannelId, Role, - Admin, Visibility, } @@ -811,7 +785,6 @@ impl Database { .select_only() .column(channel_member::Column::ChannelId) .column(channel_member::Column::Role) - .column(channel_member::Column::Admin) .column(channel::Column::Visibility) .into_values::<_, QueryChannelMembership>() .stream(&*tx) @@ -826,29 +799,16 @@ impl Database { // note these channels are not iterated in any particular order, // our current logic takes the highest permission available. while let Some(row) = rows.next().await { - let (ch_id, role, admin, visibility): ( - ChannelId, - Option, - bool, - ChannelVisibility, - ) = row?; + let (ch_id, role, visibility): (ChannelId, ChannelRole, ChannelVisibility) = row?; match role { - Some(ChannelRole::Admin) => is_admin = true, - Some(ChannelRole::Member) => is_member = true, - Some(ChannelRole::Guest) => { + ChannelRole::Admin => is_admin = true, + ChannelRole::Member => is_member = true, + ChannelRole::Guest => { if visibility == ChannelVisibility::Public { is_participant = true } } - Some(ChannelRole::Banned) => is_banned = true, - None => { - // rows created from pre-role collab server. - if admin { - is_admin = true - } else { - is_member = true - } - } + ChannelRole::Banned => is_banned = true, } if channel_id == ch_id { current_channel_visibility = Some(visibility); diff --git a/crates/collab/src/db/tables/channel_member.rs b/crates/collab/src/db/tables/channel_member.rs index e8162bfcbd133fe647dbbfa13cab1136dcbe441e..5498a008565e4db681ac588f49cf4a3332121b9a 100644 --- a/crates/collab/src/db/tables/channel_member.rs +++ b/crates/collab/src/db/tables/channel_member.rs @@ -9,9 +9,7 @@ pub struct Model { pub channel_id: ChannelId, pub user_id: UserId, pub accepted: bool, - pub admin: bool, - // only optional while migrating - pub role: Option, + pub role: ChannelRole, } impl ActiveModelBehavior for ActiveModel {} From e20bc87152291ee37f967a72103cb2bb39bcf9a5 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 14:30:20 -0600 Subject: [PATCH 11/25] Add some sanity checks for new user channel graph --- crates/collab/src/db/tests/channel_tests.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 2044310d8e025119de85dbb49cde3149ced93c6c..b96971123238d5c17c88e75fc90019eeabf70450 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -895,6 +895,18 @@ async fn test_user_is_channel_participant(db: &Arc) { ] ); + db.respond_to_channel_invite(vim_channel, guest, true) + .await + .unwrap(); + + let channels = db.get_channels_for_user(guest).await.unwrap().channels; + assert_dag(channels, &[(vim_channel, None)]); + let channels = db.get_channels_for_user(member).await.unwrap().channels; + assert_dag( + channels, + &[(active_channel, None), (vim_channel, Some(active_channel))], + ); + db.set_channel_member_role(vim_channel, admin, guest, ChannelRole::Banned) .await .unwrap(); @@ -926,7 +938,7 @@ async fn test_user_is_channel_participant(db: &Arc) { }, proto::ChannelMember { user_id: guest.to_proto(), - kind: proto::channel_member::Kind::Invitee.into(), + kind: proto::channel_member::Kind::Member.into(), role: proto::ChannelRole::Banned.into(), }, ] @@ -1015,6 +1027,12 @@ async fn test_user_is_channel_participant(db: &Arc) { }, ] ); + + let channels = db.get_channels_for_user(guest).await.unwrap().channels; + assert_dag( + channels, + &[(zed_channel, None), (vim_channel, Some(zed_channel))], + ) } #[track_caller] From af11cc6cfdd4d16e29e62864f749710ffecf4006 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 15:07:49 -0600 Subject: [PATCH 12/25] show warnings by default --- Procfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Procfile b/Procfile index 2eb7de20fb7e9cae34375dc130c6d27aea01012e..3f42c3a9677477bd7dcd04ca61a749206559be40 100644 --- a/Procfile +++ b/Procfile @@ -1,4 +1,4 @@ web: cd ../zed.dev && PORT=3000 npm run dev -collab: cd crates/collab && RUST_LOG=${RUST_LOG:-collab=info} cargo run serve +collab: cd crates/collab && RUST_LOG=${RUST_LOG:-warn,collab=info} cargo run serve livekit: livekit-server --dev postgrest: postgrest crates/collab/admin_api.conf From f8fd77b83e80743d12a38a47e960fd98e0bfddf4 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 15:08:09 -0600 Subject: [PATCH 13/25] fix migration --- crates/collab/migrations/20231011214412_add_guest_role.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/collab/migrations/20231011214412_add_guest_role.sql b/crates/collab/migrations/20231011214412_add_guest_role.sql index bd178ec63d4723a2f16bec51bdc71ab22fa13a3a..17135471583a03bd6b39e82b3644b683cfc96d57 100644 --- a/crates/collab/migrations/20231011214412_add_guest_role.sql +++ b/crates/collab/migrations/20231011214412_add_guest_role.sql @@ -1,4 +1,4 @@ ALTER TABLE channel_members ADD COLUMN role TEXT; UPDATE channel_members SET role = CASE WHEN admin THEN 'admin' ELSE 'member' END; -ALTER TABLE channels ADD COLUMN visibility TEXT NOT NULL DEFAULT 'channel_members'; +ALTER TABLE channels ADD COLUMN visibility TEXT NOT NULL DEFAULT 'members'; From f6f9b5c8cbe153c63e0bfb6431f2ea62318011d3 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 13 Oct 2023 16:59:30 -0600 Subject: [PATCH 14/25] Wire through public access toggle --- crates/channel/src/channel_store.rs | 23 ++++- .../src/channel_store/channel_index.rs | 2 + crates/collab/src/db.rs | 1 + crates/collab/src/db/ids.rs | 6 +- crates/collab/src/db/queries/channels.rs | 19 +++-- crates/collab/src/db/tests.rs | 1 + crates/collab/src/rpc.rs | 69 ++++++++++----- .../collab/src/tests/channel_buffer_tests.rs | 6 +- .../src/collab_panel/channel_modal.rs | 83 ++++++++++++++++++- crates/rpc/proto/zed.proto | 10 ++- crates/rpc/src/proto.rs | 2 + crates/theme/src/theme.rs | 2 + styles/src/style_tree/collab_modals.ts | 23 ++++- 13 files changed, 209 insertions(+), 38 deletions(-) diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 64c76a0a390c3dc5ae6ba1ef402b16d8b58efdb8..3e8fbafb6ad0fce58d8232fedf0ac95b8cf1d625 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -9,7 +9,7 @@ use db::RELEASE_CHANNEL; use futures::{channel::mpsc, future::Shared, Future, FutureExt, StreamExt}; use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Task, WeakModelHandle}; use rpc::{ - proto::{self, ChannelEdge, ChannelPermission, ChannelRole}, + proto::{self, ChannelEdge, ChannelPermission, ChannelRole, ChannelVisibility}, TypedEnvelope, }; use serde_derive::{Deserialize, Serialize}; @@ -49,6 +49,7 @@ pub type ChannelData = (Channel, ChannelPath); pub struct Channel { pub id: ChannelId, pub name: String, + pub visibility: proto::ChannelVisibility, pub unseen_note_version: Option<(u64, clock::Global)>, pub unseen_message_id: Option, } @@ -508,6 +509,25 @@ impl ChannelStore { }) } + pub fn set_channel_visibility( + &mut self, + channel_id: ChannelId, + visibility: ChannelVisibility, + cx: &mut ModelContext, + ) -> Task> { + let client = self.client.clone(); + cx.spawn(|_, _| async move { + let _ = client + .request(proto::SetChannelVisibility { + channel_id, + visibility: visibility.into(), + }) + .await?; + + Ok(()) + }) + } + pub fn invite_member( &mut self, channel_id: ChannelId, @@ -869,6 +889,7 @@ impl ChannelStore { ix, Arc::new(Channel { id: channel.id, + visibility: channel.visibility(), name: channel.name, unseen_note_version: None, unseen_message_id: None, diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index bf0de1b644cee0df8df3948e504e406a8f9cd3e7..7b54d5dcd904f8fa181a9ebcc20a0ff1e2c1e4d5 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -123,12 +123,14 @@ impl<'a> ChannelPathsInsertGuard<'a> { pub fn insert(&mut self, channel_proto: proto::Channel) { if let Some(existing_channel) = self.channels_by_id.get_mut(&channel_proto.id) { + Arc::make_mut(existing_channel).visibility = channel_proto.visibility(); Arc::make_mut(existing_channel).name = channel_proto.name; } else { self.channels_by_id.insert( channel_proto.id, Arc::new(Channel { id: channel_proto.id, + visibility: channel_proto.visibility(), name: channel_proto.name, unseen_note_version: None, unseen_message_id: None, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index e60b7cc33dfb22f030c5600cfa0d8d0794438c1d..08f78c685dc8a28392733816a8e0473c2d2ca63a 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -432,6 +432,7 @@ pub struct NewUserResult { pub struct Channel { pub id: ChannelId, pub name: String, + pub visibility: ChannelVisibility, } #[derive(Debug, PartialEq)] diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 6dd1f2f596087d1db6b2180c11cd03acc1576ac1..970d66d4cbcf45b2dd559222fb46e8be93c723b9 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -137,7 +137,7 @@ impl Into for ChannelRole { } } -#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default)] +#[derive(Eq, PartialEq, Copy, Clone, Debug, EnumIter, DeriveActiveEnum, Default, Hash)] #[sea_orm(rs_type = "String", db_type = "String(None)")] pub enum ChannelVisibility { #[sea_orm(string_value = "public")] @@ -151,7 +151,7 @@ impl From for ChannelVisibility { fn from(value: proto::ChannelVisibility) -> Self { match value { proto::ChannelVisibility::Public => ChannelVisibility::Public, - proto::ChannelVisibility::ChannelMembers => ChannelVisibility::Members, + proto::ChannelVisibility::Members => ChannelVisibility::Members, } } } @@ -160,7 +160,7 @@ impl Into for ChannelVisibility { fn into(self) -> proto::ChannelVisibility { match self { ChannelVisibility::Public => proto::ChannelVisibility::Public, - ChannelVisibility::Members => proto::ChannelVisibility::ChannelMembers, + ChannelVisibility::Members => proto::ChannelVisibility::Members, } } } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index e7db0d4cfc32197034c7c03ccd464d8a239b2fa0..0b7e9eb2d828ed78da7f2baaa20636f3bce5e74d 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -93,12 +93,12 @@ impl Database { channel_id: ChannelId, visibility: ChannelVisibility, user_id: UserId, - ) -> Result<()> { + ) -> Result { self.transaction(move |tx| async move { self.check_user_is_channel_admin(channel_id, user_id, &*tx) .await?; - channel::ActiveModel { + let channel = channel::ActiveModel { id: ActiveValue::Unchanged(channel_id), visibility: ActiveValue::Set(visibility), ..Default::default() @@ -106,7 +106,7 @@ impl Database { .update(&*tx) .await?; - Ok(()) + Ok(channel) }) .await } @@ -219,14 +219,14 @@ impl Database { channel_id: ChannelId, user_id: UserId, new_name: &str, - ) -> Result { + ) -> Result { self.transaction(move |tx| async move { let new_name = Self::sanitize_channel_name(new_name)?.to_string(); self.check_user_is_channel_admin(channel_id, user_id, &*tx) .await?; - channel::ActiveModel { + let channel = channel::ActiveModel { id: ActiveValue::Unchanged(channel_id), name: ActiveValue::Set(new_name.clone()), ..Default::default() @@ -234,7 +234,11 @@ impl Database { .update(&*tx) .await?; - Ok(new_name) + Ok(Channel { + id: channel.id, + name: channel.name, + visibility: channel.visibility, + }) }) .await } @@ -336,6 +340,7 @@ impl Database { .map(|channel| Channel { id: channel.id, name: channel.name, + visibility: channel.visibility, }) .collect(); @@ -443,6 +448,7 @@ impl Database { channels.push(Channel { id: channel.id, name: channel.name, + visibility: channel.visibility, }); if role == ChannelRole::Admin { @@ -963,6 +969,7 @@ impl Database { Ok(Some(( Channel { id: channel.id, + visibility: channel.visibility, name: channel.name, }, is_accepted, diff --git a/crates/collab/src/db/tests.rs b/crates/collab/src/db/tests.rs index 6a91fd6ffe145c1a31f9b5264029a04ddb1ef1de..99a605106eafab55f50b2b37a0a5a027b1cb9457 100644 --- a/crates/collab/src/db/tests.rs +++ b/crates/collab/src/db/tests.rs @@ -159,6 +159,7 @@ fn graph(channels: &[(ChannelId, &'static str)], edges: &[(ChannelId, ChannelId) graph.channels.push(Channel { id: *id, name: name.to_string(), + visibility: ChannelVisibility::Members, }) } diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index f8ac77325c951633df3227eab72c92f08081893b..c3d8a25ab7fb709750f1043b966dc0e1281c63e9 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -3,8 +3,8 @@ mod connection_pool; use crate::{ auth, db::{ - self, BufferId, ChannelId, ChannelsForUser, Database, MessageId, ProjectId, RoomId, - ServerId, User, UserId, + self, BufferId, ChannelId, ChannelVisibility, ChannelsForUser, Database, MessageId, + ProjectId, RoomId, ServerId, User, UserId, }, executor::Executor, AppState, Result, @@ -38,8 +38,8 @@ use lazy_static::lazy_static; use prometheus::{register_int_gauge, IntGauge}; use rpc::{ proto::{ - self, Ack, AnyTypedEnvelope, ChannelEdge, ChannelVisibility, EntityMessage, - EnvelopedMessage, LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, + self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, + LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, }, Connection, ConnectionId, Peer, Receipt, TypedEnvelope, }; @@ -255,6 +255,7 @@ impl Server { .add_request_handler(invite_channel_member) .add_request_handler(remove_channel_member) .add_request_handler(set_channel_member_role) + .add_request_handler(set_channel_visibility) .add_request_handler(rename_channel) .add_request_handler(join_channel_buffer) .add_request_handler(leave_channel_buffer) @@ -2210,8 +2211,7 @@ async fn create_channel( let channel = proto::Channel { id: id.to_proto(), name: request.name, - // TODO: Visibility - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }; response.send(proto::CreateChannelResponse { @@ -2300,9 +2300,8 @@ async fn invite_channel_member( let mut update = proto::UpdateChannels::default(); update.channel_invitations.push(proto::Channel { id: channel.id.to_proto(), + visibility: channel.visibility.into(), name: channel.name, - // TODO: Visibility - visibility: proto::ChannelVisibility::ChannelMembers as i32, }); for connection_id in session .connection_pool() @@ -2343,6 +2342,39 @@ async fn remove_channel_member( Ok(()) } +async fn set_channel_visibility( + request: proto::SetChannelVisibility, + response: Response, + session: Session, +) -> Result<()> { + let db = session.db().await; + let channel_id = ChannelId::from_proto(request.channel_id); + let visibility = request.visibility().into(); + + let channel = db + .set_channel_visibility(channel_id, visibility, session.user_id) + .await?; + + let mut update = proto::UpdateChannels::default(); + update.channels.push(proto::Channel { + id: channel.id.to_proto(), + name: channel.name, + visibility: channel.visibility.into(), + }); + + let member_ids = db.get_channel_members(channel_id).await?; + + let connection_pool = session.connection_pool().await; + for member_id in member_ids { + for connection_id in connection_pool.user_connection_ids(member_id) { + session.peer.send(connection_id, update.clone())?; + } + } + + response.send(proto::Ack {})?; + Ok(()) +} + async fn set_channel_member_role( request: proto::SetChannelMemberRole, response: Response, @@ -2391,15 +2423,14 @@ async fn rename_channel( ) -> Result<()> { let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); - let new_name = db + let channel = db .rename_channel(channel_id, session.user_id, &request.name) .await?; let channel = proto::Channel { - id: request.channel_id, - name: new_name, - // TODO: Visibility - visibility: proto::ChannelVisibility::ChannelMembers as i32, + id: channel.id.to_proto(), + name: channel.name, + visibility: channel.visibility.into(), }; response.send(proto::RenameChannelResponse { channel: Some(channel.clone()), @@ -2437,9 +2468,8 @@ async fn link_channel( .into_iter() .map(|channel| proto::Channel { id: channel.id.to_proto(), + visibility: channel.visibility.into(), name: channel.name, - // TODO: Visibility - visibility: proto::ChannelVisibility::ChannelMembers as i32, }) .collect(), insert_edge: channels_to_send.edges, @@ -2530,9 +2560,8 @@ async fn move_channel( .into_iter() .map(|channel| proto::Channel { id: channel.id.to_proto(), + visibility: channel.visibility.into(), name: channel.name, - // TODO: Visibility - visibility: proto::ChannelVisibility::ChannelMembers as i32, }) .collect(), insert_edge: channels_to_send.edges, @@ -2588,9 +2617,8 @@ async fn respond_to_channel_invite( .into_iter() .map(|channel| proto::Channel { id: channel.id.to_proto(), + visibility: channel.visibility.into(), name: channel.name, - // TODO: Visibility - visibility: ChannelVisibility::ChannelMembers.into(), }), ); update.unseen_channel_messages = result.channel_messages; @@ -3094,8 +3122,7 @@ fn build_initial_channels_update( update.channels.push(proto::Channel { id: channel.id.to_proto(), name: channel.name, - // TODO: Visibility - visibility: ChannelVisibility::Public.into(), + visibility: channel.visibility.into(), }); } diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index a0b9b524841f19e4eb6537348317ae38d23627c2..14ae159ab80fd5b46359ef1183e7ea18478f3f08 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -11,7 +11,10 @@ use collections::HashMap; use editor::{Anchor, Editor, ToOffset}; use futures::future; use gpui::{executor::Deterministic, ModelHandle, TestAppContext, ViewContext}; -use rpc::{proto::PeerId, RECEIVE_TIMEOUT}; +use rpc::{ + proto::{self, PeerId}, + RECEIVE_TIMEOUT, +}; use serde_json::json; use std::{ops::Range, sync::Arc}; @@ -445,6 +448,7 @@ fn channel(id: u64, name: &'static str) -> Channel { Channel { id, name: name.to_string(), + visibility: proto::ChannelVisibility::Members, unseen_note_version: None, unseen_message_id: None, } diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index 16d5e48f452b7acd4b220076ccb73ac7c8308854..bf04e4f7e6b257d40d52c4da9a2bcc97237684b3 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -1,6 +1,6 @@ -use channel::{ChannelId, ChannelMembership, ChannelStore}; +use channel::{Channel, ChannelId, ChannelMembership, ChannelStore}; use client::{ - proto::{self, ChannelRole}, + proto::{self, ChannelRole, ChannelVisibility}, User, UserId, UserStore, }; use context_menu::{ContextMenu, ContextMenuItem}; @@ -9,7 +9,8 @@ use gpui::{ actions, elements::*, platform::{CursorStyle, MouseButton}, - AppContext, Entity, ModelHandle, MouseState, Task, View, ViewContext, ViewHandle, + AppContext, ClipboardItem, Entity, ModelHandle, MouseState, Task, View, ViewContext, + ViewHandle, }; use picker::{Picker, PickerDelegate, PickerEvent}; use std::sync::Arc; @@ -185,6 +186,81 @@ impl View for ChannelModal { .into_any() } + fn render_visibility( + channel_id: ChannelId, + visibility: ChannelVisibility, + theme: &theme::TabbedModal, + cx: &mut ViewContext, + ) -> AnyElement { + enum TogglePublic {} + + if visibility == ChannelVisibility::Members { + return Flex::row() + .with_child( + MouseEventHandler::new::(0, cx, move |state, _| { + let style = theme.visibility_toggle.style_for(state); + Label::new(format!("{}", "Public access: OFF"), style.text.clone()) + .contained() + .with_style(style.container.clone()) + }) + .on_click(MouseButton::Left, move |_, this, cx| { + this.channel_store + .update(cx, |channel_store, cx| { + channel_store.set_channel_visibility( + channel_id, + ChannelVisibility::Public, + cx, + ) + }) + .detach_and_log_err(cx); + }) + .with_cursor_style(CursorStyle::PointingHand), + ) + .into_any(); + } + + Flex::row() + .with_child( + MouseEventHandler::new::(0, cx, move |state, _| { + let style = theme.visibility_toggle.style_for(state); + Label::new(format!("{}", "Public access: ON"), style.text.clone()) + .contained() + .with_style(style.container.clone()) + }) + .on_click(MouseButton::Left, move |_, this, cx| { + this.channel_store + .update(cx, |channel_store, cx| { + channel_store.set_channel_visibility( + channel_id, + ChannelVisibility::Members, + cx, + ) + }) + .detach_and_log_err(cx); + }) + .with_cursor_style(CursorStyle::PointingHand), + ) + .with_spacing(14.0) + .with_child( + MouseEventHandler::new::(1, cx, move |state, _| { + let style = theme.channel_link.style_for(state); + Label::new(format!("{}", "copy link"), style.text.clone()) + .contained() + .with_style(style.container.clone()) + }) + .on_click(MouseButton::Left, move |_, this, cx| { + if let Some(channel) = + this.channel_store.read(cx).channel_for_id(channel_id) + { + let item = ClipboardItem::new(channel.link()); + cx.write_to_clipboard(item); + } + }) + .with_cursor_style(CursorStyle::PointingHand), + ) + .into_any() + } + Flex::column() .with_child( Flex::column() @@ -193,6 +269,7 @@ impl View for ChannelModal { .contained() .with_style(theme.title.container.clone()), ) + .with_child(render_visibility(channel.id, channel.visibility, theme, cx)) .with_child(Flex::row().with_children([ render_mode_button::( Mode::InviteMembers, diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 90e425a39ffef4ab04b05f65e94951296768124e..f6d0dfa5d9ff2b28e6c9d6fa8fd8b31645275b7d 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -170,7 +170,8 @@ message Envelope { LinkChannel link_channel = 140; UnlinkChannel unlink_channel = 141; - MoveChannel move_channel = 142; // current max: 145 + MoveChannel move_channel = 142; + SetChannelVisibility set_channel_visibility = 146; // current max: 146 } } @@ -1049,6 +1050,11 @@ message SetChannelMemberRole { ChannelRole role = 3; } +message SetChannelVisibility { + uint64 channel_id = 1; + ChannelVisibility visibility = 2; +} + message RenameChannel { uint64 channel_id = 1; string name = 2; @@ -1542,7 +1548,7 @@ message Nonce { enum ChannelVisibility { Public = 0; - ChannelMembers = 1; + Members = 1; } message Channel { diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 57292a52ca21db846ca426945b40b2dd08738370..c60e99602e9a4c42b8f6bf857c7f9a43cd069883 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -231,6 +231,7 @@ messages!( (RenameChannel, Foreground), (RenameChannelResponse, Foreground), (SetChannelMemberRole, Foreground), + (SetChannelVisibility, Foreground), (SearchProject, Background), (SearchProjectResponse, Background), (ShareProject, Foreground), @@ -327,6 +328,7 @@ request_messages!( (RespondToContactRequest, Ack), (RespondToChannelInvite, Ack), (SetChannelMemberRole, Ack), + (SetChannelVisibility, Ack), (SendChannelMessage, SendChannelMessageResponse), (GetChannelMessages, GetChannelMessagesResponse), (GetChannelMembers, GetChannelMembersResponse), diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index e534ba4260e11b89acf2eb625701bfea921ff411..fa3db613288f162372ba200a073a2007bb01a6ce 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -286,6 +286,8 @@ pub struct TabbedModal { pub header: ContainerStyle, pub body: ContainerStyle, pub title: ContainedText, + pub visibility_toggle: Interactive, + pub channel_link: Interactive, pub picker: Picker, pub max_height: f32, pub max_width: f32, diff --git a/styles/src/style_tree/collab_modals.ts b/styles/src/style_tree/collab_modals.ts index f9b22b686799d450a0d0abebd0df6243982dc96f..586e7be3f0bb3c68a14e198ee93f1c89b35cf6fb 100644 --- a/styles/src/style_tree/collab_modals.ts +++ b/styles/src/style_tree/collab_modals.ts @@ -1,10 +1,11 @@ -import { useTheme } from "../theme" +import { StyleSet, StyleSets, Styles, useTheme } from "../theme" import { background, border, foreground, text } from "./components" import picker from "./picker" import { input } from "../component/input" import contact_finder from "./contact_finder" import { tab } from "../component/tab" import { icon_button } from "../component/icon_button" +import { interactive } from "../element/interactive" export default function channel_modal(): any { const theme = useTheme() @@ -27,6 +28,24 @@ export default function channel_modal(): any { const picker_input = input() + const interactive_text = (styleset: StyleSets) => + interactive({ + base: { + padding: { + left: 8, + top: 8 + }, + ...text(theme.middle, "sans", styleset, "default"), + }, state: { + hovered: { + ...text(theme.middle, "sans", styleset, "hovered"), + }, + clicked: { + ...text(theme.middle, "sans", styleset, "active"), + } + } + }); + const member_icon_style = icon_button({ variant: "ghost", size: "sm", @@ -88,6 +107,8 @@ export default function channel_modal(): any { left: BUTTON_OFFSET, }, }, + visibility_toggle: interactive_text("base"), + channel_link: interactive_text("accent"), picker: { empty_container: {}, item: { From 4e7b35c917745e8946bed4052351d93b45f0d3f8 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 16 Oct 2023 13:27:05 -0600 Subject: [PATCH 15/25] Make joining a channel as a guest always succeed --- crates/channel/src/channel_store.rs | 1 + crates/collab/src/db/queries/channels.rs | 129 +++++++++--- crates/collab/src/db/queries/rooms.rs | 184 +++++++++++------- crates/collab/src/db/tests/channel_tests.rs | 15 +- crates/collab/src/rpc.rs | 158 +++++++++------ crates/collab/src/tests/channel_tests.rs | 52 +++++ .../src/collab_panel/channel_modal.rs | 2 +- 7 files changed, 370 insertions(+), 171 deletions(-) diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 3e8fbafb6ad0fce58d8232fedf0ac95b8cf1d625..57b183f7debeff01350a87a887373f5dfb43a894 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -972,6 +972,7 @@ impl ChannelStore { let mut all_user_ids = Vec::new(); let channel_participants = payload.channel_participants; + dbg!(&channel_participants); for entry in &channel_participants { for user_id in entry.participant_user_ids.iter() { if let Err(ix) = all_user_ids.binary_search(user_id) { diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 0b7e9eb2d828ed78da7f2baaa20636f3bce5e74d..d4276603f915f7bd7a864d2730a2e9e49330114f 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -88,6 +88,84 @@ impl Database { .await } + pub async fn join_channel_internal( + &self, + channel_id: ChannelId, + user_id: UserId, + connection: ConnectionId, + environment: &str, + tx: &DatabaseTransaction, + ) -> Result<(JoinRoom, bool)> { + let mut joined = false; + + let channel = channel::Entity::find() + .filter(channel::Column::Id.eq(channel_id)) + .one(&*tx) + .await?; + + let mut role = self + .channel_role_for_user(channel_id, user_id, &*tx) + .await?; + + if role.is_none() { + if channel.as_ref().map(|c| c.visibility) == Some(ChannelVisibility::Public) { + channel_member::Entity::insert(channel_member::ActiveModel { + id: ActiveValue::NotSet, + channel_id: ActiveValue::Set(channel_id), + user_id: ActiveValue::Set(user_id), + accepted: ActiveValue::Set(true), + role: ActiveValue::Set(ChannelRole::Guest), + }) + .on_conflict( + OnConflict::columns([ + channel_member::Column::UserId, + channel_member::Column::ChannelId, + ]) + .update_columns([channel_member::Column::Accepted]) + .to_owned(), + ) + .exec(&*tx) + .await?; + + debug_assert!( + self.channel_role_for_user(channel_id, user_id, &*tx) + .await? + == Some(ChannelRole::Guest) + ); + + role = Some(ChannelRole::Guest); + joined = true; + } + } + + if channel.is_none() || role.is_none() || role == Some(ChannelRole::Banned) { + Err(anyhow!("no such channel, or not allowed"))? + } + + let live_kit_room = format!("channel-{}", nanoid::nanoid!(30)); + let room_id = self + .get_or_create_channel_room(channel_id, &live_kit_room, environment, &*tx) + .await?; + + self.join_channel_room_internal(channel_id, room_id, user_id, connection, &*tx) + .await + .map(|jr| (jr, joined)) + } + + pub async fn join_channel( + &self, + channel_id: ChannelId, + user_id: UserId, + connection: ConnectionId, + environment: &str, + ) -> Result<(JoinRoom, bool)> { + self.transaction(move |tx| async move { + self.join_channel_internal(channel_id, user_id, connection, environment, &*tx) + .await + }) + .await + } + pub async fn set_channel_visibility( &self, channel_id: ChannelId, @@ -981,38 +1059,39 @@ impl Database { .await } - pub async fn get_or_create_channel_room( + pub(crate) async fn get_or_create_channel_room( &self, channel_id: ChannelId, live_kit_room: &str, - enviroment: &str, + environment: &str, + tx: &DatabaseTransaction, ) -> Result { - self.transaction(|tx| async move { - let tx = tx; - - let room = room::Entity::find() - .filter(room::Column::ChannelId.eq(channel_id)) - .one(&*tx) - .await?; + let room = room::Entity::find() + .filter(room::Column::ChannelId.eq(channel_id)) + .one(&*tx) + .await?; - let room_id = if let Some(room) = room { - room.id - } else { - let result = room::Entity::insert(room::ActiveModel { - channel_id: ActiveValue::Set(Some(channel_id)), - live_kit_room: ActiveValue::Set(live_kit_room.to_string()), - enviroment: ActiveValue::Set(Some(enviroment.to_string())), - ..Default::default() - }) - .exec(&*tx) - .await?; + let room_id = if let Some(room) = room { + if let Some(env) = room.enviroment { + if &env != environment { + Err(anyhow!("must join using the {} release", env))?; + } + } + room.id + } else { + let result = room::Entity::insert(room::ActiveModel { + channel_id: ActiveValue::Set(Some(channel_id)), + live_kit_room: ActiveValue::Set(live_kit_room.to_string()), + enviroment: ActiveValue::Set(Some(environment.to_string())), + ..Default::default() + }) + .exec(&*tx) + .await?; - result.last_insert_id - }; + result.last_insert_id + }; - Ok(room_id) - }) - .await + Ok(room_id) } // Insert an edge from the given channel to the given other channel. diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index 625615db5f2b5316dc9f2fbb7a9ba2ac0e98d242..d2120495b05063550765eeb7fbe151512d524403 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -300,99 +300,139 @@ impl Database { } } - #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] - enum QueryParticipantIndices { - ParticipantIndex, - } - let existing_participant_indices: Vec = room_participant::Entity::find() - .filter( - room_participant::Column::RoomId - .eq(room_id) - .and(room_participant::Column::ParticipantIndex.is_not_null()), - ) - .select_only() - .column(room_participant::Column::ParticipantIndex) - .into_values::<_, QueryParticipantIndices>() - .all(&*tx) - .await?; - - let mut participant_index = 0; - while existing_participant_indices.contains(&participant_index) { - participant_index += 1; + if channel_id.is_some() { + Err(anyhow!("tried to join channel call directly"))? } - if let Some(channel_id) = channel_id { - self.check_user_is_channel_member(channel_id, user_id, &*tx) - .await?; + let participant_index = self + .get_next_participant_index_internal(room_id, &*tx) + .await?; - room_participant::Entity::insert_many([room_participant::ActiveModel { - room_id: ActiveValue::set(room_id), - user_id: ActiveValue::set(user_id), + let result = room_participant::Entity::update_many() + .filter( + Condition::all() + .add(room_participant::Column::RoomId.eq(room_id)) + .add(room_participant::Column::UserId.eq(user_id)) + .add(room_participant::Column::AnsweringConnectionId.is_null()), + ) + .set(room_participant::ActiveModel { + participant_index: ActiveValue::Set(Some(participant_index)), answering_connection_id: ActiveValue::set(Some(connection.id as i32)), answering_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, ))), answering_connection_lost: ActiveValue::set(false), - calling_user_id: ActiveValue::set(user_id), - calling_connection_id: ActiveValue::set(connection.id as i32), - calling_connection_server_id: ActiveValue::set(Some(ServerId( - connection.owner_id as i32, - ))), - participant_index: ActiveValue::Set(Some(participant_index)), ..Default::default() - }]) - .on_conflict( - OnConflict::columns([room_participant::Column::UserId]) - .update_columns([ - room_participant::Column::AnsweringConnectionId, - room_participant::Column::AnsweringConnectionServerId, - room_participant::Column::AnsweringConnectionLost, - room_participant::Column::ParticipantIndex, - ]) - .to_owned(), - ) + }) .exec(&*tx) .await?; - } else { - let result = room_participant::Entity::update_many() - .filter( - Condition::all() - .add(room_participant::Column::RoomId.eq(room_id)) - .add(room_participant::Column::UserId.eq(user_id)) - .add(room_participant::Column::AnsweringConnectionId.is_null()), - ) - .set(room_participant::ActiveModel { - participant_index: ActiveValue::Set(Some(participant_index)), - answering_connection_id: ActiveValue::set(Some(connection.id as i32)), - answering_connection_server_id: ActiveValue::set(Some(ServerId( - connection.owner_id as i32, - ))), - answering_connection_lost: ActiveValue::set(false), - ..Default::default() - }) - .exec(&*tx) - .await?; - if result.rows_affected == 0 { - Err(anyhow!("room does not exist or was already joined"))?; - } + if result.rows_affected == 0 { + Err(anyhow!("room does not exist or was already joined"))?; } let room = self.get_room(room_id, &tx).await?; - let channel_members = if let Some(channel_id) = channel_id { - self.get_channel_participants_internal(channel_id, &tx) - .await? - } else { - Vec::new() - }; Ok(JoinRoom { room, - channel_id, - channel_members, + channel_id: None, + channel_members: vec![], }) }) .await } + async fn get_next_participant_index_internal( + &self, + room_id: RoomId, + tx: &DatabaseTransaction, + ) -> Result { + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] + enum QueryParticipantIndices { + ParticipantIndex, + } + let existing_participant_indices: Vec = room_participant::Entity::find() + .filter( + room_participant::Column::RoomId + .eq(room_id) + .and(room_participant::Column::ParticipantIndex.is_not_null()), + ) + .select_only() + .column(room_participant::Column::ParticipantIndex) + .into_values::<_, QueryParticipantIndices>() + .all(&*tx) + .await?; + + let mut participant_index = 0; + while existing_participant_indices.contains(&participant_index) { + participant_index += 1; + } + + Ok(participant_index) + } + + pub async fn channel_id_for_room(&self, room_id: RoomId) -> Result> { + self.transaction(|tx| async move { + let room: Option = room::Entity::find() + .filter(room::Column::Id.eq(room_id)) + .one(&*tx) + .await?; + + Ok(room.and_then(|room| room.channel_id)) + }) + .await + } + + pub(crate) async fn join_channel_room_internal( + &self, + channel_id: ChannelId, + room_id: RoomId, + user_id: UserId, + connection: ConnectionId, + tx: &DatabaseTransaction, + ) -> Result { + let participant_index = self + .get_next_participant_index_internal(room_id, &*tx) + .await?; + + room_participant::Entity::insert_many([room_participant::ActiveModel { + room_id: ActiveValue::set(room_id), + user_id: ActiveValue::set(user_id), + answering_connection_id: ActiveValue::set(Some(connection.id as i32)), + answering_connection_server_id: ActiveValue::set(Some(ServerId( + connection.owner_id as i32, + ))), + answering_connection_lost: ActiveValue::set(false), + calling_user_id: ActiveValue::set(user_id), + calling_connection_id: ActiveValue::set(connection.id as i32), + calling_connection_server_id: ActiveValue::set(Some(ServerId( + connection.owner_id as i32, + ))), + participant_index: ActiveValue::Set(Some(participant_index)), + ..Default::default() + }]) + .on_conflict( + OnConflict::columns([room_participant::Column::UserId]) + .update_columns([ + room_participant::Column::AnsweringConnectionId, + room_participant::Column::AnsweringConnectionServerId, + room_participant::Column::AnsweringConnectionLost, + room_participant::Column::ParticipantIndex, + ]) + .to_owned(), + ) + .exec(&*tx) + .await?; + + let room = self.get_room(room_id, &tx).await?; + let channel_members = self + .get_channel_participants_internal(channel_id, &tx) + .await?; + Ok(JoinRoom { + room, + channel_id: Some(channel_id), + channel_members, + }) + } + pub async fn rejoin_room( &self, rejoin_room: proto::RejoinRoom, diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index b96971123238d5c17c88e75fc90019eeabf70450..9b6d8d1525b6a7f3b9a817be4a3164d2a6dd028b 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -8,7 +8,7 @@ use crate::{ db::{ queries::channels::ChannelGraph, tests::{graph, TEST_RELEASE_CHANNEL}, - ChannelId, ChannelRole, Database, NewUserParams, UserId, + ChannelId, ChannelRole, Database, NewUserParams, RoomId, UserId, }, test_both_dbs, }; @@ -207,15 +207,11 @@ async fn test_joining_channels(db: &Arc) { .user_id; let channel_1 = db.create_root_channel("channel_1", user_1).await.unwrap(); - let room_1 = db - .get_or_create_channel_room(channel_1, "1", TEST_RELEASE_CHANNEL) - .await - .unwrap(); // can join a room with membership to its channel - let joined_room = db - .join_room( - room_1, + let (joined_room, _) = db + .join_channel( + channel_1, user_1, ConnectionId { owner_id, id: 1 }, TEST_RELEASE_CHANNEL, @@ -224,11 +220,12 @@ async fn test_joining_channels(db: &Arc) { .unwrap(); assert_eq!(joined_room.room.participants.len(), 1); + let room_id = RoomId::from_proto(joined_room.room.id); drop(joined_room); // cannot join a room without membership to its channel assert!(db .join_room( - room_1, + room_id, user_2, ConnectionId { owner_id, id: 1 }, TEST_RELEASE_CHANNEL diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index c3d8a25ab7fb709750f1043b966dc0e1281c63e9..26ad2f281a0ac0d2d6c5e33e9f5dff5b5503125e 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -38,7 +38,7 @@ use lazy_static::lazy_static; use prometheus::{register_int_gauge, IntGauge}; use rpc::{ proto::{ - self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, + self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, JoinRoom, LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, }, Connection, ConnectionId, Peer, Receipt, TypedEnvelope, @@ -977,6 +977,13 @@ async fn join_room( session: Session, ) -> Result<()> { let room_id = RoomId::from_proto(request.id); + + let channel_id = session.db().await.channel_id_for_room(room_id).await?; + + if let Some(channel_id) = channel_id { + return join_channel_internal(channel_id, Box::new(response), session).await; + } + let joined_room = { let room = session .db() @@ -992,16 +999,6 @@ async fn join_room( room.into_inner() }; - if let Some(channel_id) = joined_room.channel_id { - channel_updated( - channel_id, - &joined_room.room, - &joined_room.channel_members, - &session.peer, - &*session.connection_pool().await, - ) - } - for connection_id in session .connection_pool() .await @@ -1039,7 +1036,7 @@ async fn join_room( response.send(proto::JoinRoomResponse { room: Some(joined_room.room), - channel_id: joined_room.channel_id.map(|id| id.to_proto()), + channel_id: None, live_kit_connection_info, })?; @@ -2602,54 +2599,68 @@ async fn respond_to_channel_invite( db.respond_to_channel_invite(channel_id, session.user_id, request.accept) .await?; + if request.accept { + channel_membership_updated(db, channel_id, &session).await?; + } else { + let mut update = proto::UpdateChannels::default(); + update + .remove_channel_invitations + .push(channel_id.to_proto()); + session.peer.send(session.connection_id, update)?; + } + response.send(proto::Ack {})?; + + Ok(()) +} + +async fn channel_membership_updated( + db: tokio::sync::MutexGuard<'_, DbHandle>, + channel_id: ChannelId, + session: &Session, +) -> Result<(), crate::Error> { let mut update = proto::UpdateChannels::default(); update .remove_channel_invitations .push(channel_id.to_proto()); - if request.accept { - let result = db.get_channel_for_user(channel_id, session.user_id).await?; - update + + let result = db.get_channel_for_user(channel_id, session.user_id).await?; + update.channels.extend( + result .channels - .extend( - result - .channels - .channels - .into_iter() - .map(|channel| proto::Channel { - id: channel.id.to_proto(), - visibility: channel.visibility.into(), - name: channel.name, - }), - ); - update.unseen_channel_messages = result.channel_messages; - update.unseen_channel_buffer_changes = result.unseen_buffer_changes; - update.insert_edge = result.channels.edges; - update - .channel_participants - .extend( - result - .channel_participants - .into_iter() - .map(|(channel_id, user_ids)| proto::ChannelParticipants { - channel_id: channel_id.to_proto(), - participant_user_ids: user_ids.into_iter().map(UserId::to_proto).collect(), - }), - ); - update - .channel_permissions - .extend( - result - .channels_with_admin_privileges - .into_iter() - .map(|channel_id| proto::ChannelPermission { - channel_id: channel_id.to_proto(), - role: proto::ChannelRole::Admin.into(), - }), - ); - } + .channels + .into_iter() + .map(|channel| proto::Channel { + id: channel.id.to_proto(), + visibility: channel.visibility.into(), + name: channel.name, + }), + ); + update.unseen_channel_messages = result.channel_messages; + update.unseen_channel_buffer_changes = result.unseen_buffer_changes; + update.insert_edge = result.channels.edges; + update + .channel_participants + .extend( + result + .channel_participants + .into_iter() + .map(|(channel_id, user_ids)| proto::ChannelParticipants { + channel_id: channel_id.to_proto(), + participant_user_ids: user_ids.into_iter().map(UserId::to_proto).collect(), + }), + ); + update + .channel_permissions + .extend( + result + .channels_with_admin_privileges + .into_iter() + .map(|channel_id| proto::ChannelPermission { + channel_id: channel_id.to_proto(), + role: proto::ChannelRole::Admin.into(), + }), + ); session.peer.send(session.connection_id, update)?; - response.send(proto::Ack {})?; - Ok(()) } @@ -2659,19 +2670,35 @@ async fn join_channel( session: Session, ) -> Result<()> { let channel_id = ChannelId::from_proto(request.channel_id); - let live_kit_room = format!("channel-{}", nanoid::nanoid!(30)); + join_channel_internal(channel_id, Box::new(response), session).await +} + +trait JoinChannelInternalResponse { + fn send(self, result: proto::JoinRoomResponse) -> Result<()>; +} +impl JoinChannelInternalResponse for Response { + fn send(self, result: proto::JoinRoomResponse) -> Result<()> { + Response::::send(self, result) + } +} +impl JoinChannelInternalResponse for Response { + fn send(self, result: proto::JoinRoomResponse) -> Result<()> { + Response::::send(self, result) + } +} +async fn join_channel_internal( + channel_id: ChannelId, + response: Box, + session: Session, +) -> Result<()> { let joined_room = { leave_room_for_session(&session).await?; let db = session.db().await; - let room_id = db - .get_or_create_channel_room(channel_id, &live_kit_room, &*RELEASE_CHANNEL_NAME) - .await?; - - let joined_room = db - .join_room( - room_id, + let (joined_room, joined_channel) = db + .join_channel( + channel_id, session.user_id, session.connection_id, RELEASE_CHANNEL_NAME.as_str(), @@ -2698,9 +2725,13 @@ async fn join_channel( live_kit_connection_info, })?; + if joined_channel { + channel_membership_updated(db, channel_id, &session).await? + } + room_updated(&joined_room.room, &session.peer); - joined_room.into_inner() + joined_room }; channel_updated( @@ -2712,7 +2743,6 @@ async fn join_channel( ); update_user_contacts(session.user_id, &session).await?; - Ok(()) } diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index 95a672e76c7b70e18c7deadbc1a19193655a8dcd..1700dfc5d3dd46544b5a4767198c57c603d8825b 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -912,6 +912,58 @@ async fn test_lost_channel_creation( ], ); } +#[gpui::test] +async fn test_guest_access( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + + let channels = server + .make_channel_tree(&[("channel-a", None)], (&client_a, cx_a)) + .await; + let channel_a_id = channels[0]; + + let active_call_b = cx_b.read(ActiveCall::global); + + // should not be allowed to join + assert!(active_call_b + .update(cx_b, |call, cx| call.join_channel(channel_a_id, cx)) + .await + .is_err()); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.set_channel_visibility(channel_a_id, proto::ChannelVisibility::Public, cx) + }) + .await + .unwrap(); + + active_call_b + .update(cx_b, |call, cx| call.join_channel(channel_a_id, cx)) + .await + .unwrap(); + + deterministic.run_until_parked(); + + assert!(client_b + .channel_store() + .update(cx_b, |channel_store, _| channel_store + .channel_for_id(channel_a_id) + .is_some())); + + client_a.channel_store().update(cx_a, |channel_store, _| { + let participants = channel_store.channel_participants(channel_a_id); + assert_eq!(participants.len(), 1); + assert_eq!(participants[0].id, client_b.user_id().unwrap()); + }) +} #[gpui::test] async fn test_channel_moving( diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index bf04e4f7e6b257d40d52c4da9a2bcc97237684b3..da6edbde696ae011895142b95641f969538da28e 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -1,4 +1,4 @@ -use channel::{Channel, ChannelId, ChannelMembership, ChannelStore}; +use channel::{ChannelId, ChannelMembership, ChannelStore}; use client::{ proto::{self, ChannelRole, ChannelVisibility}, User, UserId, UserStore, From 2feb091961b2c0b719cb546c39cd1752590aea38 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 16 Oct 2023 16:11:00 -0600 Subject: [PATCH 16/25] Ensure that invitees do not have permissions They have to accept the invite, (which joining the channel will do), first. --- crates/collab/src/db/queries/channels.rs | 228 +++++++++++--------- crates/collab/src/db/tests/channel_tests.rs | 72 +++---- crates/collab/src/rpc.rs | 35 ++- crates/collab/src/tests/channel_tests.rs | 63 +++++- 4 files changed, 238 insertions(+), 160 deletions(-) diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index d4276603f915f7bd7a864d2730a2e9e49330114f..e3a6170452cd21920819da7a261e248b87632b63 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -88,80 +88,87 @@ impl Database { .await } - pub async fn join_channel_internal( + pub async fn join_channel( &self, channel_id: ChannelId, user_id: UserId, connection: ConnectionId, environment: &str, - tx: &DatabaseTransaction, - ) -> Result<(JoinRoom, bool)> { - let mut joined = false; + ) -> Result<(JoinRoom, Option)> { + self.transaction(move |tx| async move { + let mut joined_channel_id = None; - let channel = channel::Entity::find() - .filter(channel::Column::Id.eq(channel_id)) - .one(&*tx) - .await?; + let channel = channel::Entity::find() + .filter(channel::Column::Id.eq(channel_id)) + .one(&*tx) + .await?; - let mut role = self - .channel_role_for_user(channel_id, user_id, &*tx) - .await?; + let mut role = self + .channel_role_for_user(channel_id, user_id, &*tx) + .await?; + + if role.is_none() && channel.is_some() { + if let Some(invitation) = self + .pending_invite_for_channel(channel_id, user_id, &*tx) + .await? + { + // note, this may be a parent channel + joined_channel_id = Some(invitation.channel_id); + role = Some(invitation.role); + + channel_member::Entity::update(channel_member::ActiveModel { + accepted: ActiveValue::Set(true), + ..invitation.into_active_model() + }) + .exec(&*tx) + .await?; + + debug_assert!( + self.channel_role_for_user(channel_id, user_id, &*tx) + .await? + == role + ); + } + } + if role.is_none() + && channel.as_ref().map(|c| c.visibility) == Some(ChannelVisibility::Public) + { + let channel_id_to_join = self + .most_public_ancestor_for_channel(channel_id, &*tx) + .await? + .unwrap_or(channel_id); + role = Some(ChannelRole::Guest); + joined_channel_id = Some(channel_id_to_join); - if role.is_none() { - if channel.as_ref().map(|c| c.visibility) == Some(ChannelVisibility::Public) { channel_member::Entity::insert(channel_member::ActiveModel { id: ActiveValue::NotSet, - channel_id: ActiveValue::Set(channel_id), + channel_id: ActiveValue::Set(channel_id_to_join), user_id: ActiveValue::Set(user_id), accepted: ActiveValue::Set(true), role: ActiveValue::Set(ChannelRole::Guest), }) - .on_conflict( - OnConflict::columns([ - channel_member::Column::UserId, - channel_member::Column::ChannelId, - ]) - .update_columns([channel_member::Column::Accepted]) - .to_owned(), - ) .exec(&*tx) .await?; debug_assert!( self.channel_role_for_user(channel_id, user_id, &*tx) .await? - == Some(ChannelRole::Guest) + == role ); - - role = Some(ChannelRole::Guest); - joined = true; } - } - - if channel.is_none() || role.is_none() || role == Some(ChannelRole::Banned) { - Err(anyhow!("no such channel, or not allowed"))? - } - let live_kit_room = format!("channel-{}", nanoid::nanoid!(30)); - let room_id = self - .get_or_create_channel_room(channel_id, &live_kit_room, environment, &*tx) - .await?; + if channel.is_none() || role.is_none() || role == Some(ChannelRole::Banned) { + Err(anyhow!("no such channel, or not allowed"))? + } - self.join_channel_room_internal(channel_id, room_id, user_id, connection, &*tx) - .await - .map(|jr| (jr, joined)) - } + let live_kit_room = format!("channel-{}", nanoid::nanoid!(30)); + let room_id = self + .get_or_create_channel_room(channel_id, &live_kit_room, environment, &*tx) + .await?; - pub async fn join_channel( - &self, - channel_id: ChannelId, - user_id: UserId, - connection: ConnectionId, - environment: &str, - ) -> Result<(JoinRoom, bool)> { - self.transaction(move |tx| async move { - self.join_channel_internal(channel_id, user_id, connection, environment, &*tx) + self.join_channel_room_internal(channel_id, room_id, user_id, connection, &*tx) .await + .map(|jr| (jr, joined_channel_id)) }) .await } @@ -624,29 +631,29 @@ impl Database { admin_id: UserId, for_user: UserId, role: ChannelRole, - ) -> Result<()> { + ) -> Result { self.transaction(|tx| async move { self.check_user_is_channel_admin(channel_id, admin_id, &*tx) .await?; - let result = channel_member::Entity::update_many() + let membership = channel_member::Entity::find() .filter( channel_member::Column::ChannelId .eq(channel_id) .and(channel_member::Column::UserId.eq(for_user)), ) - .set(channel_member::ActiveModel { - role: ActiveValue::set(role), - ..Default::default() - }) - .exec(&*tx) + .one(&*tx) .await?; - if result.rows_affected == 0 { - Err(anyhow!("no such member"))?; - } + let Some(membership) = membership else { + Err(anyhow!("no such member"))? + }; - Ok(()) + let mut update = membership.into_active_model(); + update.role = ActiveValue::Set(role); + let updated = channel_member::Entity::update(update).exec(&*tx).await?; + + Ok(updated) }) .await } @@ -844,6 +851,52 @@ impl Database { } } + pub async fn pending_invite_for_channel( + &self, + channel_id: ChannelId, + user_id: UserId, + tx: &DatabaseTransaction, + ) -> Result> { + let channel_ids = self.get_channel_ancestors(channel_id, tx).await?; + + let row = channel_member::Entity::find() + .filter(channel_member::Column::ChannelId.is_in(channel_ids)) + .filter(channel_member::Column::UserId.eq(user_id)) + .filter(channel_member::Column::Accepted.eq(false)) + .one(&*tx) + .await?; + + Ok(row) + } + + pub async fn most_public_ancestor_for_channel( + &self, + channel_id: ChannelId, + tx: &DatabaseTransaction, + ) -> Result> { + let channel_ids = self.get_channel_ancestors(channel_id, tx).await?; + + let rows = channel::Entity::find() + .filter(channel::Column::Id.is_in(channel_ids.clone())) + .filter(channel::Column::Visibility.eq(ChannelVisibility::Public)) + .all(&*tx) + .await?; + + let mut visible_channels: HashSet = HashSet::default(); + + for row in rows { + visible_channels.insert(row.id); + } + + for ancestor in channel_ids.into_iter().rev() { + if visible_channels.contains(&ancestor) { + return Ok(Some(ancestor)); + } + } + + Ok(None) + } + pub async fn channel_role_for_user( &self, channel_id: ChannelId, @@ -864,7 +917,8 @@ impl Database { .filter( channel_member::Column::ChannelId .is_in(channel_ids) - .and(channel_member::Column::UserId.eq(user_id)), + .and(channel_member::Column::UserId.eq(user_id)) + .and(channel_member::Column::Accepted.eq(true)), ) .select_only() .column(channel_member::Column::ChannelId) @@ -1009,52 +1063,22 @@ impl Database { Ok(results) } - /// Returns the channel with the given ID and: - /// - true if the user is a member - /// - false if the user hasn't accepted the invitation yet - pub async fn get_channel( - &self, - channel_id: ChannelId, - user_id: UserId, - ) -> Result> { + /// Returns the channel with the given ID + pub async fn get_channel(&self, channel_id: ChannelId, user_id: UserId) -> Result { self.transaction(|tx| async move { - let tx = tx; + self.check_user_is_channel_participant(channel_id, user_id, &*tx) + .await?; let channel = channel::Entity::find_by_id(channel_id).one(&*tx).await?; + let Some(channel) = channel else { + Err(anyhow!("no such channel"))? + }; - if let Some(channel) = channel { - if self - .check_user_is_channel_member(channel_id, user_id, &*tx) - .await - .is_err() - { - return Ok(None); - } - - let channel_membership = channel_member::Entity::find() - .filter( - channel_member::Column::ChannelId - .eq(channel_id) - .and(channel_member::Column::UserId.eq(user_id)), - ) - .one(&*tx) - .await?; - - let is_accepted = channel_membership - .map(|membership| membership.accepted) - .unwrap_or(false); - - Ok(Some(( - Channel { - id: channel.id, - visibility: channel.visibility, - name: channel.name, - }, - is_accepted, - ))) - } else { - Ok(None) - } + Ok(Channel { + id: channel.id, + visibility: channel.visibility, + name: channel.name, + }) }) .await } diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index 9b6d8d1525b6a7f3b9a817be4a3164d2a6dd028b..f08b1554bc0062e080e1bd5fdd8eee146f090c9d 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -51,7 +51,7 @@ async fn test_channels(db: &Arc) { let zed_id = db.create_root_channel("zed", a_id).await.unwrap(); // Make sure that people cannot read channels they haven't been invited to - assert!(db.get_channel(zed_id, b_id).await.unwrap().is_none()); + assert!(db.get_channel(zed_id, b_id).await.is_err()); db.invite_channel_member(zed_id, b_id, a_id, ChannelRole::Member) .await @@ -157,7 +157,7 @@ async fn test_channels(db: &Arc) { // Remove a single channel db.delete_channel(crdb_id, a_id).await.unwrap(); - assert!(db.get_channel(crdb_id, a_id).await.unwrap().is_none()); + assert!(db.get_channel(crdb_id, a_id).await.is_err()); // Remove a channel tree let (mut channel_ids, user_ids) = db.delete_channel(rust_id, a_id).await.unwrap(); @@ -165,9 +165,9 @@ async fn test_channels(db: &Arc) { assert_eq!(channel_ids, &[rust_id, cargo_id, cargo_ra_id]); assert_eq!(user_ids, &[a_id]); - assert!(db.get_channel(rust_id, a_id).await.unwrap().is_none()); - assert!(db.get_channel(cargo_id, a_id).await.unwrap().is_none()); - assert!(db.get_channel(cargo_ra_id, a_id).await.unwrap().is_none()); + assert!(db.get_channel(rust_id, a_id).await.is_err()); + assert!(db.get_channel(cargo_id, a_id).await.is_err()); + assert!(db.get_channel(cargo_ra_id, a_id).await.is_err()); } test_both_dbs!( @@ -381,11 +381,7 @@ async fn test_channel_renames(db: &Arc) { let zed_archive_id = zed_id; - let (channel, _) = db - .get_channel(zed_archive_id, user_1) - .await - .unwrap() - .unwrap(); + let channel = db.get_channel(zed_archive_id, user_1).await.unwrap(); assert_eq!(channel.name, "zed-archive"); let non_permissioned_rename = db @@ -860,12 +856,6 @@ async fn test_user_is_channel_participant(db: &Arc) { }) .await .unwrap(); - db.transaction(|tx| async move { - db.check_user_is_channel_participant(vim_channel, guest, &*tx) - .await - }) - .await - .unwrap(); let members = db .get_channel_participant_details(vim_channel, admin) @@ -896,6 +886,13 @@ async fn test_user_is_channel_participant(db: &Arc) { .await .unwrap(); + db.transaction(|tx| async move { + db.check_user_is_channel_participant(vim_channel, guest, &*tx) + .await + }) + .await + .unwrap(); + let channels = db.get_channels_for_user(guest).await.unwrap().channels; assert_dag(channels, &[(vim_channel, None)]); let channels = db.get_channels_for_user(member).await.unwrap().channels; @@ -953,29 +950,7 @@ async fn test_user_is_channel_participant(db: &Arc) { .await .unwrap(); - db.transaction(|tx| async move { - db.check_user_is_channel_participant(zed_channel, guest, &*tx) - .await - }) - .await - .unwrap(); - assert!(db - .transaction(|tx| async move { - db.check_user_is_channel_participant(active_channel, guest, &*tx) - .await - }) - .await - .is_err(),); - - db.transaction(|tx| async move { - db.check_user_is_channel_participant(vim_channel, guest, &*tx) - .await - }) - .await - .unwrap(); - // currently people invited to parent channels are not shown here - // (though they *do* have permissions!) let members = db .get_channel_participant_details(vim_channel, admin) .await @@ -1000,6 +975,27 @@ async fn test_user_is_channel_participant(db: &Arc) { .await .unwrap(); + db.transaction(|tx| async move { + db.check_user_is_channel_participant(zed_channel, guest, &*tx) + .await + }) + .await + .unwrap(); + assert!(db + .transaction(|tx| async move { + db.check_user_is_channel_participant(active_channel, guest, &*tx) + .await + }) + .await + .is_err(),); + + db.transaction(|tx| async move { + db.check_user_is_channel_participant(vim_channel, guest, &*tx) + .await + }) + .await + .unwrap(); + let members = db .get_channel_participant_details(vim_channel, admin) .await diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 26ad2f281a0ac0d2d6c5e33e9f5dff5b5503125e..4b33550c39e66a0f8e62b3757c535fb6988a68a8 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -38,7 +38,7 @@ use lazy_static::lazy_static; use prometheus::{register_int_gauge, IntGauge}; use rpc::{ proto::{ - self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, JoinRoom, + self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, }, Connection, ConnectionId, Peer, Receipt, TypedEnvelope, @@ -2289,10 +2289,7 @@ async fn invite_channel_member( ) .await?; - let (channel, _) = db - .get_channel(channel_id, session.user_id) - .await? - .ok_or_else(|| anyhow!("channel not found"))?; + let channel = db.get_channel(channel_id, session.user_id).await?; let mut update = proto::UpdateChannels::default(); update.channel_invitations.push(proto::Channel { @@ -2380,21 +2377,19 @@ async fn set_channel_member_role( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); let member_id = UserId::from_proto(request.user_id); - db.set_channel_member_role( - channel_id, - session.user_id, - member_id, - request.role().into(), - ) - .await?; + let channel_member = db + .set_channel_member_role( + channel_id, + session.user_id, + member_id, + request.role().into(), + ) + .await?; - let (channel, has_accepted) = db - .get_channel(channel_id, member_id) - .await? - .ok_or_else(|| anyhow!("channel not found"))?; + let channel = db.get_channel(channel_id, session.user_id).await?; let mut update = proto::UpdateChannels::default(); - if has_accepted { + if channel_member.accepted { update.channel_permissions.push(proto::ChannelPermission { channel_id: channel.id.to_proto(), role: request.role, @@ -2724,9 +2719,11 @@ async fn join_channel_internal( channel_id: joined_room.channel_id.map(|id| id.to_proto()), live_kit_connection_info, })?; + dbg!("Joined channel", &joined_channel); - if joined_channel { - channel_membership_updated(db, channel_id, &session).await? + if let Some(joined_channel) = joined_channel { + dbg!("CMU"); + channel_membership_updated(db, joined_channel, &session).await? } room_updated(&joined_room.room, &session.peer); diff --git a/crates/collab/src/tests/channel_tests.rs b/crates/collab/src/tests/channel_tests.rs index 1700dfc5d3dd46544b5a4767198c57c603d8825b..1bb8c92ac80ac45b78ccff76de8f15c1468acaef 100644 --- a/crates/collab/src/tests/channel_tests.rs +++ b/crates/collab/src/tests/channel_tests.rs @@ -7,7 +7,7 @@ use channel::{ChannelId, ChannelMembership, ChannelStore}; use client::User; use gpui::{executor::Deterministic, ModelHandle, TestAppContext}; use rpc::{ - proto::{self}, + proto::{self, ChannelRole}, RECEIVE_TIMEOUT, }; use std::sync::Arc; @@ -965,6 +965,67 @@ async fn test_guest_access( }) } +#[gpui::test] +async fn test_invite_access( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + + let channels = server + .make_channel_tree( + &[("channel-a", None), ("channel-b", Some("channel-a"))], + (&client_a, cx_a), + ) + .await; + let channel_a_id = channels[0]; + let channel_b_id = channels[0]; + + let active_call_b = cx_b.read(ActiveCall::global); + + // should not be allowed to join + assert!(active_call_b + .update(cx_b, |call, cx| call.join_channel(channel_b_id, cx)) + .await + .is_err()); + + client_a + .channel_store() + .update(cx_a, |channel_store, cx| { + channel_store.invite_member( + channel_a_id, + client_b.user_id().unwrap(), + ChannelRole::Member, + cx, + ) + }) + .await + .unwrap(); + + active_call_b + .update(cx_b, |call, cx| call.join_channel(channel_b_id, cx)) + .await + .unwrap(); + + deterministic.run_until_parked(); + + client_b.channel_store().update(cx_b, |channel_store, _| { + assert!(channel_store.channel_for_id(channel_b_id).is_some()); + assert!(channel_store.channel_for_id(channel_a_id).is_some()); + }); + + client_a.channel_store().update(cx_a, |channel_store, _| { + let participants = channel_store.channel_participants(channel_b_id); + assert_eq!(participants.len(), 1); + assert_eq!(participants[0].id, client_b.user_id().unwrap()); + }) +} + #[gpui::test] async fn test_channel_moving( deterministic: Arc, From 6ffbc3a0f52bd94751393ad1f0217b9692cfa230 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 16 Oct 2023 20:03:44 -0600 Subject: [PATCH 17/25] Allow pasting ZED urls in the command palette in development --- Cargo.lock | 2 + crates/collab/src/db/queries/channels.rs | 2 +- crates/command_palette/Cargo.toml | 1 + crates/command_palette/src/command_palette.rs | 17 +- crates/workspace/src/workspace.rs | 1 + crates/zed-actions/Cargo.toml | 1 + crates/zed-actions/src/lib.rs | 15 +- crates/zed/src/main.rs | 206 +----------------- crates/zed/src/open_listener.rs | 202 ++++++++++++++++- crates/zed/src/zed.rs | 6 + 10 files changed, 245 insertions(+), 208 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 72ee771f5d96093685abc019f8d1f3f5d1201c22..f68cd22ae7de47e94b488ae6c69871793f596191 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1623,6 +1623,7 @@ dependencies = [ "theme", "util", "workspace", + "zed-actions", ] [[package]] @@ -10213,6 +10214,7 @@ name = "zed-actions" version = "0.1.0" dependencies = [ "gpui", + "serde", ] [[package]] diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index e3a6170452cd21920819da7a261e248b87632b63..b10cbd14f18c701a9415bb2705f884e8c760594f 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -979,7 +979,7 @@ impl Database { }) } - /// Returns the channel ancestors, include itself, deepest first + /// Returns the channel ancestors in arbitrary order pub async fn get_channel_ancestors( &self, channel_id: ChannelId, diff --git a/crates/command_palette/Cargo.toml b/crates/command_palette/Cargo.toml index 95ba452c142dc2fa2c615b000984e2d627a1a2e8..b42a3b5f41ae780ddd877af28e3e58b3a2cba332 100644 --- a/crates/command_palette/Cargo.toml +++ b/crates/command_palette/Cargo.toml @@ -19,6 +19,7 @@ settings = { path = "../settings" } util = { path = "../util" } theme = { path = "../theme" } workspace = { path = "../workspace" } +zed-actions = { path = "../zed-actions" } [dev-dependencies] gpui = { path = "../gpui", features = ["test-support"] } diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 10c9ba7b8670e197e3d4fc805dbd15681a9edd30..9b74c13a71e70ea66b292d2232f0e4f55dc6b451 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -6,8 +6,12 @@ use gpui::{ }; use picker::{Picker, PickerDelegate, PickerEvent}; use std::cmp::{self, Reverse}; -use util::ResultExt; +use util::{ + channel::{parse_zed_link, ReleaseChannel, RELEASE_CHANNEL, RELEASE_CHANNEL_NAME}, + ResultExt, +}; use workspace::Workspace; +use zed_actions::OpenZedURL; pub fn init(cx: &mut AppContext) { cx.add_action(toggle_command_palette); @@ -167,13 +171,22 @@ impl PickerDelegate for CommandPaletteDelegate { ) .await }; - let intercept_result = cx.read(|cx| { + let mut intercept_result = cx.read(|cx| { if cx.has_global::() { cx.global::()(&query, cx) } else { None } }); + if *RELEASE_CHANNEL == ReleaseChannel::Dev { + if parse_zed_link(&query).is_some() { + intercept_result = Some(CommandInterceptResult { + action: OpenZedURL { url: query.clone() }.boxed_clone(), + string: query.clone(), + positions: vec![], + }) + } + } if let Some(CommandInterceptResult { action, string, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 8b068fa10cf984a5301cd9e6d7d73dc118218f6f..710883d7cc428eaceca51835ac54c24aee4c5b9f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -288,6 +288,7 @@ pub fn init(app_state: Arc, cx: &mut AppContext) { cx.add_global_action(restart); cx.add_async_action(Workspace::save_all); cx.add_action(Workspace::add_folder_to_project); + cx.add_action( |workspace: &mut Workspace, _: &Unfollow, cx: &mut ViewContext| { let pane = workspace.active_pane().clone(); diff --git a/crates/zed-actions/Cargo.toml b/crates/zed-actions/Cargo.toml index b3fe3cbb53c46457c6855a916dcc6ef650e6be50..353041264a93f2f66cf4daae93b121d1933b4e48 100644 --- a/crates/zed-actions/Cargo.toml +++ b/crates/zed-actions/Cargo.toml @@ -8,3 +8,4 @@ publish = false [dependencies] gpui = { path = "../gpui" } +serde.workspace = true diff --git a/crates/zed-actions/src/lib.rs b/crates/zed-actions/src/lib.rs index bcd086924d2d0b36000bd4b67d2567dbc19c589d..df6405a4b1819900aaa43a6d335339baf5ff92b6 100644 --- a/crates/zed-actions/src/lib.rs +++ b/crates/zed-actions/src/lib.rs @@ -1,4 +1,7 @@ -use gpui::actions; +use std::sync::Arc; + +use gpui::{actions, impl_actions}; +use serde::Deserialize; actions!( zed, @@ -26,3 +29,13 @@ actions!( ResetDatabase, ] ); + +#[derive(Deserialize, Clone, PartialEq)] +pub struct OpenBrowser { + pub url: Arc, +} +#[derive(Deserialize, Clone, PartialEq)] +pub struct OpenZedURL { + pub url: String, +} +impl_actions!(zed, [OpenBrowser, OpenZedURL]); diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index f89a880c715ce645cae4dbd988051b196a7f5c7a..0e3bb6ef4349f7b3eab8c7a9ea3be9a211785b7f 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -3,22 +3,16 @@ use anyhow::{anyhow, Context, Result}; use backtrace::Backtrace; -use cli::{ - ipc::{self, IpcSender}, - CliRequest, CliResponse, IpcHandshake, FORCE_CLI_MODE_ENV_VAR_NAME, -}; +use cli::FORCE_CLI_MODE_ENV_VAR_NAME; use client::{ self, Client, TelemetrySettings, UserStore, ZED_APP_VERSION, ZED_SECRET_CLIENT_TOKEN, }; use db::kvp::KEY_VALUE_STORE; -use editor::{scroll::autoscroll::Autoscroll, Editor}; -use futures::{ - channel::{mpsc, oneshot}, - FutureExt, SinkExt, StreamExt, -}; +use editor::Editor; +use futures::StreamExt; use gpui::{Action, App, AppContext, AssetSource, AsyncAppContext, Task}; use isahc::{config::Configurable, Request}; -use language::{LanguageRegistry, Point}; +use language::LanguageRegistry; use log::LevelFilter; use node_runtime::RealNodeRuntime; use parking_lot::Mutex; @@ -28,7 +22,6 @@ use settings::{default_settings, handle_settings_file_changes, watch_config_file use simplelog::ConfigBuilder; use smol::process::Command; use std::{ - collections::HashMap, env, ffi::OsStr, fs::OpenOptions, @@ -42,11 +35,9 @@ use std::{ thread, time::{Duration, SystemTime, UNIX_EPOCH}, }; -use sum_tree::Bias; use util::{ channel::{parse_zed_link, ReleaseChannel}, http::{self, HttpClient}, - paths::PathLikeWithPosition, }; use uuid::Uuid; use welcome::{show_welcome_experience, FIRST_OPEN}; @@ -58,12 +49,9 @@ use zed::{ assets::Assets, build_window_options, handle_keymap_file_changes, initialize_workspace, languages, menus, only_instance::{ensure_only_instance, IsOnlyInstance}, + open_listener::{handle_cli_connection, OpenListener, OpenRequest}, }; -use crate::open_listener::{OpenListener, OpenRequest}; - -mod open_listener; - fn main() { let http = http::client(); init_paths(); @@ -113,6 +101,7 @@ fn main() { app.run(move |cx| { cx.set_global(*RELEASE_CHANNEL); + cx.set_global(listener.clone()); let mut store = SettingsStore::default(); store @@ -729,189 +718,6 @@ async fn watch_languages(_: Arc, _: Arc) -> Option<()> #[cfg(not(debug_assertions))] fn watch_file_types(_fs: Arc, _cx: &mut AppContext) {} -fn connect_to_cli( - server_name: &str, -) -> Result<(mpsc::Receiver, IpcSender)> { - let handshake_tx = cli::ipc::IpcSender::::connect(server_name.to_string()) - .context("error connecting to cli")?; - let (request_tx, request_rx) = ipc::channel::()?; - let (response_tx, response_rx) = ipc::channel::()?; - - handshake_tx - .send(IpcHandshake { - requests: request_tx, - responses: response_rx, - }) - .context("error sending ipc handshake")?; - - let (mut async_request_tx, async_request_rx) = - futures::channel::mpsc::channel::(16); - thread::spawn(move || { - while let Ok(cli_request) = request_rx.recv() { - if smol::block_on(async_request_tx.send(cli_request)).is_err() { - break; - } - } - Ok::<_, anyhow::Error>(()) - }); - - Ok((async_request_rx, response_tx)) -} - -async fn handle_cli_connection( - (mut requests, responses): (mpsc::Receiver, IpcSender), - app_state: Arc, - mut cx: AsyncAppContext, -) { - if let Some(request) = requests.next().await { - match request { - CliRequest::Open { paths, wait } => { - let mut caret_positions = HashMap::new(); - - let paths = if paths.is_empty() { - workspace::last_opened_workspace_paths() - .await - .map(|location| location.paths().to_vec()) - .unwrap_or_default() - } else { - paths - .into_iter() - .filter_map(|path_with_position_string| { - let path_with_position = PathLikeWithPosition::parse_str( - &path_with_position_string, - |path_str| { - Ok::<_, std::convert::Infallible>( - Path::new(path_str).to_path_buf(), - ) - }, - ) - .expect("Infallible"); - let path = path_with_position.path_like; - if let Some(row) = path_with_position.row { - if path.is_file() { - let row = row.saturating_sub(1); - let col = - path_with_position.column.unwrap_or(0).saturating_sub(1); - caret_positions.insert(path.clone(), Point::new(row, col)); - } - } - Some(path) - }) - .collect() - }; - - let mut errored = false; - match cx - .update(|cx| workspace::open_paths(&paths, &app_state, None, cx)) - .await - { - Ok((workspace, items)) => { - let mut item_release_futures = Vec::new(); - - for (item, path) in items.into_iter().zip(&paths) { - match item { - Some(Ok(item)) => { - if let Some(point) = caret_positions.remove(path) { - if let Some(active_editor) = item.downcast::() { - active_editor - .downgrade() - .update(&mut cx, |editor, cx| { - let snapshot = - editor.snapshot(cx).display_snapshot; - let point = snapshot - .buffer_snapshot - .clip_point(point, Bias::Left); - editor.change_selections( - Some(Autoscroll::center()), - cx, - |s| s.select_ranges([point..point]), - ); - }) - .log_err(); - } - } - - let released = oneshot::channel(); - cx.update(|cx| { - item.on_release( - cx, - Box::new(move |_| { - let _ = released.0.send(()); - }), - ) - .detach(); - }); - item_release_futures.push(released.1); - } - Some(Err(err)) => { - responses - .send(CliResponse::Stderr { - message: format!("error opening {:?}: {}", path, err), - }) - .log_err(); - errored = true; - } - None => {} - } - } - - if wait { - let background = cx.background(); - let wait = async move { - if paths.is_empty() { - let (done_tx, done_rx) = oneshot::channel(); - if let Some(workspace) = workspace.upgrade(&cx) { - let _subscription = cx.update(|cx| { - cx.observe_release(&workspace, move |_, _| { - let _ = done_tx.send(()); - }) - }); - drop(workspace); - let _ = done_rx.await; - } - } else { - let _ = - futures::future::try_join_all(item_release_futures).await; - }; - } - .fuse(); - futures::pin_mut!(wait); - - loop { - // Repeatedly check if CLI is still open to avoid wasting resources - // waiting for files or workspaces to close. - let mut timer = background.timer(Duration::from_secs(1)).fuse(); - futures::select_biased! { - _ = wait => break, - _ = timer => { - if responses.send(CliResponse::Ping).is_err() { - break; - } - } - } - } - } - } - Err(error) => { - errored = true; - responses - .send(CliResponse::Stderr { - message: format!("error opening {:?}: {}", paths, error), - }) - .log_err(); - } - } - - responses - .send(CliResponse::Exit { - status: i32::from(errored), - }) - .log_err(); - } - } - } -} - pub fn background_actions() -> &'static [(&'static str, &'static dyn Action)] { &[ ("Go to file", &file_finder::Toggle), diff --git a/crates/zed/src/open_listener.rs b/crates/zed/src/open_listener.rs index 9b416e14be4b601cad8c6b4555a9c9459757de84..578d8cd69f1992e9e6165b303ff2f2cb7765a7ac 100644 --- a/crates/zed/src/open_listener.rs +++ b/crates/zed/src/open_listener.rs @@ -1,15 +1,26 @@ -use anyhow::anyhow; +use anyhow::{anyhow, Context, Result}; +use cli::{ipc, IpcHandshake}; use cli::{ipc::IpcSender, CliRequest, CliResponse}; -use futures::channel::mpsc; +use editor::scroll::autoscroll::Autoscroll; +use editor::Editor; use futures::channel::mpsc::{UnboundedReceiver, UnboundedSender}; +use futures::channel::{mpsc, oneshot}; +use futures::{FutureExt, SinkExt, StreamExt}; +use gpui::AsyncAppContext; +use language::{Bias, Point}; +use std::collections::HashMap; use std::ffi::OsStr; use std::os::unix::prelude::OsStrExt; +use std::path::Path; use std::sync::atomic::Ordering; +use std::sync::Arc; +use std::thread; +use std::time::Duration; use std::{path::PathBuf, sync::atomic::AtomicBool}; use util::channel::parse_zed_link; +use util::paths::PathLikeWithPosition; use util::ResultExt; - -use crate::connect_to_cli; +use workspace::AppState; pub enum OpenRequest { Paths { @@ -96,3 +107,186 @@ impl OpenListener { Some(OpenRequest::Paths { paths }) } } + +fn connect_to_cli( + server_name: &str, +) -> Result<(mpsc::Receiver, IpcSender)> { + let handshake_tx = cli::ipc::IpcSender::::connect(server_name.to_string()) + .context("error connecting to cli")?; + let (request_tx, request_rx) = ipc::channel::()?; + let (response_tx, response_rx) = ipc::channel::()?; + + handshake_tx + .send(IpcHandshake { + requests: request_tx, + responses: response_rx, + }) + .context("error sending ipc handshake")?; + + let (mut async_request_tx, async_request_rx) = + futures::channel::mpsc::channel::(16); + thread::spawn(move || { + while let Ok(cli_request) = request_rx.recv() { + if smol::block_on(async_request_tx.send(cli_request)).is_err() { + break; + } + } + Ok::<_, anyhow::Error>(()) + }); + + Ok((async_request_rx, response_tx)) +} + +pub async fn handle_cli_connection( + (mut requests, responses): (mpsc::Receiver, IpcSender), + app_state: Arc, + mut cx: AsyncAppContext, +) { + if let Some(request) = requests.next().await { + match request { + CliRequest::Open { paths, wait } => { + let mut caret_positions = HashMap::new(); + + let paths = if paths.is_empty() { + workspace::last_opened_workspace_paths() + .await + .map(|location| location.paths().to_vec()) + .unwrap_or_default() + } else { + paths + .into_iter() + .filter_map(|path_with_position_string| { + let path_with_position = PathLikeWithPosition::parse_str( + &path_with_position_string, + |path_str| { + Ok::<_, std::convert::Infallible>( + Path::new(path_str).to_path_buf(), + ) + }, + ) + .expect("Infallible"); + let path = path_with_position.path_like; + if let Some(row) = path_with_position.row { + if path.is_file() { + let row = row.saturating_sub(1); + let col = + path_with_position.column.unwrap_or(0).saturating_sub(1); + caret_positions.insert(path.clone(), Point::new(row, col)); + } + } + Some(path) + }) + .collect() + }; + + let mut errored = false; + match cx + .update(|cx| workspace::open_paths(&paths, &app_state, None, cx)) + .await + { + Ok((workspace, items)) => { + let mut item_release_futures = Vec::new(); + + for (item, path) in items.into_iter().zip(&paths) { + match item { + Some(Ok(item)) => { + if let Some(point) = caret_positions.remove(path) { + if let Some(active_editor) = item.downcast::() { + active_editor + .downgrade() + .update(&mut cx, |editor, cx| { + let snapshot = + editor.snapshot(cx).display_snapshot; + let point = snapshot + .buffer_snapshot + .clip_point(point, Bias::Left); + editor.change_selections( + Some(Autoscroll::center()), + cx, + |s| s.select_ranges([point..point]), + ); + }) + .log_err(); + } + } + + let released = oneshot::channel(); + cx.update(|cx| { + item.on_release( + cx, + Box::new(move |_| { + let _ = released.0.send(()); + }), + ) + .detach(); + }); + item_release_futures.push(released.1); + } + Some(Err(err)) => { + responses + .send(CliResponse::Stderr { + message: format!("error opening {:?}: {}", path, err), + }) + .log_err(); + errored = true; + } + None => {} + } + } + + if wait { + let background = cx.background(); + let wait = async move { + if paths.is_empty() { + let (done_tx, done_rx) = oneshot::channel(); + if let Some(workspace) = workspace.upgrade(&cx) { + let _subscription = cx.update(|cx| { + cx.observe_release(&workspace, move |_, _| { + let _ = done_tx.send(()); + }) + }); + drop(workspace); + let _ = done_rx.await; + } + } else { + let _ = + futures::future::try_join_all(item_release_futures).await; + }; + } + .fuse(); + futures::pin_mut!(wait); + + loop { + // Repeatedly check if CLI is still open to avoid wasting resources + // waiting for files or workspaces to close. + let mut timer = background.timer(Duration::from_secs(1)).fuse(); + futures::select_biased! { + _ = wait => break, + _ = timer => { + if responses.send(CliResponse::Ping).is_err() { + break; + } + } + } + } + } + } + Err(error) => { + errored = true; + responses + .send(CliResponse::Stderr { + message: format!("error opening {:?}: {}", paths, error), + }) + .log_err(); + } + } + + responses + .send(CliResponse::Exit { + status: i32::from(errored), + }) + .log_err(); + } + } + } +} diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 4e9a34c2699ab977eca0ed07b4784cc52f55922d..c2a218acae9b6d72098135e402c3aac6fa6cf6e7 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -2,6 +2,7 @@ pub mod assets; pub mod languages; pub mod menus; pub mod only_instance; +pub mod open_listener; #[cfg(any(test, feature = "test-support"))] pub mod test; @@ -28,6 +29,7 @@ use gpui::{ AppContext, AsyncAppContext, Task, ViewContext, WeakViewHandle, }; pub use lsp; +use open_listener::OpenListener; pub use project; use project_panel::ProjectPanel; use quick_action_bar::QuickActionBar; @@ -87,6 +89,10 @@ pub fn init(app_state: &Arc, cx: &mut gpui::AppContext) { }, ); cx.add_global_action(quit); + cx.add_global_action(move |action: &OpenZedURL, cx| { + cx.global::>() + .open_urls(vec![action.url.clone()]) + }); cx.add_global_action(move |action: &OpenBrowser, cx| cx.platform().open_url(&action.url)); cx.add_global_action(move |_: &IncreaseBufferFontSize, cx| { theme::adjust_font_size(cx, |size| *size += 1.0) From 465d726bd4508490b993689073f08a764d178eca Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 17 Oct 2023 03:05:01 -0700 Subject: [PATCH 18/25] Minor adjustments --- .cargo/config.toml | 2 +- crates/channel/src/channel_store.rs | 1 - .../src/channel_store/channel_index.rs | 5 +- crates/collab/src/db/ids.rs | 9 +++ crates/collab/src/db/queries/channels.rs | 61 +++++++++---------- 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index e22bdb0f2c70a1ffda714674253cc533e9e7c1d1..9da6b3be080072d89d16a199e2d60d527eeacd07 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -3,4 +3,4 @@ xtask = "run --package xtask --" [build] # v0 mangling scheme provides more detailed backtraces around closures -rustflags = ["-C", "symbol-mangling-version=v0", "-C", "link-arg=-fuse-ld=/opt/homebrew/Cellar/llvm/16.0.6/bin/ld64.lld"] +rustflags = ["-C", "symbol-mangling-version=v0"] diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 57b183f7debeff01350a87a887373f5dfb43a894..3e8fbafb6ad0fce58d8232fedf0ac95b8cf1d625 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -972,7 +972,6 @@ impl ChannelStore { let mut all_user_ids = Vec::new(); let channel_participants = payload.channel_participants; - dbg!(&channel_participants); for entry in &channel_participants { for user_id in entry.participant_user_ids.iter() { if let Err(ix) = all_user_ids.binary_search(user_id) { diff --git a/crates/channel/src/channel_store/channel_index.rs b/crates/channel/src/channel_store/channel_index.rs index 7b54d5dcd904f8fa181a9ebcc20a0ff1e2c1e4d5..36379a3942be2f2df3a24b70eb8833aabc214af2 100644 --- a/crates/channel/src/channel_store/channel_index.rs +++ b/crates/channel/src/channel_store/channel_index.rs @@ -123,8 +123,9 @@ impl<'a> ChannelPathsInsertGuard<'a> { pub fn insert(&mut self, channel_proto: proto::Channel) { if let Some(existing_channel) = self.channels_by_id.get_mut(&channel_proto.id) { - Arc::make_mut(existing_channel).visibility = channel_proto.visibility(); - Arc::make_mut(existing_channel).name = channel_proto.name; + let existing_channel = Arc::make_mut(existing_channel); + existing_channel.visibility = channel_proto.visibility(); + existing_channel.name = channel_proto.name; } else { self.channels_by_id.insert( channel_proto.id, diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 970d66d4cbcf45b2dd559222fb46e8be93c723b9..38240fd4c418c63f25a3fe3253c5fdda42bb03e7 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -106,6 +106,15 @@ impl ChannelRole { Guest => false, } } + + pub fn max(&self, other: Self) -> Self { + match (self, other) { + (ChannelRole::Admin, _) | (_, ChannelRole::Admin) => ChannelRole::Admin, + (ChannelRole::Member, _) | (_, ChannelRole::Member) => ChannelRole::Member, + (ChannelRole::Banned, _) | (_, ChannelRole::Banned) => ChannelRole::Banned, + (ChannelRole::Guest, _) | (_, ChannelRole::Guest) => ChannelRole::Guest, + } + } } impl From for ChannelRole { diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index b10cbd14f18c701a9415bb2705f884e8c760594f..0dc197aa0b6e5a36e40d2ee0d50a2bace91f95c3 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -209,7 +209,7 @@ impl Database { let mut channels_to_remove: HashSet = HashSet::default(); channels_to_remove.insert(channel_id); - let graph = self.get_channel_descendants_2([channel_id], &*tx).await?; + let graph = self.get_channel_descendants([channel_id], &*tx).await?; for edge in graph.iter() { channels_to_remove.insert(ChannelId::from_proto(edge.channel_id)); } @@ -218,7 +218,7 @@ impl Database { let mut channels_to_keep = channel_path::Entity::find() .filter( channel_path::Column::ChannelId - .is_in(channels_to_remove.clone()) + .is_in(channels_to_remove.iter().copied()) .and( channel_path::Column::IdPath .not_like(&format!("%/{}/%", channel_id)), @@ -243,7 +243,7 @@ impl Database { .await?; channel::Entity::delete_many() - .filter(channel::Column::Id.is_in(channels_to_remove.clone())) + .filter(channel::Column::Id.is_in(channels_to_remove.iter().copied())) .exec(&*tx) .await?; @@ -484,7 +484,7 @@ impl Database { tx: &DatabaseTransaction, ) -> Result { let mut edges = self - .get_channel_descendants_2(channel_memberships.iter().map(|m| m.channel_id), &*tx) + .get_channel_descendants(channel_memberships.iter().map(|m| m.channel_id), &*tx) .await?; let mut role_for_channel: HashMap = HashMap::default(); @@ -515,7 +515,7 @@ impl Database { let mut channels_to_remove: HashSet = HashSet::default(); let mut rows = channel::Entity::find() - .filter(channel::Column::Id.is_in(role_for_channel.keys().cloned())) + .filter(channel::Column::Id.is_in(role_for_channel.keys().copied())) .stream(&*tx) .await?; @@ -877,7 +877,7 @@ impl Database { let channel_ids = self.get_channel_ancestors(channel_id, tx).await?; let rows = channel::Entity::find() - .filter(channel::Column::Id.is_in(channel_ids.clone())) + .filter(channel::Column::Id.is_in(channel_ids.iter().copied())) .filter(channel::Column::Visibility.eq(ChannelVisibility::Public)) .all(&*tx) .await?; @@ -928,40 +928,39 @@ impl Database { .stream(&*tx) .await?; - let mut is_admin = false; - let mut is_member = false; + let mut user_role: Option = None; + let max_role = |role| { + user_role + .map(|user_role| user_role.max(role)) + .get_or_insert(role); + }; + let mut is_participant = false; - let mut is_banned = false; let mut current_channel_visibility = None; // note these channels are not iterated in any particular order, // our current logic takes the highest permission available. while let Some(row) = rows.next().await { - let (ch_id, role, visibility): (ChannelId, ChannelRole, ChannelVisibility) = row?; + let (membership_channel, role, visibility): ( + ChannelId, + ChannelRole, + ChannelVisibility, + ) = row?; match role { - ChannelRole::Admin => is_admin = true, - ChannelRole::Member => is_member = true, - ChannelRole::Guest => { - if visibility == ChannelVisibility::Public { - is_participant = true - } + ChannelRole::Admin | ChannelRole::Member | ChannelRole::Banned => max_role(role), + ChannelRole::Guest if visibility == ChannelVisibility::Public => { + is_participant = true } - ChannelRole::Banned => is_banned = true, + ChannelRole::Guest => {} } - if channel_id == ch_id { + if channel_id == membership_channel { current_channel_visibility = Some(visibility); } } // free up database connection drop(rows); - Ok(if is_admin { - Some(ChannelRole::Admin) - } else if is_member { - Some(ChannelRole::Member) - } else if is_banned { - Some(ChannelRole::Banned) - } else if is_participant { + if is_participant && user_role.is_none() { if current_channel_visibility.is_none() { current_channel_visibility = channel::Entity::find() .filter(channel::Column::Id.eq(channel_id)) @@ -970,13 +969,11 @@ impl Database { .map(|channel| channel.visibility); } if current_channel_visibility == Some(ChannelVisibility::Public) { - Some(ChannelRole::Guest) - } else { - None + user_role = Some(ChannelRole::Guest); } - } else { - None - }) + } + + Ok(user_role) } /// Returns the channel ancestors in arbitrary order @@ -1007,7 +1004,7 @@ impl Database { // Returns the channel desendants as a sorted list of edges for further processing. // The edges are sorted such that you will see unknown channel ids as children // before you see them as parents. - async fn get_channel_descendants_2( + async fn get_channel_descendants( &self, channel_ids: impl IntoIterator, tx: &DatabaseTransaction, From 851701cb6f1e50d0ccd272b107bad525eddf99f2 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Oct 2023 09:41:34 -0600 Subject: [PATCH 19/25] Fix get_most_public_ancestor --- crates/channel/src/channel_store.rs | 25 ++++++ crates/collab/src/db/ids.rs | 9 +-- crates/collab/src/db/queries/channels.rs | 78 +++++++++---------- crates/collab/src/db/tests/channel_tests.rs | 48 ++++++++++++ .../src/collab_panel/channel_modal.rs | 14 +++- crates/command_palette/src/command_palette.rs | 2 +- 6 files changed, 126 insertions(+), 50 deletions(-) diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index 3e8fbafb6ad0fce58d8232fedf0ac95b8cf1d625..5fb7ddc72ccded0bf326032c9393becb1f2ca672 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -82,6 +82,31 @@ pub struct ChannelMembership { pub kind: proto::channel_member::Kind, pub role: proto::ChannelRole, } +impl ChannelMembership { + pub fn sort_key(&self) -> MembershipSortKey { + MembershipSortKey { + role_order: match self.role { + proto::ChannelRole::Admin => 0, + proto::ChannelRole::Member => 1, + proto::ChannelRole::Banned => 2, + proto::ChannelRole::Guest => 3, + }, + kind_order: match self.kind { + proto::channel_member::Kind::Member => 0, + proto::channel_member::Kind::AncestorMember => 1, + proto::channel_member::Kind::Invitee => 2, + }, + username_order: self.user.github_login.as_str(), + } + } +} + +#[derive(PartialOrd, Ord, PartialEq, Eq)] +pub struct MembershipSortKey<'a> { + role_order: u8, + kind_order: u8, + username_order: &'a str, +} pub enum ChannelEvent { ChannelCreated(ChannelId), diff --git a/crates/collab/src/db/ids.rs b/crates/collab/src/db/ids.rs index 38240fd4c418c63f25a3fe3253c5fdda42bb03e7..f0de4c255edc7b13eb27656ceaccefb3c5e26c02 100644 --- a/crates/collab/src/db/ids.rs +++ b/crates/collab/src/db/ids.rs @@ -108,11 +108,10 @@ impl ChannelRole { } pub fn max(&self, other: Self) -> Self { - match (self, other) { - (ChannelRole::Admin, _) | (_, ChannelRole::Admin) => ChannelRole::Admin, - (ChannelRole::Member, _) | (_, ChannelRole::Member) => ChannelRole::Member, - (ChannelRole::Banned, _) | (_, ChannelRole::Banned) => ChannelRole::Banned, - (ChannelRole::Guest, _) | (_, ChannelRole::Guest) => ChannelRole::Guest, + if self.should_override(other) { + *self + } else { + other } } } diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 0dc197aa0b6e5a36e40d2ee0d50a2bace91f95c3..a1a618c7334b985bf70382f59f8b15ae10e60415 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -1,5 +1,3 @@ -use std::cmp::Ordering; - use super::*; use rpc::proto::{channel_member::Kind, ChannelEdge}; @@ -544,6 +542,12 @@ impl Database { if !channels_to_remove.is_empty() { // Note: this code assumes each channel has one parent. + // If there are multiple valid public paths to a channel, + // e.g. + // If both of these paths are present (* indicating public): + // - zed* -> projects -> vim* + // - zed* -> conrad -> public-projects* -> vim* + // Users would only see one of them (based on edge sort order) let mut replacement_parent: HashMap = HashMap::default(); for ChannelEdge { parent_id, @@ -707,14 +711,14 @@ impl Database { } let mut user_details: HashMap = HashMap::default(); - while let Some(row) = stream.next().await { + while let Some(user_membership) = stream.next().await { let (user_id, channel_role, is_direct_member, is_invite_accepted, visibility): ( UserId, ChannelRole, bool, bool, ChannelVisibility, - ) = row?; + ) = user_membership?; let kind = match (is_direct_member, is_invite_accepted) { (true, true) => proto::channel_member::Kind::Member, (true, false) => proto::channel_member::Kind::Invitee, @@ -745,33 +749,7 @@ impl Database { } } - // sort by permissions descending, within each section, show members, then ancestor members, then invitees. - let mut results: Vec<(UserId, UserDetail)> = user_details.into_iter().collect(); - results.sort_by(|a, b| { - if a.1.channel_role.should_override(b.1.channel_role) { - return Ordering::Less; - } else if b.1.channel_role.should_override(a.1.channel_role) { - return Ordering::Greater; - } - - if a.1.kind == Kind::Member && b.1.kind != Kind::Member { - return Ordering::Less; - } else if b.1.kind == Kind::Member && a.1.kind != Kind::Member { - return Ordering::Greater; - } - - if a.1.kind == Kind::AncestorMember && b.1.kind != Kind::AncestorMember { - return Ordering::Less; - } else if b.1.kind == Kind::AncestorMember && a.1.kind != Kind::AncestorMember { - return Ordering::Greater; - } - - // would be nice to sort alphabetically instead of by user id. - // (or defer all sorting to the UI, but we need something to help the tests) - return a.0.cmp(&b.0); - }); - - Ok(results + Ok(user_details .into_iter() .map(|(user_id, details)| proto::ChannelMember { user_id: user_id.to_proto(), @@ -810,7 +788,7 @@ impl Database { user_id: UserId, tx: &DatabaseTransaction, ) -> Result<()> { - match self.channel_role_for_user(channel_id, user_id, tx).await? { + match dbg!(self.channel_role_for_user(channel_id, user_id, tx).await)? { Some(ChannelRole::Admin) => Ok(()), Some(ChannelRole::Member) | Some(ChannelRole::Banned) @@ -874,10 +852,26 @@ impl Database { channel_id: ChannelId, tx: &DatabaseTransaction, ) -> Result> { - let channel_ids = self.get_channel_ancestors(channel_id, tx).await?; + // Note: if there are many paths to a channel, this will return just one + let arbitary_path = channel_path::Entity::find() + .filter(channel_path::Column::ChannelId.eq(channel_id)) + .order_by(channel_path::Column::IdPath, sea_orm::Order::Desc) + .one(tx) + .await?; + + let Some(path) = arbitary_path else { + return Ok(None); + }; + + let ancestor_ids: Vec = path + .id_path + .trim_matches('/') + .split('/') + .map(|id| ChannelId::from_proto(id.parse().unwrap())) + .collect(); let rows = channel::Entity::find() - .filter(channel::Column::Id.is_in(channel_ids.iter().copied())) + .filter(channel::Column::Id.is_in(ancestor_ids.iter().copied())) .filter(channel::Column::Visibility.eq(ChannelVisibility::Public)) .all(&*tx) .await?; @@ -888,7 +882,7 @@ impl Database { visible_channels.insert(row.id); } - for ancestor in channel_ids.into_iter().rev() { + for ancestor in ancestor_ids { if visible_channels.contains(&ancestor) { return Ok(Some(ancestor)); } @@ -929,11 +923,6 @@ impl Database { .await?; let mut user_role: Option = None; - let max_role = |role| { - user_role - .map(|user_role| user_role.max(role)) - .get_or_insert(role); - }; let mut is_participant = false; let mut current_channel_visibility = None; @@ -946,8 +935,15 @@ impl Database { ChannelRole, ChannelVisibility, ) = row?; + match role { - ChannelRole::Admin | ChannelRole::Member | ChannelRole::Banned => max_role(role), + ChannelRole::Admin | ChannelRole::Member | ChannelRole::Banned => { + if let Some(users_role) = user_role { + user_role = Some(users_role.max(role)); + } else { + user_role = Some(role) + } + } ChannelRole::Guest if visibility == ChannelVisibility::Public => { is_participant = true } diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index f08b1554bc0062e080e1bd5fdd8eee146f090c9d..ac272726da463f4d961f19127cedb3d551642306 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -1028,6 +1028,54 @@ async fn test_user_is_channel_participant(db: &Arc) { ) } +test_both_dbs!( + test_user_joins_correct_channel, + test_user_joins_correct_channel_postgres, + test_user_joins_correct_channel_sqlite +); + +async fn test_user_joins_correct_channel(db: &Arc) { + let admin = new_test_user(db, "admin@example.com").await; + + let zed_channel = db.create_root_channel("zed", admin).await.unwrap(); + + let active_channel = db + .create_channel("active", Some(zed_channel), admin) + .await + .unwrap(); + + let vim_channel = db + .create_channel("vim", Some(active_channel), admin) + .await + .unwrap(); + + let vim2_channel = db + .create_channel("vim2", Some(vim_channel), admin) + .await + .unwrap(); + + db.set_channel_visibility(zed_channel, crate::db::ChannelVisibility::Public, admin) + .await + .unwrap(); + + db.set_channel_visibility(vim_channel, crate::db::ChannelVisibility::Public, admin) + .await + .unwrap(); + + db.set_channel_visibility(vim2_channel, crate::db::ChannelVisibility::Public, admin) + .await + .unwrap(); + + let most_public = db + .transaction( + |tx| async move { db.most_public_ancestor_for_channel(vim_channel, &*tx).await }, + ) + .await + .unwrap(); + + assert_eq!(most_public, Some(zed_channel)) +} + #[track_caller] fn assert_dag(actual: ChannelGraph, expected: &[(ChannelId, Option)]) { let mut actual_map: HashMap> = HashMap::default(); diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index da6edbde696ae011895142b95641f969538da28e..0ccf0894b25fc1fc04ed884769c87b78bfaa72fc 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -100,11 +100,14 @@ impl ChannelModal { let channel_id = self.channel_id; cx.spawn(|this, mut cx| async move { if mode == Mode::ManageMembers { - let members = channel_store + let mut members = channel_store .update(&mut cx, |channel_store, cx| { channel_store.get_channel_member_details(channel_id, cx) }) .await?; + + members.sort_by(|a, b| a.sort_key().cmp(&b.sort_key())); + this.update(&mut cx, |this, cx| { this.picker .update(cx, |picker, _| picker.delegate_mut().members = members); @@ -675,11 +678,16 @@ impl ChannelModalDelegate { invite_member.await?; this.update(&mut cx, |this, cx| { - this.delegate_mut().members.push(ChannelMembership { + let new_member = ChannelMembership { user, kind: proto::channel_member::Kind::Invitee, role: ChannelRole::Member, - }); + }; + let members = &mut this.delegate_mut().members; + match members.binary_search_by_key(&new_member.sort_key(), |k| k.sort_key()) { + Ok(ix) | Err(ix) => members.insert(ix, new_member), + } + cx.notify(); }) }) diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 9b74c13a71e70ea66b292d2232f0e4f55dc6b451..ce762876a41238ab4e0f51ddcd29155b39274840 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -7,7 +7,7 @@ use gpui::{ use picker::{Picker, PickerDelegate, PickerEvent}; use std::cmp::{self, Reverse}; use util::{ - channel::{parse_zed_link, ReleaseChannel, RELEASE_CHANNEL, RELEASE_CHANNEL_NAME}, + channel::{parse_zed_link, ReleaseChannel, RELEASE_CHANNEL}, ResultExt, }; use workspace::Workspace; From 2456c077f698d72d411543f33fc1d3da3df14c95 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Oct 2023 10:01:31 -0600 Subject: [PATCH 20/25] Fix channel test ordering --- crates/collab/src/db/tests/channel_tests.rs | 31 +++++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/collab/src/db/tests/channel_tests.rs b/crates/collab/src/db/tests/channel_tests.rs index ac272726da463f4d961f19127cedb3d551642306..40842aff5c3bbd90fa7fc1e38297096e6c2e8bcc 100644 --- a/crates/collab/src/db/tests/channel_tests.rs +++ b/crates/collab/src/db/tests/channel_tests.rs @@ -281,10 +281,12 @@ async fn test_channel_invites(db: &Arc) { assert_eq!(user_3_invites, &[channel_1_1]); - let members = db + let mut members = db .get_channel_participant_details(channel_1_1, user_1) .await .unwrap(); + + members.sort_by_key(|member| member.user_id); assert_eq!( members, &[ @@ -294,14 +296,14 @@ async fn test_channel_invites(db: &Arc) { role: proto::ChannelRole::Admin.into(), }, proto::ChannelMember { - user_id: user_3.to_proto(), + user_id: user_2.to_proto(), kind: proto::channel_member::Kind::Invitee.into(), - role: proto::ChannelRole::Admin.into(), + role: proto::ChannelRole::Member.into(), }, proto::ChannelMember { - user_id: user_2.to_proto(), + user_id: user_3.to_proto(), kind: proto::channel_member::Kind::Invitee.into(), - role: proto::ChannelRole::Member.into(), + role: proto::ChannelRole::Admin.into(), }, ] ); @@ -857,10 +859,13 @@ async fn test_user_is_channel_participant(db: &Arc) { .await .unwrap(); - let members = db + let mut members = db .get_channel_participant_details(vim_channel, admin) .await .unwrap(); + + members.sort_by_key(|member| member.user_id); + assert_eq!( members, &[ @@ -912,11 +917,13 @@ async fn test_user_is_channel_participant(db: &Arc) { .await .is_err()); - let members = db + let mut members = db .get_channel_participant_details(vim_channel, admin) .await .unwrap(); + members.sort_by_key(|member| member.user_id); + assert_eq!( members, &[ @@ -951,10 +958,13 @@ async fn test_user_is_channel_participant(db: &Arc) { .unwrap(); // currently people invited to parent channels are not shown here - let members = db + let mut members = db .get_channel_participant_details(vim_channel, admin) .await .unwrap(); + + members.sort_by_key(|member| member.user_id); + assert_eq!( members, &[ @@ -996,10 +1006,13 @@ async fn test_user_is_channel_participant(db: &Arc) { .await .unwrap(); - let members = db + let mut members = db .get_channel_participant_details(vim_channel, admin) .await .unwrap(); + + members.sort_by_key(|member| member.user_id); + assert_eq!( members, &[ From 3412becfc53ad2551412f586ef58b2c589fe3810 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Oct 2023 10:15:20 -0600 Subject: [PATCH 21/25] Fix some tests --- crates/channel/src/channel_store_tests.rs | 16 ++++++++-------- crates/collab/src/db/queries/channels.rs | 3 ++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index ea47c7c7b7fe0f5db2c8776cebc7791d9b306a91..23f2e11a03e3fdd577412c5382ff724212e09e0f 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -18,12 +18,12 @@ fn test_update_channels(cx: &mut AppContext) { proto::Channel { id: 1, name: "b".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }, proto::Channel { id: 2, name: "a".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }, ], channel_permissions: vec![proto::ChannelPermission { @@ -51,12 +51,12 @@ fn test_update_channels(cx: &mut AppContext) { proto::Channel { id: 3, name: "x".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }, proto::Channel { id: 4, name: "y".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }, ], insert_edge: vec![ @@ -96,17 +96,17 @@ fn test_dangling_channel_paths(cx: &mut AppContext) { proto::Channel { id: 0, name: "a".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }, proto::Channel { id: 1, name: "b".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }, proto::Channel { id: 2, name: "c".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }, ], insert_edge: vec![ @@ -165,7 +165,7 @@ async fn test_channel_messages(cx: &mut TestAppContext) { channels: vec![proto::Channel { id: channel_id, name: "the-channel".to_string(), - visibility: proto::ChannelVisibility::ChannelMembers as i32, + visibility: proto::ChannelVisibility::Members as i32, }], ..Default::default() }); diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index a1a618c7334b985bf70382f59f8b15ae10e60415..07fe219330dcc2d13fd97722bd7d6885a01e034d 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -143,7 +143,8 @@ impl Database { channel_id: ActiveValue::Set(channel_id_to_join), user_id: ActiveValue::Set(user_id), accepted: ActiveValue::Set(true), - role: ActiveValue::Set(ChannelRole::Guest), + // TODO: change this back to Guest. + role: ActiveValue::Set(ChannelRole::Member), }) .exec(&*tx) .await?; From 5b39fc81232f4c7ed9aa94649f0e951f292b5f6d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Oct 2023 10:24:47 -0600 Subject: [PATCH 22/25] Temporarily join public channels as a member --- crates/collab/src/db/queries/channels.rs | 5 +++-- crates/collab/src/rpc.rs | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/collab/src/db/queries/channels.rs b/crates/collab/src/db/queries/channels.rs index 07fe219330dcc2d13fd97722bd7d6885a01e034d..ee989b2ea068ff362adc635e7616b1c25a2de573 100644 --- a/crates/collab/src/db/queries/channels.rs +++ b/crates/collab/src/db/queries/channels.rs @@ -135,7 +135,8 @@ impl Database { .most_public_ancestor_for_channel(channel_id, &*tx) .await? .unwrap_or(channel_id); - role = Some(ChannelRole::Guest); + // TODO: change this back to Guest. + role = Some(ChannelRole::Member); joined_channel_id = Some(channel_id_to_join); channel_member::Entity::insert(channel_member::ActiveModel { @@ -789,7 +790,7 @@ impl Database { user_id: UserId, tx: &DatabaseTransaction, ) -> Result<()> { - match dbg!(self.channel_role_for_user(channel_id, user_id, tx).await)? { + match self.channel_role_for_user(channel_id, user_id, tx).await? { Some(ChannelRole::Admin) => Ok(()), Some(ChannelRole::Member) | Some(ChannelRole::Banned) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 575c9d88717f79bb933c63f18b6605c152b96985..15ea3b24e133912d030ff15a850c5c3aae70bea8 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -2720,10 +2720,8 @@ async fn join_channel_internal( channel_id: joined_room.channel_id.map(|id| id.to_proto()), live_kit_connection_info, })?; - dbg!("Joined channel", &joined_channel); if let Some(joined_channel) = joined_channel { - dbg!("CMU"); channel_membership_updated(db, joined_channel, &session).await? } From 8db389313bf993d60ecf774122eea276ef0546d2 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Tue, 17 Oct 2023 13:34:51 -0400 Subject: [PATCH 23/25] Add link & public icons --- assets/icons/link.svg | 3 +++ assets/icons/public.svg | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 assets/icons/link.svg create mode 100644 assets/icons/public.svg diff --git a/assets/icons/link.svg b/assets/icons/link.svg new file mode 100644 index 0000000000000000000000000000000000000000..4925bd8e0008b3f39fdd9316e075ec03dcd29dd2 --- /dev/null +++ b/assets/icons/link.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/icons/public.svg b/assets/icons/public.svg new file mode 100644 index 0000000000000000000000000000000000000000..55a79684856cfce950785c6fcb94f5a90648ef02 --- /dev/null +++ b/assets/icons/public.svg @@ -0,0 +1,3 @@ + + + From 1c5e07f4a21f0ca4f0b80881798605c51a94a6ec Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Oct 2023 13:19:22 -0600 Subject: [PATCH 24/25] update sidebar for public channels --- assets/icons/public.svg | 2 +- .../20221109000000_test_schema.sql | 2 +- ...rojects_room_id_fkey_on_delete_cascade.sql | 8 + crates/collab_ui/src/collab_panel.rs | 246 ++++++++++++++---- 4 files changed, 208 insertions(+), 50 deletions(-) create mode 100644 crates/collab/migrations/20231017185833_projects_room_id_fkey_on_delete_cascade.sql diff --git a/assets/icons/public.svg b/assets/icons/public.svg index 55a79684856cfce950785c6fcb94f5a90648ef02..38278cdabaf203ef47fea13a88a60ed6f61d18ac 100644 --- a/assets/icons/public.svg +++ b/assets/icons/public.svg @@ -1,3 +1,3 @@ - + diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index dcb793aa513d0bde1a5e6f5787dc0d7f95fead99..8eb6b52fd8b4f0ece5c64f4c45c48da4ee97fe18 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -44,7 +44,7 @@ CREATE UNIQUE INDEX "index_rooms_on_channel_id" ON "rooms" ("channel_id"); CREATE TABLE "projects" ( "id" INTEGER PRIMARY KEY AUTOINCREMENT, - "room_id" INTEGER REFERENCES rooms (id) NOT NULL, + "room_id" INTEGER REFERENCES rooms (id) ON DELETE CASCADE NOT NULL, "host_user_id" INTEGER REFERENCES users (id) NOT NULL, "host_connection_id" INTEGER, "host_connection_server_id" INTEGER REFERENCES servers (id) ON DELETE CASCADE, diff --git a/crates/collab/migrations/20231017185833_projects_room_id_fkey_on_delete_cascade.sql b/crates/collab/migrations/20231017185833_projects_room_id_fkey_on_delete_cascade.sql new file mode 100644 index 0000000000000000000000000000000000000000..be535ff7fa6e707182b8698647ecc92d9f976183 --- /dev/null +++ b/crates/collab/migrations/20231017185833_projects_room_id_fkey_on_delete_cascade.sql @@ -0,0 +1,8 @@ +-- Add migration script here + +ALTER TABLE projects + DROP CONSTRAINT projects_room_id_fkey, + ADD CONSTRAINT projects_room_id_fkey + FOREIGN KEY (room_id) + REFERENCES rooms (id) + ON DELETE CASCADE; diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 30505b0876c054d8f1b05fcec531d6d04bde5fa1..2e68a1c939c8ba589f1c54a1c6b2abbca3ec8b49 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -11,7 +11,10 @@ use anyhow::Result; use call::ActiveCall; use channel::{Channel, ChannelData, ChannelEvent, ChannelId, ChannelPath, ChannelStore}; use channel_modal::ChannelModal; -use client::{proto::PeerId, Client, Contact, User, UserStore}; +use client::{ + proto::{self, PeerId}, + Client, Contact, User, UserStore, +}; use contact_finder::ContactFinder; use context_menu::{ContextMenu, ContextMenuItem}; use db::kvp::KEY_VALUE_STORE; @@ -428,7 +431,7 @@ enum ListEntry { is_last: bool, }, ParticipantScreen { - peer_id: PeerId, + peer_id: Option, is_last: bool, }, IncomingRequest(Arc), @@ -442,6 +445,9 @@ enum ListEntry { ChannelNotes { channel_id: ChannelId, }, + ChannelChat { + channel_id: ChannelId, + }, ChannelEditor { depth: usize, }, @@ -602,6 +608,13 @@ impl CollabPanel { ix, cx, ), + ListEntry::ChannelChat { channel_id } => this.render_channel_chat( + *channel_id, + &theme.collab_panel, + is_selected, + ix, + cx, + ), ListEntry::ChannelInvite(channel) => Self::render_channel_invite( channel.clone(), this.channel_store.clone(), @@ -804,7 +817,8 @@ impl CollabPanel { let room = room.read(cx); if let Some(channel_id) = room.channel_id() { - self.entries.push(ListEntry::ChannelNotes { channel_id }) + self.entries.push(ListEntry::ChannelNotes { channel_id }); + self.entries.push(ListEntry::ChannelChat { channel_id }) } // Populate the active user. @@ -836,7 +850,13 @@ impl CollabPanel { project_id: project.id, worktree_root_names: project.worktree_root_names.clone(), host_user_id: user_id, - is_last: projects.peek().is_none(), + is_last: projects.peek().is_none() && !room.is_screen_sharing(), + }); + } + if room.is_screen_sharing() { + self.entries.push(ListEntry::ParticipantScreen { + peer_id: None, + is_last: true, }); } } @@ -880,7 +900,7 @@ impl CollabPanel { } if !participant.video_tracks.is_empty() { self.entries.push(ListEntry::ParticipantScreen { - peer_id: participant.peer_id, + peer_id: Some(participant.peer_id), is_last: true, }); } @@ -1225,14 +1245,18 @@ impl CollabPanel { ) -> AnyElement { enum CallParticipant {} enum CallParticipantTooltip {} + enum LeaveCallButton {} + enum LeaveCallTooltip {} let collab_theme = &theme.collab_panel; let is_current_user = user_store.read(cx).current_user().map(|user| user.id) == Some(user.id); - let content = - MouseEventHandler::new::(user.id as usize, cx, |mouse_state, _| { + let content = MouseEventHandler::new::( + user.id as usize, + cx, + |mouse_state, cx| { let style = if is_current_user { *collab_theme .contact_row @@ -1268,14 +1292,32 @@ impl CollabPanel { Label::new("Calling", collab_theme.calling_indicator.text.clone()) .contained() .with_style(collab_theme.calling_indicator.container) - .aligned(), + .aligned() + .into_any(), ) } else if is_current_user { Some( - Label::new("You", collab_theme.calling_indicator.text.clone()) - .contained() - .with_style(collab_theme.calling_indicator.container) - .aligned(), + MouseEventHandler::new::(0, cx, |state, _| { + render_icon_button( + theme + .collab_panel + .leave_call_button + .style_for(is_selected, state), + "icons/exit.svg", + ) + }) + .with_cursor_style(CursorStyle::PointingHand) + .on_click(MouseButton::Left, |_, _, cx| { + Self::leave_call(cx); + }) + .with_tooltip::( + 0, + "Leave call", + None, + theme.tooltip.clone(), + cx, + ) + .into_any(), ) } else { None @@ -1284,7 +1326,8 @@ impl CollabPanel { .with_height(collab_theme.row_height) .contained() .with_style(style) - }); + }, + ); if is_current_user || is_pending || peer_id.is_none() { return content.into_any(); @@ -1406,7 +1449,7 @@ impl CollabPanel { } fn render_participant_screen( - peer_id: PeerId, + peer_id: Option, is_last: bool, is_selected: bool, theme: &theme::CollabPanel, @@ -1421,8 +1464,8 @@ impl CollabPanel { .unwrap_or(0.); let tree_branch = theme.tree_branch; - MouseEventHandler::new::( - peer_id.as_u64() as usize, + let handler = MouseEventHandler::new::( + peer_id.map(|id| id.as_u64()).unwrap_or(0) as usize, cx, |mouse_state, cx| { let tree_branch = *tree_branch.in_state(is_selected).style_for(mouse_state); @@ -1460,16 +1503,20 @@ impl CollabPanel { .contained() .with_style(row.container) }, - ) - .with_cursor_style(CursorStyle::PointingHand) - .on_click(MouseButton::Left, move |_, this, cx| { - if let Some(workspace) = this.workspace.upgrade(cx) { - workspace.update(cx, |workspace, cx| { - workspace.open_shared_screen(peer_id, cx) - }); - } - }) - .into_any() + ); + if peer_id.is_none() { + return handler.into_any(); + } + handler + .with_cursor_style(CursorStyle::PointingHand) + .on_click(MouseButton::Left, move |_, this, cx| { + if let Some(workspace) = this.workspace.upgrade(cx) { + workspace.update(cx, |workspace, cx| { + workspace.open_shared_screen(peer_id.unwrap(), cx) + }); + } + }) + .into_any() } fn take_editing_state(&mut self, cx: &mut ViewContext) -> bool { @@ -1496,23 +1543,32 @@ impl CollabPanel { enum AddChannel {} let tooltip_style = &theme.tooltip; + let mut channel_link = None; + let mut channel_tooltip_text = None; + let mut channel_icon = None; + let text = match section { Section::ActiveCall => { let channel_name = iife!({ let channel_id = ActiveCall::global(cx).read(cx).channel_id(cx)?; - let name = self - .channel_store - .read(cx) - .channel_for_id(channel_id)? - .name - .as_str(); + let channel = self.channel_store.read(cx).channel_for_id(channel_id)?; + + channel_link = Some(channel.link()); + (channel_icon, channel_tooltip_text) = match channel.visibility { + proto::ChannelVisibility::Public => { + (Some("icons/public.svg"), Some("Copy public channel link.")) + } + proto::ChannelVisibility::Members => { + (Some("icons/hash.svg"), Some("Copy private channel link.")) + } + }; - Some(name) + Some(channel.name.as_str()) }); if let Some(name) = channel_name { - Cow::Owned(format!("#{}", name)) + Cow::Owned(format!("{}", name)) } else { Cow::Borrowed("Current Call") } @@ -1527,28 +1583,30 @@ impl CollabPanel { enum AddContact {} let button = match section { - Section::ActiveCall => Some( + Section::ActiveCall => channel_link.map(|channel_link| { + let channel_link_copy = channel_link.clone(); MouseEventHandler::new::(0, cx, |state, _| { render_icon_button( theme .collab_panel .leave_call_button .style_for(is_selected, state), - "icons/exit.svg", + "icons/link.svg", ) }) .with_cursor_style(CursorStyle::PointingHand) - .on_click(MouseButton::Left, |_, _, cx| { - Self::leave_call(cx); + .on_click(MouseButton::Left, move |_, _, cx| { + let item = ClipboardItem::new(channel_link_copy.clone()); + cx.write_to_clipboard(item) }) .with_tooltip::( 0, - "Leave call", + channel_tooltip_text.unwrap(), None, tooltip_style.clone(), cx, - ), - ), + ) + }), Section::Contacts => Some( MouseEventHandler::new::(0, cx, |state, _| { render_icon_button( @@ -1633,6 +1691,21 @@ impl CollabPanel { theme.collab_panel.contact_username.container.margin.left, ), ) + } else if let Some(channel_icon) = channel_icon { + Some( + Svg::new(channel_icon) + .with_color(header_style.text.color) + .constrained() + .with_max_width(icon_size) + .with_max_height(icon_size) + .aligned() + .constrained() + .with_width(icon_size) + .contained() + .with_margin_right( + theme.collab_panel.contact_username.container.margin.left, + ), + ) } else { None }) @@ -1908,6 +1981,12 @@ impl CollabPanel { let channel_id = channel.id; let collab_theme = &theme.collab_panel; let has_children = self.channel_store.read(cx).has_children(channel_id); + let is_public = self + .channel_store + .read(cx) + .channel_for_id(channel_id) + .map(|channel| channel.visibility) + == Some(proto::ChannelVisibility::Public); let other_selected = self.selected_channel().map(|channel| channel.0.id) == Some(channel.id); let disclosed = has_children.then(|| !self.collapsed_channels.binary_search(&path).is_ok()); @@ -1965,12 +2044,16 @@ impl CollabPanel { Flex::::row() .with_child( - Svg::new("icons/hash.svg") - .with_color(collab_theme.channel_hash.color) - .constrained() - .with_width(collab_theme.channel_hash.width) - .aligned() - .left(), + Svg::new(if is_public { + "icons/public.svg" + } else { + "icons/hash.svg" + }) + .with_color(collab_theme.channel_hash.color) + .constrained() + .with_width(collab_theme.channel_hash.width) + .aligned() + .left(), ) .with_child({ let style = collab_theme.channel_name.inactive_state(); @@ -2275,7 +2358,7 @@ impl CollabPanel { .with_child(render_tree_branch( tree_branch, &row.name.text, - true, + false, vec2f(host_avatar_width, theme.row_height), cx.font_cache(), )) @@ -2308,6 +2391,62 @@ impl CollabPanel { .into_any() } + fn render_channel_chat( + &self, + channel_id: ChannelId, + theme: &theme::CollabPanel, + is_selected: bool, + ix: usize, + cx: &mut ViewContext, + ) -> AnyElement { + enum ChannelChat {} + let host_avatar_width = theme + .contact_avatar + .width + .or(theme.contact_avatar.height) + .unwrap_or(0.); + + MouseEventHandler::new::(ix as usize, cx, |state, cx| { + let tree_branch = *theme.tree_branch.in_state(is_selected).style_for(state); + let row = theme.project_row.in_state(is_selected).style_for(state); + + Flex::::row() + .with_child(render_tree_branch( + tree_branch, + &row.name.text, + true, + vec2f(host_avatar_width, theme.row_height), + cx.font_cache(), + )) + .with_child( + Svg::new("icons/conversations.svg") + .with_color(theme.channel_hash.color) + .constrained() + .with_width(theme.channel_hash.width) + .aligned() + .left(), + ) + .with_child( + Label::new("chat", theme.channel_name.text.clone()) + .contained() + .with_style(theme.channel_name.container) + .aligned() + .left() + .flex(1., true), + ) + .constrained() + .with_height(theme.row_height) + .contained() + .with_style(*theme.channel_row.style_for(is_selected, state)) + .with_padding_left(theme.channel_row.default_style().padding.left) + }) + .on_click(MouseButton::Left, move |_, this, cx| { + this.join_channel_chat(&JoinChannelChat { channel_id }, cx); + }) + .with_cursor_style(CursorStyle::PointingHand) + .into_any() + } + fn render_channel_invite( channel: Arc, channel_store: ModelHandle, @@ -2771,6 +2910,9 @@ impl CollabPanel { } } ListEntry::ParticipantScreen { peer_id, .. } => { + let Some(peer_id) = peer_id else { + return; + }; if let Some(workspace) = self.workspace.upgrade(cx) { workspace.update(cx, |workspace, cx| { workspace.open_shared_screen(*peer_id, cx) @@ -3498,6 +3640,14 @@ impl PartialEq for ListEntry { return channel_id == other_id; } } + ListEntry::ChannelChat { channel_id } => { + if let ListEntry::ChannelChat { + channel_id: other_id, + } = other + { + return channel_id == other_id; + } + } ListEntry::ChannelInvite(channel_1) => { if let ListEntry::ChannelInvite(channel_2) = other { return channel_1.id == channel_2.id; From 04a28fe831d2d044eff3405f4b034d767e07be0a Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Oct 2023 13:32:08 -0600 Subject: [PATCH 25/25] Fix lint errors --- styles/src/style_tree/collab_modals.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/styles/src/style_tree/collab_modals.ts b/styles/src/style_tree/collab_modals.ts index 586e7be3f0bb3c68a14e198ee93f1c89b35cf6fb..6132ce5ff4dd1d831ea976d5723a9d9e69550b02 100644 --- a/styles/src/style_tree/collab_modals.ts +++ b/styles/src/style_tree/collab_modals.ts @@ -1,4 +1,4 @@ -import { StyleSet, StyleSets, Styles, useTheme } from "../theme" +import { StyleSets, useTheme } from "../theme" import { background, border, foreground, text } from "./components" import picker from "./picker" import { input } from "../component/input" @@ -44,7 +44,7 @@ export default function channel_modal(): any { ...text(theme.middle, "sans", styleset, "active"), } } - }); + }) const member_icon_style = icon_button({ variant: "ghost",